Skip to content

test: synchronize timeout diagnostics before shutdown - #146

Merged
zzxwill merged 1 commit into
masterfrom
fix/timeout-fixture-output
Oct 21, 2026
Merged

zzxwill merged 1 commit into
masterfrom
fix/timeout-fixture-output

Conversation

@zzxwill

@zzxwill zzxwill commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator

The CLI timeout fixture emitted its expected partial review diagnostic only from a SIGTERM callback, then allowed just 200 ms before SIGKILL. A busy or unscheduled reviewer can correctly be killed without running that callback, making the test fail despite correct product behavior.

Write the diagnostic synchronously before the readiness marker and keep stdout silent so heartbeat assertions remain meaningful. Exercise both a responsive fixture and one whose JavaScript is deliberately blocked after readiness. The blocked variant fails before the correction with the same missing-output assertion and passes afterward. Existing failure-status, heartbeat, peer-result and pipe-drain assertions remain intact. Only the timeout test and changelog change; production code and deadlines are unchanged.

Historical evidence: Node 24/macOS job 119394920632 finished source tests with 301 passed and one missing-diagnostic assertion; package validation was skipped. The trace does not establish whether its SIGTERM callback executed, so this deterministic test flaw is verified without claiming the exact historical cause or a product output-loss fix.

Validation on 5f15a8d3d66d78faec97d5dafbab23509d05e419:

  • node --test test/timeout.test.js: 22 passed, exit 0.
  • node scripts/ci-check.mjs test: 310 passed, exit 0.
  • node scripts/ci-check.mjs test:package: 55 packaged files and installed CLI checks passed, exit 0.
  • Actual Jury: exit 0, approved after round 1 on the unchanged head. Codex reviewer raised a potential leak-while-passing concern; the Codex judge's controlled SIGKILL suppression showed the existing 15-second CLI timeout fails the test. The reviewer accepted this evidence and signed off with NO NEW FINDINGS. An independent external fault-injection harness confirmed the failure and terminated only its recorded fixture group; readback confirmed it was gone. No source amendment was needed.

Actual executed command:

GIT_AUTHOR_NAME=zzxwill GIT_COMMITTER_NAME=zzxwill node bin/jury.js review --dir /Users/bytedance/Documents/Codex/2026-10-20/task/codejury-timeout-fixture --trunk master --reviewer codex --judge codex --rounds 2 --summary 'Review this focused test-only timeout fixture correction. The old fixture emitted expected stderr only in SIGTERM, assuming its callback ran within a shortened 200ms grace. A deterministic blocked-JavaScript variant failed with the same missing partial review assertion before correction. Write stderr synchronously before readiness, then test both responsive and blocked reviewers. Product code, deadlines and existing failure, heartbeat, peer and pipe-drain assertions remain unchanged. Independent checks passed 22 timeout tests, 310 full tests and installed-package validation. Historical CI does not prove whether its signal callback ran, so do not claim its exact cause or a product output-loss fix. Assess correctness and test robustness without unrelated changes.' --web=false --push=false > ../timeout-fixture-review-evidence/jury-first.log 2>&1

@zzxwill
zzxwill marked this pull request as ready for review October 21, 2026 19:06
@zzxwill
zzxwill merged commit f2029d0 into master Oct 21, 2026
8 checks passed
@zzxwill
zzxwill deleted the fix/timeout-fixture-output branch October 21, 2026 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant