Skip to content

feat(tools): keep both ends of oversized tool output and spill the rest - #44

Open
mike-diff wants to merge 2 commits into
mainfrom
feat/recoverable-tool-output
Open

feat(tools): keep both ends of oversized tool output and spill the rest#44
mike-diff wants to merge 2 commits into
mainfrom
feat/recoverable-tool-output

Conversation

@mike-diff

Copy link
Copy Markdown
Owner

Oversized tool results were cut head-first and the discarded bytes were gone.
The measured consequence is worse than losing information: it inverts the
evidence the drive judge rules on.

The defect

agent.truncate cut at 30000 chars and kept the head. It fires on this repo's
own test command:

go test -v ./harness/   = 30745 bytes   (cap 30000)

The lost 745 bytes are PASS and ok github.com/mike-diff/sesh/harness. The
agent could not tell whether the suite passed.

Neither end alone is sufficient. On a generated 42127-byte failing run:

Signal Position Head-only cut Tail-only cut
boom: nil map write at store.go:88 51% in survives lost
FAIL<TAB>sigprobe/pkg 30 bytes from end lost survives

The diagnostic and the verdict sit at opposite ends.

Why it mattered more than it looked

The judge reads renderTranscript(_, 300), which elided each result to its
first 300 characters. End to end, for that failing run:

raw bash output     =  42127 bytes, contains FAIL verdict = true
after core truncate =  30000 bytes, contains FAIL verdict = false
judge view          =    444 bytes, contains FAIL verdict = false
judge sees 'boom'   = false

TOOL RESULT: === RUN TestAlpha1 · --- PASS: TestAlpha1 · === RUN TestAlpha2 · ...

The judge deciding whether the request is done read an unbroken wall of PASS
lines produced by a run that failed. That degrades the drive loop specifically,
which is the harness's headline behavior.

Fixing the tool result alone was not enough; the transcript elision was a second,
independent head-bias and had to change too.

The change

An over-budget result keeps its head AND its tail around an elided middle, and
the full text spills to a file the model can read:

... [lines 266-2801 of 3602 elided (67287 bytes); full output:
     ~/.sesh/out/20260819-220743-41779445/out-1.log (read it with offset 266)]

The pointer names the elided line range, so read's existing offset paging
lands directly on what was dropped. The split is a heuristic; the spilled file is
the guarantee.

Storage is keyed by chain root, not session id, so a handoff renames nothing
and pointers already in the transcript keep resolving. Verified stable across
three handoffs by driving the real seedChain. readableSeshData gains the new
directory: a pointer the read tool refuses is worse than no pointer, and every
path under ~/.sesh was refused before (-unsafe-paths did not help). Mutation
stays refused, because write/edit/bash use confine, not confineRead.

Reuses shapes already in the tree

This makes existing conventions agree rather than adding one. The repo already
elides from the middle in three places, and only the tool-result layer got it
wrong:

  • diff.go's elide drops the middle "not the tail, so a large change still
    shows where it starts"
  • renderTranscript's whole-transcript cut keeps head 1/3 + tail 2/3
  • proc's logsText keeps the tail and reports N earlier lines elided

Also reused: storeBlob's atomic write-then-rename, gcBlobs's conservative
sweep, procManager's locked id allocation, ring.append's tail eviction.

Core stays pure

No behavior added to agent/. The opposite: agent/ previously owned a
truncation direction, which is policy. It now keeps only a byte ceiling, and
the shape decision moved to harness/ as a decorator applied at tool assembly.
That covers built-ins, engines, tool mods, MCP, and subagents through one path
with no signature churn.

Untouched because each already shapes itself: read (paging footer), search
(suppression with counts), loc, write/edit (diff bounded by diff_lines),
proc logs, recall, task.

Dials

Five in tuning.json, documented in the shipped example and -help:
result_max_chars (28000), result_head_pct (25), result_spill_off (false),
result_keep_days (7), transcript_result (300).

New mount point ~/.sesh/out/, reported by -doctor.

Verification

Validated through the real binary in a real -p run, not just unit tests: 95KB
of failing-test output shaped to 27954 bytes, both ends intact, pointer
resolving, and the judge asserted to see the verdict from the captured judge
request.

Every test was run against a surgically reverted behavior to confirm it fails:

Breaker Result
head-only tool-result shaping fails
head-only transcript elision fails (unit and e2e)
no confinement carve-out fails
head-keeping cappedBuffer fails
no shaping on bash fails (e2e)
unlocked id allocation fails under -race only

gofmt clean, go vet clean, go test -race ./... green, full SESH_E2E=1
suite green. CI green on this branch.

Three pre-existing tests asserted the old head-biased behavior and were updated
with breaker comments rather than deleted: TestCappedBuffer,
TestRenderTranscript, TestMCPBigOutputTruncatedLoudly (the last now routes
through the shaper, as production does).

Two notes for review

This branch carries one CI commit. -race was added to the test step
because TestOutStoreAllocatesDistinctIDsConcurrently only fails under it, and
by this repo's own rule a test that cannot fail proves nothing. That required
updating the bar AGENTS.md declares and the command in README.md, since both
asserted CI enforced exactly three commands. The bar is still three commands:
go test ./... became go test -race ./.... It is a separate commit and
revertible on its own; if you would rather it lived with the other CI work, say
so and I will move it.

Design deviation from the plan, recorded deliberately: the plan threaded the
output store through builtinTools/childTools as a parameter. It is a
package-level var instead, matching tune and activeConsole, because that
covers subagents with zero signature churn across tool assembly and its tests.
Tests state their reset contract via withSpill.

One edge case handled and recorded: a single-line file larger than the read cap
cannot be paged by line offset. Spilled command output is always multi-line so it
is unreachable here, but headCut cuts mid-line rather than returning nothing
when no boundary exists.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant