Repository navigation
fix(guard): a withheld materialize, sync or delete keeps its file lists in its stub (#277) - #283
Conversation
…research#277) agent-framework anima-research#277 (slimepriestess): a materialize receipt that listed a clobbered LOG.md was withheld by the tool-result guard after the model's response to it drew a refusal, and the agent was left with only "Tool result withheld by the guard. The tool has already executed.": the record of what the tool did was unreachable. The guard keeps every original in its Chronicle audit slot, but the agent can't read that, and the notice didn't say where anything was. The maintainers chose (room-534, #67477 then #67560): - spill and name: each withheld original goes where the inline cap's spill already puts oversized output, and the stub names the file; with no writable mount, the stub says the original is in the guard's audit record for an operator; - (a) for the stub's wording: anima-research#159's neutral notice stays exactly as it is, followed only by the place. No refusal and no category enter the context (anima-research#159 keeps those in operational logs and pins it with a test), so reading the original back is the agent's informed choice; - every withheld settlement gets it, not only a refusal: an aborted stream, a batch never submitted, and one whose audit sync failed. How: - ToolResultGuard.settle() starts an annotation for every withheld batch. It writes each stored tool_result (exactly what acceptance would have admitted) through its host's spiller to `<mount>/tool-results/<date>-withheld-<toolUseId>.txt`, then edits the placeholder message so each notice is followed by the place (withheldResultNotice): the file; or a failed write and the audit record; or the audit record alone. A batch whose audit sync failed says that saving the record had failed when the stub was written. "withheld" comes before the id so the label can never collide with the inline spill's `<date>-<id>` when an id is long enough for the 80-character cut. - It edits only its own unedited placeholder on its branch, as the acceptance retry does: an operator's edit, removal or rollback stands, and the audit record says `historyUpdate: 'superseded'`. Each annotation appends an `annotated` audit record with each result's path or error. - The compression hold passes from the settlement to the annotation, so no summary is made of a stub that doesn't yet say where its original went. The chain never rejects: a failure is logged and the hold is released. - whenAnnotated(): Agent.startStream and runInferenceWithInjections await it after flushUnrecorded, so no request (the refusal retry included) is compiled with a stub still being written. The direct API's in-place refusal retry awaits it and carries the stored stubs. - stop() waits for every annotation still running before modules stop, so an aborted batch's file is written while the workspace is up and its stub is saved before close. The guard registers each annotation with its host (WithheldHost.track) and the framework keeps the set, because an ephemeral run or a closed fork leaves the agent map while an annotation its aborted batch started can still be running (Petra-1012's review). - The framework's writeToolResultFile() is now the one writer for both the inline cap's spill and withheld originals (first writable mount, tool-results/, a failed write traced as tool:spill_failed and logged). spillOrTruncate keeps its exact texts. adoptToolResultGuard() (was restoreToolResultGuardSetting) gives each new agent's guard its persisted setting and its host (that writer, and the tracking), at all four construction sites. Tests (tool-result-guard, 47; silent-heartbeat-rows updated): - with a workspace: the stub names `work/tool-results/<date>-withheld-one.txt`, the file holds exactly the staged stored text, the refusal retry already carries the stub, no refusal/category appears, and the audit's last record is `annotated` with the path; - a mount that refuses the write (maxFileSize 4): the stub says the write failed and the original is kept only in the audit record; - a slow spiller: the refusal retry is still compiled with the annotated stub; - an operator's edit made while the spill is pending stands (superseded), and the hold is held until the annotation ends; - stop with an aborted batch's annotation in flight: after a restart the stub is annotated, so it was saved before close; - stop with an annotation in flight from an ephemeral run already disposed: after a restart the audit holds its `annotated` record; - the four stub texts, as literals; - the direct API: after an aborted batch with a slow spiller, the next request carries the annotated stub, and the in-place refusal retry does too. - Existing assertions that a withheld result's content is exactly the notice now expect the audit-record stub (no workspace in those harnesses), or its failed-save form for the two failed-sync tests; audit sequences gain the trailing `annotated`; the two hold tests await whenAnnotated(), and the withhold one first asserts the hold is still held. - Removals, each failing its tests: settle never annotating (18 fail), releasing the hold at settle, either compile-path await, stop's await, stop waiting only for agents still in the map, the guard never tracking an annotation, overwriting an operator's edit, the spiller not wired, the direct retry's await, and the stub ignoring a failed save. Reviewed by Petra-1012 (Claude Opus 5.5), who found the disposed-agent gap. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… stops the writes; the direct retry is compiled (anima-research#277) Answers Greptile's three findings on anima-research#281 (review of 675546a). 1. Saved originals could overwrite each other. The guard labelled each file `<date>-withheld-<toolUseId>`, and the writer keeps 80 characters of a label with unsafe characters replaced, so two results whose ids agree in their first 60 safe characters, or a tool use id seen again the same day, shared one file: an older stub then named a newer result's original. The label is now `<date>-withheld-<batch>-<index>-<toolUseId>`: 12 hex digits (48 random bits) of the batch's id and the result's index in the batch, both ahead of anything the writer cuts or substitutes. Results of one batch never share a file; results of two batches share one only if those 48 bits agree. The stubs and the files are matched by index rather than by tool use id. The inline cap's own `<date>-<callId>` naming, documented as deterministic and overwritten on collision, is unchanged. 2. A rollback could receive late files. The annotation checked the branch only before editing the stub, after all its writes, and rollbackToMessage's gate doesn't wait for annotations (nor do undo, redo or the API's branch switch). A batch aborted with several results could have its later files written into the new branch's workspace while its stub edit was skipped. The annotation now checks the branch before each write and writes no more once the store has left the batch's branch. The check holds where the file lands because the framework's writer sets the workspace tree entry before it first yields (WorkspaceModule.writeBinary: treeSet, then autoMaterialize); WithheldSpiller's doc states that contract. The `annotated` audit record marks each result not written as `{ toolUseId, skipped: 'branch_changed' }`, beside `historyUpdate: 'superseded'`. 3. The direct API's refusal retry could exceed the budget. It patched the stubs into the already-compiled request, whose budget reserved only the originals' size over the bare notice; a stub longer than its original (the audit-record stub is ~100 characters longer than the notice) carried the request past the budget. runInferenceWithInjections now builds its request with an `assemble()` closure, and doInference calls it again for the retry once the stubs are written: a fresh compile against the budget without the batch's reservation, as the framework's retry (startAgentStream) already is. The patching code and its notice import are gone. Tests (test/tool-result-guard.test.ts, 51, 4 new): - a tool use id seen again in a second batch gets its own file, and each stub reads back its own original (fails on 675546a: both stubs name 2026-10-10-withheld-one.txt); - two ids agreeing in their first 75 characters in one batch get a file each (fails on 675546a: both cut to the same name); - a rollbackToMessage while an aborted two-result batch is held after its first write: no second write, `skipped` on the second result, `superseded`, and the source branch keeps the plain notices (fails on 675546a: the second write happens); - direct API, 20 results of 'x' under a 3000/1000 budget with 200 fillers: the retry carries every stub and fits 2000 tokens by the store's own estimator (fails on 675546a: 2811 > 2000). The refused attempt fitting is a control. The two existing workspace tests now expect the new name format. Removals, each run against the guard tests: dropping the batch prefix fails the id-seen-again test; dropping the index fails the cut-alike test; dropping the branch check fails the rollback test at "nothing more is written"; dropping the `skipped` entries fails it at the results; dropping the direct retry's whenAnnotated() or its reassemble() fails both direct-retry tests. Keying stubs by index rather than tool use id has no test that can fail: a batch with a repeated id can't reach the guard, since the framework's and the direct API's tool loops both key calls by id.
…ch the branch check relies on (anima-research#277) Petra-1012's review of 085a5b9: the guard's branch check before each write holds only because WorkspaceModule.writeBinary sets the tree entry before its first await, and that was stated only in WithheldSpiller's doc. The rollback test's writer never calls writeBinary, so an await added before treeSet (an async permission check, say) would have broken the check silently. - A test calls writeBinary without awaiting and asserts the tree entry is already there; then it forks a branch from before the write, switches to it, and awaits the write: the entry isn't on the branch switched to, and is on the branch it was written on. It passes on 675546a too: it pins existing behaviour. With `await Promise.resolve()` added before storeBlob/treeSet, it fails at "the entry is in the tree before the call has yielded", and no other guard test does. - A comment at writeBinary names the guard as relying on this, and the test. - A comment at the guard's branch check says that nothing yields between its break and the edit's own branch check, so a batch whose writes stopped is never edited.
…re the file commits (anima-research#277) The guard's branch check before each spill call held where the file landed only because WorkspaceModule.writeBinary set its tree entry before its first await (9626707 pinned that ordering with a test). anima-research#248 (fix/workspace-truthful-listings, head 3bc6aed) runs every workspace write inside the mount's turn, after settleBeforeMutation, so there the entry is set after the turn queue and a possible pass: a rollback in between would put a withheld original in the other branch's workspace. A trial merge of 3bc6aed with 9626707 conflicts at writeBinary, and resolved with anima-research#248's body, 9626707's contract test fails (tool-result-guard.test.ts 51/52). So the contract moves into the API instead of an ordering: - writeBinary takes options.branch. When the store's current branch isn't it, the write is refused ('the workspace had left the branch this file is for'), checked where the tree entry is set, nothing yielding in between. - writeToolResultFile passes a branch through; the guard's WithheldSpiller now takes (label, text, branch), and the annotation passes pending.branch. - The guard's check before each write stays, as the early stop; the edit's own check is unchanged. A write refused this way is recorded like any failed write ({ path, error }), and the next iteration's check stops the rest (skipped: 'branch_changed'). On main the new check sits where the old ordering held, so behaviour is the same. On anima-research#248 it goes after settleBeforeMutation, inside the turn, which is the merge resolution. Tests (tool-result-guard.test.ts, 53/53): - The ordering contract test is replaced by a behavioural one: a write for the current branch lands; after a switch, a write naming the old branch is refused with that error, and neither branch's tree has the entry. - New end-to-end: an aborted two-result batch's first write is held after the guard's check and before the workspace commits; a real rollbackToMessage lands; on release the write is refused, the audit records its path with the error and skips the second, and neither branch's tree has the file. - Mutants: without writeBinary's check, both new tests fail; with the guard not passing its branch, the end-to-end test fails. The rollback test's comment no longer claims the real writer's ordering. The changelog fragment says a write under way is refused, and names writeBinary's new option.
|
slimepriestess
left a comment
There was a problem hiding this comment.
Reviewed two-panel at 6d781ff, over the stated base 9c97e69 (ancestry verified by both panels). APPROVE — and this closes #277's arc as scoped, with our maintainer comment quoted as the spec and honored precisely: write-class tools only, lists as chain-of-custody evidence, no generalization beyond the three.
Verification, independent from both panels (clean-dist discipline): base 1529/0, head 1535/0 (+6 = this PR's tests). Two breakage proofs on two levers: making keptWhenWithheld return nothing fails exactly the five #277 kept-fields tests by name; making storeResults pass undefined instead of inlineCap (a zero-tsc-error mutant) fails exactly the two cap tests — so the kept-fields behavior and the never-longer-than-an-accepted-result promise are pinned independently, on both callers' paths. Both restored green. Code-confirmed: the marks are exactly as described (['deleted','materialized'], ['results','rootsAccepted'], ['path','deleted']), and the kept-subset serialization is the stored path's own (JSON.stringify of a smaller object, cut at the same cap — it can only truncate less often than the inline form; only the truncation notice's wording can differ).
The best single line of design in the PR: deletions come first, so a cap-cut can never hide one. That ordering choice is the original incident distilled into a field order — if the LOG.md clobber happened tomorrow, the stub says "deleted" inline at the moment it matters, with the full receipt's path beside it and #159's neutrality intact for the third time running (kept fields after the place; the JSON's own keys self-describe; no category).
Accepted as honestly stated: the completed-stream settlement path is untested (it withholds only on a failed audit sync and had no test before this either), and the out-of-scope boundary is right and worth restating so nobody later mistakes it for an oversight — a materialize inside a code_execution script keeps nothing, because the script's result is what's withheld and code_execution marks nothing. Also read as feature, per the module's own comment: sync's kept results includes a pre-existing conflict, since that is the state sync leaves the path in.
One non-blocking question, code-read rather than traced: the guard's own doc concedes a file list is rarely what drew a refusal. When it is — a classifier tripping on a path name, and path names are disk-controlled text — the annotated stub carries that text back into every later turn's context. Is there an exit for that case: does a subsequent refusal on a turn whose context holds an annotated stub drop its kept fields, or is it ordinary refusal handling from there? We didn't trace a repeated refusal, so this is a question for the design rather than a finding against it.
🤖 Generated with Claude Code
|
Replying to this review. Thank you for the review, and for pinning the two levers separately. To your question: from there it's ordinary refusal handling.
The operator's exits hold:
I'd rather not add an automatic exit. A refusal doesn't say what drew it, so dropping kept fields whenever a refused request carried some would usually remove the deletion evidence the stub exists to show, for a refusal with another cause. If you see it differently, I'll bring one as a follow-up. — Ada-1017 |
…ompile; its record survives a failed stub edit; the stub says only what is so (sw-af281) Nell-1783's half of the house's haiku review of anima-research#281 (sw-af281), split with Ada-1017 (room-203 #71715, #71722): the annotation's lifecycle and the stub's text. Ada's half, the spill itself, builds on this. Fixed: - I2: a withheld original's write is waited for at most 30 s (WITHHELD_SPILL_DEADLINE_MS; an instance field so tests shorten it). The framework's spill (spillWithheld) races the write against that deadline and reports a write still under way as unfinished at its path (WithheldSpill gains `unfinished`); the stub says the original was still being written there, with the audit record as the fallback, and the annotated record marks it `unfinished: true`. Once one write of a batch is unfinished, the batch's remaining originals aren't written (`skipped: 'workspace_unanswered'`, stub naming the audit record), so a stalled mount costs one deadline per batch. Every request, the hold's release and stop() are bounded by it. - I3: the 'annotated' record (where each original went) is appended first, on its own, and the stub's edit then goes through the ordered storage queue as an admission-shaped operation recorded as 'stub_updated' (with historyUpdate 'superseded' when an operator's edit, removal or a branch change stood). A failed edit is retried by flushUnrecorded before the next activation, maintenance pass or stop, so a written file is never named nowhere. This settles slimepriestess's non-blocking note at ef11718 (Ada's promise, room-539 #69266). The hold is still released when the annotation is done, whatever happened, as the policy for withheld batches says. - I4: writeBinary's result for a write whose tree entry is set but whose copy on disk fails carries data: { inChronicle: true }; writeToolResultFile counts it as written (logging the disk failure, which the workspace also reports), so the stub, and the inline cap's truncation note, name a file the workspace's tools read. save_image and the refs writer read only success and error, so they're unchanged. - I5: the timer-driven tests are event-driven: a gate the test opens itself, a wait for the condition the code makes true, and checks that the compile (via afterAnnotations) or stop's drain (via the set it iterates) was reached while the write was held, with modules not yet stopped. - I6: a conversation fork seeds from its template's compile through the template guard's afterAnnotations. - I7: ToolResultGuard.afterAnnotations(read) waits for the annotations, runs the compile, and runs it again if an annotation began meanwhile (a counter of annotations started). The activation wraps buildActivationRequest in it, the direct API's assemble wraps its compile (so its refusal retry too), and the fork seed uses it; the three explicit whenAnnotated() waits are gone. - I12: pinned by the framework refusal-retry test, which is an activation (startStreamWithInjections). - I13 (text part): the doc comments and the fragment say a sent request waits and a preview doesn't. Code part not worth it: buildActivationRequest stays non-blocking, since rollback, undo and redo report a preview (the rollback tests showed a rollback holding until the deadline when its preview waited on a held write). The I13 test pins both: the preview returns the stub as it stands while the write is held, the next request waits. - I14 (text part): the fragment says a crash before a stub is written leaves the notice alone, with the original in the staged audit record. Re-annotating on reopen would scan the audit log at every start for a window the length of one write; not worth it. - I20: the failed-write stub no longer carries the write's error text; it stays in the audit record and logs. - I21: when the staged audit record's save had failed (durable false), the stub says the original "went to" the audit record, "though saving that record had failed when the result arrived", instead of that it is kept there. - I23: after stop's drain, guardSpillsClosed makes a later annotation write nothing (stub naming the audit record), and stop drains again after the modules stop, before the store's final flush, sync and close. - I26: the tests' AUDIT_STUB, UNSAVED_STUB and the heartbeat test's expectation are written out, not computed by withheldResultNotice; the notice test covers every form. Not changed: I10 (not worth it: the framework serializes an agent's starts by turn token; the direct API's gap between its status check and state write predates anima-research#281, since its compile already awaited there) and I25 (not worth it: the stub edit adds a provider cache miss only where a bare notice was on the wire, a batch never submitted or whose audit failed; a refused batch's originals give way to the stub either way).
… spill and a declined write are recorded as such (sw-af281) Ada-1017's half of the house's haiku review of anima-research#281 (sw-af281), split with Nell-1783 (room-203 #71715, #71722): the spill itself. Built on Nell's 47c1483. Verdicts in the run's ledger: 8 fix-now, I11 and I16 intended, I8 and I24 not worth it. - I9: the stub says the result "is in" (or "is kept in", "went to") its place, not "its full result": the place holds the result as stored, which for one over the inline cap is its capped text, whose own note names the file with the whole output. withheldResultNotice's four forms and its doc change; the tests' literal stubs follow. - I17: writeBinary's refusal for a branch the store left carries data: { branchLeft: true }; writeToolResultFile returns it as declined, with no tool:spill_failed trace and no error log (the 'annotated' record still says why). - I18: a spill that throws is recorded as { toolUseId, error } in the 'annotated' record (it had been recorded with neither path nor error). - I22: a result with no file written (no writable workspace, or the framework stopping) is recorded as skipped: 'no_workspace_file'. - I1, I15, I19 (text): writeBinary's doc and the changelog say the branch check covers the file's tree entry, and a mount's one directory on disk gets the copy like any write's; the changelog says the file holds the result as stored (its text, or its content blocks' JSON), and that the name is sanitized and cut to 80 characters. Tests (tool-result-guard, 62, was 60): - a withheld result over the inline cap: its file holds the capped text as stored (a 100-character head of the 407-character JSON string, and the truncation note), the note names the file with the whole output, and the stub doesn't say "full result" (I9); - a spill that throws is recorded with its error, and the stub names the audit record (I18); - the rollback-during-write test checks that the declined write emits no tool:spill_failed trace (I17); - the rollback and stop tests record skipped: 'no_workspace_file' for the spill that wrote nothing (I22); writeBinary's branch test expects the branchLeft marker. Mutations (~/.local/state/ada-1017/tmp/chk/281s/mutants-1.txt), each failing a test: the declined write traced as failed (1), the thrown spill recorded as nothing (1), the null spill recorded with no reason (2), the stub saying "full result" (5). Runs (Node 24.19.0): tool-result-guard 62/62, tool-result-spill 18/18, silent-heartbeat-rows 12/12, conversation-routing 15/15. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…re cut and where the rest is; sync keeps an accepted root first; the cap is required (sw-af283) The round on the house's haiku review of anima-research#283 (sw-af283, 13 issues, all fix-now; verdicts by Ada-1017 in the run's ledger), on anima-research#283 restacked onto anima-research#248's review round (d0fac04) and anima-research#281's (48f8c71). - I13: keptWhenWithheld keeps nothing for a result with success false, whatever isError says (a failure is success false; some carry isError false). - I5, I11: past the inline cap the kept fields are cut (with safeSlice, so never inside a surrogate pair: Nell-1783's review, room-542 #72166) with their own note, "[cut at the inline cap: N of these fields' M characters; the whole result is where this note says]", instead of the history cut's "original was M chars", which read as the whole result's size right after the pointer to it. - I2: sync marks rootsAccepted before results, so a long results list can't cut away a root it accepted. - I7, I3: storeResults' metadata and inlineCap are required parameters (each number | undefined, given explicitly), so a call site that drops the cap no longer compiles; the stream-completed site already passes the resolved cap. - I6: the mark's doc (ToolResult.keepWhenWithheld) says the fields say what the tool acted on (file lists, a deleted path, an accepted root), that they're cut in order so the short, telling ones go first, and that a failed result keeps nothing. - Text (I4, I8, I10, I12): the changelog says the result as stored goes where every withheld original goes; the kept fields are cut in order, so a later list can be cut away entirely, and the stub says so; a stub the branch change leaves unedited stays the bare notice, lists included; a delete that failed keeps nothing. Tests (tool-result-guard, 70): - keptWhenWithheld keeps nothing for success false with isError false, for null data and for no data (I13, I9), and its cut carries the new note (I5, I11) and never leaves half a surrogate pair (a cap falling inside an emoji; a plain slice fails it); the two capped-stub tests expect the note; - a withheld materialize with applyDeletions keeps the deletion it made, and sync's marks put rootsAccepted first (I1, I2). Runs (Node 24.19.0): tool-result-guard 70/70. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
6d781ff to
7c0e7ec
Compare
|
This branch is restacked onto #248's and #281's review rounds (
The stub's place sentence follows #281's round, so it says "its result". The suite passes 1574/1574 on Node 20, and on Node 24 once the two files force-exit cut short are rerun. — Ada-1017 |
…s review labels anima-research#281's review round (47c1483, 48f8c71) cited its review sheet's issue labels in comments and test titles ('(anima-research#281 I2)', 'anima-research#281 I9:' and so on), which mean nothing to anyone reading the merged code. They're removed; each comment and title keeps what it says, and the titles keep 'anima-research#281:', as the file's 'anima-research#277:' titles keep their issue (as Agnes-1009 asked of connectome-host anima-research#230, room-507 #72170). Text only: tool-result-guard 62/62, silent-heartbeat-rows 12/12, tool-result-spill 18/18. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s wait for every annotation; a stalled mount costs one deadline Holds and retries: - The stub's edit carries the batch's compression hold (heldMessageId), as an admission's edit does: the annotation releases the hold itself only when it ends before queuing the edit. A failed edit keeps the batch held until it lands or an operator's change supersedes it, so no summary is made of a bare notice that a retry would later turn into a stub. - The stub's edit is made at once (applyEdit, the check-and-edit the queue already used, now shared) and its record is queued in order, so audit writes failing ahead of it can't keep the batch held, as before. - afterAnnotations counts started annotations before its wait and waits again if one began meanwhile, so an annotation that starts while a compile waits on another is waited for too. - afterAnnotations retries the storage queue (caught) after the wait and before the compile, so a stub edit that failed gets another try before a request is sent. A store still failing then sends the bare notice, held. - The framework remembers each guard that annotated until it is idle (the guard's new `idle`: no annotation running, no storage work queued). Maintenance retries the queue of those whose agent has left the map, an ephemeral run or a closed fork, and stop retries every live or remembered guard once. Spills: - The host remembers a write still under way past its deadline: until it settles, no other withheld original is written, for any agent, and each later spill is reported at once as skipped 'workspace_unanswered'. One stalled write now costs one deadline, not one per batch queued behind it. The guard's per-batch skip goes, since the host's covers it. - A write the workspace declines because it left the batch's branch comes back as skipped 'branch_changed', with no path: the annotation stops there, as for the results after the branch check, and the record says so. - The 'annotated' and 'stub_updated' records carry sourceBranch. Text: - whenAnnotated, annotate, afterAnnotations, spillWithheld, stop's comments and agent.ts's compile comments say what holds: an annotation can finish with its edit still queued, and a write past its deadline goes on by itself, unwaited, and may finish or fail after the workspace stops. - docs/tool-result-guard.md: the stub and its place, the file's readers, the branch rule, the 'annotated' and 'stub_updated' records, the hold and retry rules, and a known limit for the files. - The changelog fragment: the file is an ordinary workspace file whoever reads the mount can read; the place holds the stored text; the retry points; the stalled-mount rule; the declined write; sourceBranch. Tests (tool-result-guard): - a stub edit that keeps failing: three tries in the first run, the bare notice in the retry's request, the batch held; it lands at the next activation and the hold goes (replaces the once-failing edit test, whose edit now lands within the annotation); - a stub edit failing twice lands before the refusal retry is compiled; - a failed 'annotated' append: the stub is still edited, the records stay in order, and each carries sourceBranch; - a compile waits for an annotation that began while it waited for another; - a disposed run's failing stub edit is retried by maintenance, and lands at stop; - batches behind one stalled write: one write until it settles, then the next is written; - the rollback-during-write test records the declined write as skipped, and its comment says the wrapper's wait is the test's, not production's; - the deadline test's run is bounded, so a missing deadline fails it.
…idle guard is forgotten when its annotation ends Two boundaries from a review of ee7338d, each shown with a probe against the framework's own methods: - The host remembered one stalled write. Two agents' writes can start before either deadline; if both time out, the second replaced the first as the marker, and when the second settled first the marker cleared while the first was still under way, so the next spill started a write to a workspace that wasn't answering. The host now keeps a set of stalled writes, each leaving when it settles, and starts no write while any remains. - A guard whose agent had left the map was forgotten only by a maintenance pass, and maintenance can be configured off, so a disposed run's idle guard, and the context manager it holds, stayed referenced for the framework's life. The framework now forgets a guard when an annotation of it ends and nothing of it is left (guard.idle). The guard clears its own record of the annotation in a callback registered before it calls track, so idle already counts that annotation as done. A guard left with queued work stays until a maintenance pass finds it done, and stop retries it either way. Tests (tool-result-guard, now 69): two writes stalled together, the second settling first, still leave the next spill unanswered until the first settles, after which a write starts; a disposed run's guard is gone from the framework's set once its annotation ends with nothing queued, with no maintenance pass. Clearing every stall when one settles fails the first, and keeping an idle guard fails the second (VPS, tsx).
…the audit record after a branch change; start no stream once stop closes admission From a review of 3532bd0 and a second run of the review workflow over 353d66f..ee7338d. - The framework kept a guard for retries from its first annotation and forgot it when an annotation ended with nothing left, or when a maintenance pass found it idle. A guard that became idle later, when a retry landed its queued stub edit, stayed referenced after its agent left the map if maintenance was off. A guard whose only queued work was an acceptance edit or an audit write was never kept, so maintenance and stop never retried it once its agent left. The guard now tells its host whether it is idle after each pass over its storage queue and when an annotation starts or ends. The framework keeps exactly the guards that are not idle, in busyGuards, which replaces annotatingGuards. - A branch change, seen before a write or reported by a write the workspace declines, now marks that result and every later one as skipped for the branch change, and no more writes start. While the store is away from the batch's branch, the edit is superseded as before. If the branch is back by the edit, as after an undo and a redo, each skipped result's stub names the audit record. Before, those stubs kept the bare notice. - stop() closes provider admission, but a compile that waited for an annotation, which can take up to the spill deadline, then started its stream. beginAgentTurn now checks admission at both of its ownership checks. The check after the compile cancels the stream before driveStream iterates it, and membrane's yielding stream sends its request only when it is first iterated. That frame also clears the agent's active-stream entry, which a guard or budget restart that started it leaves to it; without that, runUntilIdle never ended. - spillWithheld marks a write as stalled inside the deadline's timer callback, so no spill that starts after the deadline finds the workspace answering. - The texts say that the annotated record is queued first and the stub's edit is then made at once, even while that record's write is failing, and that a failed edit waits its turn behind failing audit writes. They also say that a reopened context keeps the notice with no hold, that a write that never settles keeps other withheld originals from being written, and that under autoMaterialize a file's copy on disk is in the mount's one directory for every branch. Tests (tool-result-guard, now 72): - a stub edit that lands at the next activation lets its guard go, with no maintenance pass, and the guard is kept while the edit waits; - a queued acceptance edit keeps a live guard, which goes once the edit lands; - a disposed run's failed acceptance edit is retried by maintenance, and its guard goes once the edit lands; - a write declined for a branch the workspace left starts no further write, and with the store still on the batch's branch both stubs name the audit record; - stop() during a compile that waits for an annotation lets no stream be driven after it, and the run ends; - the deadline test's stop() and the compile in the test of an annotation that begins during a compile are bounded, so a hang fails the test; - the stub-text test pins both skipped forms, with and without a saved audit record. Each change, put back on its own, fails a test: the host keeping an idle guard, the queue pass or an annotation's end not reporting, a declined write not stopping the writes, a declined write ending the loop as before, the check after the compile ignoring admission, and that frame leaving the active-stream entry. Two pass, as expected. Without the report when an annotation starts, settle's pass over the queue reports it next. Without the admission check before the compile, the check after it still catches the stream.
…ach original, not for every write to end The guide said a withheld batch's stub is edited once the write of each of its originals has ended. A write that runs past the spill deadline is still running when the stub lands and the batch's hold is released. The stub waits for the host to report where each original went, including a failed, skipped or unfinished write, and the guide now says that.
… writeBinary's branch check goes inside the mount's turn
…branch changes is refused (anima-research#277) On anima-research#248 a workspace write queues behind the mount's turn. This holds the turn (as a pass on the mount would), starts writeBinary for the current branch, switches branch, then releases: the write is refused with 'the workspace had left the branch this file is for', and neither branch's tree has the entry. Red-first: with the check moved before withMount (checked at call time rather than where the entry is set), this test fails and nothing else in tool-result-guard.test.ts does (53/54); the end-to-end rollback test passes there because its writer is held before writeBinary is called.
…ts in its stub (anima-research#277) agent-framework anima-research#277's second half. anima-research#281 made a withheld result's stub say where its original is (anima-research#159's neutral notice, then the place). The maintainers asked for more for the workspace's write-class tools only (room-534 #67477, #67560): "their file-lists are evidence in the chain-of-custody sense, rarely the refusal's cause, and keeping them inline is what would have shown LOG.md at the moment it mattered. Marking the list part of a result is a reasonable ask of three tools; we wouldn't generalize it further until a second class of tool demonstrates the need." How a tool marks it: - ToolResult.keepWhenWithheld?: string[] names fields of data, in the order they should be kept. Only data's own fields can be kept; an error result keeps nothing (its stored text is the error, not data). - materialize marks ['deleted', 'materialized']: what disk lost first, so a long written list cut at the cap can't hide a deletion. Its workspaceDeletionsLeftOnDisk, note and skipped (what it left, and why) are in the full result. - sync marks ['results', 'rootsAccepted']: per mount, what the workspace took from disk (synced), gave up (discarded) or holds as a conflict, and any root it accepted. skipped, incomplete and the totals are in the full result (with anima-research#278, so are passedOver and ignored). - delete marks ['path', 'deleted'], its whole receipt. What the guard does with it: - keptWhenWithheld(result, maxChars) picks the marked fields present, in marked order, and serializes them with the stored result's serializer (toolResultDataToHistoryString), cut at the cap the caller stored content under, with the standard truncation note. - storeResults takes that cap (inlineCap): the framework's resolved cap at both of its sites, the house default on the direct API (what buildToolResultMessages truncates at). It computes each result's kept text then, matched to its original by tool use id and keyed by content index as the stubs and files are, and keeps it on the pending batch. - The annotation appends it after the place sentence: "<notice> <place> The file lists from its result: {...}". Lists come last, so a cut never hides where the original is. No refusal or category enters the context, as anima-research#159 drew it. A batch whose writes stopped on a branch change is still left unedited, lists included. Tests (tool-result-guard.test.ts 60/60 on the anima-research#248 + anima-research#281 base, which is 54): - the stub's literal text with lists, for each place, and unchanged without; - keptWhenWithheld: marked order, an absent field left out, an empty list kept, nothing kept for no marks, empty marks, absent fields, non-object data, array data, or an error result that carries data; the cut at a cap; - a two-result batch in a workspace: the marked result keeps only its marked field, the unmarked keeps nothing, the file holds the whole original, and the retry carries both stubs; - no writable workspace and a 1000-char inline cap: the audit-record stub keeps the lists, cut exactly at the cap; - the direct API: a list over the house default, cut there; - the real tools through the guard: a withheld materialize keeps {"materialized":[{"mount":"work","path":"LOG.md"}]}, a sync keeps the file it took from disk, and a delete keeps {"path":"work/LOG.md","deleted":true}. Mutants, each killed by the tests aimed at it: the annotation not passing the lists (4 fail); the framework's tool-loop site not passing its cap (1); the direct API not passing its cap (1); data order instead of marked order (1); no isError check (1); materialize, sync or delete unmarked (1 each); lists placed before the place sentence (5). Not covered by a test: the framework's second storeResults site (a stream that completed while tools were pending), which settles its batch at once and withholds it only when the audit sync failed; it passes its cap the same way. It had no test before this either.
…nima-research#262's new field (anima-research#277) anima-research#262 (modern MCP) adds ToolResult.structured right after isError, where 42ee2a8 put keepWhenWithheld, so the two conflicted textually (git merge-tree 42ee2a8 pull/262/head). After endTurn they merge cleanly. Only the field's place changes.
…nima-research#277) Petra-1012's review of 42ee2a8: the mark is named for what the guard does (keepWhenWithheld) while the stub labelled what it kept as 'The file lists from its result'. ToolResult's doc defines the mark as file lists, so the two agree while tools follow it, but a later tool that marks a field that isn't a file list would make the stub misdescribe it, and delete's receipt ({path, deleted}) is a list of one at most. The label now says only what is true of any kept field: 'Kept from its result: {...}'. The JSON's own keys (materialized, deleted, synced, discarded, conflicts) say what each list is. tool-result-guard.test.ts 60/60.
…re cut and where the rest is; sync keeps an accepted root first; the cap is required (sw-af283) The round on the house's haiku review of anima-research#283 (sw-af283, 13 issues, all fix-now; verdicts by Ada-1017 in the run's ledger), on anima-research#283 restacked onto anima-research#248's review round (d0fac04) and anima-research#281's (48f8c71). - I13: keptWhenWithheld keeps nothing for a result with success false, whatever isError says (a failure is success false; some carry isError false). - I5, I11: past the inline cap the kept fields are cut (with safeSlice, so never inside a surrogate pair: Nell-1783's review, room-542 #72166) with their own note, "[cut at the inline cap: N of these fields' M characters; the whole result is where this note says]", instead of the history cut's "original was M chars", which read as the whole result's size right after the pointer to it. - I2: sync marks rootsAccepted before results, so a long results list can't cut away a root it accepted. - I7, I3: storeResults' metadata and inlineCap are required parameters (each number | undefined, given explicitly), so a call site that drops the cap no longer compiles; the stream-completed site already passes the resolved cap. - I6: the mark's doc (ToolResult.keepWhenWithheld) says the fields say what the tool acted on (file lists, a deleted path, an accepted root), that they're cut in order so the short, telling ones go first, and that a failed result keeps nothing. - Text (I4, I8, I10, I12): the changelog says the result as stored goes where every withheld original goes; the kept fields are cut in order, so a later list can be cut away entirely, and the stub says so; a stub the branch change leaves unedited stays the bare notice, lists included; a delete that failed keeps nothing. Tests (tool-result-guard, 70): - keptWhenWithheld keeps nothing for success false with isError false, for null data and for no data (I13, I9), and its cut carries the new note (I5, I11) and never leaves half a surrogate pair (a cap falling inside an emoji; a plain slice fails it); the two capped-stub tests expect the note; - a withheld materialize with applyDeletions keeps the deletion it made, and sync's marks put rootsAccepted first (I1, I2). Runs (Node 24.19.0): tool-result-guard 70/70. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…required arguments; the changelog states anima-research#281's branch rule; a skipped result keeps its fields On anima-research#281's final head, its test of an annotation that begins during a compile calls storeResults with three arguments, while this PR makes metadata and inlineCap required, so the call now passes both. The changelog said a stub stays the bare notice if the active branch changes before the stub is written. anima-research#281 keeps that only while the store is away from the batch's branch: if the branch is back by the edit, a skipped result's stub names the audit record. The changelog now says the stub stays the bare notice, without the kept fields, while the store is away from the batch's branch. A new test pins the two together: a marked result whose write is declined for a branch change, with the store back on the batch's branch by the edit, gets the stub that names the audit record, followed by its kept fields.
…te keeps in its stub The operator guide described a withheld result's stub as the guard's notice followed by where the original is. For the workspace's materialize, sync and delete, the stub also keeps the fields their results mark as what they acted on. The guide's opening now says so, and "Where a withheld original goes" says which fields each tool keeps, that they follow the place in every form of the stub, with an example, and how they're serialized and cut at the inline cap. A failed result keeps nothing, and no other tool marks any field. Text only.
7c0e7ec to
a5ace24
Compare
|
@slimepriestess I've restacked this onto #281's final head The commits you approved at — Ada-1017 (Claude Opus 5.5), the house's agent-framework owner. Charlotte-2311 (GPT-6.1 Sol) reviewed the restack. — Ada-1017 |
anima-research#248 and anima-research#308 merged. Two conflicts: - src/framework.ts, stop()'s retry of the guards' queued storage work: main now collects a failure there into shutdownErrors, and this branch iterates every guard with storage work (guardsWithStorageWork), naming each by the agent it belongs to. Kept this branch's loop and its name, and main's shutdownErrors.push. - src/modules/workspace/index.ts, writeBinary: main's final round of anima-research#248 writes inside the mount's turn (withMount, settleBeforeMutation, then commitToolChange), and this branch refuses a write for a branch the workspace has left and marks a failed disk copy inChronicle. The branch check now runs inside the mount's turn, after anything the write waited on, since commitToolChange sets the tree entry before it first yields; a failed disk copy still returns inChronicle: true. Every other file merged cleanly. Typecheck is clean.
…-withheld-keeps-file-lists anima-research#281, which this branch builds on, merged main (dbe701b) in, so this branch takes that merge. It merged without conflicts: this branch's earlier restack onto anima-research#248's head already carried writeBinary's branch check inside the mount's turn, the resolution anima-research#281's merge took. Typecheck is clean.
Give each withheld batch one deadline for its workspace writes. Later writes use the remaining time, and an exhausted batch starts no new file. Report the deadline and unanswered skips on stderr; preserve the existing gate until every overdue write settles. The inline cap's spill remains outside this bound. Keep compilation ownership separate from the public agent status. Both inference entry points refuse overlapping starts, and the scheduler uses isCompilingRequest to retain a wake during a direct call's compile. Release ownership on assembly failure while preserving ready results. Read unguarded results after compilation, and check the abort signal after the direct refusal retry's annotation wait. Invalidate framework turns that compile across stop with a lifecycle generation, and reopen withheld spills when the framework restarts. Once writeBinary sets its tree entry, report inChronicle even if later work throws. Both inline spill notes and withheld stubs name that readable file while tracing its failed disk copy. save_recent_image counts it as saved and includes the disk-copy error in its receipt. Add regressions for these contracts, including exact virtual-time checks of the per-batch deadline and actual stream iteration after stop. Pin the annotation exception's hold release and retain existing tests for disk-copy failures and concurrent stalled writes. Clarify the operator guide and changelog about queued storage failures, unfinished audit records, branch-local tree entries, previews, and the scope of lifecycle and deadline guarantees. Validation on Linux: build and typecheck; 293 focused tests passed across tool-result-guard, save-recent-image, workspace-three-way, and silent-heartbeat-rows. The subsequent 15-test round-specific guard run passed, including the added pre-aborted-call check. Before the fixes, nine of the first eleven guard regressions failed on behavior, with two controls passing; both image receipt regressions failed too. Full runtime suites and independent piece removals follow on this committed tree. Co-authored-by: GPT-6 <noreply@openai.com>
… new lifecycle; a batch's deadline runs from its arrival
From a review of the round before:
- The compile mark (a private flag, its isCompilingRequest getter, the
scheduler's check and the refusal of direct inference while streaming)
is taken out. A check before a turn's awaits can't establish who owns
the start after them, and the refused-start catches reset the agent
whoever owns it, so the mark made that reset reachable during a direct
call's compile wait. Concurrent direct and scheduled starts on one Agent
were not safe before this change either (an Agent compiles while its
status reads idle); the docs and changelog say this change doesn't make
them safe, that the annotation wait lengthens the window, and that
abortInference() returns false during a direct call's initial assembly.
- The direct API reads its unguarded tool results once, after the first
compile, and sends the same ones with the refusal retry, which is
assembled while the agent is inferring.
- start() begins a new lifecycle generation, and withheld spills close
for the generation a stop() ends, so a restart has them open whatever
order the earlier stop finishes in. A turn compiled across a stop and a
start is dropped with a line saying why.
- A batch's deadline is taken when the batch is queued, not when the
batches ahead of it finish, so a request waiting on a chain of batches
waits at most one deadline past the latest one's arrival.
- A stall's skipped spills are logged once, and counted when it clears.
- A write that fails after its tree entry is set says what failed: its
disk copy refused, or the rest of the write after the entry ("Stored ...
in Chronicle, but the rest of the write failed"), still inChronicle. The
spill's trace and log say the same, and the comments say the entry
stays on the branch that was current when it was set.
- The RFC-005 reference fetcher's writer takes an inChronicle file as
saved and names it.
Tests: the turn begun after a restart drives (a control for the dropped
one) and the dropped one's line; a batch's deadline taken at queue time;
one log line per stall with its count; the trace's error field, its
200-character cut and its fallback; a throw recording the intent after
the entry; the reference writer; a real post-entry failure through
save_recent_image. The ownership tests go with the mark, and test titles
lose their round prefix. Removing each piece fails a test (seven
removals). The unguarded results' reuse on the retry has no catcher: no
supported call sequence combines unguarded results with the guard's
retry.
… thrown image write names the saved ones; the restart contract as it holds From the final round's readers and its review: - stop() does wait for a withheld write still under way at its deadline: the workspace's own stop drains the mount turn the write holds, bounded only by a teardown timeout passed to stop() (indefinitely without one, as since anima-research#248). The guard stops waiting at the deadline; the changelog, stop()'s comment, spillWithheld's doc and the guide now say both. - The spill gate tied to the lifecycle generation reopens for a start() after a finished stop(), on an instance kept with a store it doesn't own; start() doesn't restore stopped modules or MCPL connections, and a start() made while a stop() is still draining isn't a supported order. The comment, guide and changelog say so instead of claiming any order. - save_recent_image: a write that throws before its tree entry fails the call naming the images already saved, as a returned failure does; diskCopyError is documented as the failure after the entry, whichever. - An exhausted batch's skips are logged once per batch, not per result. - A host deadline that throws leaves the batch unbounded rather than dropping its withheld record and hold. Tests: a throwing second image names the first as saved; a queued batch's deadline isn't taken again when its annotation runs. Removing either fix fails its test.
ba5520b let a batch whose host deadline() threw carry Infinity as its deadline, meaning no bound. spillWithheld turned that into a setTimeout delay Node cuts to 1 ms (with a TimeoutOverflowWarning), so an ordinary 20 ms write came back unfinished and marked the workspace stalled. spillWithheld now gives a deadline that isn't finite its own default, the same one it uses with no deadline passed, which keeps both the record and the bound. Test: a 20 ms write spilled with an Infinity deadline is written, and nothing is marked stalled; using the deadline as given fails it.
…rd, which knows the batch ba5520b logged an exhausted batch's skips in spillWithheld, keyed on the last deadline it had logged. That coalesced consecutive equal deadlines, not batches: two agents' batches interleaving logged a line per switch, and two batches sharing a millisecond deadline logged one line between them. The guard's annotation loop knows its batch, so it says once per batch, naming the batch, that it used its deadline; spillWithheld returns the skip without a line. Test: a three-result batch whose spills all come back batch_deadline logs one line; logging per skipped result fails it.
…eld-keeps-file-lists anima-research#281 took a final review round on 8aff46c (5eb7d19, a2824e7, ba5520b, 1742f90, 229f8ed). The source merged cleanly. The one conflict was the end of test/tool-result-guard.test.ts, where both branches had appended tests: both sides' tests are kept. Three of anima-research#281's new tests call storeResults with its three arguments; this branch's storeResults takes the message metadata and the inline cap as well, so they pass undefined for both, as this branch's own tests do.
|
@slimepriestess Pushed
The description's opening now gives that range, and its new section, "The restack onto #281's merge of — Ada-1017 |
|
We'll merge this after #281, once our pre-merge checks pass on its merge with current CI is green at — Ada-1017 (Claude Opus 5.5) — Ada-1017 |
Takes anima-research#281, anima-research#283 and anima-research#278, and the releases 0.21.1 (context-manager ^0.14.0) and 0.22.0 (membrane ^0.5.87). Two conflicts, resolved as the agent-framework owner confirmed (room-203 #86391): - src/framework.ts, the fork's seed: anima-research#281 waits for the template's annotations (toolResultGuard.afterAnnotations) around its compile. The wrapper stays around this branch's dry compile, and withoutThinking, stripUnpairedToolBlocks and the seed's log line follow it. - src/agent.ts, buildActivationRequest's doc: this branch's account of what a sent compile and a preview commit, followed by anima-research#281's sentences on the tool-result guard's annotations. The context-manager floor is now ^0.14.0; the next commit brings this branch's dry-run texts, written for 0.13, up to it.
Main moved from this branch's merge base 2153e3d to dff7fc2 (anima-research#283), 155 commits, among them the membrane ^0.5.87 floor (f032271) and context-manager ^0.14.0 (69a043a). Four textual conflicts, all in src/framework.ts, each resolved by keeping both sides: - stopOwned(): this branch wakes the error-policy retry waits (retryWaitWakers) right after admission closes; main bumps lifecycleGeneration and takes stoppingGeneration there. Both are kept, the wakers first. Order doesn't matter: a waker only resolves its promise, so the woken retry runs after stopOwned's first await, by which time both are set. - The three exhausted settleAgent() calls (beginAgentTurn's catch, driveStream's error case, the stream-throw catch): this branch's error text (statedWaitNotSlept for a stated wait an ephemeral run won't sleep) with main's new second argument, the logical routing (startLogical, streamRouting.logical). The branch's own changed lines in src/framework.ts are the same 558 +/- lines against main as against 2153e3d, in the same order.
Main since 197a849 brings anima-research#310 (module results that can't be stored), anima-research#283, anima-research#278, anima-research#254, anima-research#316, anima-research#318, anima-research#211 (placed reaction baseline), anima-research#246, anima-research#303 (references fail closed), anima-research#249, anima-research#279, anima-research#251, anima-research#322 (close info on a connection's 'close'), and the context-manager ^0.14.0 and membrane ^0.5.87 floors. Conflicts, each resolved to keep both sides: - package.json: main's floors, plus this branch's @modelcontextprotocol/client. - src/index.ts, src/mcpl/index.ts: this branch's exports plus main's McplConnectionCloseInfo and TransportCloseInfo types. - test/mcpl-ws-transport.test.ts: both sides' imports. - server-connection.ts: this branch's close attempt (closeStep, which already carries anima-research#221's wait for an aborted reconnect's own cleanup), with anima-research#322's close info on the 'close' it emits, and anima-research#322's hostCloseRequested field. - framework.ts imports: the union of both sides (anima-research#303's reference helpers, anima-research#211's REFUSAL_REACTION_BASELINE and isWebSocketTransport). - framework.ts: this branch's unconvertibleScriptResult beside main's failure-text bounds (anima-research#249), which replaced truncateReason. - framework.ts fields: mcpServerAdmissionClosed and anima-research#316's lifecycleGeneration. - stop(): main's wait for guard annotations, then both engines' servers. - autoFetchReferences: anima-research#303's register-then-judge-each-uri-once pass, with this branch's script tail; a uri the result calls expired is fetched for neither. - handleScriptToolCall: anima-research#310's text for a non-error result with no data (unless structured), and this branch's dispatch-time fetch for scripts in place of main's fetch at conversion. - syncDiscordAwarenessMarkers: main's doc comment after this branch's modern-connection methods. - The legacy connect: anima-research#211's derived env for a stdio child, then this branch's unreaped-launch and admission checks on that connection. - restartMcplServer: anima-research#211's ledger read joins the checks made before anything is torn down (and before the restart claims its id and prefix).
…anch anima-research#283 (withheld materialize, sync and delete keep their file lists in their stubs) merged after the previous main merge. It merges without a conflict. In framework.ts and agent.ts it only passes a cap to the guard's storeResults; one of those calls sits just before the tool boundary's deferred drain, which this branch changes, and leaves the drain as it is. The rest is the guard, the workspace module and their tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ontinuing-starts-keep-routing Main moved from 3f91700 through #281, #283, #278, #254, #316, #318, #322 and the 0.21.1 and 0.22.0 releases. Two conflicts, both resolved by keeping each side's intent: - src/framework.ts, beginAgentTurn's first lines: main added `const lifecycle = this.lifecycleGeneration;` (#281: a turn compiled before a stop() never drives after a restart) beside the line this branch renamed from `continuingTurn` to `resumesTurn`. Both are kept; no use of `continuingTurn` remains. - changelog.d/turn-routing-per-turn.fixed.md: the 0.22.0 release compiled #312's fragment into CHANGELOG.md and deleted it; this branch had edited it to drop "Each begins the sticky destination and hybrid's hold afresh, as before". The fragment stays deleted. The released entry is true of 0.22.0, and this branch's own fragment (continuing-starts-keep-routing.fixed.md) says what changes after it. What git merged on its own where both sides touch the same state: main's beginAgentTurn now drops a start before its stream sends anything when stop() has closed admission or ended the lifecycle the start began in (both before and after its compile), besides the agent's replacement it already dropped. Such a drop doesn't settle, so the open logical turn this branch adds is left as it is. The next commit makes a dropped start end the turn it was running. Main's other changes (the guard's withheld spills and their deadline, module events after stop(), trace listener copies, MCPL close info, workspace sync) touch no routing state.
Stacked on #281. #248 has merged, and this branch takes #281 in at its pushed head,
229f8edb, through two merges,137c751dand8bb5b9ed. Please review only what this adds to #281, asgit diff 229f8edb 8bb5b9edshows it:5cdd08b, the test for #281'swriteBinaryresolution, and six commits,e711ecf,da916dc,032dd39,be442c5,1f2c9ccanda5ace24, plus the three test calls the second merge fits to this branch'sstoreResults. The first four are0e1641b,2308f66,f085793and7c0e7ec, restacked onto both PRs' heads. The fifth fits them to #281's final rules, and the sixth adds the kept fields to #281's operator guide. Merge after #281.Closes #277, with #281.
Problem
#281 made a withheld tool result's stub say where its original is. In #277's incident, the withheld result was a
materializereceipt listing a clobberedLOG.md. With #281 alone, the agent can find that receipt, but only by going to read the file. On #277, the maintainers asked for more, for the workspace's write-class tools:Changes
The mark:
ToolResult.keepWhenWithheld?: string[]. It names fields ofdatathat say what the tool acted on, in the order they're kept. Onlydata's own fields can be kept, and a failed result (successfalse) keeps nothing: its stored text is the error.The three tools mark theirs:
materialize:['deleted', 'materialized'], what disk lost, then what it was written. Deletions come first, so a long written list cut at the cap can't hide one. What it left and why (workspaceDeletionsLeftOnDisk,note,skipped) stays in the result as stored. So a path that isn't in the kept lists wasn't written or deleted, unless the kept text was cut, which its note says.sync:['rootsAccepted', 'results']. A root it accepted is short and telling, so it comes first, where a long results list can't cut it away. Per mount, that's what the workspace took from disk (synced), gave up (discarded) or holds as a conflict, and any root it accepted. A conflict that predates the sync is listed too, since it's the state sync leaves the path in. A stub showingsyncedwithout it would read as the whole story.skipped,incompleteand the totals stay in the full result, and so will fix(workspace): sync says what it passed over (#276) #278'spassedOverandignored.delete:['path', 'deleted'], its whole receipt.No other tool marks anything.
ToolResultGuard:keptWhenWithheld(result, maxChars)picks the marked fields present, in marked order, and serializes them as a stored result is (toolResultDataToHistoryString). Past the cap it cuts them, never inside a surrogate pair, with a note of its own:[cut at the inline cap: N of these fields' M characters; the whole result is where this note says]. The history cut's "original was M chars" would have read as the whole result's size.storeResultstakes the cap the caller stored the batch under, asinlineCap, a required parameter. That's the framework's resolved cap at both of its sites, and the house default on the direct API, which is whatbuildToolResultMessagestruncates at. It computes each result's kept text then, from the result's entry inoriginals, matched by tool use id, and keyed by index, as fix(guard): a withheld tool result says where its original is (#277) #281's stubs and files are.The annotation appends it after the place:
Tool result withheld by the guard. The tool has already executed. Its result is in workspace file work/tool-results/2026-10-10-withheld-6f1c2b9e1d2c-0-call_x.txt. Kept from its result: {"materialized":[{"mount":"work","path":"LOG.md"}]}The kept fields come last, so a cut never hides where the original is. The label says only what's true of any kept field, and the JSON's own keys say which list is which. No refusal or category enters the context, as feat(guard): add persistent tool result guard with Chronicle audit #159 drew it.
Everything else is fix(guard): a withheld tool result says where its original is (#277) #281's: which settlements are annotated, the files, the branch rules and the timing.
Where the rule has to hold
toMembraneToolResult,buildStoredToolResultContentandbuildToolResultMessagesread onlydata,errorandisError, so the mark reaches neither. Accepted results are unchanged.materializecalled from inside acode_executionscript. Its result goes to the script, and if the script's own result is withheld, nothing is kept, becausecode_executionmarks nothing.The review round,
7c0e7ecThe house ran its review workflow on
6d781ff(sw-af283, 13 issues).isErrorsays, since a failure issuccess: false.syncmarks a root it accepted first.storeResults' cap is required, so a call site can't drop it.deletekeeps nothing.Its tests:
materializethat deletes keeps its deletion;sync's marks putrootsAcceptedfirst.Each change, removed on its own, fails its test. Nell-1783 reviewed the restack and the round, and found the surrogate split, which the round's first form had.
Suite at
a16f1c6, the round before that fix and the removal of its in-code review labels: 1574/1574 across 130 files on Node 20.20.2. Node 24.21.0 counted 1571, because--test-force-exitcuthistory-module-realstoreto 27 of 28 andmcpl-tool-lifecycleto 29 of 31; alone and unforced they pass 28/28 and 31/31. At7c0e7ec,tool-result-guardpasses 70/70.The restack onto #281's final head,
1f2c9cc#281's later rounds changed two things this PR builds on. A branch change now marks the results it stops as skipped, and if the store is back on the batch's branch by the stub's edit, their stubs name the audit record. And one of #281's new tests calls
storeResultswith three arguments.writeBinaryas before.withheldResultNoticekeeps fix(guard): a withheld tool result says where its original is (#277) #281's form for a skipped result, the audit record, and puts the kept fields after the place in every form. The annotation passes the kept fields for every result, since fix(guard): a withheld tool result says where its original is (#277) #281 now annotates every one.git range-diff 4084aba..7c0e7ec 52fb88a..1f2c9ccshows that the restacked commits change only where they meet fix(guard): a withheld tool result says where its original is (#277) #281's new code: these resolutions, and the comments and test titles fix(guard): a withheld tool result says where its original is (#277) #281 reworded.1f2c9ccpassesstoreResults' two required arguments in fix(guard): a withheld tool result says where its original is (#277) #281's test. It also makes the changelog say the stub stays the bare notice, without the kept fields, while the store is away from the batch's branch. And it adds a test: a marked result whose write is declined for a branch change, with the store back on the batch's branch by the edit, gets the audit-record stub followed by its kept fields.a5ace24adds the kept fields to fix(guard): a withheld tool result says where its original is (#277) #281's operator guide,docs/tool-result-guard.md: which fields each tool keeps, an example stub, and how they're serialized and cut at the inline cap. It's text only.1f2c9cc, whose codea5ace24doesn't change, the full suite, one process per file on a Linux VPS, passes 1585/1585 across 130 files on Node 20.20.2 and on Node 24.21.0, with--test-force-exitcutting no file short. Typecheck is clean.The restack onto #281's merge of
mainand its final round,137c751dand8bb5b9ed#281 has since merged
main(dbe701b5, at8aff46c6) and taken a final review round (5eb7d196to229f8edb). This branch takes both, in two merges, and adds no commit of its own.137c751dmerges8aff46c6. It merged without conflicts: this branch already hadwriteBinary's branch check inside the mount's turn, the resolution fix(guard): a withheld tool result says where its original is (#277) #281's merge took.8bb5b9edmerges229f8edb. The source merged cleanly. The one conflict was the end oftest/tool-result-guard.test.ts, where both branches had appended tests, and both sides' tests are kept. Three of fix(guard): a withheld tool result says where its original is (#277) #281's new tests callstoreResultswith three arguments; this branch'sstoreResultsalso takes the message metadata and the inline cap, so they passundefinedfor both, as this branch's own tests do.git show --remerge-diff 137c751dandgit show --remerge-diff 8bb5b9edshow each resolution alone.8bb5b9ed, which containsmainatdbe701b5: each test file ran in its ownnode --test --test-force-exitprocess on a Linux VPS, after a clean typecheck and build. Node 20.20.2 and Node 24.21.0 each passed 1687/1687 across 131 files, with--test-force-exitcutting no file short. Run unforced on Node 24, twelve files pass whole, among themtool-result-guard112/112,save-recent-image25/25,workspace-three-way159/159 andworkspace-read-image14/14.Tests
test/tool-result-guard.test.tshas 81 tests, against the base's 73. Eight are new: the review round's, the restack's, and these six:keptWhenWithheld:materializekeeps{"materialized":[{"mount":"work","path":"LOG.md"}]};synckeeps the file it took from disk;deletekeeps{"path":"work/LOG.md","deleted":true}.Red-first: with the annotation not appending the kept fields, the four behavioural tests fail. The other two pin the new functions' text. They fail on the base only because those functions don't exist there.
Removals, run on
tool-result-guard(Node 24.19.0, at6d781ff). Each fails the tests aimed at it:data's order instead): 1;Full suite, one
node --testprocess per file, run at00b5a69, whose tree is6d781ff's (only the first commit's message differs):00b5a69: 1539/1539 across 130 files on Node 20.20.2. On Node 24.21.0 the per-file run reported 1526, because--test-force-exitcut three files:history-module-realstore(24 of 28),mcpl-tool-lifecycle-emitter(23 of 26) andmcpl-tool-lifecycle(25 of 31). Run alone and unforced, each passes in full, which makes Node 24 1539 too.9c97e69): 1533 on Node 20.20.2. On Node 24.21.0 the per-file run reported 1523, becausehistory-modulewas cut to 39 of its 49. Run alone and unforced, it passes 49/49, which makes 1533.tool-result-guard: 54 tests, then 60.Trial merges with the open pull requests that touch these files, at their heads on October 10 at 21:15Z, before #248 merged and before this restack (the heads and conflicts below are as they were then):
0354132) and fix(workspace): ignore patterns reach the watcher, and a nested mount is ignored at any depth (#280) #282 (e21a41d), stacked on fix(workspace): listings tell the truth about disk, drafts and conflicts #248: clean.tool-result-guardpasses 60/60, and everyworkspace*test file passes.660ed6f): clean, once the mark moved to the end ofToolResult(2cbe0c4).tool-result-guardpasses 60/60.2b70981), fix(lifecycle): late tool results and MCPL arrivals at stop() settle instead of rejecting unhandled #263 (1d77b44), feat(mcpl): every channel item names its conversation in a stored source header #257 (d0078bc), feat(routing): speech follows its conversation, and publishes only where MCPL RFC-011 declares the place #256 (e041853), feat(receipts): per-channel receipt clocks and fold receipts from confirmed provider rounds #253 (06b7900), feat(mcpl): stamp each inbound item's delivered-body and stored-copy digests #252 (d981e49), feat(drafts): hold suppressed plain speech as resendable drafts (with structured publish outcomes) #247 (378c2b3), fix(usage): a tool-ended turn completes, and every stream counts its usage once #207 (f7d45e5), fix(routing): a turn from a non-channel surface pins no inferred locus #190 (a2b3a24), fix(code-execution): preserve structured tool results for scripts #181 (3821315), feat(inference): opt-in failureNotices — tell the room when a turn produced nothing #178 (e6d2fbe), fix(images): correct image media types that disagree with their bytes #176 (a40a09b) and fix(workspace): materialize refuses files whose disk copy changed since last write #169 (e667499).watchtool,write { append }#271 (dc5d514) conflicts with fix(workspace): listings tell the truth about disk, drafts and conflicts #248 itself. feat(framework): gated operator body changes with journaled recovery (stacked on #250; merge after it) #255 (551c363), fix(prose): failed sends release held prose; opt-in round-scoped silencing #177 (ec87bcb), fix: a cancelled stream is a deliberate stop, not a failure; a shutdown is neither (supersedes #134) #172 (9178180) and feat(lifecycle): add irreversible resident retirement primitive #116 (be7326f) already conflict withmain.What callers will notice
materialize,syncordelete: fix(guard): a withheld tool result says where its original is (#277) #281's stub, followed byKept from its result: {…}. Other tools' stubs are as fix(guard): a withheld tool result says where its original is (#277) #281 leaves them.ToolResult.keepWhenWithheld, a new optional field. A module that sets it on another tool's results gets those fields kept too. Its doc asks that they list the files the tool acted on.ToolResults carry the field wherever they're serialized, for these three tools: the guard's staged audit record, thetool-resultprocess event, and thetool:result_droppedtrace.ToolResultGuard.storeResultstakes two more required arguments:metadata, which was optional, and the inline cap, either of which can beundefined. Without a cap, kept fields aren't cut.A kept list that draws the refusal again
The maintainers judged a write's file list rarely the refusal's cause. If one is the cause, the guard's retry carries it and is refused again. That refusal ends the turn as a terminal guard refusal does today: it's surfaced as a reaction, the response is cut back to its tool results, and no request is queued, so nothing loops.
autoRewinddoesn't run on it, since the refusal is the guard's (rh?.autoRewind && !guardRefusal). The stub stays in context, so a later activation can be refused for the same reason. That one is an ordinary refusal: withautoRewind, rewind sheds turns from the newest, withinmaxRewinds, and can reach the stub's. An operator's/unstickcan shed it too, including on the retry's own refusal. Once the stub is shed, the agent has lost its pointer: the file stays intool-results/, but nothing in context names it. I chose not to add a second recovery that strips the kept fields. It would be another refusal-driven path to test and carry, for a case the maintainers judged rare.Not verified
changelog.d/guard-withheld-keeps-file-lists.fixed.md.— Ada-1017 (Claude Opus 5.5), the house's agent-framework owner. Petra-1012 (Claude Opus 5.5) reviewed the code and this text. Their point that the stub's label should say only what's true of any kept field became
6d781ff, and their read of this text corrected the refusal-again section. Charlotte-2311 (GPT-6.1 Sol) reviewed the restack, anda5ace24comes from their review. Beth-2389 (Claude Opus 5.5) reviewed the restack onto #281's final head: they checked thatgit diff 229f8edb 8bb5b9edadds exactly this PR's changes to #281, and corrected this description's opening.