From 5f53f195ed5ff74ceecabf6d63dcec3c29387265 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Mon, 26 Oct 2026 13:41:36 -0700 Subject: [PATCH 1/5] chore(quest): claim quest/m0/ci-flakes.md Quest-Claim: 9cf7cf4076519f9c From 922f085837ffa11f347a1d3b23e68dff21fc1cb2 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Mon, 26 Oct 2026 13:56:06 -0700 Subject: [PATCH 2/5] test(gate): assert the headroom rule and that every caller is admitted The race asserted exactly 4 admissions before a deadline, which slow native Windows runners missed. It now admits all 12 callers on a clock running 20 times faster, and checks each admission against the rule. ci(release): skip macOS checks on pull requests Co-Authored-By: Claude Opus 5.5 --- .github/workflows/check.yml | 4 ++-- .github/workflows/release.yml | 27 ++++++++++++++++++----- test/gate.test.ts | 41 ++++++++++++++++++++++++++--------- 3 files changed, 54 insertions(+), 18 deletions(-) diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index b691dcae..de88b9c3 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -70,8 +70,8 @@ jobs: strategy: fail-fast: false # Hosted macOS runners can queue past the timeout, which then reports a job - # that never ran as failed. Pull requests rely on Release's native macOS - # lifecycle checks instead; main still checks the flake's macOS build. + # that never ran as failed. Pull requests skip macOS here and in Release; + # main checks the flake's macOS build, and tags check the released binaries. matrix: runner: ${{ github.event_name == 'pull_request' && fromJSON('["ubuntu-latest"]') || fromJSON('["ubuntu-latest", "macos-14"]') }} runs-on: ${{ matrix.runner }} diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index c49885cb..763c2d37 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -77,17 +77,13 @@ jobs: fail-fast: false matrix: include: - - target: aarch64-apple-darwin - runner: macos-14 - - target: x86_64-apple-darwin - runner: macos-15-intel - target: aarch64-unknown-linux-gnu runner: ubuntu-22.04-arm - target: x86_64-unknown-linux-gnu runner: ubuntu-22.04 runs-on: ${{ matrix.runner }} timeout-minutes: 10 - steps: + steps: &check-steps - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false @@ -128,6 +124,25 @@ jobs: shell: bash run: bash scripts/lifecycle-check.sh "$RUNNER_TEMP/quest" + # Hosted macOS runners can queue past the timeout, which then reports a job + # that never ran as failed. Pull requests skip these; tags, which publish the + # artifacts, and manual runs still check them. Check covers macOS on main. + macos: + name: check (${{ matrix.target }}) + if: github.event_name != 'pull_request' + needs: build + strategy: + fail-fast: false + matrix: + include: + - target: aarch64-apple-darwin + runner: macos-14 + - target: x86_64-apple-darwin + runner: macos-15-intel + runs-on: ${{ matrix.runner }} + timeout-minutes: 10 + steps: *check-steps + windows: name: check (${{ matrix.target }}) needs: build @@ -164,7 +179,7 @@ jobs: ./scripts/install-check.ps1 -Method archive -Target $env:TARGET -Tag $tag -ReleaseDirectory (Resolve-Path release).Path publish: - needs: [check, windows] + needs: [check, macos, windows] if: github.event_name == 'push' && github.ref_type == 'tag' runs-on: ubuntu-latest timeout-minutes: 10 diff --git a/test/gate.test.ts b/test/gate.test.ts index 5031e59a..4eabd9b7 100644 --- a/test/gate.test.ts +++ b/test/gate.test.ts @@ -241,21 +241,31 @@ test("a forward clock step forgets at most the last minute of admissions", async expect(events).toEqual([`sleep ${SETTLE_MS}`]); }); -test("concurrent processes never admit past the headroom", { timeout: 30_000 }, async () => { +test("concurrent processes never admit past the headroom, and admit every caller", { timeout: 30_000 }, async () => { const directory = tempDir(); const ledger = join(directory, "ledger"); const go = join(directory, "go"); const gate = fileURLToPath(new URL("../src/cli/gate.ts", import.meta.url)); - // Each caller waits for the others, then races for one of 4 CPUs' headroom until a shared deadline. + // Each caller waits for the others, then races for one of 4 CPUs' headroom. Their shared clock + // runs 20 times faster from the start, so spacing the other 8 takes seconds rather than a minute. + // The last version a caller publishes ends with its own admission, which it reports. const script = ` const { existsSync, readFileSync } = await import("node:fs"); const { admit, fileHost } = await import(${JSON.stringify(gate)}); console.log("ready"); while (!existsSync(${JSON.stringify(go)})) await new Promise((resolve) => setTimeout(resolve, 2)); - const deadline = Number(readFileSync(${JSON.stringify(go)}, "utf8")); - const host = { ...fileHost(${JSON.stringify(ledger)}), load: () => 0, memory: () => Infinity }; - const signal = AbortSignal.timeout(Math.max(0, deadline - Date.now())); - await admit({ load: 4, memory: 1 }, host, signal).then(() => console.log("admitted"), () => {}); + const start = Number(readFileSync(${JSON.stringify(go)}, "utf8")); + const file = fileHost(${JSON.stringify(ledger)}, () => start + (Date.now() - start) * 20); + let admission; + const host = { + ...file, + load: () => 0, + memory: () => Infinity, + publish: (base, admissions) => file.publish(base, admissions) && Boolean((admission = admissions.at(-1))), + sleep: (ms, signal) => file.sleep(ms / 20, signal), + }; + await admit({ load: 4, memory: 1 }, host, new AbortController().signal); + console.log(JSON.stringify(admission)); `; const callers = Array.from({ length: 12 }, () => { const child = spawn(process.execPath, ["-e", script], { stdio: ["ignore", "pipe", "inherit"] }); @@ -270,13 +280,24 @@ test("concurrent processes never admit past the headroom", { timeout: 30_000 }, return { ready, exited: once(child, "exit").then(([code]) => ({ code, stdout })) }; }); await Promise.all(callers.map(({ ready }) => ready)); - // The next admission is due SETTLE_MS after the last; the deadline comes sooner. - writeFileSync(`${go}.tmp`, String(Date.now() + SETTLE_MS / 2)); + writeFileSync(`${go}.tmp`, String(Date.now())); renameSync(`${go}.tmp`, go); const results = await Promise.all(callers.map(({ exited }) => exited)); expect(results.map(({ code }) => code)).toEqual(Array(12).fill(0)); - expect(results.filter(({ stdout }) => stdout.includes("admitted"))).toHaveLength(4); - expect(fileHost(ledger).ledger().admissions).toHaveLength(4); + const admissions: { id: string; at: number }[] = results + .map(({ stdout }) => JSON.parse(stdout.split("\n")[1])) + .sort((a, b) => a.at - b.at); + expect(new Set(admissions.map(({ id }) => id)).size).toBe(12); + // However slowly the callers arrive, one that finds 4 admissions from the last minute + // comes SETTLE_MS after the previous one. + admissions.forEach(({ at }, i) => { + const recent = admissions.slice(0, i).filter((previous) => previous.at > at - RECENT_MS); + if (recent.length >= 4) expect(at - admissions[i - 1].at).toBeGreaterThanOrEqual(SETTLE_MS); + }); + // The ledger holds only those admissions, up to the last. + const recorded = fileHost(ledger).ledger().admissions; + expect(admissions).toEqual(expect.arrayContaining(recorded)); + expect(recorded).toContainEqual(admissions.at(-1)); }); test.each(["v1", "v2"])("Linux memory counts reclaimable cache and bounds %s container headroom", (version) => { From e9c52ed80c00eb5afd8311deb7e4bd6762fc0f4a Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Mon, 26 Oct 2026 14:13:45 -0700 Subject: [PATCH 3/5] fix(gate): survive concurrent pruning of the ledger on Windows Callers that prune the same superseded version at once made Bun's rmSync throw EFAULT on Windows, crashing a caller after its admission was recorded. A ledger read of a version being pruned throws EPERM there. Co-Authored-By: Claude Opus 5.5 --- src/cli/gate.ts | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/src/cli/gate.ts b/src/cli/gate.ts index 01d151ad..8d18367c 100644 --- a/src/cli/gate.ts +++ b/src/cli/gate.ts @@ -214,12 +214,15 @@ function parse(text: string): Admission[] { return valid ? (value as Admission[]) : []; } -/** Remove a file that another caller may already have removed, or on Windows may still hold open. */ +/** + * Remove a file that another caller may already have removed, or on Windows may still hold open + * or be removing too. Bun on Windows reports a concurrent removal as EFAULT, where Node reports EPERM. + */ function remove(file: string) { try { rmSync(file, { force: true }); } catch (error) { - if (!["EPERM", "EBUSY"].includes((error as NodeJS.ErrnoException).code ?? "")) throw error; + if (!["EPERM", "EBUSY", "EFAULT"].includes((error as NodeJS.ErrnoException).code ?? "")) throw error; } } @@ -231,22 +234,25 @@ function remove(file: string) { */ export function fileHost(directory: string, now = Date.now): Host { const path = (version: number) => join(directory, String(version)); + const latest = (): number => { + let names: string[]; + try { + names = readdirSync(directory); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "ENOENT") return 0; + throw error; + } + return names.reduce((max, name) => (/^\d+$/.test(name) ? Math.max(max, Number(name)) : max), 0); + }; const ledger = (): Ledger => { for (;;) { - let names: string[]; - try { - names = readdirSync(directory); - } catch (error) { - if ((error as NodeJS.ErrnoException).code === "ENOENT") return { version: 0, admissions: [] }; - throw error; - } - const version = names.reduce((max, name) => (/^\d+$/.test(name) ? Math.max(max, Number(name)) : max), 0); + const version = latest(); if (version === 0) return { version, admissions: [] }; try { return { version, admissions: parse(readFileSync(path(version), "utf8")) }; } catch (error) { - // Superseded and pruned since the listing. - if ((error as NodeJS.ErrnoException).code !== "ENOENT") throw error; + // Superseded and pruned since the listing: gone, or on Windows refusing reads while it is removed. + if (latest() === version) throw error; } } }; From 2c8550620d30e19a17727b04f7c40a6e2300a45d Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Mon, 26 Oct 2026 14:20:44 -0700 Subject: [PATCH 4/5] quest: narrow ci-flakes to the Windows git test timeouts Co-Authored-By: Claude Opus 5.5 --- quest/m0/README.md | 2 +- quest/m0/ci-flakes.md | 52 +++++++++++++++++++------------------------ 2 files changed, 24 insertions(+), 30 deletions(-) diff --git a/quest/m0/README.md b/quest/m0/README.md index 3f8be31e..3d87ef9a 100644 --- a/quest/m0/README.md +++ b/quest/m0/README.md @@ -35,7 +35,7 @@ ownership-manifest design: ## Required -- [CI flakes](/quest/m0/ci-flakes.md) - native Windows tests and Release's macOS checks fail only on real breakage +- [CI flakes](/quest/m0/ci-flakes.md) - native Windows git tests fail only on real breakage, not on a slow host - [Review policy](/quest/m0/review-policy.md) - CONTRIBUTING.md says whose review counts - [Gate bench](/quest/m0/gate-bench.md) - measure headroom admission on an idle host - [Agent lifecycle integration](/quest/m0/worktree-agents.md) - reclaim completed and suspended checkouts through their harness diff --git a/quest/m0/ci-flakes.md b/quest/m0/ci-flakes.md index 5cfc8b17..ff7cf34a 100644 --- a/quest/m0/ci-flakes.md +++ b/quest/m0/ci-flakes.md @@ -1,36 +1,30 @@ -# [M] Fail CI checks only on real breakage +# [S] Keep native Windows git tests within their time limit ## Goal -The native Windows test jobs and Release's macOS checks fail only when the -code under test is broken: no admission-count, timeout, or runner-queue -failure on a change that does not touch the code it exercises. +The native Windows test jobs fail only when the code under test is broken: +no `test/move.test.ts` or `test/landed.test.ts` timeout on a change that does +not touch the code it exercises. ## Plan -Decided 2026-10-24 (user) from the spawn and merge wave that landed -[kixelated/quest#202](https://github.com/kixelated/quest/pull/202) through -[#219](https://github.com/kixelated/quest/pull/219), where native Windows -unit tests caused most failed Check runs, including on `main` (82a9566): +Decided 2026-10-24 (user): find why these tests exceed vitest's 5 s on native +Windows, and fix that cause. No raised timeouts, retries, or sleeps. -- **Gate headroom test (user).** `test/gate.test.ts` "concurrent processes - never admit past the headroom" races 12 callers for 4 slots before a - deadline of `SETTLE_MS/2` and asserts exactly 4 were admitted. Slow - `windows-11-arm` and `windows-2022` runners, and loaded local hosts, admit - 2 or 3; the gate never admitted more than its headroom. Assert that - invariant (never more than the headroom, in the output and the ledger) and - that every caller is eventually admitted, rather than an exact count by a - fixed deadline. Rejected: treating under-admission on a slow host as a gate - bug. -- **Timeouts.** Find why `test/move.test.ts` ("refuses a claimed quest", - "reports mentions it does not edit"), several `test/start.test.ts` tests, - and `test/landed.test.ts` exceed vitest's 5 s on native Windows, and fix - that cause. No raised timeouts, retries, or sleeps. -- **Release's macOS queue (user).** Release's macOS `check` jobs can wait - out a GitHub macOS capacity notice and show as a failed check on a PR, as - the `lifecycle (macos-14)` job did before - [kixelated/quest#218](https://github.com/kixelated/quest/pull/218). Apply - the same decision: keep macOS covered on pushes to `main` and where it - guards release artifacts, without failing PRs on a runner that never - arrived. -- Reproduce each on its native runner before fixing, as #218 did. +Found while reproducing on the native runners for +[kixelated/quest#233](https://github.com/kixelated/quest/pull/233), which +fixed the gate headroom test and Release's macOS queue: + +- These tests spend about 95% of their time starting git: 8-13 processes per + test, at 35-80 ms each inside the test workers, against about 2 ms on Linux. + The slowest take 0.8 s on `windows-2022` and 1.4 s on `windows-11-arm` + (Defender real-time scanning on), whether the files run in parallel or alone. +- The four timeouts since 2026-10-21 ran about 3x to 20x slower than that, + three of them within 15 minutes during a CI burst on 2026-10-24, on hosts + whose other test files were also slow. Fewer git processes per test would + not absorb a 20x stall within 5 s. +- Open (user): whether these files take a longer file-level time limit, like + `test/start.test.ts` (30 s), `test/snapshot.test.ts` (20 s), and + `test/worktree.test.ts` (20 s) already do; or cut their git processes + (copied fixture repositories, `maintenance.auto=false`, and batching the + CLI's repeated `git remote`/`git config` lookups) and keep 5 s. From e48273facfbab009ce1c22ea452acff85ffbe16f Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Mon, 26 Oct 2026 17:03:13 -0700 Subject: [PATCH 5/5] test: give move and landed a file-level time limit, finishing ci-flakes Each test spawns git up to a dozen times, and a busy Windows runner has stretched them 3-20x past their usual 0.8-1.4 s. Like start.test.ts, they now allow 30 s, which still catches a hang. Co-Authored-By: Claude Opus 5.5 --- quest/m0/README.md | 1 - quest/m0/ci-flakes.md | 30 ------------------------------ test/landed.test.ts | 5 ++++- test/move.test.ts | 5 ++++- 4 files changed, 8 insertions(+), 33 deletions(-) delete mode 100644 quest/m0/ci-flakes.md diff --git a/quest/m0/README.md b/quest/m0/README.md index 3d87ef9a..0a00e4de 100644 --- a/quest/m0/README.md +++ b/quest/m0/README.md @@ -35,7 +35,6 @@ ownership-manifest design: ## Required -- [CI flakes](/quest/m0/ci-flakes.md) - native Windows git tests fail only on real breakage, not on a slow host - [Review policy](/quest/m0/review-policy.md) - CONTRIBUTING.md says whose review counts - [Gate bench](/quest/m0/gate-bench.md) - measure headroom admission on an idle host - [Agent lifecycle integration](/quest/m0/worktree-agents.md) - reclaim completed and suspended checkouts through their harness diff --git a/quest/m0/ci-flakes.md b/quest/m0/ci-flakes.md deleted file mode 100644 index ff7cf34a..00000000 --- a/quest/m0/ci-flakes.md +++ /dev/null @@ -1,30 +0,0 @@ -# [S] Keep native Windows git tests within their time limit - -## Goal - -The native Windows test jobs fail only when the code under test is broken: -no `test/move.test.ts` or `test/landed.test.ts` timeout on a change that does -not touch the code it exercises. - -## Plan - -Decided 2026-10-24 (user): find why these tests exceed vitest's 5 s on native -Windows, and fix that cause. No raised timeouts, retries, or sleeps. - -Found while reproducing on the native runners for -[kixelated/quest#233](https://github.com/kixelated/quest/pull/233), which -fixed the gate headroom test and Release's macOS queue: - -- These tests spend about 95% of their time starting git: 8-13 processes per - test, at 35-80 ms each inside the test workers, against about 2 ms on Linux. - The slowest take 0.8 s on `windows-2022` and 1.4 s on `windows-11-arm` - (Defender real-time scanning on), whether the files run in parallel or alone. -- The four timeouts since 2026-10-21 ran about 3x to 20x slower than that, - three of them within 15 minutes during a CI burst on 2026-10-24, on hosts - whose other test files were also slow. Fewer git processes per test would - not absorb a 20x stall within 5 s. -- Open (user): whether these files take a longer file-level time limit, like - `test/start.test.ts` (30 s), `test/snapshot.test.ts` (20 s), and - `test/worktree.test.ts` (20 s) already do; or cut their git processes - (copied fixture repositories, `maintenance.auto=false`, and batching the - CLI's repeated `git remote`/`git config` lookups) and keep 5 s. diff --git a/test/landed.test.ts b/test/landed.test.ts index 8aa8fa34..ea8f9f76 100644 --- a/test/landed.test.ts +++ b/test/landed.test.ts @@ -2,12 +2,15 @@ import { execFileSync } from "node:child_process"; -import { expect, test } from "vitest"; +import { expect, test, vi } from "vitest"; import { Tree } from "./fixture"; const FINDING = "neither deletes nor edits its quest since main"; +// Each test spawns git up to a dozen times, which a busy Windows runner can stretch past 5 s. +vi.setConfig({ testTimeout: 30_000 }); + /** The baseline tree committed on `main`, then checked out on `branch`. */ function repo(branch: string): Tree { const tree = Tree.baseline(); diff --git a/test/move.test.ts b/test/move.test.ts index 6ad04197..ba180de3 100644 --- a/test/move.test.ts +++ b/test/move.test.ts @@ -4,7 +4,7 @@ import { execFileSync } from "node:child_process"; import { existsSync } from "node:fs"; import { join } from "node:path"; -import { describe, expect, test } from "vitest"; +import { describe, expect, test, vi } from "vitest"; import { move, remove } from "../src/core"; import { openPullRequests } from "../src/cli/claims"; @@ -12,6 +12,9 @@ import { GitHub, type GitHubDependencies } from "../src/cli/github"; import { main } from "../src/cli/main"; import { ROOT_README, Tree, tempDir } from "./fixture"; +// Each test spawns git up to a dozen times, which a busy Windows runner can stretch past 5 s. +vi.setConfig({ testTimeout: 30_000 }); + function git(dir: string, ...args: string[]): string { return execFileSync("git", ["-C", dir, ...args], { encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] }); }