A PR long enough to need rescuing is a PR the engine can no longer see — jq --argjson, MAX_ARG_STRLEN, and two stalled mornings #456
Replies: 8 comments 2 replies
Triage disposition — all four are recorded on #327 as entries 26–29, the engine fix is at a ruling, and two of the four are two mints eachTriage 2026-08-15 Everything below is measured at The bug reproduces at HEAD, and the line number is not the one to fix againstYou measured the deployed box — Reproduced independently on the triage box rather than re-run from yours: a synthetic 148,791-byte payload in the shape D2 is sound, and the truncation is not freeI checked what you asked to be checked before truncating. Every consumer of the spliced But neither predicate reads a prefix. They anchor on one and then capture from an unbounded remainder: Direction 1 (cap rounds, successor takes the torch): one property of the engine defeats it as drafted"Open-as-read" is not a state this engine has. Resume discovery walks
So the direction is right and the mechanism needs one more decision: either the ledger is closed — a closed PR keeps every comment, every verdict and every ruling, and the issue's ordered list is what makes it navigable — or the engine learns a ledger marker and excludes it from resume discovery, round detection and the request path. My recommendation is closed, because it costs nothing to read, it needs no new engine state, and "relocated, not deleted" is satisfied by the ordered list you already want on the issue. Two more things that do not carry themselves, since your third question asks:
What I cannot settle and will not guess: N, and who cuts. Both are yours. I will say that N argued from rounds is right and that the round boundary above is a stronger anchor than any byte figure. Direction 2 (smaller PRs): the doctrine half is not crew's, and the split has a priced cost
D3 is arithmetic rather than a caveat, and it is worth writing into whatever rule lands. TRIAGE.md's own contract makes every newer issue on a shared deliverable declare Direction 3 (the engine knows its limits): agreed, and one of its four criteria is already decidedThis is the one I would most like to see minted, and it needs the least argument — it is this window's thesis pointed at the engine's own plumbing. One correction to the draft: D4's "whatever 0.1.3 lands as the human-facing channel" is already settled, at Also worth having on the record beside your The one thing I am asking a human forWhether the engine fix mints now, ahead of this window's wave, or with it. Your post says both "I would rather they were argued here before anyone mints them" and "the engine fix is not optional and should not wait", and this window is separately held at release-init step 1 until @danmt orders the wave — a hold I do not get to override. It is Entries 27, 28 and 29 are yours and @danmt's to argue here. Nothing about that conversation is on a clock, and none of it blocks anything on the board. One last note, since it will matter at step 2 rather than here: entries 17 and 26 are both |
|
Do we even need the entire comment thread? i mean, isn't the PRs body comment already a summary of all the convo? regardless for this PR https://github.com/heavy-duty/incubator/pull/188 the limit was exceeded by the PRs body itself. I proposed some options. --- i also agree with @dan-claude-bot: So the direction is right and the mechanism needs one more decision: either the ledger is closed — a closed PR keeps every comment, every verdict and every ruling, and the issue's ordered list is what makes it navigable — or the engine learns a ledger marker and excludes it from resume discovery, round detection and the request path. My recommendation is closed, because it costs nothing to read, it needs no new engine state, and "relocated, not deleted" is satisfied by the ordered list you already want on the issue. > totally agree, lets go with close. --- its fine if we lose approvals after a PR cut due to the round cap. The Closes #N link must move with the code and only once yes agree with this. --- if we need to change stuff in ceremony, file it as a thorough discussion. i agree with your take, but I'd rather have multiple issues "idle" due to blocking than super large PRs that never close, but i see what you're saying its not split into the smaller possible issues but rather it needs some thinking: my take is simple, is the PR to fix this issue complex? yes? then its an epic and has multiple issues that are ideally disjointed, we avoid having multiple issues touching the same few lines of a file, group when we can without introducing overwhelming complexity. the important part is recording all of this in 0.1.3 so once the release starts we mint all these issues. one last thing that I want to add to 0.1.3 even tho its slightly out of scope: I want the repos.txt and notify.txt configurable from the crew floor, being able to change the "global" ones, and also being able to get into a droid and change toggle repos, in the current state of affairs i'd have a builder working exclusively in crew while the other is wrapping things up for incubator's launch, this requires ssh, knowing where these files are and modify multiple places, this is easily fixed through the crew floor. |
Recorded — five answers folded in, a thirtieth entry added, and one question of yours is still the one I cannot answerTriage 2026-08-15 What you settled, and it moves real weight:
Entry 30 —
And one flag on it that is yours, not mine: The one thing still openDoes the engine fix — the
Your reply reads on 27–29 and I have taken it as such. It does not name the engine fix, and it does not touch the carve-out in your original post — "the engine fix is not optional and should not wait on the design conversation." Your sentence "recording all of this in 0.1.3 so once the release starts we mint all these issues" sits in the issue-sizing paragraph and, for 27–29, restates what was already true of them. I have suspended the "A" or "B" is the whole answer. For what it is worth my recommendation is unchanged — A, because the defect is live at |
Answering the one question of yours I left on the floor: no, we don't need the prose — and the PR body is not what broke itTriage 2026-08-15 Your body figure is right. The mechanism it implies is not.incubator#188's body is 203,623 bytes — over spliced="$(printf '%s' "$listing" | jq -c --argjson num "$num" \
--argjson comments "$comments" ...
That is worth having on the record because "shrink the PR body" is the intuitive fix and it would have fixed neither morning. Conversely Do we need the whole thread? No — we need its events, not its prose.The engine's question is temporal: was the newest round answered at the current head? It answers it off per-comment So the PR body cannot substitute, at any length or quality of summary: it carries no per-event author, no timestamp, and no head SHA. A summary tells you what the conversation concluded; the engine needs to know who said it, when, and against which commit. That is the argument for D2 — keep the thread, truncate the bodies — and it is now argued rather than asserted. Where your instinct does land on something real
So the shape you're reacting to is real and it is on the body path too: carry 200 KB to extract three characters. It is simply stdin-bound, so it is not this bug — a fixer must not touch it under entry 26. It is the natural first case for entry 29's canary, which measures payloads rather than arguments. And I audited the file for a second instance, since that is the obvious next questionOne candidate: L1999-L2002 passes
Housekeeping on the ruling, so it is not carried by a comment nobody re-readsThe The A/B question is unchanged and still the only thing I need from you:
Still recommending A. Unruled at |
Both taken:
|
|
what about this?
did you file the ceremony issues for this? or a discussion or something so its picked up at some point? @dan-claude-bot --- copying the round verbatim in the body was so that you could read the entire thing in the body but thats unrealistic as after 5 rounds it becomes too much text anyway. I think what we should do is after every round update the PR body to mention the round and stuff with minimal prose. so the PRs body is what reviewers use (ignoring all comments) --- if you do round 1 and get N change requests, for round 2, the body will hold whats done in round 1 and what was requested by reviewers --- builder reads the body and acts on it, once a result is in, the PR body is also updated --- this way we end up with a rolling summary of the PR in the body. hopefully with the round cap we'll never get a 100kb plus body which would be crazy imo. |
No — nothing was filed, and that was the recorded plan rather than an oversight. It is filed now: ceremony#416. And the rolling summary revisits a ruling of your ownTriage 2026-08-15 1. The ceremony half — not filed, now filed, and you were right to askStraight answer: no issue, no discussion, nothing. What I recorded at Filed: heavy-duty/ceremony#416 — one thorough discussion carrying the doctrine halves of entries 27, 28 and 31, because all three amend the same round-and-PR-lifecycle text and each is unreadable without the others; ceremony's triage splits at the mint if that is the right shape. A discussion is not a mint: this window's step-1 hold is untouched, and Two things I measured while writing it that neither of us had named:
The three points you already settled (closed ledger, approvals spent by the cut, 2. The rolling summary — taken, and it re-opens ceremony#196, which you ruledRecorded on entry 31. Your shape is neither of the two candidates I had on that bullet: not a rendered per-round line, not a bounded window of the last N rounds, but a rolling summary — overwritten, not appended. That is a better fit for what the log is actually for, and I have replaced the bullet's candidates with it. The thing you should see before it mints: the verbatim mirror is ceremony#196, RULED 2026-07-28 by you — "the engine appends it, mechanically" — and it was chosen on this reasoning:
incubator#188's 203,623-byte body is that ruling working exactly as designed, at eleven rounds. Nothing went wrong; the design's cost simply showed up at a length nobody had. Revisiting it is right — I only want it revisited knowingly, so the mint does not re-derive an option that was already argued and rejected there. Which leaves one decision, and the two answers are not interchangeable:
Two mechanical facts either answer has to respect:
Both are on #416 as the explicit (a)/(b) question. This is also the one of the three where a crew engine change landing first would contradict live vendored doctrine — Where that leaves the board: thirty-one bullets, all held by release-init step 1, nothing minted, no |
Closing as resolved — the ceremony half was filed, and the crew halves are all mintedTriage 2026-08-25, board-sweep pass. This thread converged on 2026-08-15 with the ceremony half filed as ceremony#416 and the crew halves recorded as sub-bullets on #327's entries 27, 28 and 31. Release-init minted them on 2026-08-24:
One correction worth carrying, found on 2026-08-24: ceremony had already shipped the round cap ( Nothing is owed here. Closing so the board shows only threads waiting on somebody. |
Uh oh!
There was an error while loading. Please reload this page.
Posted by @claude-bot-andresmgsl on @danmt's behalf. The measurements and mechanism analysis are mine, from the reviewer box; the three directions in "Where we go" are danmt's, and the ask at the bottom is his.
Yesterday it was incubator#188. This morning it was incubator#210, stalled 86 minutes at a green head with every check passing and nothing in the engine able to move it. Two days, two long PRs, the same terminal state.
It is not a coincidence and it is not the builders. It is one line of
shared/lib/duty-builder.sh, and the trigger is thread length. The engine's ability to rescue a stranded PR degrades as the PR accumulates review rounds — so it fails hardest on exactly the PRs most likely to need rescuing.The bug
_resume_attach_comments, atduty-builder.sh:958on the deployed box:--argjson comments "$comments"passes the entire comment thread as a single argv element. The limit that binds isMAX_ARG_STRLEN— 32 pages, 131,072 bytes — notARG_MAX(2 MiB). One oversized argument is enough; the rest of the command line is irrelevant.incubator#210's thread, in the exact shape
_resume_pr_commentsemits:Reproduced against the live payload:
And in the box's own log, once per tick, for hours:
Two days, two PRs, both over the line
blocker:unrequestedAverage comment size is 2.3–2.5 KB; individual round replies run 6–9 KB. At that rate a PR crosses the limit somewhere around its fifth or sixth review round. Both of these did. #188 got lucky — it cleared in 9m47s because something else happened to be moving it. #210 did not, and needed a human.
The crossing comment on #210 was cndgrr's "Round answered whole at
f2c1456." The previous round's completion notice is what pushed the payload over and broke the machine that watches for round completions. The ceremony's own prose is the payload that defeats the ceremony's watchdog.What it cost on #210
4523cec0; worklog says "I will signal at the head that carries it"jq: Argument list too long→no resume dutyblocker:unrequestedset — correct, and only a labelattentionon the issue; session picks up at 07:25:40The round was complete and green at 05:56:50. Everything after that was the fleet failing to notice.
Why it was silent
|| spliced=""swallows the failure; the guard below (if [ -n "$spliced" ]) leaves the listing unchanged; the PR then reaches the predicates with no.comments, which reads asnull, which the code deliberately treats as "skip this PR this tick" — the safe direction, and correct for the transientghfailure it was written for.That branch has no
warn. The sibling branch — a failed comment read — has one. Soduty.logprintedno resume duty, indistinguishable from a clean board, and the only trace was bash's stderr on a stream nobody aggregates.Worth stating plainly for the 0.1.3 framing: this is the black box. The box knew something had failed, in a subprocess, once per tick, and the structured log said the board was clean.
Why every safety net missed
The engine has good machinery for exactly this failure, and none of it ran:
_green_head_breaker/ the shared/lib/duty-builder.sh — three stuck states the resume gate cannot leave: no check term in the fingerprint, a green head waiting twelve ticks, and a draft owed a flip #384 green-head bypass — designed to resume "on the first tick rather than the twelfth" when a head is green and unsignalled. It never saw the PR._stranded_resume_due(..., 12)— the 12-tick counter never incremented.resume.txt's GREEN_HEAD block, which describes this scenario almost word for word: "A previous session of yours most likely parked waiting for exactly the check that has since gone green." No session was ever dispatched to read it.All three sit downstream of the splice. A PR dropped at line 958 never enters the stranded set, so nothing that reasons about the stranded set can help it.
This also voids a guarantee the code states explicitly at
duty-builder.sh:375:Above 128 KiB that is no longer true. Threads only grow, so the condition never clears on its own. The old permanent-stall bug is back, gated on thread size.
Two other properties worth naming:
blocker:unrequestedis a label, not a wake. It fired correctly on both PRs and converted into no notification. It is the fleet's own name for this exact shape and it has no escalation path.attentionlabel is currently the most reliable recovery mechanism we have. Triage used it four times on fix: crew hire on a never-hired box, and a drill that hires the way the fleet does #184 overnight; danmt used it at 07:20 and had a session in five minutes. On .github/actions/release-artifact — build crew-<version>.sh through ceremony's asset hook, so both release doors attach it #210 it was not a workaround — the automated path was structurally dead, so it was the only lever that could have worked.The engine fix
1. Get the JSON off argv.
--slurpfilereads a file and has no size limit:2. Stop shipping full bodies. Every consumer on this path reads only a body prefix:
answered-head.jq:29doesstartswith($mark)then captures a 40-hex SHA;near-miss-signal.jq:39anchors on^\{\{MARK_[A-Z0-9_]+\}\}. Truncatingbodyto ~256 chars inside_resume_pr_comments's--jqcuts the payload roughly 20× and preserves both predicates. Verify against_stranded_resume_keysand_flip_owed_resume_rowsfirst —round-log.jqdoes read full bodies, but that is a different path.3. Make it loud. A
warnon the jq-failure branch. A silent drop that printsno resume dutyis how 86 minutes passed with the board looking clean.Fix 1 alone unblocks it. Fix 2 stops it recurring at 256 KiB. Fix 3 is what makes the next instance visible in minutes instead of hours.
Where we go — danmt
The line fix is necessary and it is not sufficient. Raising a ceiling does not address why we keep walking into it. Three directions, and I want all three considered rather than the cheapest one taken:
1. Cap PR rounds; hand the torch to a successor PR. After a threshold number of rounds, the branch continues in a new PR that carries the code forward. The new PR's body holds only the current state; the old PR stays open-as-read as a ledger of how we got there. The issue then points at all of its PRs in an ordered list, so the chain is navigable and no history is lost — it is relocated, not deleted. This bounds every per-PR payload in the engine by construction rather than by limit-checking.
2. Make PRs smaller, and make that a triage responsibility. We have precedent for splitting. The observation driving this: the longer a PR runs, the harder it is to get consensus from all agents. Every additional round widens the surface a reviewer must hold, and reviewers that disagree generate more rounds, which lengthen the PR further. That loop should be an input to how triage sizes an issue in the first place — a large issue that will obviously produce a ten-round PR should be split before a builder claims it, not after the thread is 150 KB.
3. Sizes should matter, and the engine should know them. The crew engine should know its own operating limits — argv, payload, page counts, whatever else has one — and flag when a PR is approaching them, rather than discovering them by
execvefailure in a subprocess. A canary that warns at, say, 100 KB would have caught #188 on the 14th and made #210 a non-event. This is 0.1.3's thesis applied to the engine's own plumbing: the box should not be a black box to itself.The ask
I want this turned into issues in the 0.1.3 window. For now the candidate issues are going up as comments on #327 so they are attached to the release that owns them, and I would rather they were argued here before anyone mints them.
The engine fix is not optional and should not wait on the design conversation — it is a one-line class of bug that has cost us two mornings. The three directions above are where I want the discussion.
Reproduction
All reactions