Fix: path resolution in sh:// commands reads wrong files (P0-8)

NEWS.md

@@ -2,6 +2,21 @@
 
 ## Unreleased
 
+### Fix: potentially wrong results with `sh://` commands (P0-8)
+
+- Path resolution in `sh://` commands converted every word that looked like a
+  file name to an absolute path in the launch directory without checking that
+  it exists. With `sh://cat in.txt > out.txt`, the calculation read the
+  **un-substituted** template from the launch directory instead of the case's
+  compiled file, and wrote `out.txt` outside the case directory (shared across
+  parallel cases), with no error. A word is now resolved only if it exists in
+  the launch directory and does not exist in the case directory; targets of
+  `>`/`>>` redirections are never resolved (stricter than "unless existing":
+  writing into the launch directory is the defect). Each resolved word is
+  logged at info level. Scripts located only in the launch directory
+  (`sh://bash script.sh`) still resolve. Results obtained earlier with
+  commands referencing input or output files by bare name should be re-checked.
+
 ### Breaking changes
 
 - **Dropped Python 3.8 support** (P0-5). `requires-python` is now `>=3.9` and

fz/runners/sh.py

@@ -15,7 +15,9 @@
 from .errors import classify_error
 
 
-def resolve_all_paths_in_command(command: str, original_cwd: str) -> tuple[str, bool]:
+def resolve_all_paths_in_command(
+    command: str, original_cwd: str, working_dir: str = None
+) -> tuple[str, bool]:
     """
     Resolve ALL file paths in a shell command to absolute paths
 
@@ -30,6 +32,10 @@ def resolve_all_paths_in_command(command: str, original_cwd: str) -> tuple[str,
     Args:
         command: Original shell command
         original_cwd: Original working directory for resolving relative paths
+        working_dir: Case working directory. A word is resolved only if it exists
+            in ``original_cwd`` and does not exist in ``working_dir`` (compiled
+            input files take precedence). Output redirection targets are never
+            resolved. If None, only the existence check in ``original_cwd`` applies.
 
     Returns:
         Tuple of (resolved_command, was_changed) where:
@@ -62,7 +68,7 @@ def resolve_all_paths_in_command(command: str, original_cwd: str) -> tuple[str,
 
             # Process this command segment
             resolved_segment, segment_changed = _resolve_paths_in_segment(
-                segment, original_cwd
+                segment, original_cwd, working_dir
             )
             resolved_segments.append(resolved_segment)
 
@@ -83,10 +89,15 @@ def resolve_all_paths_in_command(command: str, original_cwd: str) -> tuple[str,
         return command, False
 
 
-def _resolve_paths_in_segment(segment: str, original_cwd: str) -> tuple[str, bool]:
+def _resolve_paths_in_segment(
+    segment: str, original_cwd: str, working_dir: str = None
+) -> tuple[str, bool]:
     """
-    Resolve ALL relative paths in a command segment to absolute paths.
-    Simple approach: convert any token that looks like a file path to absolute.
+    Resolve relative paths in a command segment to absolute paths.
+
+    A token that looks like a file path is converted only if it exists in
+    ``original_cwd`` and not in ``working_dir``; the target of ``>``/``>>``
+    redirections is never converted.
     """
     import shlex
     import re
@@ -103,8 +114,15 @@ def _resolve_paths_in_segment(segment: str, original_cwd: str) -> tuple[str, boo
 
     resolved_parts = []
     was_changed = False
+    prev_part = ""
 
     for part in command_parts:
+        is_output_target = prev_part.lstrip("0123456789&") in (">", ">>", ">|")
+        prev_part = part
+        if is_output_target:
+            resolved_parts.append(part)
+            continue
+
         # Skip if already absolute path
         if os.path.isabs(part):
             resolved_parts.append(part)
@@ -210,9 +228,20 @@ def _resolve_paths_in_segment(segment: str, original_cwd: str) -> tuple[str, boo
             should_resolve = True
 
         if should_resolve:
-            # Convert to absolute path
             abs_path = os.path.abspath(os.path.join(original_cwd, part))
 
+            # Resolve only files really present in the launch directory and
+            # absent from the case directory (compiled inputs take precedence)
+            if not os.path.exists(abs_path):
+                resolved_parts.append(part)
+                continue
+            if working_dir is not None and os.path.exists(
+                os.path.join(str(working_dir), part)
+            ):
+                resolved_parts.append(part)
+                continue
+            log_info(f"Info: sh:// word resolved: {part} -> {abs_path}")
+
             # On Windows, convert path to forward slashes for bash compatibility
             # MSYS2/Git Bash/WSL all expect Unix-style paths
             if os.name == 'nt':
@@ -294,7 +323,7 @@ def run_local_calculation(
         # Construct command - resolve ALL file paths to absolute for reliable parallel execution
         if command:
             resolved_command, was_changed = resolve_all_paths_in_command(
-                command.replace("\\","/"), original_cwd
+                command.replace("\\","/"), original_cwd, "."  # cwd is the case dir after chdir
             )
 
             # Apply shell path resolution to command if FZ_SHELL_PATH is set

tests/test_comprehensive_paths.py

@@ -115,7 +115,9 @@ def test_comprehensive_path_resolution():
         {
             "name": "Archive operations",
             "calculator": "sh://tar -czf archive.tar.gz subdir/ && echo 'result = 600'",
-            "expected_status": "done" if platform.system() != "Windows" else "failed"
+            # P0-8: "failed" on Windows came from the old resolver rewriting the
+            # (non-existent) archive name to the launch directory
+            "expected_status": "done"
         },
         #{ NO: awk with file argument and redirection inside command line is not supported
         #    "name": "Multiple file arguments",

tests/test_p0_8_sh_path_resolution.py

@@ -0,0 +1,86 @@
+"""P0-8 regression tests: path resolution in sh:// commands.
+
+Only words that exist in the launch directory and not in the case directory
+are resolved; output redirection targets are never resolved.
+"""
+
+import os
+from pathlib import Path
+
+import fz
+from fz.runners.sh import resolve_all_paths_in_command
+
+
+def test_resolver_skips_nonexistent_and_case_files(tmp_path):
+    launch = tmp_path / "launch"
+    case = tmp_path / "case"
+    launch.mkdir()
+    case.mkdir()
+    (launch / "in.txt").write_text("template")
+    (launch / "tool.sh").write_text("echo tool")
+    (case / "in.txt").write_text("compiled")
+
+    cmd, _ = resolve_all_paths_in_command(
+        "bash tool.sh in.txt > out.txt", str(launch), str(case)
+    )
+    assert f"{launch.as_posix()}/tool.sh" in cmd          # only in launch dir: resolved
+    assert f"{launch.as_posix()}/in.txt" not in cmd       # in case dir: compiled file wins
+    assert f"{launch.as_posix()}/out.txt" not in cmd      # redirection target untouched
+    assert cmd.endswith("> out.txt")
+
+
+def test_resolver_never_resolves_output_redirect_even_if_exists(tmp_path):
+    launch = tmp_path / "launch"
+    case = tmp_path / "case"
+    launch.mkdir()
+    case.mkdir()
+    (launch / "out.txt").write_text("x")
+    for op in (">", ">>", "2>"):
+        cmd, _ = resolve_all_paths_in_command(
+            f"echo hi {op} out.txt", str(launch), str(case)
+        )
+        assert launch.as_posix() not in cmd
+
+
+def test_resolver_nonexistent_word_kept(tmp_path):
+    cmd, _ = resolve_all_paths_in_command("bash ghost.sh ./x", str(tmp_path), None)
+    assert tmp_path.as_posix() not in cmd
+
+
+def test_fzr_sh_cat_redirect_two_cases(tmp_path):
+    model_dir = tmp_path
+    (model_dir / "in.txt").write_text("x=${x}\n")
+    inp = model_dir / "in.txt"
+    launch = Path.cwd()
+
+    res = fz.fzr(
+        str(inp),
+        {"x": [1, 2]},
+        {"varprefix": "$", "delim": "{}", "output": {"y": "cat res.txt"}},
+        calculators="sh://cat in.txt > res.txt",
+        results_dir=str(tmp_path / "res"),
+    )
+    # fz appends the input file name to the command, so cat prints it twice
+    assert [v.splitlines()[0] for v in res["y"].astype(str)] == ["x=1", "x=2"]
+    case_dirs = sorted(p for p in (tmp_path / "res").iterdir() if p.is_dir())
+    assert len(case_dirs) == 2
+    for d in case_dirs:
+        assert (d / "res.txt").read_text().startswith("x=")
+    assert not (launch / "res.txt").exists()
+
+
+def test_fzr_sh_script_only_in_launch_dir(tmp_path):
+    (tmp_path / "in.txt").write_text("v=${x}\n")
+    script = Path.cwd() / "myscript.sh"
+    script.write_text("#!/bin/bash\ncat in.txt > out.dat\n")
+    try:
+        res = fz.fzr(
+            str(tmp_path / "in.txt"),
+            {"x": [3]},
+            {"varprefix": "$", "delim": "{}", "output": {"y": "cat out.dat"}},
+            calculators="sh://bash myscript.sh",
+            results_dir=str(tmp_path / "res"),
+        )
+        assert str(res["y"].iloc[0]).strip() == "v=3"
+    finally:
+        script.unlink()