fix: Auto-detected flaky tests (merge-group) — review for quarantine (#2692)

[zaxbysauce]

Closes #2692

Summary

Scope: Adds a single windows-only quarantine entry for tests/unit/background/completion-observer-coder.test.ts (issue #2692 merge-group retry-flake), bumps the ledger STATUS header 2→3, adds a 4-test pinning regression file, and ships the required pending release fragment.

|Verdict: APPROVE

Findings

LOW — OWNER line drops the @ prefix used by sibling entries

• Location: scripts/ci/quarantined-tests-windows.txt:117
• Evidence: New entry reads # OWNER: zaxbysauce — issue #2692 ... while both prior entries use # OWNER: @zaxbysauce. All consumers accept both: scripts/check-invariants.ts:1103 uses /^#\s*OWNER:\s*(\S.*)$/ and the new pinning test uses the same regex; the real bun scripts/check-invariants.ts run passes Check 7.
• Why it matters: Cosmetic inconsistency only; no tooling keys on the @. Purely a triage-consistency nit for the next ledger editor.
• Suggested fix: Optionally align to # OWNER: @zaxbysauce on the next renewal edit; no change required to merge.

What I Checked

• Target file present in diff: yes — scripts/ci/quarantined-tests-windows.txt (entry byte-identical to the issue's candidate line, verified via cat -A and exact-match grep -cx)
• New/updated test exercises the changed code path: yes — ci-yml-quarantine-2692.test.ts reads the real ledgers off disk and mirrors ci.yml's grep -vE '^\s*#|^\s*$' extraction and Check 7's OWNER/EXPIRY regexes; no mocks. Independently simulated ci.yml's full all-tests.txt → sort -u → comm -23 chain: target file excluded from the Windows gated set (grep -cx → 0) while remaining gated on Linux, and the similarly-named completion-observer-coder-terminal-2098.test.ts correctly stays gated on all OSes.
• Scope creep beyond the issue: no — exactly one quarantined file (the one the issue named); the release fragment is mandated by the repo's contributing contract; no source/workflow code touched.
• Anti-patterns scanned: silent-except (none), mock-without-call (none — tests are data-file pins), off-by-one (STATUS count 3 = 3 active entries; pre-existing consistency test compares declared vs actual, passes), signature-break (no code signatures changed), scope-creep (none), stale-docs (fragment claims match the diff: 4 tests, header 2→3, entry placement).
• Hidden-regression hunt on tests the wrapper's changed-file gate did NOT run: ci-yml-windows-quarantine.test.ts (no exact count pin — equality assertion 3=3 holds), ci-coverage-sharding.test.ts (asserts coverage gate does NOT consume the windows ledger — untouched), check-quarantine-metadata.test.ts (fixtures only), detect-and-quarantine-flakes.sh Rule A (reads this ledger, so the auto-filer stops re-filing — the fix's stated purpose holds), .gitattributes enforces LF so no CRLF divergence is possible.
• Ran bun scripts/check-invariants.ts against the branch: "All engineering invariant checks passed," including Check 7 on the modified ledger.

Confidence

high — the change is a data-only ledger edit whose every consumer (ci.yml exclusion chain, Check 7, flake-detection Rule A, sibling pinning tests) was read and independently exercised; I could not construct a failing scenario, and the sole finding is cosmetic.

Invariant audit

• No engineering invariants were identified as touched by this diff; the change is confined to docs/releases/pending/issue-2692-quarantine-completion-observer-coder-windows.md, scripts/ci/quarantined-tests-windows.txt, tests/unit/scripts/ci/ci-yml-quarantine-2692.test.ts

Test plan

See the linked issue and the change summary.

[github-actions]

Drift check report

Found 2 drift finding(s): 0 error, 0 warning, 2 notice.

required-check-contract (2)

• 🔵 notice scripts/required-check-contract.json: [RULESET_DIVERGENCE] intended-required context "drift" is not yet required by the captured ruleset
• 🔵 notice scripts/required-check-contract.json: [RULESET_DIVERGENCE] intended-required context "drift" is not present for every expected event in captured external workflow evidence

[Copilot]

🟢 Approval recommended

It is a low-risk, data-only quarantine edit with accurate metadata, matching STATUS/consistency assertions, and thorough new pinning coverage, with no issues found.

Pull request overview

This PR quarantines a Windows-only flaky test (tests/unit/background/completion-observer-coder.test.ts) that was auto-detected by the flake-detection workflow (issue #2692). The test is an integration-heavy suite that spawns real git subprocesses and drives async completion-observer events, making it sensitive to slow/contended windows-latest CI runners; it exhibited a passed-on-retry flake in a merge-group run while ubuntu/macOS siblings ran green. The entry is correctly scoped to the Windows-only ledger to avoid suppressing the test on other platforms.

Changes:

• Appends a Windows-only quarantine entry (with OWNER/EXPIRY metadata per Check 7 / issue #2477) and bumps the ledger # STATUS: header from 2 to 3 active entries.
• Adds a 4-test pinning regression file that reads the real ledgers off disk to guard placement, single-OS scope isolation, on-disk path presence, and OWNER/EXPIRY grammar.
• Ships the mandatory pending release fragment describing the change and its caveats.

I confirmed: the quarantined file exists and its cited spawnSync timeout (lines 26-39) is accurate; the STATUS count (3) matches the three active entries and the pre-existing ci-yml-windows-quarantine.test.ts:74 consistency assertion; the new entry satisfies Check 7 (OWNER + EXPIRY present, EXPIRY 2026-10-31 is in the future); and the test placement follows the established tests/unit/scripts/ci/ convention. I considered the OWNER line dropping the @ prefix used by the two sibling Windows entries, but the macOS ledger (quarantined-tests-macos.txt:17) also omits @, so this is not a clearly-established convention and both consumers accept either form — not worth a comment.

File summaries

File	Description
scripts/ci/quarantined-tests-windows.txt	Adds the completion-observer-coder.test.ts entry with OWNER/EXPIRY metadata and updates the STATUS header 2→3.
tests/unit/scripts/ci/ci-yml-quarantine-2692.test.ts	New 4-test pinning regression covering ledger placement, single-OS scope, path presence, and OWNER/EXPIRY grammar.
docs/releases/pending/issue-2692-quarantine-completion-observer-coder-windows.md	Mandatory pending release fragment documenting the quarantine, rationale, and caveats.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 0
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.