fix(agentic): guard Write against overwrite, allow loop recovery, and harden Write content generation#690
Merged
Conversation
…elete+Write The Write tool previously overwrote existing files unconditionally, which made it easy for models to clobber files with incomplete content when they should have used Edit. Refuse the write when the target file already exists and update the tool description to explain the intended workflow: use Edit to modify, or Delete + Write to fully rewrite. The error message returned to the model also points at both alternatives so it can self-correct on the next round.
…tected loops Previously, both the consecutive-signature and periodic-signature loop detectors terminated the round immediately on the first hit. In practice the model can often recover if it is told that it is stuck and asked to change strategy. Inject a system_reminder user message describing the detected loop and asking the model to switch approach, give it up to 3 such recovery attempts (shared across both detectors), and only then fall back to the existing terminate-with-loop_detected behavior. The reminders are also persisted via SessionManager so the recovery is visible in transcripts. This keeps the existing safety net (we still stop eventually) while making genuine transient stalls recoverable instead of fatal.
…del output
Two related improvements to the two-stage Write flow that asks the model
to emit the full file body inside <bitfun_contents> tags.
1. Prompt hardening
- Spell out the rule against omission placeholders ("// rest of the
code", "// existing code unchanged", etc.) and clarify that literal
"..." is fine when it is genuine file content (XML/JSON/docs).
- Forbid markdown fences and stray XML wrappers around the body.
- Add an assistant prefill of "<bitfun_contents>\n" to bias the model
toward emitting raw content immediately.
2. Output sanitization in extract_bitfun_contents
- Strip thinking-style XML blocks (<think>, <reasoning>,
<reflection>, <analysis>) including the non-standard <think ... >
variants that some reasoning models emit.
- Strip outer markdown code fences (```lang ... ```) when they wrap
the entire body.
- Add a conservative "omission marker" detector that warns when the
generated body contains comment-style phrases such as
"// ... rest of the code" or "<!-- snip -->". The detector is
deliberately strict (only matches phrases that are essentially never
legitimate in real source/data files) and only emits a warning — it
never blocks the write, since Write must be able to produce any
kind of file, including ones that legitimately discuss these
phrases in prose.
Adds unit tests covering thinking-block stripping, fence stripping,
preservation of legitimate XML, and both positive and negative cases for
the placeholder detector (including XML data and prose mentioning the
phrases, which must not trigger).
…ent retries - Skip Write content generation when path resolution/policy fails or the file exists - Reject existing targets in validate_input when content is absent to avoid wasted model calls - Treat identical Write content on an existing path as successful no-op (already_exists_same_content) - Add FileWriteTool unit tests and tighten assistant-facing success guidance
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three related improvements that address recurring failure modes we have hit in real agent runs:
Write tool overwrite guard (
file_write_tool.rs)Writenow refuses to overwrite an existing file and returns an error that points the model at the right alternatives:Editto modify, orDeletefollowed byWriteto fully rewrite.Write, silently losing data.Editis almost always the correct choice; full rewrites remain possible via the explicitDelete+Writesequence.Recoverable loop detection (
execution_engine.rs)<system_reminder>user message describing the detected loop and asking the model to change strategy, granting up to 3 such recovery attempts (shared across both detectors) before falling back to the previous terminate-with-loop_detectedbehavior.SessionManagerso the recovery is visible in transcripts.Write content generation: prompt + sanitization + omission warning (
round_executor.rs)// rest of the code,// existing code unchanged) and stray markdown fences / wrapper XML, while explicitly allowing literal...when it is genuine file content (XML/JSON/docs). An assistant prefill of<bitfun_contents>\nis added to bias raw-content output.extract_bitfun_contentsnow sanitizes the body: strips thinking-style XML blocks (<think>,<reasoning>,<reflection>,<analysis>, including the non-standard<think ... >variants some reasoning models emit) and strips outer markdown code fences when they wrap the entire body.detect_placeholder_patternswarning is added. It matches only comment-style omission phrases that are essentially never legitimate in real source/data files (e.g.// ... rest of the code,<!-- snip -->) and emits a warning only — it never blocks the write, becauseWritemust remain general enough to produce any kind of file (including prose / XML that legitimately mentions those phrases).Test plan
cargo check --workspacecargo test -p bitfun-core --lib agentic::execution::round_executor— 29 tests passing, including new cases:<bitfun_contents>tags)// ... rest of the code,# existing code unchanged,<!-- snip -->).../ "the rest of the story", prose discussing the phrase, plainTODO:/FIXME:comments)