diff --git a/AGENTS.md b/AGENTS.md index d567672..afdb688 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -201,7 +201,9 @@ Release npm verification/version subprocesses must remain non-interactive with s disconnected and CI mode enabled so nested commands cannot suspend a release through shell job control. Keep Git fetch/push on the separate interactive path for credential helpers, and never infer release success until the version commit, tag, and push are -all observed. See `docs/releasing.md`. +all observed. Provider-discovery shell probes must have piped I/O and a separate +process session so shell profiles cannot take the caller's controlling terminal; +discovery tests must mock those probes. See `docs/releasing.md`. Automatic provider-usage probes must treat an observed CLI crash differently from an ordinary unavailable response. Cool down the crashing interactive fallback so window diff --git a/docs/releasing.md b/docs/releasing.md index 2c68deb..1422291 100644 --- a/docs/releasing.md +++ b/docs/releasing.md @@ -41,6 +41,12 @@ configured credential helpers can still authenticate. A verification command tha needs input must fail explicitly rather than leaving a stopped release job that could later resume and mutate version/tag state. +Provider-discovery tests mock shell probes rather than loading the owner's shell +profiles. Runtime login/interactive discovery probes run in a separate process +session with piped I/O: disconnected stdin alone does not remove access to the +controlling terminal through `/dev/tty`. This prevents a shell profile from taking +terminal custody or suspending the release test process group with `SIGTTIN`. + CrewCode's desktop usage indicators probe installed provider CLIs independently of the release command. If Claude's interactive usage process exits from a crash signal, CrewCode pauses that fallback for ten minutes. This prevents focus-driven refreshes diff --git a/src/main/headless-agent-resolver.test.ts b/src/main/headless-agent-resolver.test.ts index 4dd0426..c5749d1 100644 --- a/src/main/headless-agent-resolver.test.ts +++ b/src/main/headless-agent-resolver.test.ts @@ -1,7 +1,35 @@ -import { afterEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { join } from 'node:path' +import { EventEmitter } from 'node:events' +import { PassThrough } from 'node:stream' + +function shellProbe(output = '', code = 0) { + const child = Object.assign(new EventEmitter(), { stdout: new PassThrough() }) + queueMicrotask(() => { + child.stdout.end(output) + child.emit('close', code) + }) + return child +} + +const spawn = vi.fn((_shell: string, _args: string[], _options: { detached?: boolean; timeout?: number }) => shellProbe()) +const spawnSync = vi.fn((_shell: string, _args: string[], _options: { detached?: boolean; timeout?: number }) => ({ stdout: '', status: 0 })) + +beforeEach(() => { + spawn.mockReset() + spawn.mockImplementation(() => shellProbe()) + spawnSync.mockClear() + // Discovery tests must not execute the owner's login/interactive shell profiles. + vi.doMock('child_process', async importOriginal => ({ + ...await importOriginal(), + spawn, + spawnSync, + })) +}) afterEach(() => { + vi.doUnmock('child_process') + vi.doUnmock('./agents/model-detect') vi.doUnmock('fs') vi.doUnmock('os') vi.resetModules() @@ -32,9 +60,43 @@ describe('headless agent registry', () => { available: true, path: expectedPath, }) + expect(spawnSync).not.toHaveBeenCalled() + if (process.platform !== 'win32') { + expect(spawn).toHaveBeenCalled() + for (const [, , options] of spawn.mock.calls) { + expect(options).toMatchObject({ detached: true, timeout: 3_000 }) + } + } + }) + + it.skipIf(process.platform === 'win32')('retains interactive-shell discovery without caller terminal access', async () => { + const expectedPath = join('/shell', 'codex') + vi.doMock('fs', async importOriginal => { + const actual = await importOriginal() + return { + ...actual, + promises: { ...actual.promises, access: vi.fn(async (candidate: string) => { + if (candidate !== expectedPath) throw new Error('missing') + }) }, + } + }) + spawn.mockImplementation((_shell, args) => shellProbe( + args[0] === '-ic' && args[1] === 'command -v codex' ? `${expectedPath}\n` : '', + )) + + const { headlessAgentRegistry } = await import('./headless-agent-resolver') + const registry = await headlessAgentRegistry() + expect(registry.find(agent => agent.id === 'codex')).toMatchObject({ available: true, path: expectedPath }) + expect(spawn).toHaveBeenCalledWith(expect.any(String), ['-ic', 'command -v codex'], expect.objectContaining({ + detached: true, stdio: ['ignore', 'pipe', 'ignore'], timeout: 3_000, + })) }) it('lists models from the same provider CLIs as desktop', async () => { + vi.doMock('fs', async importOriginal => ({ + ...await importOriginal(), + existsSync: () => false, + })) vi.doMock('./agents/model-detect', () => ({ listModels: vi.fn(async (provider: string) => ( provider === 'claude' @@ -47,5 +109,11 @@ describe('headless agent registry', () => { { id: 'claude-sonnet-4-6', label: 'Sonnet', provider: 'anthropic' }, ]) expect(await listHeadlessAgentModels('codex')).toEqual([]) + if (process.platform !== 'win32') { + expect(spawnSync).toHaveBeenCalled() + for (const [, , options] of spawnSync.mock.calls) { + expect(options).toMatchObject({ detached: true, timeout: 3_000 }) + } + } }) }) diff --git a/src/main/headless-agent-resolver.ts b/src/main/headless-agent-resolver.ts index 7c220df..10e2ea2 100644 --- a/src/main/headless-agent-resolver.ts +++ b/src/main/headless-agent-resolver.ts @@ -1,7 +1,7 @@ import { existsSync, constants as fsConstants, promises as fsp } from 'fs' import { homedir } from 'os' import { delimiter, join } from 'path' -import { execFile, spawnSync } from 'child_process' +import { spawn, spawnSync } from 'child_process' import { listModels } from './agents/model-detect' const PROVIDER_COMMANDS: Record = { @@ -79,8 +79,19 @@ function shellResolve(command: string, flag: '-lc' | '-ic'): Promise { - execFile(shell, [flag, `command -v ${command}`], { encoding: 'utf8', timeout: 3_000, windowsHide: true }, (_error, stdout) => { - const path = stdout?.trim().split('\n').pop()?.trim() + // Piped stdin alone leaves /dev/tty accessible to interactive shell profiles. + const child = spawn(shell, [flag, `command -v ${command}`], { + stdio: ['ignore', 'pipe', 'ignore'], detached: true, + timeout: 3_000, killSignal: 'SIGKILL', windowsHide: true, + }) + let stdout = '' + child.stdout?.setEncoding('utf8') + child.stdout?.on('data', (chunk: string) => { + stdout = (stdout + chunk).slice(-16_384) + }) + child.on('error', () => resolve(null)) + child.on('close', code => { + const path = code === 0 ? stdout.trim().split('\n').pop()?.trim() : null resolve(path || null) }) }) @@ -106,8 +117,10 @@ export function resolveHeadlessAgentPath(provider: string): string | null { for (const candidate of candidatePaths(command)) if (candidate && existsSync(candidate)) return candidate if (process.platform !== 'win32') { for (const flag of ['-lc', '-ic'] as const) { - const result = spawnSync(process.env.SHELL || '/bin/sh', [flag, `command -v ${command}`], { encoding: 'utf8', timeout: 3_000 }) - const resolved = result.stdout?.trim().split('\n').pop()?.trim() + // Shell startup must not take terminal custody from the caller. + const options = { encoding: 'utf8' as const, timeout: 3_000, detached: true, killSignal: 'SIGKILL' as const } + const result = spawnSync(process.env.SHELL || '/bin/sh', [flag, `command -v ${command}`], options) + const resolved = result.status === 0 ? result.stdout?.trim().split('\n').pop()?.trim() : null if (resolved && existsSync(resolved)) return resolved } }