Skip to content

fix(file): set success=false when file write blocked by staleness/partial-read guard (#59600) - #59685

Closed
webtecnica wants to merge 1 commit into
NousResearch:mainfrom
webtecnica:fix/59600-patch-write-success-false-on-stale
Closed

webtecnica wants to merge 1 commit into
NousResearch:mainfrom
webtecnica:fix/59600-patch-write-success-false-on-stale

Conversation

@webtecnica

Copy link
Copy Markdown
Contributor

Summary

When a file was previously read with offset/limit pagination (partial view), subsequent write_file calls returned success: true with a valid-looking diff — but the write was never applied. The only indication of failure was a _warning metadata field, which is easy to miss.

Fix

Set result_dict["success"] = False alongside the _warning on both code paths (with and without resolved path), so calling agents can detect the failure and retry with a full read.

Changes

File Δ
tools/file_tools.py +2 lines (2 sites)

Closes #59600

…tial-read guard (NousResearch#59600)

When a file was previously read with offset/limit pagination (partial
view), subsequent write_file calls returned success: true with a
valid-looking diff but the write was never applied — the only
indication was a _warning metadata field. This caused silent data
loss: calling agents saw success: true and assumed the change
landed.

Fix: set result_dict["success"] = False alongside the
_warning on both code paths (with and without resolved path), so
calling agents can detect the failure and retry with a full read.
@alt-glitch alt-glitch added type/bug Something isn't working tool/file File tools (read, write, patch, search) P2 Medium — degraded but workaround exists labels Jul 6, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Related: part of the #59600 silent-persistence cluster. This PR fixes the write_file_tool path (2 sites) in tools/file_tools.py, while the sibling PRs target the patch path — #59610 (block patch on partial-view), #59616 (promote warning to error), #59680 (file_operations patch explicit success=False). Different code sites / mechanisms, same goal; maintainer picks the combination.

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for isolating the two write_file_tool warning sites.

Automated hermes-sweeper review found that the change is based on a contradictory premise:

  • tools/file_tools.py:1718-1728 obtains file_state.check_stale(...) but always calls file_ops.write_file(...) at line 1724 before adding _warning.
  • tools/file_state.py:142-152 defines check_stale() as a warning-producing check; callers decide whether to block or warn.
  • Consequently, setting success=false at the proposed lines would report failure after a write has actually completed.

The related #59600 report concerns the patch path, and the member comment identifies separate sibling PRs for that mechanism. This write-file change cannot correctly implement its stated "write was skipped" behavior without first adding and validating an actual pre-write blocking contract.

This is an automated hermes-sweeper review.

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

Labels

P2 Medium — degraded but workaround exists sweeper:incoherent Sweeper: insufficient/incoherent to action tool/file File tools (read, write, patch, search) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

patch tool returns success:true even when file write was silently skipped

3 participants