Skip to content

🐛 Prevent passive supervision cleanup from hiding test failures - #1094

Merged
taras merged 1 commit into
mainfrom
codex/passive-supervision-stall-1091
Nov 3, 2026
Merged

taras merged 1 commit into
mainfrom
codex/passive-supervision-stall-1091

Conversation

@taras

@taras taras commented Nov 3, 2026 •

Copy link
Copy Markdown
Owner

An early supervisor failure can leave the passive-supervision test joining an entry whose cleanup waits on barriers that teardown has not released. The failure then produces no assertion verdict and stalls the CI shard. Fixes #1091.

The held-entry fixture now releases those barriers synchronously before scope destruction joins its owned work. The completed/failed cases share that fixture with a deterministic missing-supervisor regression, which checks that the initiating error returns only after entry cleanup finishes. All existing observation, progress, request-count, canonical-result and cleanup assertions remain. The change is confined to the test file.

Validation at signed feedback commit a3bdd03ecfeea9f604f0adf3538ad760007345cd:

  • The regression fails with the original cleanup ordering and passes with the finalizer; the intermediate late-ensure approach also fails.
  • Filtered Node and Bun: all three cases pass. Deno: all 14 passive-supervision steps pass.
  • Ordinary unfiltered Node: 52 tests pass, zero failures, cancellations or skips.
  • Focused lint, formatting and diff checks pass. CI owns the exhaustive runtime corpus.

The original externally terminated baseline and every first failure are retained as non-pass evidence. Two Deno filters selected zero tests and supply no evidence; the enclosing-suite run above supplies the Deno result. Local execution used macOS Node 26.5.1, Deno 2.9.7 and Bun 1.3.14. The exact initiating event in historical Linux CI remains unknown; the cleanup deadlock is reproduced and corrected independently.

Planner acceptance and independent Fable review both pass the exact signed feedback head above. Fable's three observations are nonblocking: preserve the historical-trigger qualification, optionally share the rescue timeout constant, and clarify scope-owned prompt teardown. The qualification is recorded here; the optional cleanup is deferred and no source correction is required.

Required CI2396 passes the same head. Current-main CI2394 passes exact main 98cc6f173a36e4bfb532c9f6067f1ed284989e49. Native delivery validation reports all three verification gates passed. The PR is ready for review and awaits authorized merge.

The completed Fable reply exceeded the workflow's 1200-character summary limit (1394 characters). Its original reply and joined execution are preserved. A documented invocation-local length-only exception accepts that summary with the verdict, findings, clauses, reviewer, source and other gates unchanged. No additional model call or source correction ran.

@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown

PR #1094: 🐛 Prevent passive supervision cleanup from hiding test failures

1 files, +235 / -131

Scope

✅ PR scope looks good.

Structural

✅ No structural bloat detected.

Slop

✅ Slop indicators look low.

Static Analysis

✅ Oxlint found no issues.

Correctness

No extraneous code patterns detected.

@taras taras left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent Fable review: PASS at a3bdd03ecfeea9f604f0adf3538ad760007345cd.

All recorded clauses were reviewed:

  • C1: The synchronous finalizer breaks the teardown dependency cycle; every original supervision assertion is preserved.
  • C2: The deterministic real-child failure distinguishes the original ordering from the repair, preserves the initiating error, and proves joined entry cleanup.
  • C3: Focused Node/Bun and the Deno suite pass; ordinary unfiltered Node passes 52/52. Deadlines and runtime coverage are preserved.

No blocking findings. Nonblocking observations:

  • The local baseline stall (baseline-node-permitted.log: two children, no progress, killed at 30 s) is consistent with an early failure hidden by the teardown deadlock, but its initiating event was never identified and the run immediately followed a sandbox EPERM run, so baseline-vs-ordering-node is not a controlled pair. The controlled discriminator is the original-order-regression patch versus the finalizer, which is sufficient for the accepted scope. Record that the historical Linux CI initiating event remains unknown; under the fix a recurrence will surface as a visible assertion failure instead of a stall, which is the settled contract.

  • Regression rescue hard-codes sleep(5000), duplicating deadline()'s default. Acceptable since it must yield a boolean rather than throw, but a named constant shared with deadline() would keep the two from drifting.

  • In the regression, the spawned prompt task is halted by scope destruction rather than awaited; the held entry still runs to completion because releaseBody is resolved first, and cleaned===true proves the entry cleanup joined. No change needed, noted for readers interpreting 'all owned work joined'.

Disposition: the PR description already preserves the historical-trigger limitation. The shared-timeout-constant suggestion is optional and deferred; the cleanup observation requires no change.

Workflow reconciliation: Fable's original summary is 1394 characters against a 1200-character presentation limit. An invocation-local length-only exception accepts that original summary. The raw response, actual joined execution, verdict, findings, clauses, reviewer and commit are unchanged; no second review or formatting call ran. Required CI and exact-current-main health remain delivery gates.

@taras
taras marked this pull request as ready for review November 3, 2026 21:28
@taras
taras merged commit ab314aa into main Nov 3, 2026
58 checks passed
@taras
taras deleted the codex/passive-supervision-stall-1091 branch November 3, 2026 21:42
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.

Investigate Node passive-supervision test stall

1 participant