Repository navigation
Fix local-ask tests cancelled on Node 22 CI - #35
solvyxtech wants to merge 1 commit into
Conversation
The mock stream in 'ends at the timeout while the stream is still open' holds no handle, and the ask's deadline is AbortSignal.timeout, whose timer is unref'd. Under Node 22 (CI) the loop drained before the deadline fired, cancelling that test and every one after it in the file. Node 26's test runner keeps the loop alive while a test is pending, which hid it locally. The mock now holds a timer for as long as the request is open, the way a real socket would.
solvyxtech
left a comment
solvyxtech
left a comment
There was a problem hiding this comment.
Grok review: LGTM, approve in substance (posting as a comment since this account opened the PR and GitHub won't let it approve its own PR).
Scope: test-only, one file (test/local-ask.test.ts, +16/-2). No production code touched. Base is claude/sync-local-work, head is claude/local-ask-flake, so it stays on the claude/ branches and doesn't go to main.
Pairing rules: no subscription backends, no competitor names in product copy, nothing touching fine-tune data. All clear.
The fix: the diagnosis holds up. AbortSignal.timeout uses an unref'd timer and the in-memory ReadableStream holds no handle, so under Node 22's runner the loop can drain before the 30 ms deadline and node:test cancels the rest of the file. A ref'd setInterval standing in for the socket is the right minimal fix, and cleanup is covered twice: clearInterval on abort, plus .finally(() => clearInterval(socket)) around askModel, so it can't leak even if the abort listener never runs.
Nits (non-blocking):
- The inline comment is long for a test. It could be cut to two or three lines and link the PR for the backstory.
- If other tests in this file use a never-ending mock stream, they could hit the same problem later. A small
holdLoopOpen()helper would keep them consistent. Fine to leave that for later.
CI (typecheck-and-test) was still running when I reviewed. Merge once it's green.
1 participant
The macOS check job cancelled 6 tests in test/local-ask.test.ts with "Promise resolution is still pending but the event loop has already resolved".
Root cause: the timeout test's mock fetch returns an in-memory stream that holds no event-loop handle, and askModel's deadline is AbortSignal.timeout, whose timer is unref'd. Under Node 22 (what CI runs) nothing kept the loop alive, so it drained before the 30 ms deadline fired, and node:test cancelled that test and every test after it in the file. Node 26 (local) keeps the loop alive while a test is pending, so it passed on the Mac. Nothing to do with HOME, ~/.local/bin or a local model server, apart from ~/.local/bin providing Node 26.
Fix: the mock holds a ref'd timer while the request is open, the way a real socket does, and clears it on abort. Production is unaffected because a real fetch holds its socket.
Verified: npm test with Node 22.23.3 in a fresh HOME (1953 pass, 0 cancelled; the file fails without the fix in the same setup), and npm run check green with ~/.local/bin off PATH.