Skip to content

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

Open
yannrichet wants to merge 2 commits into
mainfrom
claude/new-session-4b2704
Open

yannrichet wants to merge 2 commits into
mainfrom
claude/new-session-4b2704

Conversation

@yannrichet

Copy link
Copy Markdown
Member

Description

Fixes a critical bug in sh:// command path resolution that caused incorrect results when using input/output files by bare name. The resolver was converting every word that looked like a file path to an absolute path in the launch directory without checking existence or precedence, leading to:

  1. Reading unsubstituted template files from the launch directory instead of compiled case files
  2. Writing output files outside case directories (shared across parallel cases), causing data corruption
  3. No error indication of the problem

Related Issues

Fixes P0-8

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • Test addition/modification

Changes Made

  • Modified resolve_all_paths_in_command() to accept an optional working_dir parameter (case directory)
  • Updated path resolution logic to only resolve words that:
    • Exist in the launch directory (original_cwd)
    • Do NOT exist in the case directory (working_dir) — compiled inputs take precedence
    • Are NOT targets of output redirections (>, >>, 2>, etc.)
  • Added logging at info level for each resolved word
  • Updated run_local_calculation() to pass the case directory (.) to the resolver
  • Added comprehensive regression tests covering:
    • Precedence of case files over launch files
    • Output redirection targets never being resolved
    • Nonexistent words being left untouched
    • End-to-end behavior with fz.fzr() for both redirected output and launch-only scripts

Testing Performed

Automated Tests

  • Added 6 new regression tests in tests/test_p0_8_sh_path_resolution.py
  • Tests cover unit-level path resolution and end-to-end fz.fzr() scenarios
  • All existing tests pass (no breaking changes to public API)

Documentation

  • Updated NEWS.md with detailed explanation of the fix and migration guidance
  • Added/updated docstrings in resolve_all_paths_in_command() and _resolve_paths_in_segment()

Code Quality

  • Code follows project style guidelines
  • Self-review completed
  • Comments added for complex logic (output redirection detection, precedence checks)
  • Type hints updated for new parameter

Breaking Changes

This is a bug fix with potential impact on existing results:

  • Commands like sh://cat in.txt > out.txt will now behave correctly (read compiled file, write in case dir)
  • Results obtained earlier with commands referencing input/output files by bare name should be re-checked
  • Scripts located only in the launch directory (e.g., sh://bash script.sh) continue to work as before

Additional Notes

The fix is conservative: it only resolves paths that provably exist in the launch directory and don't shadow case files. Output redirection targets are never resolved (stricter than just checking existence, since the defect was writing outside case directories). Each resolved word is logged for auditability.

https://claude.ai/code/session_01RhG7KcNHxMhoJrsZJFtC4Y

…r and absent from case dir

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RhG7KcNHxMhoJrsZJFtC4Y
…f the old resolver)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RhG7KcNHxMhoJrsZJFtC4Y

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants