Repository navigation
feat(mcp-host): spill oversized tool results of workflow steps out of the prompt - #1454
Open
alfredolopez80 wants to merge 15 commits into
Open
alfredolopez80 wants to merge 15 commits into
alfredolopez80 wants to merge 15 commits into
Conversation
… the prompt Agentic workflow steps pasted every tool result into the prompt in full and resent it on each iteration. Wire the existing spillover store into WorkflowService.executeStep, on by default: - results at or above CLERUM_WORKFLOW_TOOL_SPILLOVER_THRESHOLD (default 32768 bytes) are written under <output dir>/.spill/spillover/<stepId>/ and the model receives the store's summary and spillover_ref; - clerum__spillover_read is offered only after the first spilled result, limited to refs of the same step and to reads of at most the threshold; - a per-step budget (CLERUM_WORKFLOW_TOOL_SPILLOVER_BUDGET_BYTES, default 16 MiB) keeps further results inline once exceeded; - the step's spill directory is removed when the step ends; - toolsCalled keeps original results; read calls record ref and byte count; - CLERUM_WORKFLOW_TOOL_SPILLOVER_ENABLED=false restores the previous behaviour. Steps whose results stay under the threshold keep the same prompts, offered tools and disk state. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C3LgMsZSv9oxr7EW78zHpu
…re removing it Resolve the step directory and verify it is inside the spillover store right before the recursive removal. The step id is already restricted to a single [A-Za-z0-9_-] segment; this makes the containment explicit at the removal site. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C3LgMsZSv9oxr7EW78zHpu
Spill files were keyed on the step id only and removed only in the step's finally block. A process killed mid-step left raw tool results on the output volume, and a later execution of the same step got the same refs (i1c0, ...) and could read them; two overlapping executions of one step also overwrote and deleted each other's files. - Each execution writes under <output dir>/.spill/<random 16-hex id>/, and cleanup removes only that directory (same resolved-path containment check as before). Refs and prompts are unchanged. - The first step of a process removes the spill directories a dead process left (.spill/<16-hex id> and the older .spill/spillover), before any store of the process exists, also when spillover is disabled. Other entries under .spill are kept. - The budget warning now says the result stays inline; the budget caps persisted bytes and never stops the step. - Tests cover the budget wiring, the write-failure latch, independence from the first-party spillover flag, cross-execution reads, overlapping executions and the leftover removal. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Workflow steps reused the first-party agent's spillover instruments, so
clerum_spillover_persisted_total, clerum_spillover_reads_total and
clerum_tool_output_bytes mixed both callers, and the histogram received
only workflow results at or above the threshold.
- SpilloverStorage takes optional instruments and keeps the first-party
ones by default; the read tool reports through its store's instruments.
First-party names, labels and values are unchanged.
- Workflow steps report to clerum_workflow_step_spillover_persisted_total
{tool} and clerum_workflow_step_spillover_reads_total{with_range}, and no
longer feed clerum_tool_output_bytes.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
alfredolopez80
marked this pull request as ready for review
November 12, 2026 22:15
…rectory The step spill code addressed `<output dir>/.spill` by path and never checked what that path was. When `.spill` was a symbolic link, the once-per-process leftover removal deleted `spillover` and execution-id entries in the link target, outside the output directory, also with the kill switch off; persist wrote tool results into the target and cleanup removed through it. A private lstat check now requires `.spill` to be a real directory (or absent) before the removal lists it, before persist writes, and before cleanup removes an execution directory. A link or a file is refused through the existing paths: the removal and cleanup log their existing warnings with errorCode ENOTDIR, and persist latches the existing write failure, so results stay inline. No new message, state or configuration. The window between the check and the following call is not closed. Orphans checked: nothing is removed by this change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tep's timeout Every step awaited the once-per-process leftover spill removal with a bare await, outside the step's timeout and abort. A removal that never settled (a stalled volume) left every step of the process pending, including timed-out, caller-aborted, kill-switch and text-only steps. The wait now goes through withTimeout with the step's abort signal, like the provider call: an abort or timeout during the wait fails the step with its normal step-timeout or caller reason. A step that is already aborted does not wait; it returns at the loop's abort check, which comes before any write to the store. The removal stays memoized and shared by every step of the process; a step that gives up leaves it running for the next step. Orphans checked: nothing is removed by this change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rst-party read counts - Leftover removal keeps four names that contain a 16-hex run but are not one (prefix, suffix, both, 32 hex); each case also removes a real execution root, so the removal ran. - The budget warning is logged once per step execution: three misses log it once, with its full text and fields; another execution logs its own. - A SpilloverStorage built without instruments counts read-tool reads under clerum_spillover_reads_total, not the workflow step counter. - The EACCES removal test is skipped as root, where directory modes do not stop the removal (repository convention). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…llover Resolve the mcp-host version conflict to 0.1.660 (dev 0.1.659 + 1) in package.json and both lock entries. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…fact quota Spilled tool results live under `<output dir>/.spill` while a step runs, and the artifact quota summed the whole output folder, so a step that spilled could make a later generated file fail with "Output quota exceeded" where it fits without spillover (owner decision S1, option 2: the spill stays in the run's output folder and the quota does not count it). enforceQuota now passes the name of the step spill directory to getDirectorySize when it measures a workflow run's output (any folder that is not a chat caller folder). Only a real directory named exactly `.spill` directly under that folder is skipped, with the spillover switch on or off. A `.spill` file or symlink, a deeper `.spill`, other names and chat caller folders are counted as before. getDirectorySize keeps its full sum unless a directory is named, and the store takes the directory name from the same constant. The enforceQuota comment now names the run volume (256Mi by default) as the hard cap instead of a 1Gi PVC. Orphans checked: nothing is removed by this change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
11 tasks done
Spilled step results were files under the run's /output folder, which the coordinator pod mounts read-write as the same uid, and mcp-host addressed them by path. No check on a path can be atomic with its use there, so the link checks could only narrow the window, not close it. The step store now keeps each spilled result in a map owned by one step execution; executeStep drops it in finally, on completion, failure, abort and timeout. Nothing is written to disk, so the leftover removal, its bounded wait, the write-failure latch and the artifact-quota exclusion for .spill are removed. The model gets the same summary as before and reads results back with clerum__spillover_read, now from memory. core/spillover, workflow/artifactOutput.ts and its test are back to the merge-base content. The only core change: SpilloverReadTool takes its store as Pick<SpilloverStorage, 'load'> and an optional read counter that defaults to the first-party one, so workflow steps keep their own counter. orphans checked: removeLeftoverStepSpillover, leftoverSpillRemoval, cleanup, SpilloverInstruments, the instruments option, STEP_SPILL_DIR, the skip argument of getDirectorySize, executionRoots, stepDir, exists, treeBytes, seedSpill, blobFile; removed: all Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… release Review round 4 found three gaps in the in-memory step store tests: - the abort test asserted its witness inside the provider mock, so a failing witness only made the step fail, which the test expected; the read is now recorded there and asserted after the step, together with the abort reason; - the copied line counter was never fed raw line feeds; three rows now compare the summary with the first-party store for raw text with and without a trailing line feed and with blank lines; - no test timed a step out after a result was stored; one now does, with a witness that the result was readable while the step ran. Also drops a comment that referred to the removed disk-write latch. Mutations caught: never store, line loop removed, trailing line feed adjustment removed, release not called, release only when not aborted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…llover Brings in A1 #1453 (spillover flag passthrough to recipe mcp-host pods). Conflicts: mcp-host/package.json and package-lock.json version lines only, resolved to the higher side. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…o 128 KiB Platform-owner decision P2: a workflow step now keeps tool results under 128 KiB inline and spills only larger ones. The read tool returns at most the threshold per call, so a spilled result is read back in 128 KiB slices. CLERUM_WORKFLOW_TOOL_SPILLOVER_THRESHOLD still overrides it (4096..1048576). Tests: the default and invalid-value fallbacks pin 131072; integration payloads are scaled above the new default; a new test keeps a 100 KiB result inline under the default. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The test that keeps a 100 KiB result inline under the default threshold now also calls a tool returning 300 KiB and asserts that result is stored, so it fails when spillover is off instead of passing vacuously. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
alfredolopez80
added a commit
that referenced
this pull request
Nov 14, 2026
…decision Answer the round-1 review of the step change policy against dev 1b1b113: - step tool-result spillover lives in mcp-host memory for one step execution (#1454), released when the step ends; the four spillover names are already on MCP_HOST_ENV_PASSTHROUGH; - no sanitizer runs on step tool results; only the <tool-result> delimiters wrap them; - name both coordinator parsers (ConfigLoader, sdk.config.getSpec()) in layer 3 and the custom-image hop; - state the snippet worker env filter; - principle 5 lists the preview exception, and a new open decision 8 covers reducing status.steps[].toolsCalled[].args and .result. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
8 of 9 tasks
List CLERUM_WORKFLOW_TOOL_SPILLOVER_ENABLED, _THRESHOLD and _BUDGET_BYTES in the Workflow table with their defaults, bounds and the warn-and-default handling of invalid values, and note that WRC forwards them from its own environment to recipe pods. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
alfredolopez80
added a commit
that referenced
this pull request
Nov 14, 2026
#1454 raised the step spillover threshold default to 131072 bytes (stepSpillover.ts, b93220e). The flag table and the rationale now say 128 KiB. Open decision 3 stays open: it is settled by the off / 128 KiB / 32 KiB comparison that #1454 requires before merge. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
Agentic workflow steps paste every tool result into the prompt in full and resend it on each
iteration. This gives
WorkflowService.executeStepa per-execution, in-memory store for largetool results. It returns the first-party summary format and reuses the first-party
clerum__spillover_readtool;mcp-host/src/core/spilloveris unchanged. It is on bydefault for every recipe that runs an mcp-host.
CLERUM_WORKFLOW_TOOL_SPILLOVER_THRESHOLDbytes (default 131072),measured on the JSON-serialized result, is kept in the mcp-host process memory for the
length of the step execution, and the model receives the store's summary (head, tail, size,
structure hint,
spillover_ref). Refs arespillover://<stepId>/<callId>.json. Nothing iswritten to the run's output directory or anywhere else on disk.
clerum__spillover_read, limited torefs of the same step, to results of the same execution, and to reads of at most the
threshold; larger results must be read with
range.prompts, same offered tools.
CLERUM_WORKFLOW_TOOL_SPILLOVER_BUDGET_BYTES, default 16 MiB,1 MiB to 256 MiB) caps the result bytes the store holds. A result that would take the total
over the budget stays inline, as without spillover; a later result that still fits is
stored. The budget never stops or fails the step.
aborted or timed out). Two executions of the same step never see each other's results. If
the mcp-host process dies, the results are gone with it; there is nothing to clean up.
store adds about 7% over inline mode (measured: +17.2 MB for a full 16 MiB budget, +16.0 MB
inline) and frees it at step end. One mcp-host serves all concurrent runs of a recipe and
/executehas no concurrency cap, so memory grows with the number of concurrent stepexecutions, as it does without spillover. The pod limit (2Gi) bounds it per recipe.
toolsCalledkeeps the original result, so artifact detection and the status report areunchanged.
clerum__spillover_readcalls record the ref and byte count, not the content.clerum_workflow_step_spillover_persisted_total{tool}andclerum_workflow_step_spillover_reads_total{with_range}. The first-party metrics(
clerum_spillover_persisted_total,clerum_spillover_reads_total,clerum_tool_output_bytes) keep their names and labels and do not receive workflow stepresults.
CLERUM_WORKFLOW_TOOL_SPILLOVER_ENABLED=falseis the operator kill switch and restores theprevious behaviour. It is a separate variable from the first-party
CLERUM_TOOL_SPILLOVER_ENABLEDbecause threshold and retention differ.Merge order: the workflow-recipes change that adds these variables to
MCP_HOST_ENV_PASSTHROUGH(#1453) is merged and deployed in the dev cluster, anddevismerged into this branch, so the kill switch reaches recipe pods.
Owner decisions (2026-11-14):
allowedTools.include, the read tool becomes callable once a result has beenspilled. Accepted as a deliberate exception to "the allowlist is the complete toolset".
A result below 128 KiB stays inline; a larger one is read back in slices of at most
128 KiB, so one read can add up to 128 KiB to every later iteration of the step.
Blocking before merge: the validation below, run by a maintainer first on the local
Minikube T2 lane and then in the dev cluster, comparing off / 128 KiB / 32 KiB. Results to be
attached here.
Also for owners: per-execution budget (16 MiB) and dropping stored results at step end.
Decided 2026-11-13: stored results live in the mcp-host process memory, not on the output volume.
Known limit: this bounds each result, not their sum. Context still grows with the number of
calls until steps run on the agent loop with compaction. When that lands, revisit lowering
the threshold to the first-party 8 KiB.
Validation script (run by a maintainer; cloud sessions cannot run it)
Prerequisite (met): the
MCP_HOST_ENV_PASSTHROUGHchange (#1453) is merged anddevismerged into this branch, so the three variables set on the WRC Deployment reach recipe
mcp-host pods.
Stage 1 — local Minikube T2 (see
AGENTS.mdanddocs/testing/minikube-t2-runbook.md;host terminal, owned branch profile):
make minikube-t2-preflight MINIKUBE_PROFILE=<owned-profile>make minikube-t2 MINIKUBE_PROFILE=<owned-profile>and keep the exact-head T2 verdict.Stage 2 — dev cluster: repeat the matrix once Stage 1 is accepted.
Matrix. Two recipes: (a) a code-review recipe whose agentic step reads files and diffs
through MCP tools; (b) a recipe that reads one document of about 100 KiB and summarizes it.
Three configurations, set as environment on the WRC Deployment (new runs pick them up):
CLERUM_WORKFLOW_TOOL_SPILLOVER_ENABLEDCLERUM_WORKFLOW_TOOL_SPILLOVER_THRESHOLDfalse32768Runs: for (a), three inputs (small, medium and large change), five runs each per configuration
in the dev cluster and two each on Minikube; for (b), five and two.
Record per run:
Step reached max iterations);iterationin theLLM call startedlog);LLM response receivedlog, ortokensUsedin the step result);clerum_workflow_step_spillover_persisted_total,clerum_workflow_step_spillover_reads_total, or the step logs);bodyBytesin the coordinator logs, when available);covers the end of the document;
.spillentry under<output dir>, and no log line with result content.Accept the default with a given threshold when all hold:
"off" for the large one;
If 128 KiB fails and 32 KiB passes, the default becomes 32768. If neither passes, the default
goes back to
false.Testing
npm testpasses for changed servicesnpx tsc --noEmitcleanRun on Node 24.22.1 at
e9a30777a:npm run typecheck(mcp-host): pass.eslintandprettier --checkon the changed files:clean.
stepSpillover.test.ts(40),stepSpilloverInMemory.test.ts(12),workflowServiceSpillover.test.ts(16),artifactOutput.test.ts(26),spilloverRead.test.ts(8),toolUseLoop.spillover.test.ts(7),
core/spillover(5 files, 34).npm test(mcp-host): 609 files, 9115 passed, 1 skipped, 1 todo.mcp-host/src/core/spillover,artifactOutput.tsand its test are identical to themerge-base with
dev(git diff --quiet 1b1b1131e).releasenot called, resultwritten to disk, one store for all executions, summary drift, call id not checked,
first-party read counter default, step read counter, default threshold back to 32 KiB,
spillover off by default against the 100 KiB inline test): all caught.
scripts/minikube/t0.sh:T0_AFFECTED_PACKAGES=mcp-host,T0_AFFECTED_CHECKS=PASS,exit 0.
Checklist
Generated by Claude Code