add validation for nested directories and unit tests

Author: kryanbeane

src/codeflare_sdk/ray/rayjobs/rayjob.py

@@ -17,6 +17,8 @@
 """
 
 import logging
+import os
+import re
 import warnings
 from typing import Dict, Any, Optional, Tuple, Union
 
@@ -160,7 +162,9 @@ def submit(self) -> str:
         if not self.entrypoint:
             raise ValueError("Entrypoint must be provided to submit a RayJob")
 
+        # Validate configuration before submitting
         self._validate_ray_version_compatibility()
+        self._validate_working_dir_entrypoint()
 
         # Extract files from entrypoint and runtime_env working_dir
         files = extract_all_local_files(self)
@@ -451,6 +455,85 @@ def _validate_cluster_config_image(self):
         elif is_warning:
             warnings.warn(f"Cluster config image: {message}")
 
+    def _validate_working_dir_entrypoint(self):
+        """
+        Validate that entrypoint doesn't redundantly reference the working_dir.
+
+        This prevents a common mistake where users specify both working_dir and
+        reference the same directory in their entrypoint, causing the job to fail.
+
+        Example problem:
+            working_dir = "./mydir"
+            entrypoint = "python ./mydir/script.py"  # Wrong! Will look for ./mydir/mydir/script.py
+
+        Should be:
+            entrypoint = "python script.py"  # Correct! Runs from within working_dir
+
+        Raises:
+            ValueError: If a redundant directory reference is detected
+        """
+        # Get working_dir from runtime_env if it exists
+        if not self.runtime_env:
+            return
+
+        runtime_env_dict = (
+            self.runtime_env.to_dict()
+            if hasattr(self.runtime_env, "to_dict")
+            else self.runtime_env
+        )
+        if not runtime_env_dict or "working_dir" not in runtime_env_dict:
+            return
+
+        working_dir = runtime_env_dict["working_dir"]
+
+        # Only validate local working_dir (not remote URLs)
+        if not os.path.isdir(working_dir):
+            return
+
+        # Extract Python file path from entrypoint using the same pattern as runtime_env.py
+        # Pattern matches: test.py, ./test.py, dir/test.py, my-dir/test.py
+        python_file_pattern = r"(?:python\s+)?([./\w/-]+\.py)"
+        matches = re.findall(python_file_pattern, self.entrypoint)
+
+        if not matches:
+            return  # No Python file found in entrypoint
+
+        entrypoint_path = matches[0]  # Get first Python file reference
+
+        # Normalize paths for comparison (remove ./, trailing /, etc.)
+        normalized_working_dir = os.path.normpath(working_dir)
+        normalized_entrypoint = os.path.normpath(entrypoint_path)
+
+        # Check if entrypoint path starts with working_dir path
+        # This indicates potential redundant directory reference
+        if normalized_entrypoint.startswith(normalized_working_dir + os.sep):
+            # Extract the path that would be searched for (the redundant nested path)
+            relative_to_working_dir = os.path.relpath(
+                normalized_entrypoint, normalized_working_dir
+            )
+            # The redundant path would be: working_dir / basename(working_dir) / relative_path
+            working_dir_basename = os.path.basename(normalized_working_dir)
+            redundant_nested_path = os.path.join(
+                normalized_working_dir, working_dir_basename, relative_to_working_dir
+            )
+
+            # Check if the redundant nested path actually exists on disk
+            # If it doesn't exist, this is likely a user error
+            # If it does exist, it's a legitimate nested directory structure
+            if not os.path.exists(redundant_nested_path):
+                # This is a user error - block it with helpful message
+                raise ValueError(
+                    f"❌ Working directory conflict detected:\n"
+                    f"   working_dir: '{working_dir}'\n"
+                    f"   entrypoint references: '{entrypoint_path}'\n"
+                    f"\n"
+                    f"This will fail because the entrypoint runs from within working_dir.\n"
+                    f"It would look for: '{redundant_nested_path}' (which doesn't exist)\n"
+                    f"\n"
+                    f"Fix: Remove the directory prefix from your entrypoint:\n"
+                    f'   entrypoint = "python {relative_to_working_dir}"'
+                )
+
     def status(
         self, print_to_console: bool = True
     ) -> Tuple[CodeflareRayJobStatus, bool]:

src/codeflare_sdk/ray/rayjobs/test/test_rayjob.py

@@ -1401,3 +1401,256 @@ def test_build_submitter_pod_template_with_files(auto_mock_setup):
     # Verify paths match keys
     for item in config_map_items:
         assert item["key"] == item["path"]
+
+
+def test_validate_working_dir_entrypoint_no_runtime_env(auto_mock_setup):
+    """
+    Test validation passes when no runtime_env is specified.
+    """
+    rayjob = RayJob(
+        job_name="test-job",
+        cluster_name="test-cluster",
+        entrypoint="python script.py",
+        namespace="test-namespace",
+    )
+
+    # Should not raise any exception
+    rayjob._validate_working_dir_entrypoint()
+
+
+def test_validate_working_dir_entrypoint_no_working_dir(auto_mock_setup):
+    """
+    Test validation passes when runtime_env has no working_dir.
+    """
+    rayjob = RayJob(
+        job_name="test-job",
+        cluster_name="test-cluster",
+        entrypoint="python script.py",
+        namespace="test-namespace",
+        runtime_env=RuntimeEnv(pip=["numpy"]),
+    )
+
+    # Should not raise any exception
+    rayjob._validate_working_dir_entrypoint()
+
+
+def test_validate_working_dir_entrypoint_remote_working_dir(auto_mock_setup):
+    """
+    Test validation skips remote working_dir (URL).
+    """
+    rayjob = RayJob(
+        job_name="test-job",
+        cluster_name="test-cluster",
+        entrypoint="python script.py",
+        namespace="test-namespace",
+        runtime_env=RuntimeEnv(
+            working_dir="https://github.com/user/repo/archive/main.zip"
+        ),
+    )
+
+    # Should not raise any exception (remote URL skipped)
+    rayjob._validate_working_dir_entrypoint()
+
+
+def test_validate_working_dir_entrypoint_no_python_file(auto_mock_setup):
+    """
+    Test validation passes when entrypoint has no Python file reference.
+    """
+    rayjob = RayJob(
+        job_name="test-job",
+        cluster_name="test-cluster",
+        entrypoint="echo 'hello world'",  # No Python file
+        namespace="test-namespace",
+        runtime_env=RuntimeEnv(working_dir="."),
+    )
+
+    # Should not raise any exception
+    rayjob._validate_working_dir_entrypoint()
+
+
+def test_validate_working_dir_entrypoint_no_redundancy(auto_mock_setup, tmp_path):
+    """
+    Test validation passes when entrypoint doesn't reference working_dir.
+    """
+    # Create test directory and file
+    test_dir = tmp_path / "testdir"
+    test_dir.mkdir()
+    script_file = test_dir / "script.py"
+    script_file.write_text("print('hello')")
+
+    rayjob = RayJob(
+        job_name="test-job",
+        cluster_name="test-cluster",
+        entrypoint="python script.py",  # No directory prefix
+        namespace="test-namespace",
+        runtime_env=RuntimeEnv(working_dir=str(test_dir)),
+    )
+
+    # Should not raise any exception
+    rayjob._validate_working_dir_entrypoint()
+
+
+def test_validate_working_dir_entrypoint_redundant_reference_error(
+    auto_mock_setup, tmp_path
+):
+    """
+    Test validation raises error when entrypoint redundantly references working_dir.
+    """
+    # Create test directory and file
+    test_dir = tmp_path / "testdir"
+    test_dir.mkdir()
+    script_file = test_dir / "script.py"
+    script_file.write_text("print('hello')")
+
+    rayjob = RayJob(
+        job_name="test-job",
+        cluster_name="test-cluster",
+        entrypoint=f"python {test_dir}/script.py",  # Redundant reference
+        namespace="test-namespace",
+        runtime_env=RuntimeEnv(working_dir=str(test_dir)),
+    )
+
+    # Should raise ValueError with helpful message
+    with pytest.raises(ValueError, match="Working directory conflict detected"):
+        rayjob._validate_working_dir_entrypoint()
+
+
+def test_validate_working_dir_entrypoint_with_dot_slash(auto_mock_setup, tmp_path):
+    """
+    Test validation handles paths with ./ prefix correctly.
+    """
+    # Create test directory and file
+    test_dir = tmp_path / "testdir"
+    test_dir.mkdir()
+    script_file = test_dir / "script.py"
+    script_file.write_text("print('hello')")
+
+    # Change to temp directory so relative paths work
+    import os
+
+    original_cwd = os.getcwd()
+    try:
+        os.chdir(tmp_path)
+
+        rayjob = RayJob(
+            job_name="test-job",
+            cluster_name="test-cluster",
+            entrypoint="python ./testdir/script.py",  # With ./ prefix
+            namespace="test-namespace",
+            runtime_env=RuntimeEnv(working_dir="./testdir"),  # With ./ prefix
+        )
+
+        # Should raise ValueError (redundant reference)
+        with pytest.raises(ValueError, match="Working directory conflict detected"):
+            rayjob._validate_working_dir_entrypoint()
+    finally:
+        os.chdir(original_cwd)
+
+
+def test_validate_working_dir_entrypoint_submit_integration(auto_mock_setup, tmp_path):
+    """
+    Test that validation is called during submit() and blocks submission.
+    """
+    mock_api_instance = auto_mock_setup["rayjob_api"]
+    mock_api_instance.submit_job.return_value = {"metadata": {"name": "test-job"}}
+
+    # Create test directory and file
+    test_dir = tmp_path / "testdir"
+    test_dir.mkdir()
+    script_file = test_dir / "script.py"
+    script_file.write_text("print('hello')")
+
+    rayjob = RayJob(
+        job_name="test-job",
+        cluster_name="test-cluster",
+        entrypoint=f"python {test_dir}/script.py",  # Redundant reference
+        namespace="test-namespace",
+        runtime_env=RuntimeEnv(working_dir=str(test_dir)),
+    )
+
+    # Should raise ValueError during submit() before API call
+    with pytest.raises(ValueError, match="Working directory conflict detected"):
+        rayjob.submit()
+
+    # Verify submit_job was never called (validation blocked it)
+    mock_api_instance.submit_job.assert_not_called()
+
+
+def test_validate_working_dir_entrypoint_error_message_format(
+    auto_mock_setup, tmp_path
+):
+    """
+    Test that error message contains helpful information.
+    """
+    # Create test directory and file
+    test_dir = tmp_path / "testdir"
+    test_dir.mkdir()
+    script_file = test_dir / "script.py"
+    script_file.write_text("print('hello')")
+
+    rayjob = RayJob(
+        job_name="test-job",
+        cluster_name="test-cluster",
+        entrypoint=f"python {test_dir}/script.py",
+        namespace="test-namespace",
+        runtime_env=RuntimeEnv(working_dir=str(test_dir)),
+    )
+
+    try:
+        rayjob._validate_working_dir_entrypoint()
+        assert False, "Should have raised ValueError"
+    except ValueError as e:
+        error_msg = str(e)
+        # Verify error message contains key information
+        assert "Working directory conflict detected" in error_msg
+        assert "working_dir:" in error_msg
+        assert "entrypoint references:" in error_msg
+        assert "Fix: Remove the directory prefix" in error_msg
+        assert "python script.py" in error_msg  # Suggested fix
+
+
+def test_validate_working_dir_entrypoint_subdirectory_valid(auto_mock_setup, tmp_path):
+    """
+    Test validation passes when entrypoint references subdirectory within working_dir.
+    """
+    # Create test directory structure: testdir/subdir/script.py
+    test_dir = tmp_path / "testdir"
+    test_dir.mkdir()
+    sub_dir = test_dir / "subdir"
+    sub_dir.mkdir()
+    script_file = sub_dir / "script.py"
+    script_file.write_text("print('hello')")
+
+    rayjob = RayJob(
+        job_name="test-job",
+        cluster_name="test-cluster",
+        entrypoint="python subdir/script.py",  # Correct: relative to working_dir
+        namespace="test-namespace",
+        runtime_env=RuntimeEnv(working_dir=str(test_dir)),
+    )
+
+    # Should not raise any exception
+    rayjob._validate_working_dir_entrypoint()
+
+
+def test_validate_working_dir_entrypoint_runtime_env_as_dict(auto_mock_setup, tmp_path):
+    """
+    Test validation works when runtime_env is passed as dict (not RuntimeEnv object).
+    """
+    # Create test directory and file
+    test_dir = tmp_path / "testdir"
+    test_dir.mkdir()
+    script_file = test_dir / "script.py"
+    script_file.write_text("print('hello')")
+
+    rayjob = RayJob(
+        job_name="test-job",
+        cluster_name="test-cluster",
+        entrypoint=f"python {test_dir}/script.py",  # Redundant reference
+        namespace="test-namespace",
+        runtime_env={"working_dir": str(test_dir)},  # Dict instead of RuntimeEnv
+    )
+
+    # Should raise ValueError even with dict runtime_env
+    with pytest.raises(ValueError, match="Working directory conflict detected"):
+        rayjob._validate_working_dir_entrypoint()