fix(review): verification-run nits — terminal zero-usage guards, TOCTOU re-emit, honest scale caveat · Entire
fix(review): verification-run nits — terminal zero-usage guards, TOCTOU re-emit, honest scale caveat
c8806af→main·
peyton-alt·1w ago·6 files·+118 added/-12 removed
The post-fix verification crew run (approve with nits) left four lows:
- Claude's terminal result emission was unconditional, so a usage-less result envelope emitted Tokens{0,0} and erased the mid-run cumulative total — the same clobber class fixed on the codex side; now gated on non-zero usage.
- The rollout tailer re-emits its last totals on stop (bypassing dedup), closing the nanosecond TOCTOU where a per-turn stdout emission races past the parser's tailerEmitted check and would otherwise stand as the final recorded value.
- The parser comment claimed turn.completed usage is per-turn scale while the test fixture comment called it running usage; both now state the honest position — unverified for real multi-turn output, identical either way for single-turn exec reviews, tailer wins whenever present.
- Two parser tests fed thread.started without the codex session-dir override, sending the tailer globbing the real ~/.codex/sessions and exposing exact-count assertions to nondeterministic injection.
Co-Authored-By: Claude Fable 5 noreply@anthropic.com
Sessions
01KWYP9YXJ19MGHWQ2RWFA4EDRView transcript
Changes
6
cmd/entire/cli/agent
claudecode
Mreviewer.go+7/-1
Mreviewer_test.go+30
codex
Mreview_tokens.go+8
Mreview_tokens_test.go+41
Mreviewer.go+15/-5
Mreviewer_test.go+17/-6
154 unmodified lines
155
156
157
158
159
160
161
162
159
163
164
165
166
167
168
154 unmodified lines
return
}
if sawResult {
// Gate on non-zero usage: a result envelope without a usage
// block would emit Tokens{0,0}, which only ever ERASES the
// mid-run cumulative total under the consumers'
// overwrite-not-sum semantics (mirrors the codex guard).
in := resultUsage.InputTokens + resultUsage.CacheReadInputTokens + resultUsage.CacheCreationInputTokens
out <- reviewtypes.Tokens{In: in, Out: resultUsage.OutputTokens}
if in > 0 || resultUsage.OutputTokens > 0 {
out <- reviewtypes.Tokens{In: in, Out: resultUsage.OutputTokens}
}
out <- reviewtypes.Finished{Success: !resultErr}
return
}
Mcmd/entire/cli/agent/claudecode/reviewer.go+7/-1
451 unmodified lines
452
453
454
455
456
457
458
459
460
461
462
463
464
465
466
467
468
469
470
471
472
473
474
475
476
477
478
479
480
481
482
483
484
485
486
487
451 unmodified lines
}
}
// TestParseClaudeOutput_UsagelessResultDoesNotClobberCumulative pins the
// terminal emission guard: a result envelope with no/zero usage must not
// emit Tokens{0,0} — under the consumers' overwrite-not-sum semantics that
// would erase the mid-run cumulative input total.
func TestParseClaudeOutput_UsagelessResultDoesNotClobberCumulative(t *testing.T) {
t.Parallel()
input := strings.Join([]string{
`{"type":"assistant","message":{"id":"msg_1","content":[{"type":"text","text":"hi"}],"usage":{"input_tokens":10,"cache_read_input_tokens":90,"cache_creation_input_tokens":0,"output_tokens":2}}}`,
`{"type":"result","subtype":"success","is_error":false}`,
"",
}, "\n")
var tokens []reviewtypes.Tokens
for ev := range parseClaudeOutput(strings.NewReader(input)) {
if tk, ok := ev.(reviewtypes.Tokens); ok {
tokens = append(tokens, tk)
}
}
if len(tokens) == 0 {
t.Fatal("expected the mid-run cumulative Tokens emission")
}
last := tokens[len(tokens)-1]
if last.In == 0 && last.Out == 0 {
t.Fatalf("final tokens = %+v — usage-less result clobbered the cumulative total", last)
}
if last.In != 100 {
t.Errorf("final tokens = %+v, want the cumulative {100, 0} to stand", last)
}
}
// collectEvents drains an event channel into a slice.
func collectEvents(ch <-chan reviewtypes.Event) []reviewtypes.Event {
var events []reviewtypes.Event
Mcmd/entire/cli/agent/claudecode/reviewer_test.go+30
79 unmodified lines
80
81
82
83
84
85
86
87
88
89
90
91
92
93
79 unmodified lines
if err := tail.drain(); err != nil {
logging.Debug(ctx, "codex token tail: final drain failed", slog.String("error", err.Error()))
}
// Re-emit the last totals unconditionally (bypassing dedup):
// a per-turn stdout emission can race past the parser's
// tailerEmitted check in the instant before this tailer's
// first send is observed, and this re-send guarantees the
// session-cumulative value is the final Tokens regardless.
if tail.lastIn >= 0 {
out <- reviewtypes.Tokens{In: tail.lastIn, Out: tail.lastOut}
}
return
case <-ticker.C:
}
Mcmd/entire/cli/agent/codex/review_tokens.go+8
328 unmodified lines
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
328 unmodified lines
}
}
// TestTailRolloutTokens_ReemitsLastTotalsOnStop pins the TOCTOU hardening:
// on stop, after the final catch-up drain, the tailer re-emits its last
// known totals. This guarantees the tailer's session-cumulative value is the
// final Tokens even if a per-turn stdout emission raced past the parser's
// tailerEmitted check in the instant before the tailer's first Store(true).
func TestTailRolloutTokens_ReemitsLastTotalsOnStop(t *testing.T) {
// Cannot t.Parallel — uses t.Setenv.
dir := t.TempDir()
t.Setenv("ENTIRE_TEST_CODEX_SESSION_DIR", dir)
rollout := filepath.Join(dir, "rollout-2026-06-03T08-57-39-"+tailTestThreadID+".jsonl")
if err := os.WriteFile(rollout, []byte(tokenLine(7000, 300)), 0o644); err != nil {
t.Fatal(err)
}
out := make(chan reviewtypes.Event, 16)
stop := make(chan struct{})
done := make(chan struct{})
go func() {
tailRolloutTokens(tailTestThreadID, out, stop, new(atomic.Bool))
close(done)
}
first := awaitTokens(t, out)
if first.In != 7000 || first.Out != 300 {
t.Fatalf("first tokens = %+v, want {7000, 300}", first)
}
close(stop)
<-done
// The stop path must have re-emitted the last totals (dedup bypassed).
select {
case ev := <-out:
tk, ok := ev.(reviewtypes.Tokens)
if !ok || tk.In != 7000 || tk.Out != 300 {
t.Fatalf("post-stop event = %#v, want re-emitted Tokens{7000, 300}", ev)
}
default:
t.Fatal("no re-emitted Tokens after stop — TOCTOU window unguarded")
}
}
func TestTailRolloutTokens_ReturnsOnStopWhenNoRollout(t *testing.T) {
// Cannot t.Parallel — uses t.Setenv.
t.Setenv("ENTIRE_TEST_CODEX_SESSION_DIR", t.TempDir())