Repository navigation
VC-824: Keep ticket list under the CLI response limit and name an oversized answer - #934
Merged
hussainph merged 4 commits intoOct 19, 2026
Conversation
…very fixture (VC-778) The fixture set followUpAdmissionTimeoutMs: 50 for the whole runtime, but that bound covers every adapter.attach (workspace + executor stages) and every cold binding rebuild, not only queued messages. A slow but healthy real Pi attach under CI load then came back rejected with queue_admission_timeout. Genuine work now keeps the production 30 s bound; the 50 ms bound applies only once the fixture enters an attach/admission gate. Adds a regression test with a real slow attach, and receipts in the accepted-status assertion messages.
…rsized answer The ticket list table asked for every ticket body, which it never prints; on a long-lived board that answer passes the CLI's 4 MiB limit and failed as SOCKET_PROTOCOL with a build-compatibility hint. - ticket.list accepts body: false and omits body from each row; the human table sends it, --json keeps full rows unless --no-body is passed. - An answer past the limit is RESPONSE_TOO_LARGE, naming the verb; for ticket list it suggests --status, --limit, --no-body and ticket show. - Record both in the capability changes (volli help changes).
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…limit constant - Lead ticket list's overflow advice with --status/--limit, which shrink the answer from any app build; --no-body only helps with a matching app. - The advice lives on the verb's registry entry (overflowNext), read by the socket client through verbEntry, not a per-verb branch in the client. - AGENT_RESPONSE_LIMIT_BYTES/_LABEL in @volli/shared: the client guard, its refusal, the error guidance and the help-changes record share one value. - ticket-list.test builds the 2,000-ticket board once; run.test covers an older app that ignores body: false.
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.
Stacked on #929 (VC-778 crash-recovery fixture fix, head 637c7d1, test-only). Its head is merged into this branch additively (merge commit 935009c), because the VC-778 flake kept failing
Test (packages)here. Once #929 merges, the diff against main is this PR's own change.Problem
volli ticket listsends every non-archived ticket with its full markdown body (read-verbs.ts,ticketListVerb). The CLI stops reading any answer over 4 MiB (client.ts,MAX_RESPONSE_BYTES). On a long-lived board (about 1,350 tickets at this board's average size, fewer when bodies are long), the command exits 1 withSOCKET_PROTOCOL: The app response is too large.The recovery it prints is "confirm the CLI bundle and running app come from a compatible build", which sends people looking for a version mismatch that doesn't exist. The human table never prints a body, but it was still sent all of them.Fix
ticket.listacceptsbody: falseand leavesbodyout of each row. Every other field stays. Any other non-boolean value isINVALID_REQUEST.body: false(run.ts), so its answer is about a sixth of the size. The table it prints is byte-identical.--jsonkeeps full rows by default. New opt-in flag--no-bodydrops the bodies.RESPONSE_TOO_LARGE(new error code, exit 1), and the message names the verb. Forticket list, the recovery comes from the verb's registry entry (overflowNext). It leads with--statusand--limit, then mentions--no-body, which only helps with a matching app build, andvolli ticket show <id>. Verbs without an entry get the generic "narrow the request with the verb's filters" guidance. The build-compatibility hint is never shown for this case.volli help changeshas a new VC-824 entry recording the flag, the new code, the table change and the fix.Decisions
--jsonrows keepbody, so existing consumers are not moved. Only the human table, which never showed bodies, stops asking for them. I did not add a server-side byte bound with anomittedcount: it would silently truncate the default--jsonanswer. Instead, an oversized--jsonanswer fails with guidance, as the ticket's acceptance allows.bodyLengthor excerpt field on summary rows. It isn't needed to fix this, and it would be one more field to freeze.ticket showreads a body.bodyand answers full rows. The table renders the same as before, with the old size behaviour. An older CLI sends nobodyand gets today's rows. The app never sendsRESPONSE_TOO_LARGE, because only the CLI produces it.pnpm check:protocol-schema --base origin/mainpasses, sinceticket.listis CLI-only and has no published schema.volli boardalso returns bodies in its rows. With this change, its overflow reportsRESPONSE_TOO_LARGEwith the generic guidance instead of the build hint, but it still carries the bodies.Tests
All run single-file with
--maxWorkers=1through the machine's serial runner:packages/host-core/src/agent-dispatch/ticket-list.test.ts(new): drives the real dispatch on 2,000 tickets with 3 KB bodies. Withbody: falsethe frame is under 4 MiB and has all 2,000 rows and no bodies. Default rows keepbody, and that frame is over 4 MiB. A non-booleanbodyis refused. Before the fix it failed (6,557,827-byte frame, and the bad selector was accepted).packages/cli/src/run.test.ts(new block): runsrunCliover a real Unix socket against a server answering with the same 2,000-ticket board.volli ticket listexits 0 and prints exactly the table the full rows would render.--json --no-bodyexits 0 with bodiless rows.--jsonexits 1 withRESPONSE_TOO_LARGE, the verb named, and--status/--limit/--no-bodyguidance, and noSOCKET_PROTOCOLor compatible-build text. Before the fix the table case failed witherror[SOCKET_PROTOCOL] … compatible build.packages/cli/src/client.test.ts: the oversize case now expectsRESPONSE_TOO_LARGEwith the verb named. A generic verb gets the default guidance, andticket.listgets its registryoverflowNext.packages/cli:parser.test.ts,help.test.ts, andcli-reference.test.ts(snapshot regenerated: the new flag, the new error code, the changes entry).packages/shared:agent-product.test.ts(the newest-entry pin moved to VC-824),agent-surface.test.ts,harness/skill-content.test.ts,verb-registry.test.ts.vp checkon all touched files;pnpm check:protocol-schema --base origin/main.Review
Changes from the review of 935009c, in commit 1bc0f54:
ticket listadvice now leads with--status/--limit, which shrink the answer from any app build. It words--no-bodyas "with a matching app build", because an older app ignoresbody. The capability record says so too.overflowNextfield onVerbEntry.ticket.listdeclares it, and the socket client reads it throughverbEntry(request.cmd). The client no longer has a per-verb branch, soboardcan declare its own advice later.AGENT_RESPONSE_LIMIT_BYTESandAGENT_RESPONSE_LIMIT_LABELare exported from@volli/shared. The client guard, the refusal message, the RESPONSE_TOO_LARGEwhytext, the help-changes entry andticket-list.test.tsall use them. The literal "4 MiB" left inclient.test.ts/run.test.tsis the expected output those tests check.ticket-list.test.tsbuilds the 2,000-ticket board once inbeforeAll. The body-selector refusal runs on an empty board.run.test.tshas a new case where an older app ignoresbody: falseon a 20-ticket board. The table is identical to the one rendered from bodiless rows.CI reruns
Test (packages)failed only insession-runtime/crash-recovery.integration.test.ts("reconciles an accepted command with a different persisted timestamp…" and "abandons a pending flat ask durably…", bothexpected 'rejected' to be 'accepted'). That is the known flake VC-778, and this diff doesn't touch session runtime. I reran the failed jobs once.Test (packages)failed only in the same VC-778 test ("reconciles an accepted command…",expected { closed: 1, errors: [ … ] } to deeply equal { closed: 0, errors: [] }). The fix for that flake is VC-778: Bound only deliberately stalled stages in the crash-recovery fixture #929, still open and test-only. I reran the failed jobs once more.Test (packages)failed only inworktree/watch.test.ts("500 Done tickets merged in an earlier launch and 10 open PRs cost 10 gh calls a cycle"). That is the known timeout flake VC-856, unrelated to this diff. VC-856 was outside my allowed rerun list. After the supervising Session approved it, I reran the failed jobs once (attempt 4).Test (packages)failed only in the VC-778 test again ("reconciles an accepted command…",{ closed: 1, errors: [ … ] }), its third failure in four attempts. I have not rerun it again. The plan is to merge origin/main once VC-778: Bound only deliberately stalled stages in the crash-recovery fixture #929 (the VC-778 fix) lands, then re-run.