fix(chat): agent write_file/edit_file no longer return false success receipts (BUG-29) #100

2026-07-22T21:19:58Z

tim.lingo commented

2026-07-22 21:19:58 +00:00

Root cause (BUG-29)

dispatch_tool in chat.el fed the model false success receipts on its two file-mutating agent tools:

write_file returned {"ok":true} without ever checking fs_write's result. A write into a missing/unwritable directory (or any fs failure) still reported success.
edit_file returned {"ok":true} even when old_text was not in the file — str_replace silently no-ops — and its fs_write was also unchecked. The model was told the edit landed when the file was untouched.

The model repeats these receipts to the user as fact, so every downstream claim built on them is a confabulation the soul itself manufactured.

The change

Implements Receipt Contract rule 1 (docs repo, docs/specs/RECEIPT-CONTRACT-2026-07-22.md): a tool result must reflect what actually happened.

write_file: check fs_write's return (1 = all bytes written, 0 = fail); on failure return {"error":"write failed"} — same error-JSON shape the handler already uses ({"error":"file not found"}).
edit_file: reject empty old_text; verify old_text is actually present (str_contains) before replacing ({"error":"old_text not found in file"}); check the fs_write result the same way.
Verification is by operation result, not an fs_read read-back: fs_read arms the runtime's one-shot binary send length — the exact mechanism that truncated the safety-contact response (#96). This is the same honest-write pattern that fix established in safety.el.

Minimal diff, no new architecture: 26 insertions in the two handlers.

How to test (exact sandbox repro)

No HTTP route reaches dispatch_tool without a live LLM (the agentic loop creates the only suspension blobs /api/sessions/{id}/approve and /tool_result will resume — verified empirically: both return no pending tool / unknown session_id on a fresh soul). So the repro drives dispatch_tool directly with a throwaway .el entry (not committed):

import "safety.el"
import "chat.el"
import "sessions.el"
let w: String = dispatch_tool("write_file", "{\"path\":\"<SANDBOX>/locked/f.txt\",\"content\":\"hello\"}")
println("WRITE_FILE(unwritable-dir) -> " + w)
let e: String = dispatch_tool("edit_file", "{\"path\":\"<SANDBOX>/target.txt\",\"old_text\":\"THIS_TEXT_IS_NOT_IN_THE_FILE\",\"new_text\":\"replacement\"}")
println("EDIT_FILE(absent-old-text) -> " + e)

Setup: mkdir <SANDBOX>/locked && chmod 000 <SANDBOX>/locked; printf 'hello world\n' > <SANDBOX>/target.txt. Build with elb (--out=<builddir> pre-seeded with dist/elp-c-decls.h) against the vendored release runtime v1.0.0-20260501 (the dev/AR "latest" runtime no longer defines engram_prune_telemetry — same pin as #97's CI fix), manifest entry temporarily pointed at the driver.

Before (unpatched, this branch's parent `b784750`)

WRITE_FILE(unwritable-dir) -> {"ok":true}     <- lie: nothing was written
EDIT_FILE(absent-old-text) -> {"ok":true}     <- lie: file untouched

After (this branch)

WRITE_FILE(unwritable-dir) -> {"error":"write failed"}
EDIT_FILE(absent-old-text) -> {"error":"old_text not found in file"}
WRITE_FILE(writable)       -> {"ok":true}     (file content read-back-verified)
EDIT_FILE(present-old-text)-> {"ok":true}     (replacement verified on disk)

Regression gate

scripts/verify-soul-contract.sh (from main) against the patched elb-built soul on a throwaway port: GATE PASS — PRESENCE 27/27 answered, IMMUTABILITY pass (all mutation routes tombstone/supersede, no hard-delete).

Note on dist/

dist/soul.c regen is intentionally not included — the combined-unit assembly step is Will's, and the committed dist already lags the .el sources by 8 modules.

🤖 Generated with Claude Code

## Root cause (BUG-29) `dispatch_tool` in chat.el fed the model **false success receipts** on its two file-mutating agent tools: - **write_file** returned `{"ok":true}` without ever checking `fs_write`'s result. A write into a missing/unwritable directory (or any fs failure) still reported success. - **edit_file** returned `{"ok":true}` even when `old_text` was not in the file — `str_replace` silently no-ops — and its `fs_write` was also unchecked. The model was told the edit landed when the file was untouched. The model repeats these receipts to the user as fact, so every downstream claim built on them is a confabulation the soul itself manufactured. ## The change Implements **Receipt Contract rule 1** (docs repo, `docs/specs/RECEIPT-CONTRACT-2026-07-22.md`): a tool result must reflect what actually happened. - **write_file**: check `fs_write`'s return (1 = all bytes written, 0 = fail); on failure return `{"error":"write failed"}` — same error-JSON shape the handler already uses (`{"error":"file not found"}`). - **edit_file**: reject empty `old_text`; verify `old_text` is actually present (`str_contains`) before replacing (`{"error":"old_text not found in file"}`); check the `fs_write` result the same way. - Verification is by **operation result, not an fs_read read-back**: `fs_read` arms the runtime's one-shot binary send length — the exact mechanism that truncated the safety-contact response (#96). This is the same honest-write pattern that fix established in safety.el. Minimal diff, no new architecture: 26 insertions in the two handlers. ## How to test (exact sandbox repro) No HTTP route reaches `dispatch_tool` without a live LLM (the agentic loop creates the only suspension blobs `/api/sessions/{id}/approve` and `/tool_result` will resume — verified empirically: both return `no pending tool` / `unknown session_id` on a fresh soul). So the repro drives `dispatch_tool` directly with a throwaway `.el` entry (not committed): ```el import "safety.el" import "chat.el" import "sessions.el" let w: String = dispatch_tool("write_file", "{\"path\":\"<SANDBOX>/locked/f.txt\",\"content\":\"hello\"}") println("WRITE_FILE(unwritable-dir) -> " + w) let e: String = dispatch_tool("edit_file", "{\"path\":\"<SANDBOX>/target.txt\",\"old_text\":\"THIS_TEXT_IS_NOT_IN_THE_FILE\",\"new_text\":\"replacement\"}") println("EDIT_FILE(absent-old-text) -> " + e) ``` Setup: `mkdir <SANDBOX>/locked && chmod 000 <SANDBOX>/locked; printf 'hello world\n' > <SANDBOX>/target.txt`. Build with elb (`--out=<builddir>` pre-seeded with `dist/elp-c-decls.h`) against the vendored release runtime `v1.0.0-20260501` (the dev/AR "latest" runtime no longer defines `engram_prune_telemetry` — same pin as #97's CI fix), manifest entry temporarily pointed at the driver. ### Before (unpatched, this branch's parent b784750) ``` WRITE_FILE(unwritable-dir) -> {"ok":true} <- lie: nothing was written EDIT_FILE(absent-old-text) -> {"ok":true} <- lie: file untouched ``` ### After (this branch) ``` WRITE_FILE(unwritable-dir) -> {"error":"write failed"} EDIT_FILE(absent-old-text) -> {"error":"old_text not found in file"} WRITE_FILE(writable) -> {"ok":true} (file content read-back-verified) EDIT_FILE(present-old-text)-> {"ok":true} (replacement verified on disk) ``` ## Regression gate `scripts/verify-soul-contract.sh` (from main) against the patched elb-built soul on a throwaway port: **GATE PASS** — PRESENCE 27/27 answered, IMMUTABILITY pass (all mutation routes tombstone/supersede, no hard-delete). ## Note on dist/ `dist/soul.c` regen is intentionally **not** included — the combined-unit assembly step is Will's, and the committed dist already lags the `.el` sources by 8 modules. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

will.anderson referenced this pull request

2026-07-22 22:00:26 +00:00

Unscoped agent write_file lands files in the soul process cwd — deny like run_command #75

tim.lingo referenced this pull request

2026-07-22 22:55:46 +00:00

Agent launch-readiness package: one regen (narrated runs + sources ledger + text-drop + WS2 + fabrication law) + 4 new engine bugs + 3 decisions #78

will.anderson reviewed 2026-07-28 03:19:57 +00:00

will.anderson left a comment

Review: honest write_file/edit_file receipts (BUG-29)

Verdict: looks good / approve. The change does exactly what the title claims and does not break the tool contract.

Correctness

fs_write returning Int (1 = all bytes written, 0 = fail) is the established contract -- safety.el:477 uses the identical let write_ok: Int = fs_write(...); if write_ok == 0 pattern cited in the PR body. Both new checks match it.
Error shape {"error":"..."} returned through json_safe(...) is consistent with the handler's pre-existing convention ({"error":"file not found"} already went through json_safe the same way at chat.el:1795). The success path still returns {"ok":true} -- contract preserved.
str_contains / str_replace are established runtime builtins (used across awareness.el, elp-input.el). The empty-old_text guard + presence check + write-result check are all correct.

Safety / consent

write_file/edit_file are the reversible risk tier in classify_tool_risk (chat.el:1672) -- they auto-run inside a chosen workspace root and rely on the run receipt as the client's undo path. That makes an honest receipt safety-critical, not cosmetic: before this fix a failed reversible write still landed {"ok":true} on the very receipt the undo path and the user's mental model depend on. #100 hardens the reversible tier. It does not alter the consent model (unscoped writes still escalate), which is the right scope. Good.

One optional follow-up (pre-existing, not introduced here)

str_replace(content, old_text, new_text) replaces all occurrences, while the new str_contains(content, old_text) guard only confirms at least one. A non-unique old_text will silently edit every match and still report ok:true. That is a different class of false receipt than BUG-29 targets, so not a blocker -- but since this PR now owns edit_file's correctness contract, consider a follow-up that requires a unique match (or returns a replacement count) so "the edit landed" cannot mean "landed in N places I did not intend."

Base-branch flag

This PR targets hotfix/elc-source-typos, not main. main is the authoritative branch now, and there is already a reconcile/hotfix-to-main-launch line in flight. As-is, this honest-receipt fix lands on a stale hotfix branch and risks being stranded there rather than reaching main. @tim.lingo / @will: please confirm the intended landing path -- either retarget to main, or guarantee this flows through the reconcile branch. Flagging, not blocking.

## Review: honest write_file/edit_file receipts (BUG-29) **Verdict: looks good / approve.** The change does exactly what the title claims and does not break the tool contract. ### Correctness - `fs_write` returning `Int` (1 = all bytes written, 0 = fail) is the established contract -- `safety.el:477` uses the identical `let write_ok: Int = fs_write(...); if write_ok == 0` pattern cited in the PR body. Both new checks match it. - Error shape `{"error":"..."}` returned through `json_safe(...)` is consistent with the handler's pre-existing convention (`{"error":"file not found"}` already went through `json_safe` the same way at chat.el:1795). The success path still returns `{"ok":true}` -- contract preserved. - `str_contains` / `str_replace` are established runtime builtins (used across awareness.el, elp-input.el). The empty-`old_text` guard + presence check + write-result check are all correct. ### Safety / consent `write_file`/`edit_file` are the **`reversible`** risk tier in `classify_tool_risk` (chat.el:1672) -- they auto-run inside a chosen workspace root and rely on the **run receipt as the client's undo path**. That makes an honest receipt safety-critical, not cosmetic: before this fix a *failed* reversible write still landed `{"ok":true}` on the very receipt the undo path and the user's mental model depend on. #100 hardens the reversible tier. It does not alter the consent model (unscoped writes still escalate), which is the right scope. Good. ### One optional follow-up (pre-existing, not introduced here) `str_replace(content, old_text, new_text)` replaces **all** occurrences, while the new `str_contains(content, old_text)` guard only confirms *at least one*. A non-unique `old_text` will silently edit every match and still report `ok:true`. That is a different class of false receipt than BUG-29 targets, so not a blocker -- but since this PR now owns edit_file's correctness contract, consider a follow-up that requires a unique match (or returns a replacement count) so "the edit landed" cannot mean "landed in N places I did not intend." ### Base-branch flag This PR targets `hotfix/elc-source-typos`, not `main`. `main` is the authoritative branch now, and there is already a `reconcile/hotfix-to-main-launch` line in flight. As-is, this honest-receipt fix lands on a stale hotfix branch and risks being stranded there rather than reaching `main`. @tim.lingo / @will: please confirm the intended landing path -- either retarget to `main`, or guarantee this flows through the reconcile branch. Flagging, not blocking.

will.anderson changed target branch from hotfix/elc-source-typos to main

2026-08-01 15:56:16 +00:00

will.anderson added 1 commit 2026-08-01 15:56:16 +00:00

fix(chat): agent write_file/edit_file no longer return false success receipts (BUG-29) 62c562a3f1

Root cause: dispatch_tool's write_file returned {"ok":true} without checking
fs_write's result, and edit_file returned ok:true even when old_text was absent
(str_replace silently no-ops) and its fs_write was also unchecked. Any failed
or no-op write fed the model a false success receipt, which it then repeated
to the user as fact.

The change (Receipt Contract rule 1 — a tool result must reflect what actually
happened):
- write_file: check fs_write's return (1 = all bytes written, 0 = fail);
  on failure return {"error":"write failed"} in the handler's existing
  error-JSON shape.
- edit_file: reject empty old_text, verify old_text is actually present
  (str_contains) before replacing, and check the fs_write result the same way.
- Verification is by operation result, NOT an fs_read read-back: fs_read arms
  the runtime's one-shot binary send length, the exact mechanism that truncated
  the safety-contact response (#96). Same honest-write pattern as that fix.

E2E evidence (sandboxed elb build, dispatch_tool driven directly):
- unpatched: write_file into a chmod-000 dir -> {"ok":true} (lie);
  edit_file with absent old_text -> {"ok":true} (lie, file untouched)
- patched:   same calls -> {"error":"write failed"} /
  {"error":"old_text not found in file"}; happy paths still ok:true
- scripts/verify-soul-contract.sh on the patched soul: GATE PASS (27/27
  presence + immutability)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

will.anderson merged commit 731efaedaf into main

2026-08-01 15:56:32 +00:00

will.anderson referenced this pull request

2026-08-03 15:40:05 +00:00

Agent launch-readiness package: one regen (narrated runs + sources ledger + text-drop + WS2 + fabrication law) + 4 new engine bugs + 3 decisions #78

will.anderson referenced this pull request

2026-08-03 19:08:07 +00:00

Agent launch-readiness package: one regen (narrated runs + sources ledger + text-drop + WS2 + fabrication law) + 4 new engine bugs + 3 decisions #78

tim.lingo referenced this pull request

2026-08-06 21:23:24 +00:00

MCP proxy blind-retries mutating calls and synthesizes replies when retries run out #118

Sign in to join this conversation.