From 9644e8f08f0c3e7873d92854860dd802476dd200 Mon Sep 17 00:00:00 2001 From: Eric Rodriguez Date: Thu, 9 Jul 2026 09:45:18 +0200 Subject: [PATCH 1/3] fix(ci): install node-jq binary explicitly; npm 11 blocks its preinstall The v0.19.0 release gate hung: every --jq test failed with 'spawn node_modules/node-jq/bin/jq ENOENT'. CI upgrades to npm@latest (needed for OIDC trusted publishing), and npm >= 11 blocks dependency lifecycle scripts by default, so node-jq's preinstall never downloaded its jq binary. Node 24's bundled npm 11 broke the CI matrix leg the same way. Run node-jq's installer directly (a plain node invocation, not an npm lifecycle hook) after npm ci in both workflows. Also corrects the testTimeout rationale: the real driver is the client retry tests' real exponential-backoff sleeps (~14s worst case), not jq; bump to 30s for safe headroom on coverage-instrumented CI. Claude-Session: https://claude.ai/code/session_018dRMUUj9vpKp3tqKQFsvE9 --- .github/workflows/ci.yml | 4 ++++ .github/workflows/release.yml | 5 +++++ vitest.config.js | 11 +++++------ 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 06beb53..83f2473 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -38,6 +38,10 @@ jobs: node-version: ${{ matrix.node }} cache: npm - run: npm ci + # npm >= 11 blocks dependency lifecycle scripts by default, so node-jq's + # preinstall never downloads its jq binary and every --jq call would spawn + # a missing binary. Run its installer directly (not an npm lifecycle hook). + - run: node node_modules/node-jq/scripts/install-binary.mjs - run: npm run test:coverage - name: Upload coverage to Codecov if: matrix.os == 'ubuntu-latest' && matrix.node == 22 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index b25cdd7..f323218 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -27,6 +27,11 @@ jobs: - run: npm ci + # npm >= 11 (installed just above for trusted publishing) blocks + # dependency lifecycle scripts, so node-jq's preinstall never fetches its + # jq binary. Run its installer directly so --jq works in the test gate. + - run: node node_modules/node-jq/scripts/install-binary.mjs + # Gate the release on the same checks as CI. - run: npm run lint - run: npm run test:coverage diff --git a/vitest.config.js b/vitest.config.js index 3a6d352..d4473af 100644 --- a/vitest.config.js +++ b/vitest.config.js @@ -5,12 +5,11 @@ export default defineConfig({ globals: true, include: ['test/**/*.test.js'], setupFiles: ['test/setup.js'], - // node-jq shells out to the jq binary; its subprocess cold-start under - // coverage instrumentation on shared CI runners can spike well past the - // 5s default, timing out --jq tests that run in ~30ms locally. Give CI - // headroom without masking a genuine hang. - testTimeout: 20000, - hookTimeout: 20000, + // The client retry tests exercise real exponential backoff (sleeps that + // sum to ~14s in the worst-case exhaustion path), so the 5s default is too + // tight; give generous headroom, more so on coverage-instrumented CI. + testTimeout: 30000, + hookTimeout: 30000, coverage: { include: ['src/**/*.js'], exclude: ['src/hooks/**'], From c135d27d90f247040302630cd4942cf9583a1649 Mon Sep 17 00:00:00 2001 From: Eric Rodriguez Date: Thu, 9 Jul 2026 09:54:17 +0200 Subject: [PATCH 2/3] test(mcp): normalize Windows path separators in the command-audit walk realCommandIds() split file paths on '/' only, so on Windows readdirSync's backslash paths produced ids like 'deal\list' that missed the audited map (4 failures on the windows-latest CI legs). Split on both separators. Claude-Session: https://claude.ai/code/session_018dRMUUj9vpKp3tqKQFsvE9 --- test/lib/mcp/catalog.test.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/test/lib/mcp/catalog.test.js b/test/lib/mcp/catalog.test.js index efd300b..970dd94 100644 --- a/test/lib/mcp/catalog.test.js +++ b/test/lib/mcp/catalog.test.js @@ -308,10 +308,14 @@ describe('real-config classification audit', () => { // which would import every command module and skew coverage merging. function realCommandIds() { const dir = fileURLToPath(new URL('../../../src/commands', import.meta.url)) - return readdirSync(dir, { recursive: true }) - .filter((f) => f.endsWith('.js')) - .map((f) => f.replace(/\.js$/, '').split('/').join(':')) - .sort() + return ( + readdirSync(dir, { recursive: true }) + .filter((f) => f.endsWith('.js')) + // readdirSync yields OS-native separators (backslashes on Windows); + // split on both so command ids normalize to colon form everywhere. + .map((f) => f.replace(/\.js$/, '').split(/[/\\]/).join(':')) + .sort() + ) } it('classifies every real command exactly as audited', () => { From ed28ccfdc84938c89311b1e3f9fbc611ed015887 Mon Sep 17 00:00:00 2001 From: Eric Rodriguez Date: Thu, 9 Jul 2026 10:05:40 +0200 Subject: [PATCH 3/3] test(mcp): make the SIGKILL escalation coverable on Windows The 100% coverage gate failed on the windows-latest legs: makeExec's SIGTERM->SIGKILL grace escalation is unreachable there (Windows kills a child on the first signal), leaving its callback uncovered. Extract it as scheduleHardKill() and unit-test it directly with fake timers, so the escalation is exercised OS-independently while the real-process test keeps asserting end-to-end behavior on POSIX. Claude-Session: https://claude.ai/code/session_018dRMUUj9vpKp3tqKQFsvE9 --- src/lib/mcp/invoke.js | 14 +++++++++++++- test/lib/mcp/invoke.test.js | 20 +++++++++++++++++++- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/lib/mcp/invoke.js b/src/lib/mcp/invoke.js index f3e0800..a78766a 100644 --- a/src/lib/mcp/invoke.js +++ b/src/lib/mcp/invoke.js @@ -91,6 +91,18 @@ export function errMessage(e) { return String(e?.message || e) } +/** + * After a graceful SIGTERM, force-kill the child if it has not exited within + * `grace` ms. Extracted so the escalation is unit-testable without depending on + * OS signal semantics: Windows terminates a child on the first kill, so a + * real-process test can never exercise this escalation there. + * @param {import('node:child_process').ChildProcess} child + * @param {number} grace + */ +export function scheduleHardKill(child, grace) { + return setTimeout(() => child.kill('SIGKILL'), grace) +} + /** * Build an executor that spawns the pdcli CLI as a child process. Keeping * command output in a child process keeps the parent's stdout (the MCP stdio @@ -126,7 +138,7 @@ export function makeExec({ const timer = setTimeout(() => { timedOut = true child.kill('SIGTERM') - killTimer = setTimeout(() => child.kill('SIGKILL'), grace) + killTimer = scheduleHardKill(child, grace) }, timeout) const guard = () => { if (stdout.length + stderr.length > maxBuffer) { diff --git a/test/lib/mcp/invoke.test.js b/test/lib/mcp/invoke.test.js index 15b94ba..f89f4b3 100644 --- a/test/lib/mcp/invoke.test.js +++ b/test/lib/mcp/invoke.test.js @@ -1,10 +1,11 @@ -import { describe, it, expect } from 'vitest' +import { describe, it, expect, vi } from 'vitest' import { toArgv, runTool, makeExec, normalizeExit, errMessage, + scheduleHardKill, } from '../../../src/lib/mcp/invoke.js' const readEntry = { @@ -298,6 +299,23 @@ describe('makeExec', () => { expect((await exec([])).signal).toBe('tool timed out after 0.15s') }) + // Directly exercises the SIGKILL escalation with fake timers and a fake + // child, so it is covered on every OS (a real process on Windows dies on the + // first signal and never reaches the grace escalation). + it('scheduleHardKill force-kills with SIGKILL after the grace period', () => { + vi.useFakeTimers() + try { + const kills = [] + const child = { kill: (sig) => kills.push(sig) } + scheduleHardKill(child, 100) + expect(kills).toEqual([]) + vi.advanceTimersByTime(100) + expect(kills).toEqual(['SIGKILL']) + } finally { + vi.useRealTimers() + } + }) + it('escalates to SIGKILL when the child ignores SIGTERM', async () => { const exec = makeExec({ command: process.execPath,