From 675546aeed6f9c4b2838da8e58006fdba3af332b Mon Sep 17 00:00:00 2001 From: Ada-1017 Date: Sat, 10 Oct 2026 15:12:11 -0400 Subject: [PATCH 01/16] fix(guard): a withheld tool result says where its original is (#277) agent-framework #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: #159's neutral notice stays exactly as it is, followed only by the place. No refusal and no category enter the context (#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 `/tool-results/-withheld-.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 `-` 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/-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 --- .../guard-withheld-names-original.fixed.md | 12 + src/agent.ts | 16 +- src/framework.ts | 125 +++++---- src/tool-result-guard.ts | 111 +++++++- test/silent-heartbeat-rows.test.ts | 4 +- test/tool-result-guard.test.ts | 251 ++++++++++++++++-- 6 files changed, 452 insertions(+), 67 deletions(-) create mode 100644 changelog.d/guard-withheld-names-original.fixed.md diff --git a/changelog.d/guard-withheld-names-original.fixed.md b/changelog.d/guard-withheld-names-original.fixed.md new file mode 100644 index 000000000..3e341796f --- /dev/null +++ b/changelog.d/guard-withheld-names-original.fixed.md @@ -0,0 +1,12 @@ +- A tool result the guard withholds now says where its original is. The + guard's notice ("Tool result withheld by the guard. The tool has already + executed.") is followed by the original's place: a workspace file under the + first writable mount's `tool-results/` (named `-withheld-`), + or, with no writable workspace or a failed write, the guard's audit record + (`framework/tool-result-guard`), for an operator. This holds for every + withheld settlement: a refusal, an aborted stream, a batch that was never + submitted, and one whose audit sync failed. The notice still names no + refusal and no category. The stub is written before any request is compiled, + including the refusal retry, and before stop closes the store. Before this, + a withheld result left only the notice, so the agent couldn't tell what the + tool had done (agent-framework #277). diff --git a/src/agent.ts b/src/agent.ts index 412707a33..bf4d48d0c 100644 --- a/src/agent.ts +++ b/src/agent.ts @@ -655,6 +655,8 @@ export class Agent { } this.toolResultGuard.flushUnrecorded(); + // A withheld stub is compiled only once it says where its original went. + await this.toolResultGuard.whenAnnotated(); // Filter tools to only allowed ones const tools = availableTools.filter((t) => this.canUseTool(t.name)); @@ -935,6 +937,8 @@ export class Agent { // Retry accepted history edits before assembling a real request. Pure // preview/compile entry points must not perform these writes. this.toolResultGuard.flushUnrecorded(); + // A withheld stub is compiled only once it says where its original went. + await this.toolResultGuard.whenAnnotated(); // A new activation supersedes every receipt flight the previous one left // open. Membrane fires the kv-unified wire receipt per provider attempt, @@ -1192,9 +1196,19 @@ export class Agent { if (ids) { abandonedUsage = response.usage; const withheld = new Set(ids); + // The retry carries the stubs as stored: each says where its original + // went (agent-framework #277), once the guard has written that. + await this.toolResultGuard.whenAnnotated(); + const stubs = new Map['content']>(); + for (const message of this.contextManager.getAllMessages()) { + for (const block of message.content) { + if (block.type === 'tool_result' && withheld.has(block.toolUseId)) stubs.set(block.toolUseId, block.content); + } + } request = { ...request, messages: request.messages.map((message) => ({ ...message, content: message.content.map((block) => block.type === 'tool_result' && withheld.has(block.toolUseId) - ? { type: 'tool_result', toolUseId: block.toolUseId, content: TOOL_RESULT_GUARD_NOTICE, isError: block.isError } : block), + ? { type: 'tool_result', toolUseId: block.toolUseId, + content: stubs.get(block.toolUseId) ?? TOOL_RESULT_GUARD_NOTICE, isError: block.isError } : block), })) }; response = await this.membrane.stream(request, { signal }); } diff --git a/src/framework.ts b/src/framework.ts index 349bdd693..5f449de67 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -2018,6 +2018,12 @@ export class AgentFramework { this.eventGate?.dispose(); this.livenessWatchdog?.stop(); + // A stopping stream's batch settles as withheld, and its stub names where + // the original went: let those writes finish while the workspace is up. + // Tracked here rather than per agent, since an ephemeral run or a closed + // fork leaves the agent map while its annotation can still be running. + while (this.guardAnnotations.size > 0) await Promise.all([...this.guardAnnotations]); + // Stop modules and MCPL servers in parallel const shutdownPromises: Promise[] = [this.moduleRegistry.stopAll()]; if (this.mcplServerRegistry) { @@ -2694,7 +2700,13 @@ export class AgentFramework { }; } - private restoreToolResultGuardSetting(agent: Agent): void { + /** Every guard's annotation still running, for stop() (agent-framework #277). */ + private readonly guardAnnotations = new Set>(); + + /** A new agent's guard: its persisted setting, the tool-results spill its + * withheld originals are written to, and the set stop() waits on + * (agent-framework #277). */ + private adoptToolResultGuard(agent: Agent): void { const enabled = (this.store.getStateJson(FRAMEWORK_STATE_ID) as { toolResultGuards?: Record; } | null)?.toolResultGuards?.[agent.name]; @@ -2702,6 +2714,13 @@ export class AgentFramework { throw new Error(`Invalid persisted tool_result_guard for ${agent.name}: expected a boolean`); } agent.toolResultGuard.setOverride(enabled); + agent.toolResultGuard.setHost({ + spill: (label, text) => this.writeToolResultFile(label, text), + track: (annotation) => { + this.guardAnnotations.add(annotation); + void annotation.finally(() => this.guardAnnotations.delete(annotation)); + }, + }); } private buildThinkTool( @@ -4151,7 +4170,7 @@ export class AgentFramework { }); const agent = new Agent(config, contextManager, this.membrane); - this.restoreToolResultGuardSetting(agent); + this.adoptToolResultGuard(agent); agent.markContextConsumed(); // RFC-006: nothing stored before now is replaceable this.ephemeralCandidates.set(agent, contextManager); @@ -6457,7 +6476,7 @@ export class AgentFramework { }); const agent = new Agent(config, contextManager, this.membrane); - this.restoreToolResultGuardSetting(agent); + this.adoptToolResultGuard(agent); agent.markContextConsumed(); // RFC-006: nothing stored before now is replaceable this.sharedSlotAgents.add(agent); const restoredSettings = this.readAgentRuntimeSettings(config.name); @@ -6534,7 +6553,7 @@ export class AgentFramework { allowedTools: [...SUBCONSCIOUS_TOOL_NAMES, 'think', 'skip_reply', 'end_turn'], }; const agent = new Agent(agentConfig, contextManager, this.membrane); - this.restoreToolResultGuardSetting(agent); + this.adoptToolResultGuard(agent); agent.markContextConsumed(); // RFC-006: nothing stored before now is replaceable this.sharedSlotAgents.add(agent); // reads the shared slot through its merged view this.agents.set(name, agent); @@ -7386,7 +7405,7 @@ export class AgentFramework { const config: AgentConfig = { ...templateConfig, name, strategy: undefined }; const agent = new Agent(config, contextManager, this.membrane); - this.restoreToolResultGuardSetting(agent); + this.adoptToolResultGuard(agent); agent.markContextConsumed(); // RFC-006: nothing stored before now is replaceable this.agents.set(name, agent); this.agentConfigs.set(name, config); @@ -11704,6 +11723,40 @@ export class AgentFramework { return { blocks, spilled }; } + /** + * Write one tool result's text to `/tool-results/ + *