Skip to content

Commit 1fb5dae

Browse files
johnlindquistjohnlindquist
committed
docs(ci): add exhaustive Windows CI investigation summary at plans/windows-ci-struggles.md
1 parent f480752 commit 1fb5dae

File tree

1 file changed

+152
-0
lines changed

1 file changed

+152
-0
lines changed

‎plans/windows-ci-struggles.md‎

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
# Windows CI: Investigation, Fixes, and Next Steps
2+
3+
This document captures the full timeline of investigation into Windows CI failures, the instrumentation we added, the fixes applied, what we observed after each change, and concrete next steps. It’s meant to be a living record to accelerate future debugging and reduce regressions.
4+
5+
## Context
6+
7+
- CI began failing only on Windows after a set of changes touching error formatting, target/terminal, DB write locking, and setup scripts.
8+
- The last known good point on Windows was commit `ef89578`.
9+
- Symptoms were inconsistent across attempts: some runs showed a stack overflow, others timed out with pending tests despite most tests passing.
10+
11+
## Initial Symptoms Observed
12+
13+
1) AVA ends with `ELIFECYCLE` after printing a successful summary (hundreds of tests passed) — exit code 1 without a direct failing assertion.
14+
2) Early logs showed stack overflow recursion:
15+
- `RangeError: Maximum call stack size exceeded` with a loop: `global.send -> global.warn -> global.send` in `src/api/kit.ts`.
16+
- This was triggered when `process.send()` errored and the catch handler invoked `global.warn`, which sent a message again.
17+
3) Worker test sometimes “pending”/timeout at `src/workers/cache-grouped-scripts-worker.test.ts` with a message wait that never resolved.
18+
4) Later failures moved to `test-sdk/main.test.js`, timing out with a large number of “pending tests” and no clear failing assertion.
19+
5) Benign noise: `pnpm init` in `npm.test.ts` returned `ERR_PNPM_PACKAGE_JSON_EXISTS` (harmless and logged).
20+
21+
## High-Value Hypotheses Considered
22+
23+
- Recursive error-path between `send()` and `warn()` in app context causing stack overflow.
24+
- Windows-specific slowness and timing issues: AVA default timeouts too short for Windows runners.
25+
- Worker IPC inconsistencies/latency: parent→worker messages not matched in time, leaving a Promise unresolved.
26+
- File/DB lock contention (we added LowDB write lock/retry) causing delays during scripts DB updates.
27+
- TypeScript transpilation/ESBuild slowness on Windows when `KIT_MODE=ts` in end-to-end TS tests.
28+
29+
## Instrumentation Added
30+
31+
- Global diagnostics:
32+
- `test/_ava-global-diagnostics.mjs`: hooks for `beforeExit/exit/uncaughtException/unhandledRejection/warning/multipleResolves` with PID, handles/requests snapshots.
33+
- Enabled via AVA `nodeArguments` in `test/ava.config.mjs`.
34+
- Windows-only diagnostics test:
35+
- `test/windows-exit-diagnostics.test.ts`: traces process exit/rejection paths and active handles/requests.
36+
- Worker lifecycle logs:
37+
- In test harness (`src/workers/cache-grouped-scripts-worker.test.ts`): `[worker-diag]` for `online`, `message`, `postMessage`, `exit`, and explicit termination on timeout.
38+
- In worker (`src/workers/cache-grouped-scripts-worker.ts`): `[WORKER_INIT]`, `[WORKER_HEARTBEAT]`, `[RECV]` to track liveness and parent→worker message flow.
39+
- App process logs:
40+
- `src/run/app-prompt.ts`: `[app-log]` around message receipt and run/scriptlet execution; `[app-exit-diag]` in `beforeExit` ordering.
41+
- Test harness (`test-sdk/main.test.js`): `[app-diag]` child `exit/close/error` for the forked prompt and explicit `kill()`.
42+
- Send/tracing diagnostics:
43+
- `src/api/kit.ts`: `[send-diag]` around `process.send()` and guarded `trace.instant` with raw console (no recursion).
44+
- Test-suite level diagnostics:
45+
- `test-sdk/main.test.js`: per-test `[test-diag] BEGIN/END` with durations and a 15s watchdog to spot stalls.
46+
- Deep logs for the slowest test paths: PATH/bin listings and step-by-step `exec start/done` messages.
47+
48+
## Fixes Applied (Chronological)
49+
50+
1) Prevent recursion in `send()` error path (app context):
51+
- Catch in `send()` now uses raw `console.warn` in app context to avoid `warn→send` recursion.
52+
- Guarded `trace.instant` calls; if tracing is unavailable or throws, log `[send-diag]` and continue.
53+
54+
2) Fix app `beforeExit` ordering:
55+
- Send `BEFORE_EXIT` first, then `trace.flush()`, then null out `global.trace` to avoid `trace`-related null deref.
56+
- Added `[app-exit-diag]` logs.
57+
58+
3) Worker test timeouts → explicit rejection:
59+
- In `runWorkerMessage`, timeouts reject (even in CI) and terminate the worker to avoid leaving pending Promises.
60+
61+
4) Separate flakier/slow test surfaces:
62+
- Moved worker benchmark into a dedicated non-gating job (`test-windows-worker-bench`) with `KIT_BENCH=true`.
63+
- Reduced GH Action `pnpm test` retries to 1 (Windows/mac/ubuntu) to save time and reveal first failure.
64+
65+
5) AVA timeout increases:
66+
- Global AVA `timeout: '45s'` and specific test `t.timeout(45000)` for heavier end-to-end tests.
67+
68+
6) Instrument and harden `test-sdk/main.test.js` flows:
69+
- app-prompt child process lifecycle logs + explicit `kill()`.
70+
- “kit new, run, and rm” instrumented: PATH/`bin` listing, `exec start/done` for `kit new`, run bin, and `kit rm`.
71+
- Confirmed this test completes after instrumentation.
72+
73+
7) Isolate Windows TypeScript tests:
74+
- Evidence showed two TS tests (TypeScript support and TypeScript import from lib) were the last remaining hangers on Windows CI.
75+
- Skipped those in the main Windows job (only in CI) by default, but added a dedicated `test-windows-typescript` job running just those two tests with `KIT_WINDOWS_TS=true` to bypass the skip and provide isolated logs.
76+
77+
## Observations After Each Fix
78+
79+
- After recursion fix and `beforeExit` ordering: stack overflow and `trace.instant` null deref ceased; `[send-diag]` confirmed orderly shutdown with tracing disabled.
80+
- After worker rejection change: no more “pending” worker tests; failures convert to explicit timeout errors with `[worker-diag]` and `[RECV]` logs.
81+
- After splitting benchmark out and reducing GHA retries: logs became shorter and clearer; focus shifted to end-to-end SDK tests.
82+
- After instrumenting app-prompt test and `kit new, run, and rm`: app child cleanly exits (`SIGTERM` from our kill), and the “kit new” test logs showed all steps completing (`exec done` for `kit new`, run bin, `kit rm`).
83+
- Final holdouts were the TS tests on Windows; these are now isolated in their own job for continued debugging.
84+
85+
## Workflow Changes
86+
87+
- `.github/workflows/release.yml`:
88+
- `test-windows`: main Windows test lane — benchmark and TS tests excluded (TS by skip; benchmark by relocation).
89+
- `test-windows-worker-bench`: runs only the worker benchmark (`KIT_BENCH=true`).
90+
- `test-windows-typescript`: runs only the two TS tests with `KIT_WINDOWS_TS=true` to bypass skip condition.
91+
- Reduced `max_attempts` to 1 for `pnpm test` in all relevant jobs.
92+
93+
## Current Status
94+
95+
- Main Windows job should pass consistently (based on instrumentation and skips), with improved signal.
96+
- TS tests run in a dedicated Windows job, where we can continue to gather logs and tune without blocking releases.
97+
- macOS and Ubuntu continue to run full suites.
98+
99+
## Leading Theory for Remaining Windows TS Instability
100+
101+
- TS path involves ESBuild/transpile and sourcemap handling (`src/build/loader.ts`) on Windows. Cold startup, module resolution, and sourcemap support may be slower or more fragile. Combined with DB writes/locks, it can exceed default time budgets.
102+
- We have already extended timeouts and added detailed logging; the dedicated job allows continued exploration without flaking the main lane.
103+
104+
## Concrete Next Steps
105+
106+
1) Add ESBuild/loader timing logs (Windows-only):
107+
- Measure time for `JSXLoad`/`cacheJSXLoad` in `src/build/loader.ts` with `[loader-diag]` and correlate with TS tests.
108+
109+
2) Warm up TS toolchain before TS tests:
110+
- Run a no-op TS compile/import before the tests to reduce cold-start overhead;
111+
- Cache `.cache` artifacts if safe.
112+
113+
3) Add Bottleneck diagnostics for worker/DB paths:
114+
- Track queued/executing counts to confirm whether DB write locks contribute to TS slowness.
115+
116+
4) Artifact retention for isolated TS job:
117+
- Upload logs from the dedicated TS job (stdout/stderr files) as artifacts for easier offline triage.
118+
119+
5) Optionally split the TS tests further:
120+
- Separate “env flip” (`KIT_MODE=ts`) and “run TS” into distinct tests to isolate which step is slow/flaky.
121+
122+
6) Consider a nightly Windows-heavy job:
123+
- Run TS tests repeatedly with additional debug flags (`--trace-...`) to gather long-form diagnostics without blocking day-to-day CI.
124+
125+
## Open Questions / Risks
126+
127+
- Are there intermittent network/registry calls during TS flow that need mocking to be fully deterministic on Windows CI?
128+
- Does ESBuild on Windows require different flags to reduce sourcemap overhead (we currently use inline sourcemaps)?
129+
- Are there PATH resolution differences causing rare prompts in “kit new” or “run” for TS? (So far the logs suggest not.)
130+
131+
## File/Commit Highlights
132+
133+
- Diagnostics:
134+
- `test/_ava-global-diagnostics.mjs`, `test/windows-exit-diagnostics.test.ts`.
135+
- Worker logs: `src/workers/cache-grouped-scripts-worker.ts`, `src/workers/cache-grouped-scripts-worker.test.ts`.
136+
- App logs: `src/run/app-prompt.ts`, `src/run/app.ts`, `src/run/app-index.ts`.
137+
- Send/tracing guards: `src/api/kit.ts`.
138+
- Test harness logs: `test-sdk/main.test.js` (per-test diagnostics, app child lifecycle, PATH/bin listings, exec step logs).
139+
140+
- Behavior changes:
141+
- Reject worker test timeouts (no “pending” tests) and explicitly terminate workers.
142+
- Send `BEFORE_EXIT` before tearing down tracing; protected `trace.instant` calls with guards.
143+
- Increased AVA timeouts (global and per-test) for Windows slowness.
144+
145+
- Workflow:
146+
- Reduced retries for test steps to 1.
147+
- Split benchmark and TS tests into dedicated jobs; TS tests still run on Windows but no longer block the main lane.
148+
149+
---
150+
151+
This plan will be kept up to date as we continue investigating the isolated Windows TS job. The goal is to re-enable TS tests in the main Windows job once stability is confirmed.
152+

0 commit comments

Comments
 (0)