fix(ci): give the Windows leg the budgets and the crash retry it never had

.github/workflows/ci.yml

@@ -516,7 +516,7 @@ jobs:
             if [ "$suite_status" -eq 0 ]; then
               exit 0
             fi
-            if ! grep -Eqi 'oh no: Bun has crashed|Segmentation fault at address|Illegal instruction|Bus error|Aborted \(core dumped\)' "$suite_log"; then
+            if ! grep -Eqi 'oh no: Bun has crashed|Internal assertion failure|Segmentation fault at address|Illegal instruction|Bus error|Aborted \(core dumped\)' "$suite_log"; then
               echo "::error::macOS suite failed on attempt ${attempt} (exit ${suite_status}); assertion failures are not retried."
               exit "$suite_status"
             fi
@@ -611,7 +611,31 @@ jobs:
         # the only one left on Bun's 5s default, and it is the slowest hardware on the board.
         # Three of its failures were the default firing on tests that had not hung — the
         # composed-acceptance cases spawn a real `ocx start` and were still working at 41s.
-        run: bun test --isolate --timeout 60000 tests --shard=${{ matrix.shard }}/4
+        #
+        # The retry is the same one the macOS leg already carries, for the same reason: a Bun
+        # runtime panic is a crash in the interpreter, not a test result, and failing the shard
+        # on it reports a defect this repository does not have (#2152). An ordinary assertion
+        # failure returns its status immediately — only the crash signatures below are retried,
+        # and only once, so a genuinely broken build cannot be retried into green.
+        shell: bash
+        run: |
+          set +e
+          set -uo pipefail
+          suite_log="$(mktemp -t ocx-windows-suite.XXXXXX)"
+          for attempt in 1 2; do
+            bun test --isolate --timeout 60000 tests --shard=${{ matrix.shard }}/4 2>&1 | tee "$suite_log"
+            suite_status="${PIPESTATUS[0]}"
+            if [ "$suite_status" -eq 0 ]; then
+              exit 0
+            fi
+            if ! grep -Eqi 'oh no: Bun has crashed|Internal assertion failure|Segmentation fault at address|Illegal instruction|Bus error|Aborted \(core dumped\)' "$suite_log"; then
+              echo "::error::Windows shard ${{ matrix.shard }}/4 failed on attempt ${attempt} (exit ${suite_status}); assertion failures are not retried."
+              exit "$suite_status"
+            fi
+            echo "::warning::Bun runtime crash in Windows shard ${{ matrix.shard }}/4 (exit ${suite_status}, attempt ${attempt})."
+          done
+          echo "::error::Bun runtime crash repeated on Windows shard ${{ matrix.shard }}/4; failing after one retry."
+          exit 1
 
       - name: CLI help smoke
         run: bun run src/cli/index.ts help

scripts/ci/run-bun-test-batches.sh

@@ -78,7 +78,7 @@ is_bun_runtime_crash() {
   fi
 
   grep -Eqi \
-    'oh no: Bun has crashed|Segmentation fault at address|Illegal instruction|Bus error|Aborted \(core dumped\)' \
+    'oh no: Bun has crashed|Internal assertion failure|Segmentation fault at address|Illegal instruction|Bus error|Aborted \(core dumped\)' \
     "$log_file"
 }
 

tests/ci-workflows.test.ts

@@ -48,6 +48,21 @@ function hasExactShellCommand(run: string | undefined, expected: string): boolea
     .includes(expected);
 }
 
+/**
+ * Same intent as {@link hasExactShellCommand}, but for a command that is the HEAD of a
+ * pipeline. The retry loops capture the suite with `… 2>&1 | tee "$suite_log"`, so an exact
+ * whole-line match would reject the very shape the retry requires. Anchoring at the start of
+ * the line still rejects an `echo` of the command or a commented-out copy, which is what the
+ * exact match was protecting against.
+ */
+function hasShellCommandHead(run: string | undefined, expected: string): boolean {
+  return (run ?? "")
+    .split(/\r?\n/)
+    .map(line => line.trim())
+    .filter(line => line.length > 0 && !line.startsWith("#"))
+    .some(line => line === expected || line.startsWith(`${expected} `));
+}
+
 function expectSecureLinuxKeyringBootstrap(workflow: string): void {
   const smokeStep = workflow
     .split("- name: OS keyring create/read/delete smoke")[1]
@@ -224,17 +239,52 @@ describe("GitHub Actions hardening", () => {
     // Three composed-acceptance failures were that default firing on tests still working
     // at 41s. Pin the flag so the leg cannot silently drift back to the default.
     const windowsTestCommand = `bun test --isolate --timeout 60000 tests --shard=\${{ matrix.shard }}/${windowsShards.length}`;
-    expect(hasExactShellCommand(`echo ${windowsTestCommand}`, windowsTestCommand)).toBe(false);
+    expect(hasShellCommandHead(`echo ${windowsTestCommand}`, windowsTestCommand)).toBe(false);
     // Binding the assertion to an executable line is only half the guarantee: a
     // step carrying the exact command still runs nothing under `if: false`, and
     // the suite would stay green against a Windows leg that never tests. Require
     // the matching step to be unconditional.
-    const windowsTestSteps = winSteps.filter(step => hasExactShellCommand(step.run, windowsTestCommand));
+    const windowsTestSteps = winSteps.filter(step => hasShellCommandHead(step.run, windowsTestCommand));
     expect(windowsTestSteps.length).toBeGreaterThan(0);
     expect(windowsTestSteps.every(step => step.if === undefined)).toBe(true);
     expect(winSteps.some(step => step.if === "runner.environment == 'self-hosted'"
       && step.run?.includes("git clean -xffd"))).toBe(true);
 
+    // The three crash-signature lists must stay identical, and they must not key on
+    // `panic(thread`.
+    //
+    // Bun emits BOTH `panic(thread 2852)` and `panic(main thread)` for the same class of
+    // failure, so a grep anchored on the numbered form silently misses half of them and the
+    // shard fails on a crash it was supposed to retry. This repository already learned that
+    // once — `devlog/_fin/260731_pr_issue_triage_round/050_windows_ci_flake_rca.md` names
+    // `Internal assertion failure` as the stable fingerprint — and #2152 reintroduced it.
+    // Three copies of one list is the real hazard, so pin the sync rather than the text.
+    const crashSignatures = [
+      "oh no: Bun has crashed",
+      "Internal assertion failure",
+      "Segmentation fault at address",
+      "Illegal instruction",
+      "Bus error",
+    ];
+    const windowsTestRun = windowsTestSteps[0]?.run ?? "";
+    const batchScript = await readText("scripts/ci/run-bun-test-batches.sh");
+    for (const signature of crashSignatures) {
+      expect(`macos:${signature}:${macosTestRun.includes(signature)}`).toBe(`macos:${signature}:true`);
+      expect(`windows:${signature}:${windowsTestRun.includes(signature)}`).toBe(`windows:${signature}:true`);
+      expect(`script:${signature}:${batchScript.includes(signature)}`).toBe(`script:${signature}:true`);
+    }
+    // The thread-numbered form must not be the anchor anywhere.
+    expect(macosTestRun).not.toContain("panic\\(thread");
+    expect(windowsTestRun).not.toContain("panic\\(thread");
+    expect(batchScript).not.toContain("panic\\(thread");
+
+    // Windows carries the same bounded retry as macOS: one attempt, crash-only.
+    expect(hasExactShellCommand(windowsTestRun, "set +e")).toBe(true);
+    expect(windowsTestRun).toContain("for attempt in 1 2");
+    expect(windowsTestRun).not.toContain("while true");
+    expect(windowsTestRun).toContain("assertion failures are not retried");
+    expect(windowsTestRun).toContain("failing after one retry");
+
     // Every job that runs the root suite must build the GUI first, unconditionally.
     // Tests that fetch the served dashboard read their session bootstrap out of
     // `gui/dist/index.html`; with no build the server has no index to serve and the

tests/codex-composed-acceptance.test.ts

@@ -266,7 +266,11 @@ class Fixture {
     runtime: RuntimeRecord,
     path: string,
     init: RequestInit = {},
-    timeoutMs = 10_000,
+    // Scaled like every other budget in this file. This one was left unscaled, and it is what
+    // actually failed `A-reduced` on Windows: the case has a 150 s ceiling and reported ~80 s
+    // elapsed, so the outer budget was never the constraint — a single request hit this fixed
+    // 10 s AbortSignal and aborted the case from inside (#2152).
+    timeoutMs = watchdogMs(10_000),
   ): Promise<{ status: number; body: Record<string, unknown> }> {
     const response = await fetch(`http://127.0.0.1:${runtime.port}${path}`, {
       ...init,
@@ -619,7 +623,18 @@ describe("WP13 composed toggle acceptance", () => {
     const release = join(fx.root, "release");
     const holder = Bun.spawn([process.execPath, lockChildPath], {
       cwd: repoRoot,
-      env: { ...fx.env(fx.homeA, fx.userprofileA), OCX_LOCK_CHILD_PAYLOAD: JSON.stringify({ timeoutMs: 5_000, holdMarker: held, releaseMarker: release }) },
+      // The hold has to outlast the contender's process spawn, which is the slow part on a
+      // Windows shard. The release marker below still ends it early everywhere else, so this
+      // is a ceiling rather than a sleep the test pays for.
+      env: {
+        ...fx.env(fx.homeA, fx.userprofileA),
+        OCX_LOCK_CHILD_PAYLOAD: JSON.stringify({
+          timeoutMs: 5_000,
+          holdMarker: held,
+          releaseMarker: release,
+          holdMs: watchdogMs(3_000),
+        }),
+      },
       stdout: "pipe", stderr: "pipe",
     });
     fx.children.push(holder);

tests/helpers/ci-watchdog.ts

@@ -11,7 +11,14 @@
  * hung test, not to assert latency. Local behaviour is unchanged. Bun's own
  * per-test timeout (`--timeout`, 60 s on CI) would pre-empt a 30 s watchdog,
  * so the lane timeout and this floor move together.
+ *
+ * Windows needs a higher floor still. Its shards run four Bun pools on one runner, and process
+ * spawn there is slower than on the POSIX lanes to begin with — a `ocx restore --json` child
+ * that finishes comfortably elsewhere was observed failing the 30 s floor at 30,147 ms (#2152).
+ * 45 s keeps the watchdog meaningful while staying under the lane's own 60 s per-test timeout,
+ * so a genuinely hung test is still bounded by something rather than running to the ceiling.
  */
 export function watchdogMs(base: number): number {
-  return process.env.CI === "true" ? Math.max(base, 30_000) : base;
+  if (process.env.CI !== "true") return base;
+  return Math.max(base, process.platform === "win32" ? 45_000 : 30_000);
 }

tests/helpers/codex-write-lock-child.ts

@@ -17,6 +17,7 @@ const payload = JSON.parse(process.env.OCX_LOCK_CHILD_PAYLOAD ?? "{}") as {
   timeoutMs?: number;
   holdMarker?: string;
   releaseMarker?: string;
+  holdMs?: number;
 };
 
 const admitted = { authoritySnapshotId: "authority-child" } as AdmissionSnapshot;
@@ -40,7 +41,12 @@ const result = await withCodexWriteLock(
       // an unheld lock and saw `acquired` where the test demands `busy`, which
       // reads exactly like a broken exclusion invariant rather than a late marker.
       writeFileSync(payload.holdMarker, "held");
-      const until = Date.now() + 3_000;
+      // The release marker is the real signal; this is only the ceiling for how long we wait
+      // to see it. Three seconds was enough where the contender starts quickly, but on a
+      // Windows shard the contender's process spawn can outlast the hold — the holder then
+      // releases first and the parent sees `acquired` where it demands `busy`, which reads as
+      // a broken exclusion invariant rather than as a hold that expired too early (#2152).
+      const until = Date.now() + (payload.holdMs ?? 3_000);
       while (Date.now() < until) {
         if (payload.releaseMarker && Bun.file(payload.releaseMarker).size > 0) break;
       }

tests/update-npm-cache-preflight.test.ts

@@ -27,6 +27,20 @@ const canSymlink = (() => {
   }
 })();
 
+/**
+ * Windows needs its own guard, separate from `canSymlink`.
+ *
+ * The capability probe answers "may this user create a symlink", and on a GitHub-hosted
+ * Windows runner the answer is YES — so these cases ran and then failed in the fixture with
+ * `cache_entry_inaccessible`, because what actually differs there is how the preflight reads
+ * mode and access through a Windows symlink, not whether the link can be made (#2152).
+ *
+ * Two neighbouring cases in this file already skip on `process.platform === "win32"` for the
+ * same reason, so this reuses that guard rather than inventing a second mechanism. The
+ * capability check stays: an unprivileged POSIX-like environment still skips honestly.
+ */
+const WINDOWS = process.platform === "win32";
+
 function tempRoot(name: string): string {
   const root = join(tmpdir(), `ocx-cache-preflight-${name}-${process.pid}-${Date.now()}-${Math.random().toString(36).slice(2)}`);
   mkdirSync(root, { recursive: true });
@@ -67,7 +81,7 @@ describe("npm cache access pre-flight", () => {
     }
   });
 
-  test.skipIf(!canSymlink)("lstats normal nested symlinks but never traverses their targets", () => {
+  test.skipIf(WINDOWS || !canSymlink)("lstats normal nested symlinks but never traverses their targets", () => {
     const cache = tempRoot("symlink-cache");
     const missingTarget = join(tempRoot("symlink-target"), "does-not-exist");
     const npx = join(cache, "_npx");
@@ -79,7 +93,7 @@ describe("npm cache access pre-flight", () => {
     expect(inspectNpmCacheDirectory(cache)).toEqual({ ok: true, reason: "cache_accessible" });
   });
 
-  test.skipIf(!canSymlink)("a foreign-owned nested symlink does not block the update", () => {
+  test.skipIf(WINDOWS || !canSymlink)("a foreign-owned nested symlink does not block the update", () => {
     // The distinction that decides whether this feature is usable. A real npm cache is full of
     // symlinks below _npx/node_modules/.bin, and their owner is irrelevant because we never
     // follow them. Rejecting on ownership before skipping the link would abort updates for
@@ -164,7 +178,7 @@ describe("npm cache access pre-flight", () => {
     })).toEqual({ ok: false, reason: "worker_output_malformed" });
   });
 
-  test.skipIf(!canSymlink)("a cache root symlinked to another volume is inspected, not rejected", () => {
+  test.skipIf(WINDOWS || !canSymlink)("a cache root symlinked to another volume is inspected, not rejected", () => {
     // Pointing ~/.npm at another volume is ordinary npm configuration. Rejecting it outright was
     // the same class of false positive as failing on a large cache: it blocks updates for users
     // whose setup is fine. The root is resolved once; nested links are still never followed.