Skip to content

refactor(showcase/scripts): tame vitest birpc cliff — split validate-pins + audit#4081

Merged
jpr5 merged 9 commits into
mainfrom
fix/split-validate-pins-tests
Apr 19, 2026
Merged

refactor(showcase/scripts): tame vitest birpc cliff — split validate-pins + audit#4081
jpr5 merged 9 commits into
mainfrom
fix/split-validate-pins-tests

Conversation

@jpr5

@jpr5 jpr5 commented Apr 18, 2026

Copy link
Copy Markdown
Contributor

Problem

vitest 3.2.4's hardcoded 60s birpc onTaskUpdate timeout (vitest-dev/vitest#6129) was tripping unit(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.ts 50.6s, audit.test.ts 71.4s) individually approaching/exceeding the cliff. Deterministic failure on #4068/#4018/#4079.

Fix

  • Split validate-pins.test.ts into 5 files (cli, eacces, parsers, r-scenarios, validate-all) plus shared helpers. 50.6s → 12.8s.
  • Split audit.test.ts into 3 files (unit, audit-package, cli) plus shared helpers. 71.4s → ~16s.
  • Isolated bundle-demo-content.test.ts / generate-registry.test.ts / create-integration.test.ts from .git/index.lock races by serializing every restoreFromGitHead invocation through a cross-process advisory lock in test-cleanup.ts.
  • Isolated create-integration.test.ts from the real showcase/packages/ tree. The generator now honors CREATE_INTEGRATION_{PACKAGES,WORKFLOWS}_DIR env overrides; the test runs entirely inside a per-suite os.tmpdir().
  • Flipped fileParallelism: true — safe now that pool: 'forks' gives every file its own process AND FS isolation is in place.

Results

  • Full suite: 158s → 12s locally, 1061/1061 passing across 3 consecutive pnpm nx run @copilotkit/showcase-scripts:test --skip-nx-cache runs.
  • Zero onTaskUpdate timeouts, zero ENOENT, zero index.lock contention across all 3 runs.
  • Every parallel file gets a fresh 60s birpc budget, eliminating the cumulative back-pressure cliff.

Test plan

  • CI green on unit(20.x), unit(22.x), unit(24.x).
  • Manual: pnpm nx run @copilotkit/showcase-scripts:test --skip-nx-cache three times in a row — all 1061/1061, no timeout warnings, no tree drift.

@vercel

vercel Bot commented Apr 18, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview, Comment Apr 19, 2026 2:28am
research-canvas Ready Ready Preview, Comment Apr 19, 2026 2:28am
travel Ready Ready Preview, Comment Apr 19, 2026 2:28am
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
chat-with-your-data Skipped Skipped Apr 19, 2026 2:28am
form-filling Skipped Skipped Apr 19, 2026 2:28am

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📣 Social Copy Generator

Generate social media copies (Twitter/X, LinkedIn, Blog Post) for this PR using Claude.

  • Generate social media copies

@jpr5
jpr5 force-pushed the fix/split-validate-pins-tests branch from 33fcae4 to 50bc568 Compare April 18, 2026 23:14
@vercel
vercel Bot temporarily deployed to Preview – chat-with-your-data April 18, 2026 23:14 Inactive
@vercel
vercel Bot temporarily deployed to Preview – form-filling April 18, 2026 23:14 Inactive
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.
@vercel
vercel Bot temporarily deployed to Preview – form-filling April 18, 2026 23:46 Inactive
@vercel
vercel Bot temporarily deployed to Preview – chat-with-your-data April 18, 2026 23:46 Inactive
@jpr5 jpr5 changed the title refactor(showcase/scripts): split validate-pins.test.ts to dodge vitest birpc cliff refactor(showcase/scripts): tame vitest birpc cliff — split validate-pins + audit Apr 18, 2026
@jpr5
jpr5 force-pushed the fix/split-validate-pins-tests branch from 8420703 to 1650f13 Compare April 18, 2026 23:47
@vercel
vercel Bot temporarily deployed to Preview – form-filling April 18, 2026 23:47 Inactive
@vercel
vercel Bot temporarily deployed to Preview – chat-with-your-data April 18, 2026 23:47 Inactive
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
jpr5 force-pushed the fix/split-validate-pins-tests branch from 1650f13 to e63003a Compare April 19, 2026 00:16
@vercel
vercel Bot temporarily deployed to Preview – chat-with-your-data April 19, 2026 00:16 Inactive
@vercel
vercel Bot temporarily deployed to Preview – form-filling April 19, 2026 00:16 Inactive
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
jpr5 force-pushed the fix/split-validate-pins-tests branch from 36dba03 to f394fcd Compare April 19, 2026 02:10
@vercel
vercel Bot temporarily deployed to Preview – chat-with-your-data April 19, 2026 02:10 Inactive
@vercel
vercel Bot temporarily deployed to Preview – form-filling April 19, 2026 02:10 Inactive
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.
jpr5 added 8 commits April 18, 2026 19:17
… 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
jpr5 force-pushed the fix/split-validate-pins-tests branch from f394fcd to 2dc98b3 Compare April 19, 2026 02:17
@vercel
vercel Bot temporarily deployed to Preview – form-filling April 19, 2026 02:17 Inactive
@vercel
vercel Bot temporarily deployed to Preview – chat-with-your-data April 19, 2026 02:17 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant