Skip to content

fix(file_write): refuse an overwrite the approval probe saw as a create - #98

Merged
M3gA-Mind merged 1 commit into
tinyhumansai:mainfrom
M3gA-Mind:fix/file-write-reprobe-approval
Oct 23, 2026
Merged

M3gA-Mind merged 1 commit into
tinyhumansai:mainfrom
M3gA-Mind:fix/file-write-reprobe-approval

Conversation

@M3gA-Mind

Copy link
Copy Markdown
Collaborator

Summary

file_write decides whether a write needs approval in external_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_write with no workspace descriptor, so the probe uses the action dir. Sub-agents reuse the parent's tool belt (parent_tools) but run with child_context.workspace set to their own descriptor; spawn_parallel workers 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.

execute now 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 it and 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_write whose 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 2024 on the touched files only.
  • N/A: cargo clippy --all-targets --all-features -- -D warnings was left to CI.
  • N/A: cargo build --all-targets --all-features was left to CI.
  • N/A: cargo test --all-features was left to CI. Only the file_write tests ran locally:
    • an_overwrite_the_approval_probe_saw_as_a_create_is_refused before the fix failed (Written 9 bytes to a.txt, the existing file clobbered without approval); after the fix it passed, along with the other 22 file_write tests.

Tests

filesystem/file_write/mod_tests.rs::an_overwrite_the_approval_probe_saw_as_a_create_is_refused sets up a supervised gate rooted at an empty directory and a context workspace that holds a.txt. The test asserts that the probe reports a create (no approval), that execute_with_context refuses, and that a.txt keeps its contents.

Documentation

The doc comment on external_effect_with_args used to say execute() never re-probes. It now describes the one case where it does.

Checklist

  • The change is focused on one logical change
  • No new #[allow(...)], #[ignore], or relaxed lints
  • No secrets, tokens, or .env contents in the diff or the description

@tinysweeper

tinysweeper Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

Reviewed head: c24c1e8bd83dded45052853ea7e7ac7cf15df126

Verdict: Ready for maintainer review · Priority: none
Coverage: Reviewed 2 of 2 files.
Pull request purpose (title): fix(file_write): ref…

Diagnostics and supporting details

Full summary

Pull 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

Change surface Files Review signal Count
Production 1 Active findings 0
Tests 1 Noted findings 0
Documentation 0 Resolved findings 0
Configuration 0 Pending checks/questions 0

Completeness: Complete
Test assessment: No supported feature-to-test mapping was available; this does not mean tests are absent or passed.

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

  • Added — Re-probe refusing unapproved overwrites in a context workspace: In ask-before-edit mode, an overwrite that the approval probe saw as a create (because it resolved against a different root) is now refused rather than silently clobbering an existing file; it can only add refusals, not widen writes. (crates/tinytools-std/src/filesystem/file_write/mod.rs impl FileWriteTool {, crates/tinytools-std/src/filesystem/file_write/mod.rs impl Tool for FileWriteTool {)
  • Modified — Doc comment on `external_effect_with_args` re-probe behavior: Documentation now accurately states that `execute()` does re-probe for the context-workspace case, replacing the prior claim that it never re-probes. (crates/tinytools-std/src/filesystem/file_write/mod.rs impl Tool for FileWriteTool {)

Tests

No supported feature-to-test mapping was produced. Test execution is not inferred.

Findings

No active actionable findings.

Before merge

None.

Agent review details

critique

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: Critique lane retained one model finding at no more than Likely confidence due to degraded retrieval (code index behind the PR head), which was not elevated into a concrete defect.
  • Lane summary: Critique findings were capped at Likely confidence because retrieval was degraded: Only refuse the overwrite when the approval probe saw a create. (1 model finding retained at no more than Likely confidence because retrieval was degraded) _The code index is behind this pull request (indexed at `28ea0933b938`), so retrieved context may be out of date._

security

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: Security lane reports no defect: the predicate is conservative (only adds refusals), re-probes with the same args the gate saw, and the new test exercises the exact scenario.
  • Lane summary: The change adds a re-probe in `file_write::execute_in_context` that refuses an unapproved overwrite when the approval probe resolved the path against a different root, plus a test for it. The predicate is conservative (it can only add refusals, never widen writes), the probe is repeated with the same args the gate saw, and the test exercises the exact scenario. No security defect found. _The code index is behind this pull request (indexed at `28ea0933b938`), so retrieved context may be out of date._

tests

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: Tests lane found no defects and confirmed the test asserts the refusal, the error output, and untouched contents, plus an in-test check that the probe saw a create.
  • Lane summary: The new re-probe branch in `execute_in_context` closes a real gap and the added test exercises it end to end — asserting the refusal, the error output, and that the file's contents are untouched — plus an in-test check that the approval probe saw the path as a create, which is what makes the case meaningful. The diff's comment claim ("a context workspace can resolve the path somewhere other than this probe's root") is exactly the situation the test constructs. No defects found; worth noting the existing-file-with-approval path (where `external_effect_with_args` returns true) is not covered by a new test, but that path's behaviour is unchanged by this diff and pre-existing coverage already exists in `contract_tests.rs`. _The code index is behind this pull request (indexed at `28ea0933b938`), so retrieved context may be out of date._

commits

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: Commits lane found no sensitive content in committed files.
  • Lane summary: No sensitive content in committed files. _The code index is behind this pull request (indexed at `28ea0933b938`), so retrieved context may be out of date._

description

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: Description lane confirms the description matches the diff (root-cause mismatch, re-probe, refusal message, new test, doc update), but could not verify the claim that prompted overwrites remain unaffected, which it flags as a code question outside its lane.
  • Lane summary: The description matches the diff: it accurately explains the root-cause mismatch between the probe's approval root and the context workspace, the new re-probe in `execute_in_context`, the exact refusal message, the unchanged paths (full/read-only/create/prompted overwrite), the new test, and the doc-comment update. One thing I could not verify is the claim that a prompted overwrite remains unaffected — the re-probe condition looks like it could also fire for paths the probe classified as an existing file if the invoked `external_effect_with_args` re-evaluates differently, but that is a code question rather than a description problem and falls outside this lane. _The code index is behind this pull request (indexed at `28ea0933b938`), so retrieved context may be out of date._
Evidence and run details
  • Models: ladder/vectors-oai3, deepseek-v4.1-flash
  • Spend: $0.000524
  • Tokens: 199963 input · 4366 output · 16896 cached · 208 embedding
  • Updated: 2026-10-22T23:26:11Z
Head State Pass summary
c24c1e8bd83d ready for maintainer review 0 active finding(s), 0 resolved finding(s) (at 2026-10-22T23:26:11Z)

tinysweeper 0.1.2

@coderabbitai

coderabbitai Bot commented Oct 22, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: cdb61ef8-25ab-4a45-a72a-f3808fbec192
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tinysweeper found nothing blocking. Approving.

Diagnostics

Total spend: $0.0005

@M3gA-Mind
M3gA-Mind merged commit 75e42ba into tinyhumansai:main Oct 23, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant