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

[zaxbysauce]

Closes #2660

Summary

|Scope: Adds tests/unit/telemetry/init-rehome.test.ts to the general quarantine ledger (scripts/ci/quarantined-tests.txt) with Check-7-compliant OWNER/EXPIRY metadata, a regression pinning test that reads the real ledgers, and a release fragment.
|Verdict: APPROVE
|
|### Findings
|
|#### MEDIUM — Blast-radius claim in ledger comment and release fragment is false: general-ledger entries suppress the file on ALL unit-shard OSes, not just coverage
|- Location: scripts/ci/quarantined-tests.txt:26-31 (comment: "Flaky on ubuntu-latest merge-group coverage-shard 1 only") and docs/releases/pending/issue-2660-init-rehome-quarantine.md:9-11 ("The test still runs on every other platform/shard")
|- Evidence: .github/workflows/ci.yml:628-638 applies scripts/ci/quarantined-tests.txt via comm -23 on every RUNNER_OS (the per-OS ledgers at ci.yml:632-636 are additive); the in-file comment at ci.yml:631 states merge_group runs the full 3-OS matrix. The file is therefore skipped in all 18 ubuntu/macos/windows unit cells plus the coverage shard, not "every other platform/shard". The green-sibling-shards evidence in the same run shows attempt-level flakiness distribution, but the mechanism does not discriminate coverage from unit.
|- Why it matters: Gating behavior is still correct (coverage honors only this ledger, so it is the only effective target), but the misstatement of the mechanism could misdirect the future root-cause attempt the EXPIRY demands (someone may chase a coverage-only harness cause for a failure mode that also fires in plain unit shards), and it contradicts the #2185 precedent test's stated semantics ("the general ledger applies on every RUNNER_OS").
|- Suggested fix: Reword the ledger comment block and fragment to "flake observed on ubuntu-latest coverage-shard 1; the general ledger suppresses the file in all unit/coverage invocations because coverage consumes only this ledger — accepted over-provision until root fix."
|
|#### LOW — Timestamp inconsistency in evidence prose
|- Location: scripts/ci/quarantined-tests.txt:27 vs tests/unit/scripts/ci/ci-yml-quarantine-2660.test.ts:11
|- Evidence: The ledger attributes 2026-09-09T04:41:50Z to CI run 34309871068; the test attributes 04:09:35Z to the same run; commit c2d48d4 assigns 04:41:50Z to the detection run 34311972198.
|- Why it matters: Comments only (nothing parses them beyond the OWNER/EXPIRY prefix checks), but one attribution is wrong and this ledger block is the durable evidence record for the quarantine.
|- Suggested fix: Pick one timestamp per run ID consistently across ledger, test comment, and fragment.
|
|#### LOW — "Retry loop discards attempt-1 output" is asserted, not evidenced
|- Location: scripts/ci/quarantined-tests.txt:31-32, docs/releases/pending/issue-2660-init-rehome-quarantine.md:29
|- Evidence: No diff or context line demonstrates the ci.yml retry loop's log-overwrite behavior; the claim is load-bearing for "no assertion text exists to drive a root fix".
|- Why it matters: Weakens the documented justification for quarantining instead of fixing.
|- Suggested fix: Cite the specific ci.yml retry-step lines, or soften to "attempt-1 output was not retained in the run artifacts reviewed".
|
|### What I Checked
|- Target file present in diff: yes (scripts/ci/quarantined-tests.txt; quarantined path tests/unit/telemetry/init-rehome.test.ts exists at that exact repo-relative path, found at line 2753 of the replicated all-tests inventory)
|- New/updated test exercises the changed code path: yes — ci-yml-quarantine-2660.test.ts reads the real ledger files with readFileSync/existsSync (no mocks); independently replicated ci.yml's grep -vE | sort | comm -23 chain: 3537 total → 1 quarantined → 3536 gated, entry active and excluding exactly that file
|- Check 7 (issue #2477) against the real ledger: bun run check:invariants exits 0, "All active quarantine entries carry OWNER + EXPIRY metadata" (EXPIRY 2026-10-15 is future-dated; OWNER/EXPIRY grammar matches checkQuarantineMetadata patterns at scripts/check-invariants.ts:1103-1105)
|- Ledger parse scoping: blank line at ledger line 14 terminates both Check 7's and the pinning test's upward block walks before the stale "# No active global test quarantines" line; no test pins that comment's truth or a ledger entry count for the general ledger
|- Scope creep beyond the issue: no — exactly the ledger entry, one pinning test, one release fragment; ci.yml and run-coverage-gate.sh untouched (both asserted textually by pre-existing ci-yml-integration.test.ts / ci-coverage-sharding.test.ts, which this diff cannot break)
|- Anti-patterns scanned: silent-except (n/a — no code), mock-without-call (none; test is filesystem-real), off-by-one (n/a), signature-break (n/a), scope-creep (none), stale-docs (found: the MEDIUM above), duplicate-entry (single active entry; detector Rule A drops re-detections; #2730 collision disclosed)
|
|### Confidence
|high — the gating mechanics were verified end-to-end (real gate run + replicated comm/gate math); only a verified root-cause demonstrating the flake is not coverage-specific (making the general ledger over-broad) would change the verdict.

Invariant audit

• No engineering invariants were identified as touched by this diff; the change is confined to docs/releases/pending/issue-2660-init-rehome-quarantine.md, scripts/ci/quarantined-tests.txt, tests/unit/scripts/ci/ci-yml-quarantine-2660.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]

🟡 Changes recommended

The quarantine mechanics and metadata are correct, but the release fragment inaccurately describes the suppression blast radius and Check 7's grace-window behavior, and the ledger evidence timestamp is internally inconsistent.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR quarantines the flaky test tests/unit/telemetry/init-rehome.test.ts (auto-detected in a merge-group CI run, issue #2660) by adding it to the general quarantine ledger with Check-7-compliant OWNER/EXPIRY metadata. It fits into the repository's test-stability workflow (issue #1782 / #2477), where merge-group flake detection files issues and quarantine ledgers exclude the affected files from the CI gate until a root fix lands. The change is CI/docs-only — no runtime, plugin, or source code is touched.

Changes:

• Adds tests/unit/telemetry/init-rehome.test.ts to scripts/ci/quarantined-tests.txt (general ledger) with an OWNER/EXPIRY comment block (EXPIRY 2026-10-15).
• Adds a filesystem-real regression pinning test (ci-yml-quarantine-2660.test.ts) asserting the entry is active in the general ledger, absent from per-OS/integration ledgers, carries OWNER/EXPIRY, and that the on-disk path exists.
• Adds a pending release fragment documenting the quarantine, rationale, and caveats.

File summaries

File	Description
scripts/ci/quarantined-tests.txt	Adds the quarantine entry and OWNER/EXPIRY metadata block; evidence timestamp is inconsistent with the pinning test.
tests/unit/scripts/ci/ci-yml-quarantine-2660.test.ts	New pinning test reading the real ledgers; assertions are correct and filesystem-real.
docs/releases/pending/issue-2660-init-rehome-quarantine.md	Release fragment; overstates that the test still runs on non-coverage shards and mischaracterizes the Check 7 grace window.

Review details

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

---

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