Repository navigation
test(handoff): bound missing pipe output and CI execution - #372
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex review Please review the current head |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary
An interrupted watch can return 130 after failing to deliver its final frame. The test retained its pipe write descriptor and then performed a blocking read, so missing output could leave CI waiting indefinitely.
-X faulthandlerfor fatal-error diagnostics. No background traceback timer or shell-specific wrapper is added. These are execution limits, not runner queue-time guarantees.Refs #370. This addresses the reproduced fixture hazard, not an established cause of the historical 23-minute CI stall. Product runtime/source/version/dependencies and native delivery remain unchanged. Separately, #316 was reopened with monitoring after an accidental closing phrase was found in PR #320; #268/#180 remain P1/Long-term. No relationship between these historical observations is asserted.
Validation
-X dev -W error::ResourceWarning: 132 handoff watch/runtime/lifecycle/process tests pass.-X faulthandleragainst the watch module: 37 pass. Documentation/matrix checks: 91 pass. Generator consistency andgit diff --checkpass.os.forkoccurrences in this suite are subprocess fixture source strings, not direct calls in the watchdog-hosting test process.Local broad-suite limitation: an earlier full Core run (980 tests) was not green: 5 failures, 32 errors, 6 skips involving private-storage guards on the external test volume. Two representative failures also reproduce on an unchanged HEAD source copy. This is not presented as a passing full suite or proof of every failure's cause; no permissions were relaxed and no internal-volume fallback was used. Hosted Linux/macOS/Windows CI remains required.
Independent review
The current independent-cli-review skill ran Claude Code 2.1.314 / metadata-confirmed claude-opus-5-5 and Antigravity CLI 1.5.0 / init-confirmed gemini-4-argon-medium (medium), in parallel, fresh non-interactive read-only sessions with no tools/tests/network/writes requested. Both produced successful substantive responses.
Codex checked the feedback against source. Follow-ups preserve constructor-failure diagnostics, restrict the short negative timeout to the final read, propagate child warning settings, bound child summaries and improve timeout diagnostics. The final review's conditional Windows-shell finding led to an initial explicit-shell correction. Hosted CI then exposed the old matrix guard's literal command expectation and a separate macOS3.9 KeyboardInterrupt during cleanup. The optional threaded traceback wrapper was removed; the final workflow keeps the standard, cross-platform unittest command, finite execution caps and fatal-error faulthandler flag. Existing matrix guard tests remain unchanged and pass locally. This simplification was tested after the paired reviews; it is not represented as a third full source review. The fixture refinements (including one executed, non-skipped child assertion) were retested. No cause is assigned to the separate cleanup interruption or the historical stall; current-head CI is required.
This PR prepares tests/CI only. It authorizes no merge, tag/release, installation, service changes, model session activation or live message experiment. Automatic Codex review is left to trigger without a manual mention.