Fix/no op file mutations by wm0018 · Pull Request #97008 · openclaw/openclaw

wm0018 · 2026-06-26T09:34:46Z

What Problem This Solves

Fixes an issue where agents can repeatedly call write with identical content or edit with identical old/new text, and the tool reports success each time. The agent interprets this as progress and keeps retrying the same no-op mutation indefinitely, wasting tokens and inflating the tool loop without making any actual change.

Why This Change Was Made

The write tool already had a content comparison precheck (readOriginalWriteState) that could detect identical content, but it was not used to short-circuit the write. Even when precheck.state === "same", the tool would write the file again and return "Successfully wrote N bytes".

Similarly, the edit tool had no check for edits where oldText === newText, and no detection for edits that produce no diff after application.

The fix returns a terminal result with terminate: true. This ensures:

The runtime classifies the result as terminal (not retryable) — the terminate field is already used for this purpose at agent-tool-definition-adapter.ts:560
The agent does not interpret the mutation as progress
No new types are needed; the existing AgentToolResult.terminate signal is used

Three no-op paths are covered:

Tool	Path	Detection
`write`	Content identical to existing file	`readOriginalWriteState` returns `"same"`
`edit`	`oldText === newText` in any edit entry	Pre-apply check
`edit`	All edits applied but content unchanged	`baseContent === newContent` after apply

Non-goals:

This does not add a generic circuit-breaker or retry budget — that remains a separate concern tracked in other issues.
This does not change behavior for real writes/edits: different content still writes normally, new files still create normally.
apply_patch no-change detection is left as follow-up work.

User Impact

Agents will no longer waste tokens re-writing identical file content or re-editing with identical old/new text. When a write or edit would produce no change, the tool returns a structured terminal { status: "blocked", reason: "no-op-write" } result, allowing the runtime to classify it properly and the agent to move on.

Evidence

Structured result shape

No-op results now include:

{
  content: [{ type: "text", text: "No changes: ..." }],
  details: undefined,
  terminate: true,
}

This uses the textResult helper from src/agents/tools/common.ts and adds terminate: true (already used by the tool adapter for client-side tools at agent-tool-definition-adapter.ts:560).

Real behavior proof

Live tool invocation output (Node.js, Linux):

write with identical content:

{
  "content": [{
    "type": "text",
    "text": "No changes: content is identical to existing /tmp/openclaw-proof-xxx/demo.txt"
  }],
  "details": undefined,
  "terminate": true
}

File mtime confirmed unchanged after the call — the file was not rewritten.

edit with identical oldText/newText:

{
  "content": [{
    "type": "text",
    "text": "No changes: one or more edits have identical old and new text in /tmp/openclaw-proof-xxx/demo.ts."
  }],
  "details": undefined,
  "terminate": true
}

Regression — write with different content still writes:

text: Successfully wrote 12 bytes to /tmp/openclaw-proof-xxx/write-diff.txt

Regression — write to new file still creates:

text: Successfully wrote 10 bytes to /tmp/openclaw-proof-xxx/new-file.txt
file exists: "brand new\n"

Regression — edit with non-matching oldText still errors (not no-opped):

Error: Could not find the exact text in ... The old text must match exactly including all whitespace and newlines.

Unit test coverage

// write.test.ts
expect(result.content[0]).toEqual({
  type: "text",
  text: `No changes: content is identical to existing ${filePath}`,
});
expect(result.details).toBeUndefined();
expect(result.terminate).toBe(true);

Regression coverage preserved

write with different content still writes successfully
write to a new file still creates it
write post-write timeout recovery (recoverSuccessfulWrite) is unchanged
edit with real changes still applies and returns diff

Changes

File	Lines	Change
`src/agents/sessions/tools/write.ts`	+5/-2	Return `{ terminate: true }` on identical content (uses `textResult` + `terminate`)
`src/agents/sessions/tools/edit.ts`	+7/-2	Check `oldText === newText` pre-apply AND `baseContent === newContent` post-apply; return `terminate: true`
`src/agents/sessions/tools/write.test.ts`	+2	Assert `details` and `terminate` fields on no-op result

Closes #96983

clawsweeper · 2026-06-26T09:35:43Z

Codex review: needs changes before merge. Reviewed June 26, 2026, 6:09 AM ET / 10:09 UTC.

Summary
The PR changes the built-in write and edit session tools to return terminate: true no-op results for identical writes and identical or no-diff edits, with a focused write-tool test update.

PR surface: Source +27, Tests +10. Total +37 across 3 files.

Reproducibility: yes. at source level: current main can detect same-content writes but still writes and returns success, and edit no-change currently throws before any structured terminal no-op result is produced. I did not run a live agent-loop replay in this read-only review.

Review metrics: 1 noteworthy metric.

Linked no-op surfaces: 2 covered, 1 omitted. The branch handles built-in write and edit, while the canonical issue also names apply_patch, so closing scope needs maintainer attention before merge.

Stored data model
Persistent data-model change detected: serialized state: src/agents/sessions/tools/edit.ts, serialized state: src/agents/sessions/tools/write.test.ts, serialized state: src/agents/sessions/tools/write.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #96983
Summary: This PR is a candidate fix for the canonical no-op file-mutation issue; the broader circuit-breaker issue is adjacent, and the other open PR is a same-root candidate rather than a safe superseding canonical PR.

Members:

canonical: fix(tools): treat no-op edits and identical writes as terminal tool-loop failures #96983 - The PR body uses closing syntax for the exact no-op file-mutation terminal-loop issue.
partial_overlap: Tool call circuit breaker needed — LLMs will blindly retry forever without one #78865 - The broader circuit-breaker issue shares the repeated tool-loop failure mode but tracks retry, cooldown, and confirmation policy beyond this narrow file-tool fix.
same_root_cause: fix(tools): treat no-op writes and edits as terminal tool-loop failures (fixes #96983) #97004 - That open PR targets the same canonical issue but is not merged or established as a safe superseding replacement.
superseded: fix(tools): classify no-op file mutations #97000 - That earlier fuller attempt covered the same issue but is closed unmerged, so it is related context rather than a canonical landing path.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦞 diamond lobster
Patch quality: 🦪 silver shellfish
Result: blocked by patch quality or review findings.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

[P2] Fix mixed edit batches so valid sibling replacements are not skipped.
[P2] Classify the helper-thrown No changes made path and add focused edit-tool regression coverage.
Either cover apply_patch no-op classification or keep the canonical issue open for that remaining surface.

Risk before merge

[P1] A mixed edit call with one identical entry and one real replacement can now return a terminal no-op before applying the real replacement.
[P1] The post-apply no-diff branch does not catch the helper's existing No changes made throw path, so some canonical no-change edits can still surface as ordinary errors.
[P1] Same-content write changes from idempotent overwrite success to an early terminal no-op result, so maintainers should intentionally accept that tool-result and mtime compatibility change.
[P1] The PR body closes the canonical linked issue while apply_patch no-change classification is explicitly left as follow-up work.

Maintainer options:

Fix Edit No-Op Semantics Before Merge (recommended)
Preserve or reject only the offending no-op edit entry without skipping valid sibling edits, and classify the helper's No changes made error through the same terminal no-op path.
Accept The Write Compatibility Change
Maintainers may intentionally accept same-content write becoming an early terminal no-op, but the PR should document that behavior and keep real writes/new-file creation unchanged.
Keep The Canonical Issue Open
If apply_patch is not included here, remove or narrow the closing scope so the remaining no-op file-mutation work stays tracked.

Copy recommended automerge instruction

@clawsweeper automerge

Special instructions:
Fix the no-op edit handling in `src/agents/sessions/tools/edit.ts` so mixed edit batches do not skip valid replacements, catch/classify `applyEditsToNormalizedContent` no-change errors through the terminal no-op contract, and add focused regression coverage in `src/agents/sessions/tools/edit.test.ts` plus any necessary write/edit test updates.

Next step before merge

[P2] A bounded automated repair can address the two edit-path findings on this maintainer-editable branch; the remaining write compatibility and issue-scope concerns are visible merge risks for maintainers.

Security
Cleared: The diff only changes built-in agent file-tool behavior and a focused test; it adds no dependency, workflow, credential, or supply-chain execution surface.

Review findings

[P2] Preserve valid edits in mixed no-op batches — src/agents/sessions/tools/edit.ts:392-399
[P2] Classify helper-thrown no-change edits — src/agents/sessions/tools/edit.ts:431-435

Review details

Best possible solution:

Land a narrow file-tool fix that preserves valid multi-edit siblings, classifies helper-thrown no-change edits through the same terminal contract, and either covers apply_patch or leaves the canonical issue open for that remaining surface.

Do we have a high-confidence way to reproduce the issue?

Yes, at source level: current main can detect same-content writes but still writes and returns success, and edit no-change currently throws before any structured terminal no-op result is produced. I did not run a live agent-loop replay in this read-only review.

Is this the best way to solve the issue?

No. The PR is in the right built-in file-tool boundary, but the edit no-op check is too broad for multi-edit calls and the post-apply no-diff check misses the helper-thrown no-change path.

Full review comments:

[P2] Preserve valid edits in mixed no-op batches — src/agents/sessions/tools/edit.ts:392-399
This returns No changes for the entire edit call when any entry has identical old/new text. The edit tool supports multiple disjoint edits in one call, so a batch with one no-op and one real edit would silently skip the real edit instead of applying it or rejecting only the offending entry.
Confidence: 0.9
[P2] Classify helper-thrown no-change edits — src/agents/sessions/tools/edit.ts:431-435
applyEditsToNormalizedContent already throws when the computed content is unchanged, so this post-return equality check cannot handle that no-diff path. Catch the helper's no-change error or move classification into the helper; otherwise some canonical no-change edits still surface as ordinary errors instead of the terminal no-op result this PR adds.
Confidence: 0.87

Overall correctness: patch is incorrect
Overall confidence: 0.9

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against ec737ee74d9b.

Label changes

Label justifications:

P2: The PR targets a normal-priority agent tool-loop reliability bug with bounded but real session impact.
merge-risk: 🚨 compatibility: The branch changes same-content write from idempotent overwrite success to a terminal no-op result, affecting tool-result and file-metadata expectations.
merge-risk: 🚨 session-state: The new terminal no-op paths affect whether an agent session treats file mutations as progress, error, or an early stop.
rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦞 diamond lobster and patch quality is 🦪 silver shellfish.
status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): The PR body includes copied live Linux/Node output for after-fix write/edit no-op calls and regression cases, which is sufficient real behavior proof for this non-visual tool change.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied live Linux/Node output for after-fix write/edit no-op calls and regression cases, which is sufficient real behavior proof for this non-visual tool change.

Evidence reviewed

PR surface:

Source +27, Tests +10. Total +37 across 3 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	2	27	0	+27
Tests	1	15	5	+10
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	3	42	5	+37

Acceptance criteria:

[P1] node scripts/run-vitest.mjs src/agents/sessions/tools/write.test.ts src/agents/sessions/tools/edit.test.ts.
[P1] git diff --check.

What I checked:

Repository policy read: Root AGENTS.md and scoped src/agents/AGENTS.md/src/agents/tools/AGENTS.md were read; their review guidance required checking the agent tool/result path beyond the diff. (AGENTS.md:8, ec737ee74d9b)
Current main write behavior: Current main computes readOriginalWriteState, but the same-content state is not short-circuited before ops.writeFile, so the linked bug is still real on main. (src/agents/sessions/tools/write.ts:395, ec737ee74d9b)
Current main edit no-change behavior: applyEditsToNormalizedContent throws getNoChangeError when baseContent === newContent, so a caller-side equality check after the helper returns cannot classify that no-change path. (src/agents/sessions/tools/edit-diff.ts:263, ec737ee74d9b)
PR head mixed-edit behavior: At PR head, edits.some((e) => e.oldText === e.newText) returns a terminal no-op for the whole edit call before reading or applying any valid sibling edits. (src/agents/sessions/tools/edit.ts:392, 7caf70222b35)
PR head unreachable no-diff branch: The PR adds a post-helper baseContent === newContent no-op branch, but current helper behavior throws before returning that pair for canonical no-change edits. (src/agents/sessions/tools/edit.ts:431, 7caf70222b35)
Agent terminate contract: Agent core stops a tool batch only when every finalized result has result.terminate === true; resolved tool executions are otherwise non-error results unless hooks override them. (packages/agent-core/src/agent-loop.ts:768, ec737ee74d9b)

Likely related people:

zhang-guiping: Local blame attributes the current write, edit-diff, and agent-loop lines central to this review to commit f12ade0082ad; the commit is broad, so this is a routing hint rather than precise feature ownership. (role: recent current-main file committer; confidence: low; commits: f12ade0082ad; files: src/agents/sessions/tools/write.ts, src/agents/sessions/tools/edit-diff.ts, packages/agent-core/src/agent-loop.ts)
Ayaan Zaidi: git show for f12ade0082ad lists Ayaan Zaidi as the committer for the broad current-main commit that carries the reviewed files. (role: recent current-main committer; confidence: low; commits: f12ade0082ad; files: src/agents/sessions/tools/write.ts, src/agents/sessions/tools/edit-diff.ts, packages/agent-core/src/agent-loop.ts)
steipete: History for the agent loop-detection area shows Peter Steinberger introduced configurable loop detection and later guarded loop-detection parsing, which is adjacent to terminal tool-loop behavior. (role: adjacent loop-detection contributor; confidence: medium; commits: 076df941a326, 83a8b78a4248; files: src/agents/tool-loop-detection.ts)
Sk Akram: History shows Sk Akram added stuck-loop detection and polling backoff infrastructure in the same runtime-safety family as this no-op tool-loop fix. (role: original adjacent loop-hardening contributor; confidence: medium; commits: e5eb5b3e43d7; files: src/agents/tool-loop-detection.ts, src/agents/tool-loop-detection.test.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

…esults

openclaw-barnacle Bot added app: web-ui App: web-ui agents Agent runtime and tooling size: XS labels Jun 26, 2026

fix(tools): treat no-op file writes and edits as terminal tool-loop r…

c4c7b21

…esults

wm0018 force-pushed the fix/no-op-file-mutations branch from 6a68a63 to c4c7b21 Compare June 26, 2026 09:39

openclaw-barnacle Bot removed the app: web-ui App: web-ui label Jun 26, 2026

clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 26, 2026

fix(tools): treat no-op file writes and edits as terminal tool-loop r…

053f313

…esults

openclaw-barnacle Bot added size: S and removed size: XS labels Jun 26, 2026

clawsweeper Bot added P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels Jun 26, 2026

fix(tools): treat no-op file writes and edits as terminal tool-loop r…

7caf702

…esults

openclaw-barnacle Bot added size: XS and removed size: S labels Jun 26, 2026

clawsweeper Bot mentioned this pull request Jun 26, 2026

fix(tools): treat no-op writes and edits as terminal tool-loop failures (fixes #96983) #97004

Closed

1 task

zw-xysk mentioned this pull request Jun 26, 2026

fix(tools): treat no-op writes and edits as terminal tool-loop failures (fixes #96983) #97044

Merged

1 task

wm0018 closed this Jun 27, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Fix/no op file mutations#97008

Fix/no op file mutations#97008
wm0018 wants to merge 3 commits into
openclaw:mainfrom
wm0018:fix/no-op-file-mutations

wm0018 commented Jun 26, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 26, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

wm0018 commented Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Structured result shape

Real behavior proof

Unit test coverage

Regression coverage preserved

Changes

Uh oh!

clawsweeper Bot commented Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

wm0018 commented Jun 26, 2026 •

edited

Loading

clawsweeper Bot commented Jun 26, 2026 •

edited

Loading