fix(file-tools): stale write_file overwrites are refused before disk mutation (#65604, salvage #65605) - #91238
Conversation
૮ >ﻌ< ა ci reviewran on 378fb2c — fix: paged, extracted and post-compaction reads count as a w
|
|
Cross-reference: #69403 (open, CI green) also touches |
|
#67216 ( |
42fc2d1 to
aff7920
Compare
Require an explicit full-file baseline before replacing existing host-visible files with write_file, and fail closed when that baseline is stale. This prevents stale conversation context from clobbering manual or external edits.\n\nRefs #65604
…seline, docs - test_file_staleness redacted-read case now force-enables redaction (matches tests/agent/test_redact.py convention) so it exercises the sentinel path in hermetic CI where security.redact_secrets is unset. - test_write_verification CRLF case establishes a read baseline first (the new guard refuses unread existing-file overwrites by design). - tools-reference.md documents the read-before-overwrite contract. - contributors/emails mapping for DanSpicyTaco.
…with schema The stale-write guard now refuses write_file on an existing file the task never read in full, so test_write_file_rewrite_hint's overwrite-without-read fixtures were refused before the hint could be computed. Reading first is the exact read->whole-file-rewrite pattern the hint exists for. tools-reference.md's write_file row now mirrors the WRITE_FILE_SCHEMA description (one-sentence contract + the recovery step) instead of a longer paraphrase.
… baseline The stale-overwrite refusal made write_file permanently unusable for any existing file it could not show in one read_file page: every >2000-line (or >100K-char) page was recorded as partial, no full baseline ever existed, and the refusal told the model to "re-read the whole file", which the tool cannot do. Track the line ranges each task pages through per path at one mtime; contiguous pages from line 1 to total_lines are a full read (a new mtime between pages restarts the coverage). The same gap hit two siblings: the extracted-document branch (.ipynb, text-authorable) returned before any read bookkeeping, so an existing notebook could never be overwritten; and reset_file_dedup dropped every baseline on compaction while keeping read_timestamps, so every write after compaction was refused even for files unchanged on disk. Baselines now survive compaction exactly like the dedup mtime map does — only while the recorded mtime still matches. Refusal texts no longer embed the pre-PR "Warning: … Consider re-reading" copy inside "Refusing to overwrite", and every refusal names a recovery the model can perform: read the remaining pages, or use patch.
aff7920 to
378fb2c
Compare
Summary
write_filecan no longer overwrite an existing file the task never read, or a file that changed on disk after the last read — the stale-write refusal now fires BEFORE disk mutation instead of warning after the data loss already happened. Salvage of #65605 by @danspicytaco (fixes #65604), cherry-picked with authorship preserved, plus hardening on top.Also scouted from MoonshotAI/kimi-code#3096 (their agent-core-v2
staleGuardfeature, merged this week) — same design conclusion independently: veto Edit/Write pre-execution on never-read or mtime-changed targets, refresh the baseline after own writes so consecutive self-writes stay unblocked.Changes
tools/file_tools.py(contributor commits, cherry-picked):full_write_baselinesset: populated by full, non-partial, unredactedread_fileand by successfulwrite_file;patchintentionally does not qualify.write_file_toolrefuses (pre-mutation,stale_write_blocked: true) when the target exists but has no baseline, or when cross-agent/mtime staleness is detected; refusal message tells the model to re-read → merge → retry.«redacted:…»sentinel back into credential files — the [Bug]: redact_sensitive_text corrupts API keys in config files when read via read_file/search_files, causing 401 #35519 corruption class).patchremains warning-only; new-file creation unaffected; capped container (_FULL_WRITE_BASELINES_CAP) +_new_read_tracker_task()consolidation.tests/agent/test_redact.pyconvention.test_write_verificationCRLF case establishes a read baseline first — sibling test pinning the old overwrite-without-read behavior, updated to the new contract.website/docs/reference/tools-reference.mddocuments the read-before-overwrite contract.contributors/emails/mapping for DanSpicyTaco.Validation
Targeted suites: 139 passed, 2 skipped (
test_file_staleness,test_file_read_guards,test_file_tools,test_file_state_registry,test_cross_profile_guard,test_write_verification,test_line_ending_preservation). E2E against real file I/O in an isolatedHERMES_HOME: 7/7 scenarios (unread-refusal, read→write, consecutive write, external-edit refusal, re-read heal, new file, patch untouched).Closes #65605 (salvaged, authorship preserved). Fixes #65604.
Infographic