fix(ci): make unit (windows) green — core git-suite timeouts, step budget, skill-refresh rename hardening (LAC-2717) - #38
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds a 30-second timeout to the test script in packages/core/package.json and documents the reasoning in bunfig.toml due to slow git-subprocess tests on Windows runners. The reviewer suggests isolating these slow tests into a separate test suite or script to prevent hiding performance regressions in tests that should run quickly.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| "migration": "bun run script/migration.ts", | ||
| "fix-node-pty": "bun run script/fix-node-pty.ts", | ||
| "test": "bun test --only-failures", | ||
| "test": "bun test --timeout 30000 --only-failures", |
There was a problem hiding this comment.
While this fixes the immediate issue on Windows runners, applying a 30s timeout to all tests in the package could hide future performance regressions in tests that are expected to be fast.
A more targeted approach would be to isolate the slow, git-heavy tests and apply the longer timeout only to them. You could achieve this by:
- Grouping the slow tests into a specific directory or using a naming convention (e.g.,
*.slow.test.ts). - Creating a separate
test:slowscript inpackage.jsonthat runs only these tests with--timeout 30000. - Adjusting the main
testscript to exclude these slow tests and run with the default timeout. - Ensuring your CI pipeline runs both test scripts.
This would maintain a stricter performance baseline for the majority of your tests.
There was a problem hiding this comment.
Addressed in d03e065. Agreed — the package-wide --timeout 30000 is gone. The testEffect helper now accepts a per-suite default (testEffect(layer, { timeout: 30_000 })), applied only in project-copy.test.ts and repository-cache.test.ts (the two git-subprocess-heavy suites). All other core tests keep bun's 5s default, so fast-test regressions stay visible. Per-test opts still override the suite default. One note on the exact suggestion: setDefaultTimeout() was avoided because bun runs all test files in one process, so it would leak to files loaded afterwards.
…avy suites (LAC-2717) RepositoryCache/ProjectCopy tests chain many git subprocess operations. On GitHub-hosted Windows runners each op costs 100ms-2.8s (measured via a temporary per-op timing harness: no hangs, no orphaned processes, all ops complete; cold-start clone peaked at 2.8s). Under full-suite load the chains exceed bun's 5s default per-test timeout. Matches the existing packages/opencode precedent (bun test --timeout 30000); bunfig.toml cannot set this (oven-sh/bun#7789). At 30s all 17 tests in both suites pass on windows-2025 in <7s per file.
…avy suites (LAC-2717) Three fixes from diagnosing the 20-minute step kill on run 29071790348: - Scope the 30s test timeout to the two git-heavy suites via a per-suite default in the testEffect helper, instead of package-wide --timeout (addresses Gemini review: a blanket 30s hides regressions in fast tests). - Raise the unit step timeout-minutes 20 -> 30: with core now green, the opencode suite (~18.5 min alone on 4-core hosted Windows runners) starts ~2.5 min in and cannot finish inside 20; the step was killed mid-run, not hung. - Stream turbo output (--log-order=stream) so a killed step keeps partial logs; grouped mode buffered 18 min of opencode output that was lost. - Pass env through to @opencode-ai/core#test in turbo.json: turbo 2.x strict env mode silently stripped the workflow's OPENCODE_EXPERIMENTAL_DISABLE_FILEWATCHER=true, so the watcher booted in core tests on Windows against the workflow's stated intent.
…ll refresh renames on Windows (LAC-2717) Post-rebase onto the integration branch (which now carries the LAC-2693 windows fixes and a dev merge), run 29077959954 shows two remaining unit (windows) failures, both in core's Snapshot suite dying at bun's 5s default per-test timeout — the same git-subprocess class LAC-2717 fixed in ProjectCopy/RepositoryCache. Apply the same per-suite 30s default. Also retry directory renames in Discovery.pull skill refresh on win32: Defender/indexer briefly holds handles on freshly written directories, so a single rename can fail with EPERM even though the swap is valid (seen as "refreshes a remote skill when its version changes" returning stale content on run 29073953968).
d03e065 to
d750d59
Compare
e192ee4
into
LAC-2385/blacksmith-to-github-runners
Paperclip issue
LAC-2717 — Fix unit (windows) CI failures: @opencode-ai/core ProjectCopy/RepositoryCache tests time out at 5s on GitHub-hosted runners.
Summary
Makes the
unit (windows)job pass on GitHub-hosted runners. The scope grew as each fix exposed the next failure layer (the job had never run to completion on hosted Windows):project-copy,repository-cache, and nowsnapshot) via atestEffect(layer, defaults)per-suite default instead of a package-wide--timeout— a blanket 30s would hide regressions in fast tests (per Gemini review). These suites chain git subprocess ops that cost 100ms–2.8s each under full-suite load on 4-core hosted Windows runners, blowing bun's 5s default.timeout-minuteson the unit step (the opencode suite alone takes ~18.5–20 min on hosted Windows; the old 20-min budget killed the step mid-run once earlier suites stopped failing fast) and--log-order=streamso a killed step keeps partial turbo output instead of losing 18 min of buffered logs.@opencode-ai/core#test: turbo 2.x strict env mode silently stripped the workflow'sOPENCODE_EXPERIMENTAL_DISABLE_FILEWATCHER=true, so the file watcher booted in Windows core tests against the workflow's stated intent.Discovery.pullskill refresh: Defender/indexer briefly holds handles on freshly written directories, so the staging→root directory swap can fail with EPERM even though the swap is valid; retry like graceful-fs does. Seen asDiscovery.pull > refreshes a remote skill when its version changesreturning stale content on run 29073953968.Rebased onto
LAC-2385/blacksmith-to-github-runnersafter the LAC-2693 fixes and dev merge landed there; overlapping test fixes (cwd expectations, routing casing, path-variant tests, prompt loop timeouts) were dropped in favor of the versions already on the base branch.Acceptance criteria
unit (windows)reachessuccesson GitHub-hosted runners on the integration branch.Testing
snapshot,discovery,project-copy,repository-cache, and touched opencode suites pass;tsgo --noEmitclean in both packages.unit (windows)green. Prior run 29071790348 already showed core 1046 pass / 0 fail for the first time on hosted Windows.