feat(agents): raise the command-result result cap to 5 MB, matching stdout/stderr - #3283
Merged
Merged
Conversation
… stdout/stderr Follow-up to #3267 (issue #3001), approved after that fix landed. `result` was capped at 1 MiB while `stdout` and `stderr` in the same schema allowed 5,000,000. That split was not incidental — it caused #3001. The backup forwarder assigns its run body to `result` rather than `stdout`, so the payload silently inherited a limit five times tighter than the one it was sized against, and every backup over ~2,000 files lost its terminal result. Making the three equal removes the trap. Value is 5_000_000, NOT 5 * 1024 * 1024: the point is parity with the sibling caps, and rounding up would leave `result` 242,880 bytes looser than `stdout` and re-create a smaller version of the same mismatch. Effect: a snapshot keeps a browsable file index to ~9,500 files instead of ~2,000 (~522 B/entry). The agent's tiered degradation is unchanged and still guarantees the terminal status past that — a 100k-file index is ~52 MB and fits no sane cap. This widens the good path; it does not replace the machinery. Both sides of the mirrored contract move together (schemas.ts and wire/limits.go), along with both literal pins. The two cross-language parsers now also accept digit separators, since the Go literal gained underscores. CommandResultHeadroom stays at 64 KiB: it is an absolute allowance for encoding differences, not a percentage, so scaling it with the cap would have widened it to 320 KiB for no reason. Test fixtures resized so each still exercises the band it exists for — their preconditions caught this themselves rather than going vacuous: - the 4,000-file QA reproduction now asserts it arrives INTACT (it used to be rejected, then degraded); that regression is the raise's whole point - a new 16,000-file fixture (~6.0 MB) keeps the degradation path pinned - the non-object array and websocket-backstop fixtures grew past the new budget; "just under the budget" is now sized off the budget itself - new assertion that `result`, `stdout` and `stderr` caps are equal AND still independent checks, so the inconsistency cannot creep back Refs #3001, #3267. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Deploying breeze with
|
| Latest commit: |
6d5f0df
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9bc9114b.breeze-9te.pages.dev |
| Branch Preview URL: | https://toddhebebrand-raise-command.breeze-9te.pages.dev |
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.
Follow-up to #3267, approved after that fix landed. Refs #3001 — no closing keyword; that issue's residual is already fixed and this only widens the good path.
Why
resultwas capped at 1 MiB whilestdoutandstderrin the same schema allowed 5,000,000. That split was not an incidental inconsistency — it is what caused #3001. The backup forwarder assigns its run body toresultrather thanstdout(agent/internal/heartbeat/heartbeat.go,case TypeBackupResult), so the payload silently inherited a limit five times tighter than the one anyone had reasoned about, and every backup over ~2,000 files lost its terminal result with no log on either side. Making the three equal removes the trap rather than documenting it.Effect: a snapshot keeps a browsable restore file index to ~9,500 files instead of ~2,000 (at ~522 B/entry). Concretely, the v0.104.0 QA reproduction — the 4,000-file run — now delivers its index intact, where it was previously rejected outright and then (post-#3267) degraded to a bare terminal status.
The agent's tiered degradation is unchanged and still guarantees the terminal status past that point: a 100k-file index is ~52 MB and fits no sane cap. This widens the good path; it does not replace the machinery.
Value: 5_000_000, not 5 * 1024 * 1024
Deliberate. The point is parity with the sibling caps, and rounding to
5 * 1024 * 1024would leaveresult242,880 bytes looser thanstdout— a smaller version of the exact mismatch being fixed. Both sides carry a comment saying so, and a new test asserts the three caps are equal.Changes
Both halves of the mirrored contract move together, plus both literal pins:
apps/api/src/routes/agents/schemas.ts—MAX_COMMAND_RESULT_BYTES1_048_576 → 5_000_000agent/internal/wire/limits.go—MaxCommandResultBytes1024 * 1024→5_000_0005_000_000as5until its character class was widened. Caught by the pin itself.CommandResultHeadroomstays at 64 KiB: it is an absolute allowance for JSON re-encoding differences (HTML escaping, number formatting, string quoting), not a percentage of the payload. Scaling it with the cap would have widened it to 320 KiB for no reason. Documented at the constant.result_bounds.go,wire/limits.go,schemas.tsand the test narratives. Historical references are kept where they describe what actually happened, and marked as history.Fixtures: each still exercises the band it exists for
The self-checking preconditions from #3267 did their job — every stale fixture failed loudly with an explicit message rather than going vacuously green. Two rounds of that caught both the resize list and a wrong per-entry estimate of mine (
buildLargeRunJobencodes ~375 B/entry, not the field report's ~522).TestFourThousandFileRunSendsItsIndexIntact— the QA reproduction, inverted. It now asserts the result arrives whole, and guards that the fixture is still over the original 1 MiB cap so it keeps representing the payload that reproduced [Agent] Successful backup reported as stalled — 64MB result payload exceeds the 16MB IPC cap, result is dropped, reaper fails the job #3001. A regression to either earlier behaviour (rejected, or degraded) is invisible without this.TestOversizeIndexIsDegradedForTheServerCap— new 16,000-file (~6.0 MB) fixture keeping the degradation path pinned, in the band only the server cap owns (over the ~4.93 MB budget, under the ~15.9 MiB IPC budget). Without it the raise would have silently deleted coverage of the entire reason the tiers exist.equals the stdout/stderr caps in the same schematest, plus one asserting the caps remain independent checks — a message may legitimately carry a full-sizestdoutand a full-sizeresult, and a refactor folding them into one shared budget would break that.Confirmations requested
maxPayloadis configured on the agent WebSocket server, sowsapplies its 100 MiB default — 20x the new cap.MAX_PRECISE_RESULT_MEASURE_BYTESis 8,000,000, above the new 5,000,000 cap, so a rejected result is still measured precisely rather than falling into the unmeasured branch.Swept for other 1 MiB assumptions: the only nearby hits are unrelated —
CRITICAL_RESULT_STDOUT_MAX_BYTES(a separatestdoutlimit on the critical-result validation path) andTERMINAL_BYTES_LIMIT. Neither touchesresult.Verification
go test -race ./internal/wire/... ./internal/websocket/... ./cmd/breeze-backup/...go test -race ./...(whole agent module)go vet ./...gofmt -lon touched packagesGOOS=windows/GOOS=linux/ native buildsvitest run— schemas, schemas.commandResult, agentWs (×3), commands, backupResultPersistencetsc --noEmit(apps/api)🤖 Generated with Claude Code