Skip to content

Agent-loop hardening for small local models (Code mode) - #655

Open
timtoole02 wants to merge 7 commits into
codex/web-code-revivalfrom
codex/web-code-revival-agentwork
Open

Agent-loop hardening for small local models (Code mode)#655
timtoole02 wants to merge 7 commits into
codex/web-code-revivalfrom
codex/web-code-revival-agentwork

Conversation

@timtoole02

Copy link
Copy Markdown
Owner

Three commits of agent-loop hardening on top of codex/web-code-revival, sized for the
local-4B reality where every wasted round trip is a full decode pass.

What's here

fix(parse): the JSON escape repair desynced after an escaped quote, so a shell command
containing \" and \$ together stayed unparseable and the turn died on the malformed-syntax
guard. The escape pair is now consumed atomically; the live failing call is pinned as a test.

fix(checkpoint): undo now restores before forgetting (a failed restore no longer erases the
checkpoint and truncates the target), drops unrecoverable entries instead of wedging, sequences
undone_* parks, and rejects journal lines whose backup path resolves outside
.camelid/checkpoints — the journal is workspace-writable, so an absolute path smuggled into it
could read arbitrary files into /changes output.

feat(agent): small-model steering and recovery for the Code loop —

  • confined-path guidance in the system prompt + an actionable sandbox-escape error
  • one [hint: …] line on failed run_shell results for known failure classes (head+tail scan)
  • reciprocal tool-routing clauses (file tools name the shell commands they replace, and vice versa)
  • tool-name repair ladder (WriteFile/write-file/write_file_tool execute; ambiguity refuses;
    echoed/garbage names get a terse catalog-free error)
  • output-cap truncation classified before the malformed-syntax guard
  • oversized tool batches clamp and defer instead of killing the turn
  • run_shell honors Stop inside its wait loop (~50ms instead of the full shell timeout)
  • successful shell work satisfies the completion contract via a bounded fail-closed mtime scan
    (1s slack for HFS+ volumes, VCS/build dirs skipped)
  • budget exhaustion asks for one toolless final summary; terminal outcome stays honest
  • reasoning-only replies resume from their own reasoning instead of being re-asked

Verification

  • cargo test --all-targets: 2406 passed / 0 failed (66 binaries), macOS arm64
  • cargo clippy --all-targets -- -D warnings: clean (also fixes two pre-existing failures of
    this gate on the non-Windows legs)
  • Every behavior change carries a unit test; live receipts on an M4: a sandbox-denied command
    was not retried, a bulk-create request completed as one shell loop with verified files, and a
    truncated write_file received the cap correction with zero malformed strikes.

🤖 Generated with Claude Code

The in-string tracker flipped on every raw '"', so an escaped quote inside a
tool-call argument desynced it and the later invalid escapes (a shell \$ in a
run_shell command) were never doubled. The call stayed unparseable, the model
re-emitted it verbatim, and the turn died on the malformed-syntax guard.
Consume the escape pair atomically instead, and pin the live failing call as a
regression test.
- Restore BEFORE forgetting: a failed restore used to erase the checkpoint from
  both stores and leave the target truncated. Now a retryable failure re-pushes
  the entry (idempotently), while an unrecoverable one (missing/out-of-store
  backup blob) drops it with an explanatory error instead of wedging undo.
- Re-validate the backup path at use time and reject journal lines whose backup
  resolves outside .camelid/checkpoints: the journal is workspace-writable, and
  an absolute path smuggled into it read arbitrary files into /changes output.
- Sequence undone_* parks like prepare() sequences backups, so a second undo of
  the same file no longer overwrites the only copy of the intermediate state.
Tests cover the failed-restore retry, the escaping-backup rejection, and park
collisions.
Steering and recovery, sized for a local 4B where every wasted round trip is a
full decode pass:

- Confined sessions now state the path rule (workspace-relative, never absolute)
  in the system prompt; the sandbox-escape error names the correction instead of
  advertising the CLI-only --allow-fs flag.
- Failed run_shell results append one actionable [hint: ...] line for known
  failure classes (sandbox denial, missing interpreter, compile/test verdicts,
  full disk, denied network), scanning both the head and tail of the output.
- Tool descriptions route both directions: file tools name the shell commands
  they replace, run_shell defers to them and to one-loop bulk work.
- Near-miss tool names (WriteFile, write-file, write_file_tool) are normalized
  and fuzzy-repaired to the advertised name instead of burning a validation
  strike; echoed/garbage names get a terse catalog-free error so phantom calls
  are not primed. Repairs refuse ambiguity and never swap one real tool for
  another.
- A step cut at the output cap is classified as capped BEFORE the malformed-
  syntax guard, so a truncated write_file gets the split-the-work correction
  rather than a malformed strike.
- Oversized tool batches clamp to the per-step limit and defer the remainder
  with an explicit note, instead of killing the turn.
- run_shell honors the turn's cancel flag inside its wait loop: Stop lands
  within one poll instead of being ignored for the whole shell timeout.
- A successful shell command that actually changed the tree satisfies the
  completion contract: a bounded fail-closed mtime scan (1s slack for HFS+
  volumes, VCS/build dirs skipped) reports the real changed paths into the
  semantic post-change capture.
- Budget exhaustion asks for one toolless final summary so a spent turn leaves
  a partial deliverable; the terminal outcome stays StepCapped/Repeated.
- A reasoning-only reply resumes from its own reasoning (carried inside the
  correction message) instead of being re-asked or committed as the answer.
- cfg_attr allows for the two Windows-only paths that failed clippy -D warnings
  on the macOS/Linux legs.

Each behavior is pinned by a unit test; the live receipts are a sandbox-denial
turn that stopped retrying and a bulk-create turn that finished in one shell
loop with verified files.
The scrub guard rejects any /Users/<name> literal; a hint test used one as a
sample denied path, so swap it for /etc/probe (the arm matches on the error
phrase, not the path). Also apply rustfmt to the new conditionals.
Killing the direct child does not necessarily close the pipe write ends on
Unix: a pipe reports EOF only once EVERY writer has closed, and `/bin/sh -c`
may leave a descendant holding the inherited fd. Joining the reader threads
after the kill therefore waited for that orphan to exit on its own, so both the
deadline and a user Stop were silently unbounded — Linux CI measured a full 30s
for a cancel that was requested before the first poll.

Detach the readers on both abnormal-exit paths instead; their output is
discarded there, and they own only their own buffer. The regression test now
uses a backgrounded descendant that inherits the pipes, which is the shape that
reproduced on Linux while passing on macOS (where `sh -c` execs a lone command
and the fds die with it).
Both predate this PR — the branch's own CI run at e169eda failed the same two
jobs — but they block every PR that targets it, so they are fixed here rather
than left to fail alongside work that is green.

validation-scripts: the SmolLM3 qualification pins record the git blob SHA of
src/api/mod.rs as provenance for the renderer. This branch edited that file for
the web-code routes, so the blob moved and four derived pins went stale (the
runtime-envelope fixture, the parity script's RENDERER_GIT_BLOB_SHA1 and its
copy of the fixture digest, and the test's double-entry tripwires for both).
Regenerated all four. The thing the pins protect is unchanged: the renderer
fn is byte-identical to main (45 lines, sha1 7aecfc97) and the branch's diff to
src/api/mod.rs contains no smollm3 lines, so this is stale provenance, not a
behavior change.

rust (windows): windows_store_alias_failure_requires_py_launcher_probe drives a
REAL `python --version` and asserts it hits the Store alias stub, so it can only
pass on a host where that is what python resolves to. GitHub's windows runner
ships a working Python, so the assertions could never hold there and the job
failed on every run regardless of the code under test. It now self-skips when
the host does not reproduce the alias, the same way the Metal tests skip with no
device. Verified on a Windows box where the alias IS in play: the test passes
there, and `where python` shows the WindowsApps stub shadowing a real Python
3.10 — so the guard keys on the observed behavior of `python --version`, not on
whether Python is installed anywhere.
The auto `py -m py_compile` probe borrows the caller's shell timeout, so on a
loaded machine it can fail for reasons that say nothing about the file: a
timeout, a spawn failure, a missing launcher. Every such failure was recorded as
"Python syntax validation failed", which lies to the model about its own
correct source AND — because semantic_contract_findings is sticky — re-arms the
completion gate on a finding that can never be re-derived, so the turn ends
Repeated instead of Answered.

That is the flake on the Windows CI leg: the same commit (811def4) failed three
tests on one run and passed 1907/0 on another, and the three failures are
exactly this shape (two Repeated-instead-of-Answered, one missing SyntaxError
that the probe never got to produce).

Classify before blaming the file, mirroring the discrimination the alias and
traceback checks in this same file already apply. A host failure is now
disclosed as unverified rather than reported as a defect. The rule lives in a
platform-independent helper so it is testable on every host, though its only
caller is Windows-only.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant