Skip to content

Commit 362ead0

Browse files
committed
fix(cli): stop the Pi extension from killing pi on startup
`codetime` installs as a .cmd shim on Windows and spawn() does no PATHEXT resolution, so spawn("codetime") always raised ENOENT there. The unhandled 'error' event fires on a later tick, outside pi's own handler guard, so the very first report — session_start — took the whole process down: installing the extension made pi unstartable on Windows. Run the hook through the shell on win32, mute spawn/stdin errors, and unref the child: the hook triggers a full local sync that was pinning pi's event loop for its entire duration (26s to exit in a local repro).
1 parent 3dfa33c commit 362ead0

2 files changed

Lines changed: 102 additions & 5 deletions

File tree

packages/cli/src/adapters/pi.ts

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -432,12 +432,32 @@ function piExtensionContent(): string {
432432
import { spawn } from "node:child_process";
433433
import type { ExtensionAPI } from "@mariozechner/pi-coding-agent";
434434
435+
// Fire-and-forget: the extension runs inside pi's own process, so a failure
436+
// here must never surface. Three hazards to keep muzzled:
437+
// - on Windows \`codetime\` is a .cmd shim and spawn() does no PATHEXT
438+
// resolution, so a plain spawn always fails there — run it through the
439+
// shell instead;
440+
// - a spawn failure emits an unhandled 'error' event, which kills pi outright
441+
// (the throw lands on a later tick, outside any try/catch or pi's own
442+
// handler guard) — and the first report fires on session_start, so pi dies
443+
// at startup;
444+
// - the hook triggers a full local sync, so an attached child keeps pi's
445+
// event loop alive for as long as that sync runs — unref/detach it.
435446
function report(payload: Record<string, unknown>) {
436-
const child = spawn("codetime", ["hook", "--agent", "pi"], {
437-
stdio: ["pipe", "ignore", "ignore"],
438-
});
439-
child.stdin.write(JSON.stringify(payload));
440-
child.stdin.end();
447+
const isWindows = process.platform === "win32";
448+
try {
449+
const child = spawn("codetime", ["hook", "--agent", "pi"], {
450+
stdio: ["pipe", "ignore", "ignore"],
451+
shell: isWindows,
452+
windowsHide: true,
453+
// A detached child under cmd.exe can surface a stray console window.
454+
detached: !isWindows,
455+
});
456+
child.on("error", () => {});
457+
child.stdin.on("error", () => {});
458+
child.stdin.end(JSON.stringify(payload));
459+
child.unref();
460+
} catch {}
441461
}
442462
443463
export default function (pi: ExtensionAPI) {

packages/cli/test/cli.test.ts

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
import type { RunContext } from '../src/lib/types.ts'
22
import assert from 'node:assert/strict'
3+
import { spawn } from 'node:child_process'
34
import { mkdir, mkdtemp, readdir, readFile, stat, symlink, utimes, writeFile } from 'node:fs/promises'
45
import { tmpdir } from 'node:os'
56
import path from 'node:path'
67
import { Readable } from 'node:stream'
78
// eslint-disable-next-line test/no-import-node-test -- This repo uses node:test as the runner.
89
import { test } from 'node:test'
10+
import { fileURLToPath, pathToFileURL } from 'node:url'
911
import { codexBackfillFiles, createCodexAdapter } from '../src/adapters/codex.ts'
1012
import { run, syncLocalRunnerEntryArgs } from '../src/cli.ts'
1113
import { ensureLocalMachineId, machineIdPath, readConfig, writeConfig } from '../src/lib/config.ts'
@@ -1284,6 +1286,81 @@ test('PI_CODING_AGENT_DIR relocates Pi detect and install paths', async () => {
12841286
assert.match(extension, /"--agent", "pi"/)
12851287
})
12861288

1289+
// The Pi extension runs inside pi's own process, so anything report() does
1290+
// wrong takes pi down with it. These two cases are the ones that actually bit:
1291+
// a missing `codetime` binary used to raise an unhandled 'error' event (pi died
1292+
// on session_start, i.e. it would not start at all), and an attached child kept
1293+
// pi's event loop alive for the whole duration of the sync the hook triggers.
1294+
async function runPiExtensionHarness(env: NodeJS.ProcessEnv): Promise<{ code: number | null, elapsedMs: number }> {
1295+
const home = await mkdtemp(path.join(tmpdir(), 'codetime-'))
1296+
const piDir = await mkdtemp(path.join(tmpdir(), 'pi-agent-'))
1297+
const exitCode = await run(['install', '--target', 'pi', '--home', home], testContext({
1298+
env: { HOME: home, PI_CODING_AGENT_DIR: piDir },
1299+
}))
1300+
assert.equal(exitCode, 0)
1301+
1302+
const extensionPath = path.join(piDir, 'extensions', 'codetime.ts')
1303+
const harnessPath = path.join(piDir, 'harness.mjs')
1304+
await writeFile(harnessPath, `
1305+
const mod = await import(${JSON.stringify(pathToFileURL(extensionPath).href)})
1306+
const handlers = new Map()
1307+
mod.default({ on: (name, fn) => handlers.set(name, fn) })
1308+
await handlers.get('session_start')({ id: 'sess', cwd: process.cwd() })
1309+
`, 'utf8')
1310+
1311+
const startedAt = Date.now()
1312+
const child = spawn(process.execPath, ['--import', 'tsx', harnessPath], {
1313+
// tsx resolves from cwd, and the harness imports the generated extension.
1314+
cwd: path.resolve(fileURLToPath(new URL('..', import.meta.url))),
1315+
env,
1316+
stdio: ['ignore', 'ignore', 'ignore'],
1317+
})
1318+
const code = await new Promise<number | null>((resolve) => {
1319+
child.on('exit', resolve)
1320+
child.on('error', () => resolve(-1))
1321+
})
1322+
return { code, elapsedMs: Date.now() - startedAt }
1323+
}
1324+
1325+
test('Pi extension survives a codetime binary that is not on PATH', async () => {
1326+
const emptyBin = await mkdtemp(path.join(tmpdir(), 'empty-bin-'))
1327+
const { code } = await runPiExtensionHarness({
1328+
...process.env,
1329+
PATH: emptyBin,
1330+
Path: emptyBin,
1331+
})
1332+
assert.equal(code, 0, 'reporting must not crash the host agent when codetime is missing')
1333+
})
1334+
1335+
test('Pi extension does not hold the host process open while the hook runs', async () => {
1336+
const fakeBin = await mkdtemp(path.join(tmpdir(), 'fake-bin-'))
1337+
const fakeCodetime = path.join(fakeBin, 'codetime')
1338+
await writeFile(fakeCodetime, '#!/bin/sh\nsleep 30\n', { encoding: 'utf8', mode: 0o755 })
1339+
1340+
const { code, elapsedMs } = await runPiExtensionHarness({
1341+
...process.env,
1342+
PATH: `${fakeBin}${path.delimiter}${process.env.PATH ?? ''}`,
1343+
})
1344+
assert.equal(code, 0)
1345+
assert.ok(elapsedMs < 15_000, `host exited in ${elapsedMs}ms; the hook child is keeping it alive`)
1346+
})
1347+
1348+
test('Pi extension spawns through the shell on Windows', async () => {
1349+
// `codetime` is installed as a .cmd shim on Windows and spawn() does no
1350+
// PATHEXT resolution, so a plain spawn always raises ENOENT there. This
1351+
// branch cannot be exercised on POSIX, so assert on the emitted source.
1352+
const home = await mkdtemp(path.join(tmpdir(), 'codetime-'))
1353+
const piDir = await mkdtemp(path.join(tmpdir(), 'pi-agent-'))
1354+
await run(['install', '--target', 'pi', '--home', home], testContext({
1355+
env: { HOME: home, PI_CODING_AGENT_DIR: piDir },
1356+
}))
1357+
1358+
const extension = await readFile(path.join(piDir, 'extensions', 'codetime.ts'), 'utf8')
1359+
assert.match(extension, /const isWindows = process\.platform === "win32"/)
1360+
assert.match(extension, /shell: isWindows/)
1361+
assert.match(extension, /detached: !isWindows/)
1362+
})
1363+
12871364
test('PI_CODING_AGENT_SESSION_DIR relocates only the Pi sessions dir, not the install path', async () => {
12881365
// Plan a backfill against a relocated sessions dir while leaving the agent
12891366
// dir at its default — this proves PI_CODING_AGENT_SESSION_DIR is honored

0 commit comments

Comments
 (0)