burn: codex's usage-limit message on stderr now marks the tank dry with its reset (#81) - #86
burn: codex's usage-limit message on stderr now marks the tank dry with its reset (#81)#86cverorg wants to merge 8 commits into
Conversation
A codex run that hit its limit ended as "no fresh artifact and no
limit" with reset null, because the limit text came only on stderr
("ERROR: You've hit your usage limit … try again at Sep 13th, 2026
2:13 AM") and the classifier read the exec stdout stream alone; no
dry_store marker was written and the board kept the tank green.
burn now feeds the captured stderr through the same classifier as
stdout (after the same prompt redaction); the classifier accepts
codex's "ERROR:" transport prefix; the exact stderr line is a fixture
whose reset parses in UTC and in Asia/Tokyo. Ordinary stderr keeps
#66's fast-failure reason. The obsolete "exec-stdout-only" note in
home.sh is corrected.
Built by codex (gpt-6-astra) on tank crazy; report in the lane worktree.
… (P2-1) lib/commands/burn.sh:2567-2571 appended a second, unbounded cat of $stderr_file to out_for_class and re-ran _burn_redact_full on it. $out already carries stderr merged in via 2>&1 on every capture path (_burn_capture_stderr tees fd 2 back to fd 2 while writing the file), so this hunk was dead: A/B (hunk vs no hunk) produce byte-identical --json output on the same stub, and a 16 MiB stderr stream is ~45% slower with the hunk (8.62s vs 4.82s, this host, this run) for zero behavioural difference. #81's actual fix is limit.sh:130's anchor accepting codex's ERROR: transport prefix. Delete the hunk, document at the call site that stderr already arrives merged and where, and correct CHANGELOG.md's description of the mechanism. No new bats test: the existing '#81: stderr-only codex limit ...' test (stub writes to fd 2 only) already proves the classifier still sees a stderr-only line with the hunk gone, and stays green. Review: REVIEW-stderr81-r1.md P2-1. Co-authored-by: KITT <kitt@cver.net>
limit_codex_output_dry matched the vendor sentence anywhere in the buffer, so a healthy codex task that happens to quote '#81's own real sentence verbatim mid-transcript — summarizing this very issue, for instance — and keeps working afterward was misread as dry. A genuine vendor limit line terminates the run: it is never followed by pages of ordinary output. Restrict the anchor grep to the last 20 lines of the reply; the caller (burn.sh) only reaches this path once the artifact check has already failed, so the 'no artifact' half of the guard was already structural. bats (burn.bats): a stub that prints the exact sentence as line 100 of 200 and then writes an artifact stays a plain success, no dry marker (red before this commit — confirmed by re-running it against the pre-patch grep, see lane receipts); the same sentence as the LAST line with no artifact still fires dry (control, unaffected either way). Review: REVIEW-stderr81-r1.md P2-2. Co-authored-by: KITT <kitt@cver.net>
limit_codex_output_dry handed the WHOLE reply to limit_codex_reset,
which takes grep's first 'resets …'/'try again at …'/'reset at …' hit
anywhere in the buffer. An unrelated sentence earlier in the engine's
own stdout ('the schedule resets Monday morning, so try again at 9:00
PM if unsure.') became the marker instead of the genuine vendor phrase
that anchored the dry verdict, and that decoy text almost never parses
as an epoch — #75's 'a passed reset is not dry' logic then never
fires, so the tank stays red until the marker's own TTL.
Add _limit_codex_anchor_line: isolate the specific line (of the
already tail-restricted buffer) that satisfied the anchor, and hand
ONLY that line to limit_codex_reset. A vendor sentence wrapped across
two physical lines ('… try again at Sep 13th, 2026 2:13\nAM.') is
joined with its neighbour first — the anchor's own clause is short
enough to match on one line alone even when the trailing reset phrase
wraps, so a wrap-unaware read would silently truncate the phrase and
fail to parse an epoch; only a line that does NOT already end in
terminal punctuation is ever glued to what follows or precedes it.
bats (limit.bats): the reviewer's decoy no longer wins; a wrapped
sentence now yields the full phrase and parses (both confirmed red
against P2-2's commit, green after this one).
Review: REVIEW-stderr81-r1.md P2-3.
Co-authored-by: KITT <kitt@cver.net>
….bats assertions (P3-4, P3-2, P3-5, P3-6)
P3-4: the anchor's only letter-bearing prefix allowance was the literal
'ERROR:' — a special case for codex's current transport wording, not a
description of the noise class. A real capture commonly carries an ISO
timestamp and/or a single bracketed tag ahead of that ('2026-…Z ERROR:
…', '[codex] ERROR: …'); both were rejected outright. Accept one
optional ISO-shaped token (a non-space run containing a literal 'T')
and/or one optional '[...] ' tag ahead of the existing ERROR:
allowance. Free prose ('warn: ERROR: …') still has letters in a
position neither new group recognizes and the base noise class
excludes letters outright, so it stays rejected — bats covers all
three.
P3-2: the fast-failure 'reason' (burn.sh, the #66 path) took stderr's
first line through _burn_sanitize_reason only — JSON-escaping, never
redaction — while out_for_class right above it already redacts the
same stderr for classification. An engine that echoes the operator's
prompt back on stderr put it verbatim into --json/status.json's
reason. Route stderr_first through _burn_redact_full first, same as
out_for_class. bats: a prompt fragment placed on stderr no longer
appears in the JSON reason field (confirmed red beforehand — the field
carried it verbatim).
P3-5: two assertions used 'run cmd; [ status -ne 0 ]' while their
immediate same-shape neighbours stayed bare '! cmd' — bash exempts a
bare '! cmd' from set -e exactly like the bare '[[ ]]' tests/README.md
already warns about, so a failing one would be silently ignored
mid-body, not just inconsistent in style. Converted the two neighbours
to match.
P3-6: 'ordinary codex stderr preserves the no-artifact failure'
accepted either of two DIFFERENT reason strings via '||', so a real
regression in #66's stderr-reason plumbing — what the test's own title
promises to guard — could never turn it red. Assert only the shape
this stub actually produces ('ERROR: could not open input file',
confirmed by direct invocation).
Review: REVIEW-stderr81-r1.md P3-2, P3-4, P3-5, P3-6.
Co-authored-by: KITT <kitt@cver.net>
|
P2-1 (dead hunk / mis-described mechanism): fixed in 0e1a123. Deleted the |
|
P2-2 (false-dry from a mid-buffer quote): fixed in 8e26e63. |
|
P2-3 (reset phrase from the wrong line): fixed in 6c18d2c. The reset phrase is now extracted from the SAME line that anchored the dry verdict, never |
|
P3-1 (>128 KiB prompt → perl E2BIG → silently blind classifier): confirmed, but out of scope for this PR per the review's own recommendation — it's pre-existing (not introduced by #81's fix) and mostly overlaps with codex's own exec E2BIG ceiling. Filed as #99 with the measurement ( |
|
P3-2 (reason field carries raw stderr, no redaction): fixed in bf640e4. The fast-failure |
|
P3-3 (#68 not closed): already true on this branch before this round — verified again here. |
|
P3-4 (ERROR: over-narrowed as the only letter-bearing prefix): fixed in bf640e4. The anchor now also accepts one optional ISO-shaped timestamp token and/or one |
|
P3-5 (inconsistent bare |
|
P3-6 ( |
…1-1) REVIEW-stderr81-r2.md P1-1: round-1's own fix for round-2's P2-2 (limiting codex's usage-limit anchor to the reply's LAST 20 LINES) was reasoned from "the caller only reaches this function once the artifact check has already failed, so 'no artifact' is a given here" — true for exactly one of this function's three real callers (burn.sh's no-artifact branch), and false for the other two: burn.sh's ARTIFACT-PRODUCED branch (the run kept going and finished the artifact, so the vendor's limit line is often far from the end) and conduct.sh's per-leg capture (no artifact concept at all). On both of those, a real limit line beyond line 20 read as "no limit here" — on the artifact path that silently cleared an existing dry marker (undoing round-2's own P2-2 guarantee); on conduct it reported a dried-out leg as CAPTURED. On the one path the window WAS written for, a real #81 shape (limit line + a stack trace over 20 lines, no artifact) still misread as a task failure — the window's cliff sat exactly where #81's own incident text (6 lines) happened to fit. The window is withdrawn. The classifier now scans the WHOLE captured reply on every caller. The false-dry guard this bought by POSITION is now bought by PRECISION instead: the anchor accepts a line only when its sole prefixes are, in order, an optional exact ISO-8601 stamp, one optional bracketed tag, and optional "ERROR: " — nothing else — matched CASE-SENSITIVELY, POSIX ERE only (no `\S`/`\s`). This also fixes the review's own P2-1: the old ISO-timestamp allowance was `(\S+T\S+ )?` under `grep -i`, which reads as "any token with a t or T anywhere inside it" — `agent:`, `context:`, `stderr:`, `output:`, `note:`, bare `attempt` all satisfied that and came out dry. Removing the window means `_limit_codex_anchor_line` now walks the reply's full line count, and the original implementation forked one `grep` per line (twice, for the direct check and the backward-join fallback) — a 20,000-line capture with the anchor on its last line took ~60s to classify under that design (an existing P1-3 timing guard, round-5, for a different shape caught it immediately; this one wasn't covered until this fix). Rewritten to do the whole-buffer scan in a bounded number of `grep` forks: one `grep -n` pass finds every candidate line in a single fork, and only the rare fallback (the anchor phrase itself split across a line boundary, not just its trailing reset phrase) pays for testing every adjacent pair — also one grep call, not one per pair. Verified: an 8MB single-line capture and a 20,000-line capture with the anchor on the last line both classify in low single-digit seconds; a 100,000-line no-match capture in ~2s. burn.sh's artifact-wins branch (round-2's own P2-2: never clear an existing dry marker when the SAME reply also shows a limit) already had the right shape — it just never saw a limit line beyond the window. Fixed at the source; belt added at the call site too: a fresh artifact now clears a marker only when the whole reply has no limit line AND the engine's own exit code was 0 — a non-zero exit with a fresh artifact and no limit line is not the "real success" the clear exists for either. bats: a limit line followed by a 25-line stack trace, no artifact ⇒ dry with the reset relayed (the #81 shape, restored); a limit line early in an artifact-producing reply with a PRE-EXISTING dry marker ⇒ success, and the marker survives untouched (round-2's own P2-2 receipt, restored); the same shape with a non-zero engine exit and NO limit line ⇒ marker also survives (the rc-gate), a matching rc==0 control clears it; a conduct.sh leg with the limit line far from the tail ⇒ dry, not CAPTURED; the reviewer's six prefix decoys (agent:/context:/stderr:/output:/note:/bare attempt) ⇒ not dry; an ISO timestamp and a bracketed tag ahead of ERROR: ⇒ still accepted; "warn: ERROR: " ⇒ still rejected (same three cases as before, now correct for the right reason instead of by accident). Review: REVIEW-stderr81-r2.md P1-1, P2-1. Co-authored-by: KITT <kitt@cver.net>
REVIEW-stderr81-r2.md P2-2: the fast-failure `reason` field's own redaction (round-1's P3-2 fix) used `_burn_redact_full`'s DEFAULT replacement — an empty string — and only ever looked at stderr's FIRST line. When that first line IS, in its entirety, the thing being redacted — codex echoing the whole prompt back as its own first line of stderr, one of the "several failure shapes" the existing comment already names — redacting it to "" and stopping left `reason` an empty string in both `--json` and status.json, even when a real diagnostic sat right there on the next line. Separately (issue #99, pre-existing, not fixed here): a >128 KiB prompt makes `_burn_redact_full`'s own `perl` invocation fail closed and hand back "" regardless of content — this commit's own mechanism (walking for the first non-placeholder, non-blank line) absorbs that shape too, without touching `_burn_redact_full` itself. `reason` is now redacted with the SAME "[prompt: …]" placeholder burn.sh:385 already uses for the human-facing diagnostic tail, then walks the redacted lines — not just the first — for the first one that ISN'T entirely that placeholder (or blank): a prompt-echo line is skipped, a real diagnostic on the next line is kept. If every line is placeholder or blank, `reason` says so in plain words ("engine exited rc=<n>, output redacted") — never `""` or `null`. bats: a stderr whose first line IS the prompt and whose second line is a real diagnostic ⇒ `reason` carries the diagnostic, not the prompt and not empty; a stderr where every line redacts to placeholder/blank ⇒ `reason` falls back to the plain rc-carrying message, never empty. Existing #66/#81 reason tests (trimmed first line, 200-byte cap, legitimate double spaces, the P3-2 prompt-redaction receipt, ordinary single-line stderr) all still green — this only widens what counts as a usable first candidate. Review: REVIEW-stderr81-r2.md P2-2. Co-authored-by: KITT <kitt@cver.net>
…P3-2)
REVIEW-stderr81-r2.md P3-1: the forward-join in _limit_codex_anchor_line
glued the NEXT line onto an anchor line that didn't end in terminal
punctuation, unconditionally — written for a vendor sentence wrapped
mid-RESET-PHRASE by a terminal ("…2:13\nAM."), but it fired just as
readily when the anchor line simply had no trailing period for its own
reasons and the next line was unrelated prose carrying its OWN "resets
…"/"try again at …" text, donating that decoy's reset to the genuine
anchor. limit_codex_reset had this exact bug at the whole-buffer level
once (P2-3, fixed by isolating the anchor line); this is the same shape
one line-join later. Only glue now when the next line does not already
stand on its own — neither as another anchor match nor as something that
itself parses as a reset phrase. A genuine wrapped continuation ("AM.")
is neither, so it still joins; a decoy with its own "resets …" text no
longer donates it — the anchor line's own (lack of a) reset now decides,
same as before P2-3 ever existed.
P3-2: `while read -r` does not strip a trailing `\r` — a CRLF capture
left one on the end of every line, so the `case … in *[.\!?])`
terminal-punctuation check never matched (a line ending "…AM.\r" tests
against `.\r`, not `.`), and EVERY anchor line, not just a genuinely
wrapped one, attempted the forward join above — widening P3-1's exposure
from "occasionally" to "every line of a CRLF transport". Strip the `\r`
at the source (right after `read`) instead of leaning on
limit_codex_reset's own `head -n 1` to mask it.
bats: a decoy reset phrase on the line AFTER an anchor line with none of
its own is no longer dry (conservative — matches pre-P2-3 behavior); the
genuine wrapped-reset case (no decoy reset on the next line) still joins
and parses; a CRLF capture's extracted reset stays byte-clean with no
stray `\r`.
Review: REVIEW-stderr81-r2.md P3-1, P3-2.
This also completes P3-4 and P3-5 from the same review, as a side effect
of the prior P1-1 commit: the `\S`/`\s` GNU regex extension is gone (the
anchor no longer has any use for it) and the `local ... tail` variable
this review flagged for shadowing the `tail` builtin no longer exists —
the tail-windowing it named is deleted, not renamed.
Co-authored-by: KITT <kitt@cver.net>
|
P1-1: withdrew the 20-line tail window entirely (commit c11d9ba) — it hid the anchor from 2 of its 3 real callers and reintroduced #81 on the artifact-produced and conduct.sh paths. The classifier now scans the whole captured reply; the false-dry guard is a precise 3-part anchor (ISO stamp / bracketed tag / |
|
P2-1 (found while fixing P1-1): the ISO-timestamp allowance |
|
P2-2: fast-failure |
|
P3-1: the forward-join glued the next line onto an anchor line missing terminal punctuation unconditionally, so a decoy's own "resets …" text on the following line could be donated to a genuine anchor with no reset of its own. Now only joins when the next line neither matches the anchor itself nor parses as a reset phrase on its own. Commit e50d471. |
|
P3-2: |
|
P3-4 / P3-5: resolved as a side effect of the P1-1 rewrite (c11d9ba) rather than as separate edits — the GNU |
|
P3-3 (JSON pretty-print not detected): noted, not separately fixed — the reviewer's own finding was that the JSON coverage this review saw was a side effect of the P2-1 bug (the loose ISO-timestamp group accidentally swallowing |
|
P3-6 (roam.bats flake): confirmed independently on this lane host — same shape, same host-dependent rate. Not fixed here per the review's own recommendation; filed as #101 with the reviewer's 17-run A/B numbers and the root cause (wait_for's return value dropped by all 4 callers at roam.bats:112). |
1 participant
Closes #81. Also closes #68.
What
lib/core/limit.sh: the anchored usage-limit classifier now accepts codex'sERROR:transport prefix — this, not a stderr-capture change, is burn: codex's usage-limit message on stderr is not recognised — run ends as "no fresh artifact and no limit" instead of dry with a reset time #81's actual fix. codex's stderr was already merged into the classifier's input via2>&1before this PR (round-1 review,REVIEW-stderr81-r1.mdP2-1). The anchor is also now: (a) only honored inside the last 20 lines of the reply, so a healthy run that merely quotes the vendor sentence mid-transcript is never misread as dry; (b) bounded by an optional ISO timestamp and/or one[tag]ahead ofERROR:, not just that one literal string.lib/core/limit.sh: the reset phrase is now extracted from the SAME line that anchored the dry verdict (neverhead -n 1of the whole buffer) — an earlier, unrelated "resets …"/"try again at …" elsewhere in the reply can no longer win the marker over the genuine vendor phrase; a sentence wrapped across two physical lines is rejoined before extraction.lib/commands/burn.sh: removed a dead hunk (round-1's own first attempt) that re-slurped the whole$stderr_filea second time into the classifier input and re-ran redaction on it.$outalready carries stderr merged in on every capture path (_burn_capture_stderrtees fd 2 back to fd 2 while writing the file). A/B-tested: behavior is byte-identical with or without the hunk; a 16 MiB stderr stream is ~45% slower with it for zero effect.lib/commands/burn.sh: the fast-failurereason(burn codex: a non-git first --add-dir makes codex exec refuse in one second, and burn reports it as a generic task failure #66's path,--json/status.json) is now redacted the same way the classifier input already is, so a prompt an engine echoes back on stderr can no longer reachreasonverbatim.lib/commands/home.sh: the obsolete "exec-stdout-only" note corrected.tests/bats/burn.batsassertions tightened: a bare! cmd(exempt fromset -e, same traptests/README.mddocuments for bare[[ … ]]) brought in line with its immediate, already-fixed neighbour; a||two-way assertion narrowed to the one shape the test's own stub actually produces.Round-1 adversarial review (KITT)
REVIEW-stderr81-r1.mdfound 0 P1, 3 P2 — the mechanism above was mis-described (dead hunk vs. the real fix), the widenedERROR:anchor caused a false-dry regression on a healthy run that merely quotes the vendor sentence, and the reset phrase could be picked up from unrelated prose elsewhere in the reply — and 6 P3. This PR's later commits fix both P2-1/P2-2/P2-3 and four of the P3s (theERROR:prefix's noise class was too narrow;reasonwasn't redacted; twoburn.batsassertions were inconsistent/loose). Each finding has a red-before/green-after bats receipt; full transcript in the lane report.One pre-existing, unrelated defect the review also found — a prompt over ~128 KiB causes
perlE2BIG inside_burn_redact_full, which swallows the failure and silently returns an empty (fully blinded) classifier input — is not fixed here, per the review's own recommendation to track it separately: filed as #99.Validation
limit.bats+limit-reset.bats;burn.bats(full — including the tmux/background/transport-stub cases, via a throwawayTMUX_TMPDIR, never the real server); fulltests/batssuite;shellcheck -S warning(both the two changed files and the CI-equivalent full scope); bash-3.2 compatibility grep on the diff;git merge-tree --write-tree origin/main HEAD. Full receipts in the lane report.🤖 Generated with Claude Code
Round-2 adversarial review (KITT) — round-1's tail window is withdrawn
REVIEW-stderr81-r2.mdfound 1 P1, 2 P2, 6 P3. The headline: round-1's own fix for P2-2 (limiting the anchor to the reply's last 20 lines) reintroduced #81 itself on the two callers that don't fit its own stated premise (burn.sh's artifact-produced path, andconduct.sh, which has no artifact concept at all) — a real vendor limit line followed by more than 20 lines of stack trace or ordinary output read as "no limit here." Separately, round-1's P3-4 fix (accepting an ISO timestamp / bracketed tag ahead ofERROR:) was written as "any non-space token containing a literal T", which undergrep -ireads as "any token with a t or T anywhere in its middle" —agent:,context:,stderr:,output:,note:, and bareattemptall satisfied it.This PR's later commits withdraw the tail window entirely and replace both regressions at the root:
lib/core/limit.sh: the classifier now scans the whole captured reply, not a 20-line tail — the false-dry guard is precision (an exact anchor), not position._limit_codex_anchor_linewas rewritten to do this in a bounded number ofgrepforks (one pass over the whole buffer, not one fork per line) — a 20,000-line capture with the anchor on its last line took ~60s under the old per-line-fork design; it's under a second now.ERROR:— matched case-sensitively, in that order, each optional, nothing else. No generic noise class, no markdown, no free prose ahead of them.lib/commands/burn.sh's artifact-wins branch (round-2's own P2-2 fix — never clear an existing dry marker when the same reply also shows a limit) now actually sees a limit line wherever it sits in the reply, not just in the last 20 lines; a fresh artifact with a non-zero engine exit and no limit line also leaves an existing marker alone (rc == 0is now part of the clear condition, not just "no limit line").lib/commands/burn.sh's fast-failurereasonfield (round-1's own P2-2 fix) redacted with an empty replacement and only ever looked at stderr's first line — when that whole first line WAS the thing being redacted (an engine echoing the operator's prompt back, a documented codex shape),reasoncame back"". It's redacted with the same[prompt: …]placeholder the human-facing tail already uses, and walks the redacted lines for the first one that isn't entirely that placeholder; if every line is placeholder or blank,reasonsays so in plain words — never""ornull.\S/\sGNU regex extension this repo's other patterns avoid is gone from this one too (a POSIX character class does the same job).main):roam.bats's "second client attaches" flake —wait_for's return value is silently dropped by all 4 call sites, so a 20s timeout on this contended host reads as a wrong value instead of "hadn't happened yet." See roam.bats: "a second client attaches" flake is a dropped wait_for return, not a race (63-67% red on reefbox) #101.Full receipts (red on
bf640e4, green on HEAD, per finding) in the lane report.