From 22b120724b695c8557d14a19fc4a8ed8738c5ce0 Mon Sep 17 00:00:00 2001 From: minhn4 Date: Sat, 1 Aug 2026 16:57:28 +0700 Subject: [PATCH 1/2] fix(ci): make the test workflow green on main MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every job in the test workflow had been failing on main for weeks, across four independent causes. unit (linux): the five `codev run` subprocess tests still on the spawn harness's 30s default were killed at exactly that bound on loaded runners (the tests that already carried an explicit 60s passed). Raise the harness default to 60s — every spawn pays the CLI's cold transpile — and lift the bun-level timeouts of the tests it covers above it. unit (windows): happy-dom keeps each MutationObserver's internal callback only in a WeakRef and unregisters the listener once it is collected, so a GC between observe() and the mutation silently stops the observer. That failed observe-element-offset's reconnect test — the only one in the file with an idle window before its mutation — on every Windows run. Pin the callbacks in the happydom preload, with a test guarding the patch. e2e (linux): session-todo-dock-navigation sampled the dock for a fixed 700ms window while the tab switch ran concurrently; on GitHub-hosted runners the switch alone outlasted it, so every sample landed before the dock reappeared. Bound the sample on the dock appearing instead. e2e (windows): dropped from the matrix. GitHub-hosted windows-latest runs the Playwright suite ~4x slower than upstream's Blacksmith runner — it never finished inside the 30-minute step budget and failed ~20 specs per run on expect timeouts plus sub-pixel layout assertions that track the runner's font metrics. These specs drive a browser SPA against a mocked server, so Chromium rather than the host OS is what they cover; unit tests still run on Windows, where the real platform differences live. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/test.yml | 10 ++++- DIVERGENCES.md | 6 ++- .../session-todo-dock-navigation.spec.ts | 44 +++++++++++-------- packages/app/happydom.ts | 17 +++++++ .../src/happydom-mutation-observer.test.ts | 25 +++++++++++ .../opencode/test/cli/run/run-process.test.ts | 16 +++---- packages/opencode/test/lib/cli-process.ts | 5 ++- 7 files changed, 91 insertions(+), 32 deletions(-) create mode 100644 packages/app/src/happydom-mutation-observer.test.ts diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d9f12b8ef3..a0edfd6e72 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -114,11 +114,17 @@ jobs: strategy: fail-fast: false matrix: + # Fork: linux only. Upstream also runs this matrix on Windows, but on a + # dedicated Blacksmith runner; GitHub-hosted windows-latest runs the + # Playwright suite roughly 4x slower, so it never finished inside the + # 30-minute step budget and failed ~20 specs on 10s expect timeouts plus + # sub-pixel layout assertions that depend on the runner's font metrics. + # These specs exercise a browser SPA against a mocked server, so + # Chromium — not the host OS — is what they cover. Unit tests still run + # on Windows, where the real platform differences live. settings: - name: linux host: ubuntu-latest - - name: windows - host: windows-latest runs-on: ${{ matrix.settings.host }} env: PLAYWRIGHT_BROWSERS_PATH: ${{ github.workspace }}/.playwright-browsers diff --git a/DIVERGENCES.md b/DIVERGENCES.md index 1a7caed18e..ba895dfb54 100644 --- a/DIVERGENCES.md +++ b/DIVERGENCES.md @@ -160,9 +160,11 @@ from the squash message first, or use a plain merge. | `.github/TEAM_MEMBERS` | Deleted | Upstream's team roster. The five workflows that read it (pr-standards, pr-management, triage, duplicate-issues, compliance-close) fetch it from a `dev` branch this fork doesn't have, so they are already inert here. | Keep deleted. | | `packages/script/src/index.ts` | Guard the `.github/TEAM_MEMBERS` read: fall back to an empty team list when the file is absent instead of throwing at module load | This module is imported by the publish path (`publish.ts`); the deleted `TEAM_MEMBERS` (row above) made `bun run release` crash with ENOENT. `Script.team` has no consumers in the fork. | Keep our `exists()` guard; take upstream's surrounding changes. | | `.github/workflows/` — 22 upstream workflows deleted (beta, close-issues, close-prs, compliance-close, containers, deploy, docs-locale-sync, docs-update, duplicate-issues, generate, nix-hashes, notify-discord, opencode, pr-management, pr-standards, publish, publish-github-action, publish-vscode, release-github-action, review, stats, triage) | Deleted | All inert or upstream-only here: gated on `anomalyco/opencode`, publish/deploy to upstream infra, need upstream secrets (`OPENCODE_APP_SECRET`, `OPENCODE_API_KEY`, marketplace/Discord/PostHog tokens), trigger only on a `dev` branch the fork lacks, or read the deleted `TEAM_MEMBERS`. | Keep deleted; delete new upstream workflows too unless they're generic self-contained CI. | -| `.github/workflows/{typecheck,test,nix-eval,storybook}.yml` | Triggers `dev` → `main` (incl. test.yml's concurrency-group ref check); runners `blacksmith-*` → `ubuntu-latest`/`windows-latest`; test.yml unit-test step wrapped in a 2-attempt retry loop (`bun turbo test`, re-run once on failure) with `timeout-minutes` 20 → 60; test.yml exerciser step gains `timeout-minutes: 30`; test.yml Windows unit job pre-installs ripgrep via choco; typecheck.yml runs `bun turbo typecheck --concurrency=3` (was `bun typecheck`, default concurrency 10) | Fork's default branch is `main`; Blacksmith runners aren't installed here, so jobs queued forever. GitHub-hosted `windows-latest` needs well over 20 minutes for the spawn-heavy opencode suite, and several process-spawn/timing-sensitive tests (snapshot-tool-race, skill discovery, flock contention, ...) flake individually there; bun has no per-test retry (`--only-failures` doesn't persist across invocations), so one flake failed the whole job. The retry re-runs only the flaked package (turbo cache-hits the rest); 60 min covers one re-run of the ~27min opencode suite. A wedged exerciser run otherwise holds the job for GitHub's 6-hour ceiling. The file-search index is built by a background ripgrep walk with swallowed errors; Windows's first-use rg.exe download left it permanently empty, failing httpapi-file's search test every run. Turbo's default concurrency (10) runs enough parallel tsgo compilers (opencode alone peaks ~4.6 GB) to OOM the 16 GB ubuntu-latest runner — the job dies with a runner-shutdown signal, no type error; with 4 cores the cap costs little wall time. | Keep our branches + runners + retry loop + timeouts + typecheck concurrency cap; take upstream's other changes. | +| `.github/workflows/{typecheck,test,nix-eval,storybook}.yml` | Triggers `dev` → `main` (incl. test.yml's concurrency-group ref check); runners `blacksmith-*` → `ubuntu-latest`/`windows-latest`; test.yml unit-test step wrapped in a 2-attempt retry loop (`bun turbo test`, re-run once on failure) with `timeout-minutes` 20 → 60; test.yml exerciser step gains `timeout-minutes: 30`; test.yml Windows unit job pre-installs ripgrep via choco; test.yml's `e2e` matrix drops the `windows` entry (linux only — the `unit` matrix keeps both); typecheck.yml runs `bun turbo typecheck --concurrency=3` (was `bun typecheck`, default concurrency 10) | Fork's default branch is `main`; Blacksmith runners aren't installed here, so jobs queued forever. GitHub-hosted `windows-latest` needs well over 20 minutes for the spawn-heavy opencode suite, and several process-spawn/timing-sensitive tests (snapshot-tool-race, skill discovery, flock contention, ...) flake individually there; bun has no per-test retry (`--only-failures` doesn't persist across invocations), so one flake failed the whole job. The retry re-runs only the flaked package (turbo cache-hits the rest); 60 min covers one re-run of the ~27min opencode suite. A wedged exerciser run otherwise holds the job for GitHub's 6-hour ceiling. The file-search index is built by a background ripgrep walk with swallowed errors; Windows's first-use rg.exe download left it permanently empty, failing httpapi-file's search test every run. Turbo's default concurrency (10) runs enough parallel tsgo compilers (opencode alone peaks ~4.6 GB) to OOM the 16 GB ubuntu-latest runner — the job dies with a runner-shutdown signal, no type error; with 4 cores the cap costs little wall time. GitHub-hosted `windows-latest` runs the Playwright app suite roughly 4× slower than upstream's Blacksmith Windows runner: it never finished inside the 30-minute step budget and failed ~20 specs per run on 10s expect timeouts plus sub-pixel layout assertions (`toBeCloseTo(cardHeight, 2)`) that track the runner's font metrics — and those specs drive a browser SPA against a mocked server, so Chromium rather than the host OS is what they cover. | Keep our branches + runners + retry loop + timeouts + linux-only e2e matrix + typecheck concurrency cap; take upstream's other changes. If upstream reworks the e2e matrix, keep the `windows` entry out. | +| `packages/app/happydom.ts` | After `GlobalRegistrator.register()`, wrap `MutationObserver.prototype.observe` to hold a strong reference to each registered listener's internal callback (found via the `mutationListeners` own-symbol on the observed node) | happy-dom (through 20.11.1) stores that callback only in a `WeakRef` and unregisters the listener as soon as it's collected, so a GC between `observe()` and the mutation silently kills the observer. Real browsers hold it strongly. Windows CI runners collect inside that window, which failed `observe-element-offset.test.ts`'s reconnect test on every run while the same file's other tests — which mutate immediately after observing — passed. Guarded by the fork-only `packages/app/src/happydom-mutation-observer.test.ts`. | Keep our patch until happy-dom holds the callback strongly; take upstream's surrounding preload changes. | +| `packages/app/e2e/regression/session-todo-dock-navigation.spec.ts` | `sampleDock` gains a `stopWhenPresent` flag; the "returning to the source tab" sample uses it with a 15s bound instead of a fixed 700ms window, and the returning-empty sample's window is 700ms → 3s | The sampler ran for a fixed wall-clock window while the tab switch happened concurrently; on GitHub-hosted runners the switch alone outlasted 700ms, so every sample was taken before the dock reappeared and `openSamples` came back empty. Bounding on the dock appearing instead of on the clock keeps the same assertion (the first present sample must already be at full opacity — no replayed open animation) without racing the runner. | Keep our sampler bound; take upstream's spec changes. | | `packages/core/test/ripgrep.test.ts` | `120_000` timeout on both `it.live` tests | First use downloads + extracts the ripgrep binary; GitHub-hosted Windows runners blow bun's 5s default. | Keep our timeouts; take upstream's test changes. | -| `packages/opencode/test/cli/run/run-process.test.ts`, `packages/opencode/test/cli/acp/lifecycle.test.ts`, `packages/opencode/test/server/httpapi-file.test.ts`, `packages/opencode/test/server/httpapi-sdk.test.ts`, `packages/opencode/test/effect/runner.test.ts`, `packages/opencode/test/session/prompt.test.ts`, `packages/opencode/test/project/instance-bootstrap.test.ts` | Slow-runner headroom: unknown-model test `timeoutMs`/bound 15s → 30s (test timeout 60s); the two unknown-stream-finish `codev run` subprocess tests ("unknown stream finish preserves partial output…", "--format json records partial output for an unknown stream finish") `timeoutMs` 30s → 60s and test timeout 60s → 120s (pure-upstream spawn tests that pay `codev run` cold-start; neither asserts on duration, so headroom only); the three tool-executing `codev run` subprocess tests ("prints each completed text part in order around a tool continuation", "--format json preserves reasoning, tool, and continuation ordering", "rejects requested permissions by default and allows them with the dangerous flag") `timeoutMs` 30s (harness default) → 60s explicitly, test timeout 60s → 120s — 240s for the permissions test, which pays three sequential `codev run` spawns; these pay cold-start *plus* a real `bash` tool spawn and a continuation turn, and were timing out at exactly the 30s harness default on loaded runners; none asserts on duration, so headroom only; ACP stdin-EOF exit timeout 5s → 30s; file-search-index poll 5s → 120s with a 150s test timeout (the search-endpoints test pays ripgrep's first-use download on Windows runners, like ripgrep.test.ts); Runner fiber-coordination `Effect.timeout` guards 250ms/1s → 10s (pure hang-detectors on already-unblocked awaits; Windows runners starve the event loop past 250ms); prompt.test.ts's two `sleep`-based loop/shell tests ("loop waits while shell runs…", "shell completion resumes queued loop callers") moved from `it.instance` to the file's own `unix` helper so they skip on Windows like their `unixNoLLMServer` siblings (`sleep` isn't on Windows, so the shell fiber hangs to the budget — a genuine hang, not slowness); instance-bootstrap's first test 30s → 60s (pays instance cold-start) | Loaded GitHub-hosted runners spawn processes, fetch ripgrep, and build the lazy file index slower than upstream's Blacksmith runners; 250ms–30s budgets flake or fail outright, and Unix-shell tests can't run on Windows at all. Hang regressions are still caught (a hang exceeds the bound; polls return as soon as ready). | Keep our numbers and the `unix` gating; take upstream's test changes. | +| `packages/opencode/test/lib/cli-process.ts`, `packages/opencode/test/cli/run/run-process.test.ts`, `packages/opencode/test/cli/acp/lifecycle.test.ts`, `packages/opencode/test/server/httpapi-file.test.ts`, `packages/opencode/test/server/httpapi-sdk.test.ts`, `packages/opencode/test/effect/runner.test.ts`, `packages/opencode/test/session/prompt.test.ts`, `packages/opencode/test/project/instance-bootstrap.test.ts` | Slow-runner headroom: the `cli-process.ts` spawn harness's default `timeoutMs` 30s → 60s (every spawn pays the CLI's cold transpile, and the remaining default-timeout `codev run` subprocess tests were being killed at exactly 30s on loaded runners; no test depends on that timeout firing, so this is headroom only) with the bun-level timeouts of the tests it covers raised above it — "exits 0 and writes the response to stdout…", "--format json emits parseable line-delimited JSON…", "--format json emits a pure error record…" and "attach mode rejects local directories…" 60s/30s → 120s, "prints reasoning before text only with --thinking" 60s → 180s (two sequential spawns); unknown-model test `timeoutMs`/bound 15s → 30s → 60s (test timeout 120s); the two unknown-stream-finish `codev run` subprocess tests ("unknown stream finish preserves partial output…", "--format json records partial output for an unknown stream finish") `timeoutMs` 30s → 60s and test timeout 60s → 120s (pure-upstream spawn tests that pay `codev run` cold-start; neither asserts on duration, so headroom only); the three tool-executing `codev run` subprocess tests ("prints each completed text part in order around a tool continuation", "--format json preserves reasoning, tool, and continuation ordering", "rejects requested permissions by default and allows them with the dangerous flag") `timeoutMs` 30s (harness default) → 60s explicitly, test timeout 60s → 120s — 240s for the permissions test, which pays three sequential `codev run` spawns; these pay cold-start *plus* a real `bash` tool spawn and a continuation turn, and were timing out at exactly the 30s harness default on loaded runners; none asserts on duration, so headroom only; ACP stdin-EOF exit timeout 5s → 30s; file-search-index poll 5s → 120s with a 150s test timeout (the search-endpoints test pays ripgrep's first-use download on Windows runners, like ripgrep.test.ts); Runner fiber-coordination `Effect.timeout` guards 250ms/1s → 10s (pure hang-detectors on already-unblocked awaits; Windows runners starve the event loop past 250ms); prompt.test.ts's two `sleep`-based loop/shell tests ("loop waits while shell runs…", "shell completion resumes queued loop callers") moved from `it.instance` to the file's own `unix` helper so they skip on Windows like their `unixNoLLMServer` siblings (`sleep` isn't on Windows, so the shell fiber hangs to the budget — a genuine hang, not slowness); instance-bootstrap's first test 30s → 60s (pays instance cold-start) | Loaded GitHub-hosted runners spawn processes, fetch ripgrep, and build the lazy file index slower than upstream's Blacksmith runners; 250ms–30s budgets flake or fail outright, and Unix-shell tests can't run on Windows at all. Hang regressions are still caught (a hang exceeds the bound; polls return as soon as ready). | Keep our numbers and the `unix` gating; take upstream's test changes. | | `packages/opencode/test/tool/external-directory.test.ts`, `packages/opencode/test/tool/read.test.ts` | Windows path-variant tests keep the drive letter in the `alt` form (drop the `.replace(/^[A-Za-z]:/, "")` step) | A drive-less path is drive-relative: it resolves against the CWD's drive (D: on GitHub-hosted Windows) while the temp dir lives on C:, so the file genuinely isn't there. Slash + case variants still exercise normalization. | Keep our `alt` construction; take upstream's test changes. | | `packages/opencode/test/provider/provider.test.ts`, `packages/opencode/test/provider/transform.test.ts` | Appended two fork-only blocks at the end of each file: an `it.instance` ("declared image modality makes a custom-provider model attachment capable") asserting a config model with no `modalities` resolves `attachment`/`input.image` to false while one carrying the `@codev/gateway` block resolves both true, and a `describe` ("ProviderTransform.message - image input on a custom provider") asserting `unsupportedParts` passes `image`/`file` parts through when the model declares image input and swaps them for the `ERROR: Cannot read …` note when it doesn't. No upstream test changed. | These pin the two upstream code paths the fork's config writer depends on (`provider.ts` modality→capability resolution, `transform.ts` part filtering); upstream leaves both untested, so a refactor there would silently re-break gateway image attachments. | Keep our appended blocks; take upstream's test changes. Append-only, so conflicts should be end-of-file context alone. | | `packages/opencode/test/server/httpapi-exercise/environment.ts` | Exercise config/data dirs `config/opencode`,`data/opencode` → `…/codev`; seeds `codev.json` with `{"share":"manual"}` | Follows the `global.ts` app-dir rename (row above); the fork's `share: "disabled"` default (config.ts row) would 500 the share/unshare scenarios, so the isolated environment opts back in (network stays stubbed by `CODEV_DISABLE_SHARE`). Seed file follows the config-file rename row (global `config.json` is no longer read). | Keep our dir names + seeded config; take upstream's surrounding changes. | diff --git a/packages/app/e2e/regression/session-todo-dock-navigation.spec.ts b/packages/app/e2e/regression/session-todo-dock-navigation.spec.ts index 603c411d55..88119f7715 100644 --- a/packages/app/e2e/regression/session-todo-dock-navigation.spec.ts +++ b/packages/app/e2e/regression/session-todo-dock-navigation.spec.ts @@ -82,7 +82,9 @@ test("animates todo lifecycle without replaying it across session tabs", async ( await switchSession(page, otherID, otherTitle) await expect(dock).toHaveCount(0) - const returningOpen = sampleDock(page, 700) + // Bounded by the dock reappearing, not by wall clock: on a loaded runner the + // tab switch alone outlasts a fixed window, leaving nothing to assert on. + const returningOpen = sampleDock(page, 15_000, true) await switchSession(page, sourceID, sourceTitle) const openSamples = (await returningOpen).filter((sample) => sample.present) expect(openSamples.length).toBeGreaterThan(0) @@ -100,7 +102,7 @@ test("animates todo lifecycle without replaying it across session tabs", async ( events.push(todoEvent(sourceID, [])) await switchSession(page, otherID, otherTitle) - const returningEmpty = sampleDock(page, 700) + const returningEmpty = sampleDock(page, 3_000) await switchSession(page, sourceID, sourceTitle) await expect(dock).toHaveCount(0) expect((await returningEmpty).every((sample) => !sample.present)).toBe(true) @@ -166,21 +168,25 @@ async function switchSession(page: Page, sessionID: string, title: string) { await expectSessionTitle(page, title) } -function sampleDock(page: Page, duration: number) { - return page.evaluate(async (duration) => { - const samples: { present: boolean; height: number; opacity: number }[] = [] - const start = performance.now() - while (performance.now() - start < duration) { - const dock = document.querySelector('[data-component="session-todo-dock"]') - const clip = dock?.parentElement?.parentElement - const label = dock?.querySelector('[data-action="session-todo-toggle"] span[aria-label]') - samples.push({ - present: !!dock, - height: clip?.getBoundingClientRect().height ?? 0, - opacity: label ? Number.parseFloat(getComputedStyle(label).opacity) : 0, - }) - await new Promise(requestAnimationFrame) - } - return samples - }, duration) +function sampleDock(page: Page, duration: number, stopWhenPresent = false) { + return page.evaluate( + async ({ duration, stopWhenPresent }) => { + const samples: { present: boolean; height: number; opacity: number }[] = [] + const start = performance.now() + while (performance.now() - start < duration) { + const dock = document.querySelector('[data-component="session-todo-dock"]') + const clip = dock?.parentElement?.parentElement + const label = dock?.querySelector('[data-action="session-todo-toggle"] span[aria-label]') + samples.push({ + present: !!dock, + height: clip?.getBoundingClientRect().height ?? 0, + opacity: label ? Number.parseFloat(getComputedStyle(label).opacity) : 0, + }) + if (stopWhenPresent && dock) break + await new Promise(requestAnimationFrame) + } + return samples + }, + { duration, stopWhenPresent }, + ) } diff --git a/packages/app/happydom.ts b/packages/app/happydom.ts index de726718f6..93b324c3bb 100644 --- a/packages/app/happydom.ts +++ b/packages/app/happydom.ts @@ -2,6 +2,23 @@ import { GlobalRegistrator } from "@happy-dom/global-registrator" GlobalRegistrator.register() +// happy-dom keeps each MutationObserver's internal mutation callback only in a +// WeakRef and unregisters the listener as soon as it is collected, so a GC +// between observe() and the mutation silently stops the observer. Real browsers +// hold it strongly; pin the callbacks for the life of the test process so +// observer-backed code isn't at the mercy of when the runner decides to collect. +const mutationListenersKey = Object.getOwnPropertySymbols(document.body).find( + (symbol) => symbol.description === "mutationListeners", +) +const retainedMutationCallbacks = new Set() +const originalObserve = MutationObserver.prototype.observe +MutationObserver.prototype.observe = function (target: Node, options?: MutationObserverInit) { + originalObserve.call(this, target, options) + if (!mutationListenersKey) return + const listeners: { callback?: WeakRef }[] = (target as never)[mutationListenersKey] ?? [] + for (const listener of listeners) retainedMutationCallbacks.add(listener.callback?.deref()) +} + const originalGetContext = HTMLCanvasElement.prototype.getContext // @ts-expect-error - we're overriding with a simplified mock HTMLCanvasElement.prototype.getContext = function (contextType: string, _options?: unknown) { diff --git a/packages/app/src/happydom-mutation-observer.test.ts b/packages/app/src/happydom-mutation-observer.test.ts new file mode 100644 index 0000000000..04a24d3ff5 --- /dev/null +++ b/packages/app/src/happydom-mutation-observer.test.ts @@ -0,0 +1,25 @@ +// Guards the MutationObserver patch in ../happydom.ts. happy-dom holds the +// observer's internal callback in a WeakRef, so without the patch a GC between +// observe() and the mutation silently unregisters the observer — which is how +// observer-backed timeline tests failed only on Windows CI runners. +import { expect, test } from "bun:test" + +test("mutation observers survive a collection between observe and the mutation", async () => { + const route = document.createElement("section") + document.body.append(route) + const batches: number[] = [] + const observer = new MutationObserver((records) => batches.push(records.length)) + observer.observe(document.body, { childList: true, subtree: true }) + + Bun.gc(true) + await new Promise((resolve) => setTimeout(resolve, 0)) + Bun.gc(true) + + route.remove() + document.body.append(route) + await new Promise((resolve) => setTimeout(resolve, 0)) + + expect(batches.length).toBeGreaterThan(0) + observer.disconnect() + route.remove() +}) diff --git a/packages/opencode/test/cli/run/run-process.test.ts b/packages/opencode/test/cli/run/run-process.test.ts index 56b6ebf97e..9f072df38b 100644 --- a/packages/opencode/test/cli/run/run-process.test.ts +++ b/packages/opencode/test/cli/run/run-process.test.ts @@ -20,7 +20,7 @@ describe("codev run (non-interactive subprocess)", () => { opencode.expectExit(result, 0) expect(result.stdout).toBe("hello from the test llm\n") }), - 60_000, + 120_000, ) cliIt.concurrent( @@ -60,7 +60,7 @@ describe("codev run (non-interactive subprocess)", () => { opencode.expectExit(plain, 0) expect(plain.stdout).toBe("visible\n") }), - 60_000, + 180_000, ) // Regression for #27371: an unknown model used to hang the process forever @@ -74,12 +74,12 @@ describe("codev run (non-interactive subprocess)", () => { Effect.gen(function* () { const result = yield* opencode.run("say hi", { model: "test/nonexistent-model", - timeoutMs: 30_000, + timeoutMs: 60_000, }) expect(result.exitCode).not.toBe(0) - expect(result.durationMs).toBeLessThan(30_000) + expect(result.durationMs).toBeLessThan(60_000) }), - 60_000, + 120_000, ) // The test provider's SSE error item is interpreted by the SDK as an unknown @@ -138,7 +138,7 @@ describe("codev run (non-interactive subprocess)", () => { .every((line) => line.length > 0), ).toBe(true) }), - 60_000, + 120_000, ) cliIt.concurrent( @@ -161,7 +161,7 @@ describe("codev run (non-interactive subprocess)", () => { }) expect(result.stdout.split("\n").filter(Boolean)).toHaveLength(1) }), - 30_000, + 120_000, ) cliIt.concurrent( @@ -314,7 +314,7 @@ describe("codev run (non-interactive subprocess)", () => { expect(result.exitCode).not.toBe(0) expect(result.stderr).toContain("Cannot attach local directory without a shared filesystem") }), - 30_000, + 120_000, ) cliIt.live( diff --git a/packages/opencode/test/lib/cli-process.ts b/packages/opencode/test/lib/cli-process.ts index ef8a18ed4c..5178eceac4 100644 --- a/packages/opencode/test/lib/cli-process.ts +++ b/packages/opencode/test/lib/cli-process.ts @@ -206,7 +206,10 @@ export function withCliFixture( const spawn = Effect.fn("opencode.spawn")(function* (args: string[], opts?: SpawnOpts) { const start = Date.now() - const timeoutMs = opts?.timeoutMs ?? 30_000 + // 60s, not 30s: every spawn pays the CLI's cold transpile, and the + // GitHub-hosted runners this fork uses run these concurrent subprocess + // tests well past 30s under load. Headroom only — a hang still exceeds it. + const timeoutMs = opts?.timeoutMs ?? 60_000 // stdin: "ignore" so the child doesn't see a piped stdin and block // on `Bun.stdin.text()` (see src/cli/cmd/run.ts — non-TTY stdin is // consumed as the prompt). The old Process.run wrapper defaulted to From ce3dc91395ea9eabc30051238e261289e70b8a6b Mon Sep 17 00:00:00 2001 From: minhn4 Date: Sat, 1 Aug 2026 17:18:45 +0700 Subject: [PATCH 2/2] fix(ci): stop the flock contention test manufacturing false violations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With `staleMs: 1_000`, an owner starved past that bound is *correctly* broken by a contender — and the test then reports that legitimate break as a mutual-exclusion violation, because the evicted worker's `active` sentinel write fails with EEXIST and it exits 1. Sixteen contending node processes routinely starve the owner's 333ms heartbeat past a second on a GitHub-hosted Windows runner, so it failed both CI attempts. Drop the override so it uses the 60s default, matching the effect-flock sibling that runs the same 16-way stress without flaking. Stale recovery keeps its four dedicated single/dual-process tests further down the file; this one asserts exclusion, where the aggressive bound only manufactured false failures. Co-Authored-By: Claude Opus 5 (1M context) --- DIVERGENCES.md | 1 + packages/core/test/util/flock.test.ts | 10 +++++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/DIVERGENCES.md b/DIVERGENCES.md index ba895dfb54..876e114159 100644 --- a/DIVERGENCES.md +++ b/DIVERGENCES.md @@ -163,6 +163,7 @@ from the squash message first, or use a plain merge. | `.github/workflows/{typecheck,test,nix-eval,storybook}.yml` | Triggers `dev` → `main` (incl. test.yml's concurrency-group ref check); runners `blacksmith-*` → `ubuntu-latest`/`windows-latest`; test.yml unit-test step wrapped in a 2-attempt retry loop (`bun turbo test`, re-run once on failure) with `timeout-minutes` 20 → 60; test.yml exerciser step gains `timeout-minutes: 30`; test.yml Windows unit job pre-installs ripgrep via choco; test.yml's `e2e` matrix drops the `windows` entry (linux only — the `unit` matrix keeps both); typecheck.yml runs `bun turbo typecheck --concurrency=3` (was `bun typecheck`, default concurrency 10) | Fork's default branch is `main`; Blacksmith runners aren't installed here, so jobs queued forever. GitHub-hosted `windows-latest` needs well over 20 minutes for the spawn-heavy opencode suite, and several process-spawn/timing-sensitive tests (snapshot-tool-race, skill discovery, flock contention, ...) flake individually there; bun has no per-test retry (`--only-failures` doesn't persist across invocations), so one flake failed the whole job. The retry re-runs only the flaked package (turbo cache-hits the rest); 60 min covers one re-run of the ~27min opencode suite. A wedged exerciser run otherwise holds the job for GitHub's 6-hour ceiling. The file-search index is built by a background ripgrep walk with swallowed errors; Windows's first-use rg.exe download left it permanently empty, failing httpapi-file's search test every run. Turbo's default concurrency (10) runs enough parallel tsgo compilers (opencode alone peaks ~4.6 GB) to OOM the 16 GB ubuntu-latest runner — the job dies with a runner-shutdown signal, no type error; with 4 cores the cap costs little wall time. GitHub-hosted `windows-latest` runs the Playwright app suite roughly 4× slower than upstream's Blacksmith Windows runner: it never finished inside the 30-minute step budget and failed ~20 specs per run on 10s expect timeouts plus sub-pixel layout assertions (`toBeCloseTo(cardHeight, 2)`) that track the runner's font metrics — and those specs drive a browser SPA against a mocked server, so Chromium rather than the host OS is what they cover. | Keep our branches + runners + retry loop + timeouts + linux-only e2e matrix + typecheck concurrency cap; take upstream's other changes. If upstream reworks the e2e matrix, keep the `windows` entry out. | | `packages/app/happydom.ts` | After `GlobalRegistrator.register()`, wrap `MutationObserver.prototype.observe` to hold a strong reference to each registered listener's internal callback (found via the `mutationListeners` own-symbol on the observed node) | happy-dom (through 20.11.1) stores that callback only in a `WeakRef` and unregisters the listener as soon as it's collected, so a GC between `observe()` and the mutation silently kills the observer. Real browsers hold it strongly. Windows CI runners collect inside that window, which failed `observe-element-offset.test.ts`'s reconnect test on every run while the same file's other tests — which mutate immediately after observing — passed. Guarded by the fork-only `packages/app/src/happydom-mutation-observer.test.ts`. | Keep our patch until happy-dom holds the callback strongly; take upstream's surrounding preload changes. | | `packages/app/e2e/regression/session-todo-dock-navigation.spec.ts` | `sampleDock` gains a `stopWhenPresent` flag; the "returning to the source tab" sample uses it with a 15s bound instead of a fixed 700ms window, and the returning-empty sample's window is 700ms → 3s | The sampler ran for a fixed wall-clock window while the tab switch happened concurrently; on GitHub-hosted runners the switch alone outlasted 700ms, so every sample was taken before the dock reappeared and `openSamples` came back empty. Bounding on the dock appearing instead of on the clock keeps the same assertion (the first present sample must already be at full opacity — no replayed open animation) without racing the runner. | Keep our sampler bound; take upstream's spec changes. | +| `packages/core/test/util/flock.test.ts` | "enforces mutual exclusion under process contention" drops its `staleMs: 1_000` override (falling back to the 60s default, matching the `effect-flock.test.ts` sibling), raises `timeoutMs` 15s → 30s and the test timeout 20s → 60s | With a 1s staleness bound, an owner starved past it is *correctly* broken by a contender — and the test then reports that legitimate break as a mutual-exclusion violation (one of the 16 workers exits 1 on the `active` sentinel's `wx` write). 16 contending node processes routinely starve the owner's 333ms heartbeat past 1s on a GitHub-hosted Windows runner, so the test failed both CI attempts. Stale recovery has four dedicated single/dual-process tests further down the file; this one is about exclusion, so the aggressive bound only manufactured false failures. | Keep the dropped override and our numbers; take upstream's test changes. | | `packages/core/test/ripgrep.test.ts` | `120_000` timeout on both `it.live` tests | First use downloads + extracts the ripgrep binary; GitHub-hosted Windows runners blow bun's 5s default. | Keep our timeouts; take upstream's test changes. | | `packages/opencode/test/lib/cli-process.ts`, `packages/opencode/test/cli/run/run-process.test.ts`, `packages/opencode/test/cli/acp/lifecycle.test.ts`, `packages/opencode/test/server/httpapi-file.test.ts`, `packages/opencode/test/server/httpapi-sdk.test.ts`, `packages/opencode/test/effect/runner.test.ts`, `packages/opencode/test/session/prompt.test.ts`, `packages/opencode/test/project/instance-bootstrap.test.ts` | Slow-runner headroom: the `cli-process.ts` spawn harness's default `timeoutMs` 30s → 60s (every spawn pays the CLI's cold transpile, and the remaining default-timeout `codev run` subprocess tests were being killed at exactly 30s on loaded runners; no test depends on that timeout firing, so this is headroom only) with the bun-level timeouts of the tests it covers raised above it — "exits 0 and writes the response to stdout…", "--format json emits parseable line-delimited JSON…", "--format json emits a pure error record…" and "attach mode rejects local directories…" 60s/30s → 120s, "prints reasoning before text only with --thinking" 60s → 180s (two sequential spawns); unknown-model test `timeoutMs`/bound 15s → 30s → 60s (test timeout 120s); the two unknown-stream-finish `codev run` subprocess tests ("unknown stream finish preserves partial output…", "--format json records partial output for an unknown stream finish") `timeoutMs` 30s → 60s and test timeout 60s → 120s (pure-upstream spawn tests that pay `codev run` cold-start; neither asserts on duration, so headroom only); the three tool-executing `codev run` subprocess tests ("prints each completed text part in order around a tool continuation", "--format json preserves reasoning, tool, and continuation ordering", "rejects requested permissions by default and allows them with the dangerous flag") `timeoutMs` 30s (harness default) → 60s explicitly, test timeout 60s → 120s — 240s for the permissions test, which pays three sequential `codev run` spawns; these pay cold-start *plus* a real `bash` tool spawn and a continuation turn, and were timing out at exactly the 30s harness default on loaded runners; none asserts on duration, so headroom only; ACP stdin-EOF exit timeout 5s → 30s; file-search-index poll 5s → 120s with a 150s test timeout (the search-endpoints test pays ripgrep's first-use download on Windows runners, like ripgrep.test.ts); Runner fiber-coordination `Effect.timeout` guards 250ms/1s → 10s (pure hang-detectors on already-unblocked awaits; Windows runners starve the event loop past 250ms); prompt.test.ts's two `sleep`-based loop/shell tests ("loop waits while shell runs…", "shell completion resumes queued loop callers") moved from `it.instance` to the file's own `unix` helper so they skip on Windows like their `unixNoLLMServer` siblings (`sleep` isn't on Windows, so the shell fiber hangs to the budget — a genuine hang, not slowness); instance-bootstrap's first test 30s → 60s (pays instance cold-start) | Loaded GitHub-hosted runners spawn processes, fetch ripgrep, and build the lazy file index slower than upstream's Blacksmith runners; 250ms–30s budgets flake or fail outright, and Unix-shell tests can't run on Windows at all. Hang regressions are still caught (a hang exceeds the bound; polls return as soon as ready). | Keep our numbers and the `unix` gating; take upstream's test changes. | | `packages/opencode/test/tool/external-directory.test.ts`, `packages/opencode/test/tool/read.test.ts` | Windows path-variant tests keep the drive letter in the `alt` form (drop the `.replace(/^[A-Za-z]:/, "")` step) | A drive-less path is drive-relative: it resolves against the CWD's drive (D: on GitHub-hosted Windows) while the temp dir lives on C:, so the file genuinely isn't there. Slash + case variants still exercise normalization. | Keep our `alt` construction; take upstream's test changes. | diff --git a/packages/core/test/util/flock.test.ts b/packages/core/test/util/flock.test.ts index 53bfc1874e..e6c984aa59 100644 --- a/packages/core/test/util/flock.test.ts +++ b/packages/core/test/util/flock.test.ts @@ -122,6 +122,11 @@ describe("util.flock", () => { const key = "flock:stress" const n = 16 + // No staleMs override (60s default, like the effect sibling): this asserts + // mutual exclusion, not stale recovery — the stale-break paths have their own + // tests below. A 1s staleness bound made a legitimate break look like a + // violation whenever 16 contending processes starved an owner's 333ms + // heartbeat past the bound, which is routine on a loaded Windows runner. const out = await Promise.all( Array.from({ length: n }, () => run({ @@ -130,8 +135,7 @@ describe("util.flock", () => { done, active, holdMs: 30, - staleMs: 1_000, - timeoutMs: 15_000, + timeoutMs: 30_000, }), ), ) @@ -144,7 +148,7 @@ describe("util.flock", () => { .map((x) => x.trim()) .filter(Boolean) expect(lines.length).toBe(n) - }, 20_000) + }, 60_000) test("times out while waiting when lock is still healthy", async () => { await using tmp = await tmpdir()