feat(tools): keep both ends of oversized tool output and spill the rest - #44
Open
mike-diff wants to merge 2 commits into
Open
feat(tools): keep both ends of oversized tool output and spill the rest#44mike-diff wants to merge 2 commits into
mike-diff wants to merge 2 commits into
Conversation
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.
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.truncatecut at 30000 chars and kept the head. It fires on this repo'sown test command:
The lost 745 bytes are
PASSandok github.com/mike-diff/sesh/harness. Theagent could not tell whether the suite passed.
Neither end alone is sufficient. On a generated 42127-byte failing run:
boom: nil map write at store.go:88FAIL<TAB>sigprobe/pkgThe 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 itsfirst 300 characters. End to end, for that failing run:
The judge deciding whether the request is
doneread an unbroken wall ofPASSlines 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:
The pointer names the elided line range, so
read's existingoffsetpaginglands 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.readableSeshDatagains the newdirectory: a pointer the read tool refuses is worse than no pointer, and every
path under
~/.seshwas refused before (-unsafe-pathsdid not help). Mutationstays refused, because
write/edit/bashuseconfine, notconfineRead.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'selidedrops the middle "not the tail, so a large change stillshows where it starts"
renderTranscript's whole-transcript cut keeps head 1/3 + tail 2/3proc'slogsTextkeeps the tail and reportsN earlier lines elidedAlso reused:
storeBlob's atomic write-then-rename,gcBlobs's conservativesweep,
procManager's locked id allocation,ring.append's tail eviction.Core stays pure
No behavior added to
agent/. The opposite:agent/previously owned atruncation 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 bydiff_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
-prun, not just unit tests: 95KBof 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:
cappedBufferbash-raceonlygofmtclean,go vetclean,go test -race ./...green, fullSESH_E2E=1suite 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 routesthrough the shaper, as production does).
Two notes for review
This branch carries one CI commit.
-racewas added to the test stepbecause
TestOutStoreAllocatesDistinctIDsConcurrentlyonly fails under it, andby this repo's own rule a test that cannot fail proves nothing. That required
updating the bar
AGENTS.mddeclares and the command inREADME.md, since bothasserted CI enforced exactly three commands. The bar is still three commands:
go test ./...becamego test -race ./.... It is a separate commit andrevertible 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/childToolsas a parameter. It is apackage-level var instead, matching
tuneandactiveConsole, because thatcovers 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
headCutcuts mid-line rather than returning nothingwhen no boundary exists.