refactor(showcase/scripts): tame vitest birpc cliff — split validate-pins + audit#4081
Merged
Conversation
jpr5
requested review from
marthakelly,
mme,
ranst91 and
tylerslaton
as code owners
April 18, 2026 23:12
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Contributor
📣 Social Copy GeneratorGenerate social media copies (Twitter/X, LinkedIn, Blog Post) for this PR using Claude.
|
jpr5
force-pushed
the
fix/split-validate-pins-tests
branch
from
April 18, 2026 23:14
33fcae4 to
50bc568
Compare
jpr5
added a commit
that referenced
this pull request
Apr 18, 2026
audit.test.ts is 3034 lines / 119 tests and on Node 22 CI its single-file runtime grew from 36.7s (PR #4071) to 71.4s (PR #4081) — over the hardcoded 60s birpc onTaskUpdate RPC window (vitest #6129). Same cliff that motivated the validate-pins split earlier in this PR. Extract makeTmpTree, makeConfig, writePackage, makeExampleDir, anomalyStrings, and the AUDIT_SCRIPT path constant into audit.shared.ts so the forthcoming split files can share them without duplication. No behavior change — the original audit.test.ts still re-declares its own local copies until the split commit removes them.
jpr5
force-pushed
the
fix/split-validate-pins-tests
branch
from
April 18, 2026 23:47
8420703 to
1650f13
Compare
jpr5
added a commit
that referenced
this pull request
Apr 19, 2026
audit.test.ts is 3034 lines / 119 tests and on Node 22 CI its single-file runtime grew from 36.7s (PR #4071) to 71.4s (PR #4081) — over the hardcoded 60s birpc onTaskUpdate RPC window (vitest #6129). Same cliff that motivated the validate-pins split earlier in this PR. Extract makeTmpTree, makeConfig, writePackage, makeExampleDir, anomalyStrings, and the AUDIT_SCRIPT path constant into audit.shared.ts so the forthcoming split files can share them without duplication. No behavior change — the original audit.test.ts still re-declares its own local copies until the split commit removes them.
jpr5
force-pushed
the
fix/split-validate-pins-tests
branch
from
April 19, 2026 00:16
1650f13 to
e63003a
Compare
jpr5
added a commit
that referenced
this pull request
Apr 19, 2026
audit.test.ts is 3034 lines / 119 tests and on Node 22 CI its single-file runtime grew from 36.7s (PR #4071) to 71.4s (PR #4081) — over the hardcoded 60s birpc onTaskUpdate RPC window (vitest #6129). Same cliff that motivated the validate-pins split earlier in this PR. Extract makeTmpTree, makeConfig, writePackage, makeExampleDir, anomalyStrings, and the AUDIT_SCRIPT path constant into audit.shared.ts so the forthcoming split files can share them without duplication. No behavior change — the original audit.test.ts still re-declares its own local copies until the split commit removes them.
jpr5
force-pushed
the
fix/split-validate-pins-tests
branch
from
April 19, 2026 02:10
36dba03 to
f394fcd
Compare
jpr5
added a commit
that referenced
this pull request
Apr 19, 2026
…ate timeout (#4087) ## Problem `showcase/scripts` test runs intermittently fail with birpc `onTaskUpdate` timeouts, caused by an upstream bug in vitest 3.x's subprocess reporter transport ([vitest-dev/vitest#8164](vitest-dev/vitest#8164)). Fix landed upstream in [#8297](vitest-dev/vitest#8297), shipped in v4.0.0-beta.4+. ## Why this is safe - **Blast radius is this one package.** Root `package.json` already declares `"vitest": "^4.1.3"`. The only holdout was `showcase/scripts/package.json` at `"vitest": "^3.0.0"`. All 10 subprocess-spawning test files in the monorepo live in `showcase/scripts/__tests__/` — zero elsewhere — so this is the only package actually affected by the bug. - **Zero vitest API breaks in the diff.** Spot-checked: no `vi.mock`, no `expect.extend`, no `.concurrent`, no snapshots, no coverage config, no custom reporters. Only `vi.spyOn` (unchanged in v4) and standard `describe`/`it`/`expect`. - **Pool config unchanged.** `pool: 'forks'` + `fileParallelism` behave identically in v3 and v4. - **Lockfile diff is minimal.** Only `showcase/scripts` import resolution changes + a new `@vitest/[email protected]` snapshot. No other package was edited; no other package's resolution changed. vitest 3.2.4 and 2.1.9 continue to resolve side-by-side for the packages that still pin them. ## Verification 3 consecutive `pnpm nx run @copilotkit/showcase-scripts:test --skip-nx-cache` runs on Node 20.20.2: | Run | Result | Wall time | Tests | |-----|--------|-----------|-------| | 1 | green | 34.45s | 1061/1061 | | 2 | green | 29.64s | 1061/1061 | | 3 | green | 29.46s | 1061/1061 | Zero `onTaskUpdate` errors, zero unhandled exceptions across all three runs. ## Test plan - [ ] CI unit(20.x) green — this has been the red one on recent PRs due to this exact bug - [ ] CI unit(22.x) green - [ ] CI unit(24.x) green ## Relationship to #4081 Complementary, not overlapping. #4081 splits / isolates the showcase-scripts test suite as a workaround at the Nx/pool-config layer. This PR fixes the root cause in the test runner itself. Once this merges, the `onTaskUpdate` failures #4081 has been chasing should clear; #4081 can then rebase on a clean main and its remaining work (the split/isolation improvements) can be evaluated on its own merits rather than as a bug workaround.
… lock `restoreFromGitHead` runs `git checkout HEAD -- <paths>` inside three sibling suites (bundle-demo-content, generate-registry, create-integration) plus concurrent `git` from the pre-commit hook. Every one of those grabs `.git/index.lock` — parallel callers race for it and flake with "fatal: Unable to create .git/index.lock: File exists". Acquire a cross-process advisory lock (atomic `fs.mkdirSync` of `/tmp/copilotkit-showcase-git-restore.lock`) around every git invocation in this module: partition, pre-heal diff, checkout, post-heal diff. Held for the entire sequence so intermediate state is consistent from the caller's perspective. Stale locks (> 60s) are reaped before the wait loop so a hard-killed previous run can't wedge subsequent runs. Unblocks enabling vitest `fileParallelism: true` — the three consumer suites can now run in parallel forks without stepping on each other's git operations or on the pre-commit hook's.
… tmpdirs create-integration.test.ts invoked the real generator against `showcase/packages/` and `.github/workflows/`, then healed the mutations in afterEach. Under `fileParallelism: true` that collided with generate-registry.test.ts (concurrent readdirSync of `showcase/packages/` observed partial state → ENOENT) and with every suite that restored workflow YAMLs from git (`.git/index.lock` contention). Teach `create-integration/index.ts` to honor two env overrides — `CREATE_INTEGRATION_PACKAGES_DIR` and `CREATE_INTEGRATION_WORKFLOWS_DIR` — that redirect its writes to any directory. Production behavior unchanged (defaults resolve to the real paths as before). Rewrite the test to create a per-suite `os.tmpdir()`-backed root, seed it with copies of the three real workflow YAMLs so the generator's regex-based edits still match, and point both env vars there. The test now never mutates a tracked file — no restorer, no git invocation, no cross-suite shared state. generate-registry can scan real `showcase/packages/` concurrently without collisions. The regression-guard test still exercises the same cleanup semantics, just against the tmpdir-backed baseline map instead of `workflowRestorer.snapshotMap`.
…red module Hoist tmpdir/write/withTmp helpers and FIXTURES_DIR/VALIDATE_PINS_SCRIPT path constants out of validate-pins.test.ts so the forthcoming split files can share them without duplication. Behaviour-neutral.
…ategory Break the 3567-line validate-pins.test.ts into five smaller test files so each one fits comfortably under vitest's hardcoded 60s birpc onTaskUpdate RPC window (upstream vitest #6129: DEFAULT_TIMEOUT = 6e4 in the bundled birpc). pool: 'forks' + fileParallelism: false already gives each file a fresh 60s RPC budget; splitting ensures no single file is anywhere near that cliff even when CI is slow. Split buckets, chosen for logical cohesion and balanced subprocess load: - validate-pins.parsers.test.ts - pure parsers (no validateAll, no subprocess) - validate-pins.validate-all.test.ts - in-process validateAll + drift detection - validate-pins.cli.test.ts - CLI subprocess exit codes - validate-pins.eacces.test.ts - chmod/EACCES-routed subprocess tests - validate-pins.r-scenarios.test.ts - R29/R33 regression scenarios Test count is identical pre/post-split (134 tests). Behaviour-neutral refactor: no test logic changes, only file boundaries + shared helper imports.
audit.test.ts is 3034 lines / 119 tests and on Node 22 CI its single-file runtime grew from 36.7s (PR #4071) to 71.4s (PR #4081) — over the hardcoded 60s birpc onTaskUpdate RPC window (vitest #6129). Same cliff that motivated the validate-pins split earlier in this PR. Extract makeTmpTree, makeConfig, writePackage, makeExampleDir, anomalyStrings, and the AUDIT_SCRIPT path constant into audit.shared.ts so the forthcoming split files can share them without duplication. No behavior change — the original audit.test.ts still re-declares its own local copies until the split commit removes them.
Replaces the monolithic audit.test.ts (3034 lines, 119 tests, ~71s single-file on Node 22 CI) with three scenario-scoped files, each comfortably under the 60s birpc onTaskUpdate RPC window (vitest #6129): - audit.unit.test.ts (59 tests, ~40ms local) — readManifest, countFiles, EACCES-on-spec-dir, findExamplesSource (all three describes), parseArgs, BORN_IN_SHOWCASE, SLUG_TO_EXAMPLES, isProgrammerBug, UnreadableDirError, canonicalizeForIsMain, listShowcasePackageSlugs. - audit.audit-package.test.ts (44 tests, ~510ms local) — auditPackage (main + direct-caller invariants), buildReport (main + scalar summary + --strict exit code), parseArgs --strict/--columns, computeExitCode --strict semantics. - audit.cli.test.ts (16 tests, ~8.2s local) — main() CLI exit codes, --columns filtering, module isMain guard. Isolated here so the per-file fork window absorbs all the subprocess cost. Tests moved, not copied — 119 + 0 + 0 = 119 total preserved. Helpers live in audit.shared.ts (extracted in the previous commit). Per-file caps hold with generous margin on CI: even the subprocess-heavy CLI file (the worst case) is ~8s locally vs the 60s budget.
…s in place Prior reasons for `fileParallelism: false` are resolved: - Env-var mutation races (VALIDATE_PINS_REPO_ROOT, SHOWCASE_AUDIT_ROOT, VALIDATE_PARITY_REPO_ROOT) are moot under `pool: 'forks'` — every file already gets its own node process with its own `process.env`. - `.git/index.lock` races between suites that call `restoreFromGitHead` are fixed by the cross-process lock in test-cleanup.ts. - The create-integration vs generate-registry collision on `showcase/packages/` is fixed by the create-integration tmpdir isolation. Under fork-per-file + parallel, each test file also gets a fresh 60s birpc `onTaskUpdate` budget (vitest #6129), eliminating the cumulative RPC back-pressure that tripped unit(20.x/22.x/24.x) on #4018/#4068/#4079. Empirical: local full suite 158s → 12s, 1061/1061 passing across three consecutive `--skip-nx-cache` runs with zero timeouts, zero ENOENT, zero index.lock contention.
… refactor
- test-cleanup.ts: `new Error(msg, { cause })` is ES2022; workspace lib is
ES2020 so the two-arg overload is missing. Replaced with an
`errorWithCause()` helper that assigns `.cause` after construction.
Runtime is identical (Node >=16.9); only the TS signature differs.
- test-cleanup.ts: retyped `SAFE_STDIO` as `StdioOptions` (still frozen at
runtime to keep `test-cleanup.test.ts` freeze assertion green) so
spreading `SAFE_EXEC_OPTS` into `execFileSync(..., opts)` no longer trips
the readonly-vs-mutable-array mismatch on `stdio` (fixes the error at
create-integration.test.ts:132).
- create-integration/index.ts: dropped unused `devCmd` local and unused
`args` parameter on `generateDemoPage` (+ call site); both were dead code
introduced during the parallel-isolation refactor.
- validate-pins.parsers.test.ts: annotated all `withTmp((tmp) => ...)`
callbacks as `(tmp: string)` for robustness under LSP module-resolution
glitches. Matches the contract in `validate-pins.shared.ts`.
Tests: 1061/1061 pass (`pnpm nx run @copilotkit/showcase-scripts:test`).
jpr5
force-pushed
the
fix/split-validate-pins-tests
branch
from
April 19, 2026 02:17
f394fcd to
2dc98b3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
vitest 3.2.4's hardcoded 60s birpc
onTaskUpdatetimeout (vitest-dev/vitest#6129) was trippingunit(20.x/22.x/24.x)on CopilotKit showcase-scripts tests. Root driver: cumulative RPC back-pressure from serial file execution plus two oversized test files (validate-pins.test.ts50.6s,audit.test.ts71.4s) individually approaching/exceeding the cliff. Deterministic failure on #4068/#4018/#4079.Fix
validate-pins.test.tsinto 5 files (cli,eacces,parsers,r-scenarios,validate-all) plus shared helpers. 50.6s → 12.8s.audit.test.tsinto 3 files (unit,audit-package,cli) plus shared helpers. 71.4s → ~16s.bundle-demo-content.test.ts/generate-registry.test.ts/create-integration.test.tsfrom.git/index.lockraces by serializing everyrestoreFromGitHeadinvocation through a cross-process advisory lock intest-cleanup.ts.create-integration.test.tsfrom the realshowcase/packages/tree. The generator now honorsCREATE_INTEGRATION_{PACKAGES,WORKFLOWS}_DIRenv overrides; the test runs entirely inside a per-suiteos.tmpdir().fileParallelism: true— safe now thatpool: 'forks'gives every file its own process AND FS isolation is in place.Results
pnpm nx run @copilotkit/showcase-scripts:test --skip-nx-cacheruns.onTaskUpdatetimeouts, zero ENOENT, zeroindex.lockcontention across all 3 runs.Test plan
unit(20.x),unit(22.x),unit(24.x).pnpm nx run @copilotkit/showcase-scripts:test --skip-nx-cachethree times in a row — all 1061/1061, no timeout warnings, no tree drift.