Repository navigation
fix(file_write): refuse an overwrite the approval probe saw as a create - #98
Conversation
Tiny Sweeper reviewReviewed head: Verdict: Ready for maintainer review · Priority: none Diagnostics and supporting detailsFull summaryPull request purpose (title): fix(file_write): refuse an overwrite the approval probe saw as a create. This pull-request fixes an unapproved-overwrite gap in `file_write`. In ask-before-edit mode the approval probe resolves a relative path against the tool's approval root, while the write itself resolves against the run's context workspace; when the two roots differ, a path that is new under the approval root but already exists under the context workspace was treated as a free create and overwritten without approval. The fix adds a re-probe in `execute_in_context` that refuses the write when approval is needed, the probe classified the path as a create, and something already exists at the resolved target. The latest-push-delta section is unavailable, so this reflects the whole-pull-request diff rather than a proven push delta. Review snapshot
Completeness: Complete What changed`FileWriteTool::execute_in_context` now re-checks the resolved target after computing it: if the gate needs approval and `external_effect_with_args` classified the path as a create, but `symlink_metadata` finds an existing entry, the write is refused with `<path> already exists in this workspace; overwriting it needs approval. Use the edit tool to change it` and nothing is written. The doc comment on `external_effect_with_args` is updated to describe this single re-probe case instead of stating that `execute()` never re-probes. A new test constructs the mismatch between the approval root and the context workspace. Features
TestsNo supported feature-to-test mapping was produced. Test execution is not inferred. FindingsNo active actionable findings. Before mergeNone. Agent review detailscritique
security
tests
commits
description
Evidence and run details
|
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Comment |
Summary
file_writedecides whether a write needs approval inexternal_effect_with_args. In ask-before-edit mode, overwriting an existing file prompts and creating a new file does not. That probe resolves a relative path against the tool's approval root (approval_workspace_root, or the gate's action dir). The write itself resolves against the run's context workspace when one is threaded, and the probe never sees that context. When the two roots differ, a path that is new under the approval root but already exists under the context workspace was classified as a free create and then overwritten without approval.This happens in a real host. OpenHuman's session builder constructs
file_writewith no workspace descriptor, so the probe uses the action dir. Sub-agents reuse the parent's tool belt (parent_tools) but run withchild_context.workspaceset to their own descriptor;spawn_parallelworkers get a fresh git worktree. In a worktree sub-agent, a file that exists in the worktree but not in the parent's action dir could be overwritten without a prompt.executenow re-probes. If writes need approval, the probe classified the path as a create, and something already exists at the resolved target, the write is refused with<path> already exists in this workspace; overwriting it needs approval. Use the edit tool to change itand nothing is written. When the roots agree this case cannot happen, short of an external process creating the file between the probe and the write. If that does happen, the write is refused too.Related issue
None.
API or behavior changes
In ask-before-edit mode only: a
file_writewhose approval probe saw a new file, but whose real target already exists, now returns an error and does not overwrite. Full mode, read-only mode, creates, and overwrites that were prompted for are unchanged. The model is pointed to the edit tool, which asks for approval.Validation
Commands actually run, with their outcome:
cargo fmt --all -- --check:rustfmt --edition 2024on the touched files only.cargo clippy --all-targets --all-features -- -D warningswas left to CI.cargo build --all-targets --all-featureswas left to CI.cargo test --all-featureswas left to CI. Only thefile_writetests ran locally:an_overwrite_the_approval_probe_saw_as_a_create_is_refusedbefore the fix failed (Written 9 bytes to a.txt, the existing file clobbered without approval); after the fix it passed, along with the other 22file_writetests.Tests
filesystem/file_write/mod_tests.rs::an_overwrite_the_approval_probe_saw_as_a_create_is_refusedsets up a supervised gate rooted at an empty directory and a context workspace that holdsa.txt. The test asserts that the probe reports a create (no approval), thatexecute_with_contextrefuses, and thata.txtkeeps its contents.Documentation
The doc comment on
external_effect_with_argsused to sayexecute()never re-probes. It now describes the one case where it does.Checklist
#[allow(...)],#[ignore], or relaxed lints.envcontents in the diff or the description