(MOT-4363) fix(harness): accept absolute workspace paths - #716
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe E2E harness now preserves transcript call IDs, matches results to individual invocations, normalizes workspace paths, and validates operation-specific outputs for coder and host execution scenarios. ChangesSandbox invocation validation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Transcript
participant function_invocations
participant shell_coder_sandbox
participant function_result
Transcript->>function_invocations: Parse calls and call IDs
function_invocations-->>shell_coder_sandbox: Return observed invocations
shell_coder_sandbox->>shell_coder_sandbox: Normalize workspace paths
shell_coder_sandbox->>function_result: Find result for matching invocation
function_result->>Transcript: Match call ID and function ID
Transcript-->>function_result: Return successful result
function_result-->>shell_coder_sandbox: Return correlated result
shell_coder_sandbox->>shell_coder_sandbox: Validate operation output
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
skill-check — worker0 verified, 54 skipped (no docs/).
Four for four. Nicely done. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@harness/tests/e2e/src/scenarios/shell_coder_sandbox.rs`:
- Around line 426-429: Update the argument validation around
workspace_path_matches so FINAL_NAME must be the Python command’s actual script
argument, not merely any argument. Accept only the supported Python invocation
with exactly one script argument, and reject earlier script arguments and
python3 -c forms; add negative tests covering both cases.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5c81ee92-871c-45bc-94f6-2b7aa1a553ea
📒 Files selected for processing (2)
harness/tests/e2e/src/scenarios/common.rsharness/tests/e2e/src/scenarios/shell_coder_sandbox.rs
| .is_some_and(|args| { | ||
| args.iter() | ||
| .any(|arg| workspace_path_matches(arg.as_str(), root, FINAL_NAME)) | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Require the final file to be Python's script argument.
Line 426 accepts any argument that names FINAL_NAME. Therefore, python3 other.py /workspace/final_check.py or python3 -c "..." /workspace/final_check.py can pass this gate while Python does not execute the final file. Require exactly one script argument, or validate the exact supported Python command form.
Proposed fix
- .is_some_and(|args| {
- args.iter()
- .any(|arg| workspace_path_matches(arg.as_str(), root, FINAL_NAME))
- });
+ .is_some_and(|args| {
+ args.len() == 1 && workspace_path_matches(args[0].as_str(), root, FINAL_NAME)
+ });Add negative tests for an earlier script argument and for python3 -c. This conflicts with the PR objective for an exact host execution gate.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .is_some_and(|args| { | |
| args.iter() | |
| .any(|arg| workspace_path_matches(arg.as_str(), root, FINAL_NAME)) | |
| }); | |
| .is_some_and(|args| { | |
| args.len() == 1 && workspace_path_matches(args[0].as_str(), root, FINAL_NAME) | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@harness/tests/e2e/src/scenarios/shell_coder_sandbox.rs` around lines 426 -
429, Update the argument validation around workspace_path_matches so FINAL_NAME
must be the Python command’s actual script argument, not merely any argument.
Accept only the supported Python invocation with exactly one script argument,
and reject earlier script arguments and python3 -c forms; add negative tests
covering both cases.
1 participant
Summary
function_call_idWhy
The
shell_coder_sandboxevaluator compared raw path strings with relative filenames. The DeepSeek V4 Flash daily execution completed the required file workflow with absolute in-workspace paths, but the evaluator reported a hard-gate failure despite the exact final file and successful outputs.Impact
Valid workflows no longer fail solely because they use an absolute workspace path. A successful result from an unrelated invocation can no longer satisfy the affected Coder or host gate. The scenario prompt, score weights, threshold, and hard-gate policy remain unchanged.
Validation
cargo fmt --manifest-path harness/Cargo.toml --all -- --checkcargo test --locked --manifest-path harness/Cargo.toml -p harness-e2e(91 passed)cargo clippy --locked --manifest-path harness/Cargo.toml -p harness-e2e -- -D warningsFixes MOT-4363
Refs MOT-4279
Summary by CodeRabbit
Bug Fixes
Tests