refactor(runtime): make the tool-result archive capability indivisible - #2255
Merged
Merged
Conversation
The pruned tool-result placeholder is a runtime-generated protocol value that names ArchiveRead as the way back to the content. Writer, replay reader, ref reader and that decoder were four independent optional fields across two configuration surfaces, so "archives but cannot read back" was representable — and it shipped twice, on the headless surface (#2025) and on the child-agent path, where CHILD_RECOVERY_TOOL_NAMES only widens the candidate pool that buildToolsForAgentDefinition then narrows again. Introduce the archive authority as one value that carries its own decoder, and let the backend bind it from the session that archives. Which host remembered to register a tool stops being part of the question, because there is no longer a second delivery address to forget. Hosts are untouched here. The binding is idempotent, so a host that still binds ArchiveRead through its own tool surface keeps exactly one of it while the surfaces migrate. Refs #2026
The negative case is the point of the optional capability: a session that archives nothing (the CLI) must not carry a decoder for placeholders it will never emit. The round trip is the other half — the tool the model is told to call has to reach the storage the writer used, which is what "indivisible" actually buys over two co-present fields.
Four hosts assembled this capability, and each had two addresses to send it to: the writer and replay reader went to the backend, the ref reader went to the tool builder. Nothing tied the two together, so "archives but cannot decode" stayed spellable at every one of them — headless shipped it (#2025), and the child-agent path shipped it a second time. Both addresses collapse into one. Hosts hand the backend a whole `ToolResultArchiveCapability` and nothing else; the decoder rides along and the backend advertises it. `archiveResources`, the second desktop and runtime-host registrations, and the separate headless resource-reader builder are gone, along with `CHILD_RECOVERY_TOOL_NAMES` and the `ArchiveRead` entry in the deep-research allowlist — both were pool-level pass-throughs that every later narrowing step undid. The child ceiling assertions move rather than relax: a child now receives its exact allowlist plus the decoder, which is the point. The ceiling governs agent-permission tools, and a decoder for a placeholder the runtime itself wrote is not one.
Every existing test observed advertisement — that the decoder reaches the provider's tool list. None observed execution. That left the second half of the invariant unpinned: a tool can be advertised and still be unreachable at dispatch, and "the model was told to call a tool it cannot use" is exactly the failure that shipped twice. This drives a real turn: the budget prunes an oversized result, the model reads the ref out of the placeholder it gets back, calls ArchiveRead with it, and the body lands in the next request. Deleting the decoder binding fails it. Also re-homes the `activityKind` assertion the deleted built-in tool test owned. The tool still exists; only its birthplace moved.
Three leftovers from moving the capability, each a second source of truth: - `ToolResultArchiveRecorder` stayed in the compaction contract, which now imports the capability back — a type cycle. It belongs with the capability: it is one of the three services, and its input spans both prune paths. - `'ArchiveRead'` was spelled in both the tool and the constant guarding against duplicate bindings. If those drift the guard silently stops guarding, so the tool now owns its own name. - `ToolResultArchiveReader` was still imported by the backend after the field that used it was removed. `ARCHIVE_READ_TOOL_NAME` also stops being public — nothing outside the runtime uses it. Docblocks corrected where the prose outlived the migration it described: the dedup guard is not a migration affordance any more, "no spelling" overclaimed (a capability with failing readers is constructible; a forgotten tool is not), and the session-scoping duty the ceiling carve-out leans on was unwritten.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The tool-result archive was three optional fields spread across two configuration surfaces. The writer (
archiveToolResult) and the replay reader (readToolResultArchive) went to the backend; the ref-addressed reader (archiveResources) went to the tool builder. Nothing tied them together, so "this session archives but has no way to decode a placeholder" was a spellable state at all four hosts that assemble the capability — embedded desktop, runtime-host server, CLI, and headless/Harbor.It shipped twice. #2025 was the headless surface. The child-agent path was the second instance, and
CHILD_RECOVERY_TOOL_NAMESdid not fix it: every child surface is re-narrowed bybuildToolsForAgentDefinitionagainstdefinition.tools, no built-in definition listsArchiveRead, and durable snapshots record[...definition.tools]. The pass-through was undone by the very next step every time.The fix is not a stricter assertion at the assembly points. It is noticing what the decoder actually is. When the context budget prunes a large tool result, the placeholder that replaces it is a runtime-generated protocol value naming
ArchiveReadas the way back. Which tool decodes a value the runtime itself wrote is not a host configuration question and not an agent permission. So:ToolResultArchiveCapabilitybundles the writer, both readers, and the decoder tool. It is constructible only throughcreateToolResultArchiveCapability, so a half-built capability has no spelling.AiSdkBackendInputfield replaces the two backend options, and the backend binds the decoder itself at the one placeinput.toolsis consumed.archiveResources, the hand-wired desktop and runtime-hostArchiveReadregistrations, and the separate headless resource-reader builder are deleted. So areCHILD_RECOVERY_TOOL_NAMESand theArchiveReadentry in the deep-research allowlist — both were pool-level pass-throughs that later narrowing undid.Closes #2026. Refs #2025, #2036, #2028.
Breaking change
BackendFactoryContext.toolsis now documented as a ceiling on agent-permission tools, not on everything the provider sees. A child receives its exact allowlist plus the archive decoder. That is the point of the change: withholding the decoder grants no safety — it only strands content the model was explicitly instructed to retrieve, using a ref the runtime minted from the parent's own tool output. The contract comment atpackages/runtime/src/session-manager.tssays so explicitly rather than leaving the carve-out implicit.The product tool surface identity no longer lists
ArchiveRead, for the same reason: it records the host's product binding, and the decoder is not part of it. Nothing consumes that identity as "what the model will see" — the benchmark arm classifiers compare connection slug, model, reasoning effort,agentTools, prompt hash and pricing profile, neverproductToolNames, andvalidateHarborCellExecutionIdentityonly checks catalog membership. So the under-report has no reader.Review
Reviewed by four independent passes (correctness/completeness, first-principles design, test quality, leftovers). No P0/P1. The one finding worth acting on was that every test observed advertisement and none observed execution, leaving half the invariant unpinned — a tool can reach the provider's list and still be unreachable at dispatch. Fixed with an end-to-end test that drives a real turn (prune → placeholder → model calls
ArchiveReadwith the ref → body returns), verified by mutation: deleting the binding fails it. Also cleaned three second-sources-of-truth the move left behind (a type cycle, a duplicated tool-name literal, a dead import) and corrected docblocks whose prose outlived the migration they described.Declined: extracting the two 6-line desktop test helpers into a new
test-onlypackage export — a public-surface change costing more than the duplication it removes. Deferred: a second round-trip test for the runtime-host ref reader, which is literally the samereadArchivefunction the replay reader already covers.Verification
npm run lint,npm run format:check,npm run build— clean.@maka/runtime— 3195 pass, 0 fail (9 skipped).@maka/runtime-host— 699 pass, 0 fail. The child-ceiling assertions moved from['Glob','Grep','Read']to['ArchiveRead','Glob','Grep','Read']against a real provider wire, which is the observable proof that a narrowed child now keeps its decoder.@maka/headless— 1409 pass, 0 fail. Includes the fix(headless): register ArchiveRead when Harbor enables tool-result archiving #2025 round trip, now driven end to end from one capability.@maka/core— 784 pass, 0 fail.@maka/desktop— 13 failures, all pre-existing: the same set fails onmain(33 failures there, a strict superset), and they are macOS temp-path and project-root resolution tests untouched by this change.@maka/storage— 15 failures, 16 onmain; Git-worktree tests, andpackages/storagedoes not depend on@maka/runtime.New tests pin both halves of the contract at the provider-visible boundary — a session that archives advertises the tool its placeholders name, a session that archives nothing does not, the decoder reads back exactly what the writer stored, and it reads as the invoking session.