starred/orca
1
0
Fork 0
mirror of https://github.com/stablyai/orca.git synced 2026-11-02 01:57:57 +00:00
Code Issues Packages Projects Releases Wiki Activity
Files
de5470a9003aaec451ff3ea36a314fc46355c64a
orca/src/main/text-generation/commit-message-text-generation-local-subprocess.test.ts
T
Brennan Benson 19661dfca6 fix: stop Claude and agent helper processes when Orca quits or crashes (STA-9254, 1 of 2) (#25715) 
* fix(supervisor): add a one-shot lifetime that runs past stdin end, and relay a provider's last output

A one-shot CLI (claude -p, codex exec -) reads its request until stdin ends, so the
supervisor's session rule (stdin end means the owner is closing) would stop it about
1 s into its answer. The one-shot lifetime passes stdin end through and leaves only
the owner-death watch and explicit signals to stop it.

The supervisor also exited as soon as its provider did, dropping output still in the
pipes when the owner reads slowly. It now waits, bounded, for the provider's output
to be relayed before exiting.

* fix(text-generation): run agent one-shots under the provider supervisor on POSIX

The Claude model-list probe, model discovery for every agent, and commit message,
pull request and branch name generation all spawned the agent CLI as a plain child
of Orca. If Orca quit, crashed or was killed while one was running, nothing stopped
it, and a CLI that hung kept running after Orca was gone.

They now run under the provider supervisor that native chat already uses, in its
one-shot lifetime, so Orca's exit stops the agent's whole process group however
Orca exits. A timeout or cancel asks the supervisor to stop (SIGTERM) and only
tears the tree down once it has had its full stop time; killing the supervisor
first would orphan the agent's group. A missing binary now reaches Orca as the
supervisor's exit 127, which is mapped back to the existing not-found message.
Windows and WSL keep spawning the agent directly.

* fix(supervisor): carry the provider argv on the supervisor's argv, map every spawn error back, and share one stop ladder

- The supervisor read the provider's command and arguments from one base64 JSON env string.
  Linux caps a single env string at 128 KiB, so an argv prompt of about 90-120 KiB, which
  passes the 120 KiB per-argument guard, failed execve with E2BIG. The provider argv now
  follows the supervisor script's '--' as real arguments; the env keeps only small fields.
- A supervisor that cannot start its provider reports Node's spawn error line and exits 127.
  That line now becomes the same error a direct spawn emits, so ENOENT still reads as
  'not found on PATH' and EACCES or any other spawn error reads as 'failed to start'.
- stopSupervisedProvider is the one ask, wait and force ladder: it gives a supervisor its full
  stop time before forcing. Agent one-shots, the Codex app-server close and the Claude child
  exit proof now share it, with the same requests, bounds and forced steps as before.

* fix(supervisor): report every provider spawn failure on one marked stderr line

Node throws most spawn failures (ENOEXEC, ENOTDIR, ELOOP, EPERM, ...) instead of emitting
them, and the supervisor had no catch, so it died with exit 1 and a stack trace that the
user saw as the agent's failure. A thrown or emitted spawn failure now exits 127 with one
marked line carrying whether it was thrown, its code and its message. Orca reads only the
last stderr line, so a runtime warning printed earlier cannot hide it, and maps it to the
message a direct spawn gave: thrown is 'could not be started', ENOENT is 'not found on
PATH', and any other emitted error is 'failed to start'.

* fix(supervisor): keep the user's Node options away from the supervisor and hand them to the provider

The supervisor runs Electron in Node mode, which honours NODE_OPTIONS and
NODE_REPL_EXTERNAL_MODULE. A user value such as a --require of a missing file stopped
the supervisor from starting, breaking even native CLIs like Codex that never load it.
The launch now takes both out of the supervisor's environment, carries them in its spec,
and restores them for the provider only, so a Node-based CLI still gets them.

* fix(text-generation): file a forced agent one-shot teardown under its own breadcrumb site

The forced tree teardown recorded every self-initiated kill as the Codex app-server's, so
a forced commit-message or model-discovery stop read as a Codex teardown in crash
breadcrumbs. The teardown now takes the caller's site; source-control stops pass the
site their Windows tree kill already uses.

* test(text-generation): cover the supervised stop under timeout and output limit; name the direct-child suites

The commit-message suites that drive fake children spawn them directly, the unsupervised
shape Windows and WSL use, so they now say so. The supervised POSIX stop gets its own
compositions: a timed-out Codex generation settles at once but holds the Codex home until
its supervisor has stopped (faithful fake, fake timers), and an agent that floods past the
output limit is stopped through its real supervisor with no process left behind.

* test(text-generation): run the direct-child suites on the Windows path and cover supervised discovery

The commit-message suites that drive fake children mocked the supervisor away on POSIX, so
they asserted a direct root SIGKILL that production no longer takes there. They now pin the
platform to Windows (with an empty PATH, so host installs cannot answer a bare agent name)
and assert the Windows kill, taskkill included. The three tests that check the host's own
discovery spawn shape run on the host and read the agent argv past the supervisor's '--'.
Model discovery gets its supervised composition: a timed-out Codex discovery settles at once
but holds the Codex home until its supervisor has stopped.

* fix(supervisor): show a supervised spawn failure in native chat as the spawn error it was

Native chat's exit errors carry the provider's stderr tail into Details. Under the supervisor
a missing CLI left the supervisor's internal spawn-failure report there instead of Node's own
'spawn <cmd> ENOENT'. The report, its parser and a display formatter now live in one module;
the Codex app-server and Claude stream-json exit errors pass the tail through the formatter,
which turns a report back into the spawn error and leaves any other stderr unchanged.

* fix(supervisor): report a spawn that failed without a pid instead of crashing on its missing pipes

When the provider spawn fails outright (EMFILE, ENFILE), Node emits 'error' later and leaves
the child with no pid and no stdio. Piping stdin into the missing pipe threw first, so the
supervisor died with exit 1 and a stack trace and never wrote its spawn-failure report. The
pipes are now wired only for a provider that started.

* refactor(supervisor): share the stop of a supervised child process

Agent one-shots stop their supervisor with SIGTERM through the shared stop ladder, watching
the child's own exit. That adapter moves into one helper beside the ladder, so other
supervised children can use it with their own stop request instead of a copy. The caller's
breadcrumb site still reaches the forced teardown. Same request, wait and force as before.

* fix(supervisor): give a session provider its stdin end and grace when its owner dies

The owner-death watch went straight to the group SIGTERM and cancelled any stdin-end grace,
so when Orca quit or crashed a session provider such as the Codex app-server never saw the
EOF that lets it finish writing its state (auth.json, the state database). A session whose
owner is gone now closes as an owner's stdin end does: the provider's stdin is ended, it
gets the stdin-end grace, and only then the SIGTERM and SIGKILL ladder. A one-shot already
had its EOF at the end of its request, so its owner's death still stops it at once.

* fix(supervisor): kill the rest of the provider group once the provider exits on a stop

A requested stop waited out the whole SIGTERM grace for the provider's group even after the
provider itself had exited, so a SIGTERM-ignoring helper it left behind held every stop for
up to 3 s. Under a stop, the rest of the group is now SIGKILLed as soon as the provider has
exited, the same rule its own exit already follows.

* test(text-generation): cover a supervised Codex discovery past its output limit

Model discovery's supervised stop was covered only under timeout. A Codex discovery that
floods past the output limit now runs through a real supervisor: it settles with the
too-much-data error, its agent is stopped through the supervisor, and the next discovery on
the same Codex home starts only after that agent is gone. The direct-child suites' headers
now list exactly the supervised cases that are covered.

* fix(supervisor): close a provider whose owner is gone the way its owner closes it

Owner death gave every session provider the stdin-end grace, so after an Orca crash a
Claude session, whose close is a stdin end plus SIGTERM, could keep working on its turn
for a second with nobody watching. The spawn spec now names the provider's close request:
'stdin-end' (the Codex app-server drains and exits on EOF, then gets its grace) or
'stdin-end-and-sigterm' (Claude; the default). A gone owner gets that same request. One
constant per provider feeds both its spawn spec and its owner-side close, through one
requestProviderClose, so the two cannot drift. One-shots still stop at once.

* fix(supervisor): keep the SIGTERM grace for a session's group after its provider exits

Killing the rest of the group the moment the provider exited under a stop also reached
native chat's closes, so an MCP server, a tool's child or a dev server still in Claude's or
Codex's group was SIGKILLed mid-cleanup instead of getting the rest of the SIGTERM grace.
The early group kill now applies only to one-shots, where the saved wait was the point;
a session's stop is back to waiting out the grace for its group.

* test(claude): pin that Claude's spawn passes its close request explicitly

The spawn-spec assertion matched the default close request, so dropping Claude's explicit
request still passed. The test now checks that the spec is built with the exit-proof ladder's
own constant.

* refactor(codex): give the Codex app-server close request its own module

Other Codex app-server spawns will name the same close request as the connection does.
Holding it in its own small module lets them import it without the connection itself.

* build(cli): list the Codex close request and the provider supervisor in the CLI project

The command-line build runs the short-lived Codex app-server session, which will name the
same close request as the Codex connection. Listing the close request, the provider
supervisor it takes its type from, and the spawn-failure report the supervisor uses lets the
CLI project typecheck that import without pulling the connection in.

* test(text-generation): check the ENOEXEC start failure only where Node reports one

On Linux, glibc's execvp hands an executable that is not a program to /bin/sh, so both a
direct and a supervised spawn run it and it exits 127; only macOS throws ENOEXEC. The
not-a-program case now runs on macOS only; the path-through-a-file case (ENOTDIR) still
covers a thrown start failure everywhere.
2026-10-06 12:15:49 -07:00

287 lines
9.5 KiB
TypeScript
Raw Blame History

import { spawn } from 'node:child_process'
import type * as ChildProcess from 'node:child_process'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { generateCommitMessageFromContext } from './commit-message-text-generation'
import {
createChildTerminationExpectation,
withPlatform
} from './commit-message-text-generation-test-harness'
const { terminateWindowsProcessTreeMock } = vi.hoisted(() => ({
terminateWindowsProcessTreeMock: vi.fn(async () => {})
}))
vi.mock('../windows-process-tree-kill', () => ({
terminateWindowsProcessTree: terminateWindowsProcessTreeMock
}))
vi.mock('child_process', async (importOriginal) => {
const actual = await importOriginal<typeof ChildProcess>()
return {
...actual,
spawn: vi.fn(actual.spawn)
}
})
const spawnMock = vi.mocked(spawn)
const expectChildTerminated = createChildTerminationExpectation(terminateWindowsProcessTreeMock)
// These suites drive fake children down the Windows direct-child path, taskkill included. The POSIX
// supervised stop with the Codex home lock (generation: timeout, cancel, output limit; discovery:
// timeout, output limit) is in source-control-local-process.test.ts.
const hostPlatform = process.platform
afterEach(() => {
Object.defineProperty(process, 'platform', { configurable: true, value: hostPlatform })
vi.unstubAllEnvs()
})
beforeEach(() => {
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
// Windows resolves a bare agent name on PATH; the host's own installs must not answer it.
vi.stubEnv('PATH', '')
terminateWindowsProcessTreeMock.mockClear()
terminateWindowsProcessTreeMock.mockResolvedValue(undefined)
spawnMock.mockClear()
})
describe('generateCommitMessageFromContext on the Windows direct-child path', () => {
it('caps local agent output before buffering unbounded data', async () => {
const listeners = new Map<string, (value: unknown) => void>()
const child = {
pid: 123,
kill: vi.fn(),
stdout: { on: vi.fn((event, callback) => listeners.set(`stdout:${event}`, callback)) },
stderr: { on: vi.fn((event, callback) => listeners.set(`stderr:${event}`, callback)) },
stdin: { end: vi.fn() },
on: vi.fn((event, callback) => listeners.set(event, callback))
}
spawnMock.mockReturnValue(child as never)
const pending = generateCommitMessageFromContext(
{
branch: 'main',
stagedSummary: 'M\tREADME.md',
stagedPatch: '+hello'
},
{
agentId: 'custom',
model: '',
customAgentCommand: 'agent'
},
{
kind: 'local',
cwd: '/repo'
}
)
listeners.get('stdout:data')?.(Buffer.alloc(4 * 1024 * 1024 + 1))
listeners.get('close')?.(null)
await expect(pending).resolves.toEqual({
success: false,
error:
'agent CLI command produced too much output. Check the agent CLI configuration and try again.'
})
await expectChildTerminated(child)
})
it('passes prepared provider environment to local agent subprocesses', async () => {
const listeners = new Map<string, (value: unknown) => void>()
const child = {
pid: 123,
kill: vi.fn(),
stdout: { on: vi.fn((event, callback) => listeners.set(`stdout:${event}`, callback)) },
stderr: { on: vi.fn((event, callback) => listeners.set(`stderr:${event}`, callback)) },
stdin: { end: vi.fn() },
on: vi.fn((event, callback) => listeners.set(event, callback))
}
spawnMock.mockReturnValue(child as never)
const pending = generateCommitMessageFromContext(
{
branch: 'main',
stagedSummary: 'M\tREADME.md',
stagedPatch: '+hello'
},
{
agentId: 'custom',
model: '',
customAgentCommand: 'orca-test-agent-nope'
},
{
kind: 'local',
cwd: '/repo',
env: { ...process.env, CODEX_HOME: '/managed/codex-home' }
}
)
listeners.get('stdout:data')?.(Buffer.from('Add README note\n'))
listeners.get('close')?.(0)
await expect(pending).resolves.toMatchObject({
success: true,
message: 'Add README note'
})
expect(spawnMock).toHaveBeenCalledWith(
'orca-test-agent-nope',
[],
expect.objectContaining({
env: expect.objectContaining({ CODEX_HOME: '/managed/codex-home' })
})
)
})
it('routes WSL local commit generation through the selected distro login shell', async () => {
await withPlatform('win32', async () => {
process.env.ORCA_HOST_ONLY_SECRET = 'do-not-leak'
const listeners = new Map<string, (value: unknown) => void>()
const child = {
pid: 123,
kill: vi.fn(),
stdout: { on: vi.fn((event, callback) => listeners.set(`stdout:${event}`, callback)) },
stderr: { on: vi.fn((event, callback) => listeners.set(`stderr:${event}`, callback)) },
stdin: { end: vi.fn() },
on: vi.fn((event, callback) => listeners.set(event, callback))
}
spawnMock.mockReturnValue(child as never)
const pending = generateCommitMessageFromContext(
{
branch: 'main',
stagedSummary: 'M\tREADME.md',
stagedPatch: '+hello'
},
{
agentId: 'custom',
model: '',
customAgentCommand: 'agent --mode fast'
},
{
kind: 'local',
cwd: 'C:\\repo',
wslDistro: 'Ubuntu 24.04',
env: { ...process.env, CODEX_HOME: '/home/tester/.codex' }
}
)
listeners.get('stdout:data')?.(Buffer.from('Update README\n'))
listeners.get('close')?.(0)
await expect(pending).resolves.toMatchObject({
success: true,
message: 'Update README'
})
expect(spawnMock).toHaveBeenCalledWith(
'wsl.exe',
['-d', 'Ubuntu 24.04', '--exec', 'sh', '-lc', expect.any(String)],
expect.objectContaining({
// Why a concrete directory (#16463): `undefined` makes CreateProcessW inherit
// Orca's own cwd, a deletable WSL UNC path when it was launched from a
// worktree. The Linux directory still rides inside the command (/mnt/c/repo,
// asserted below), so the Windows-side cwd never decides where the agent runs.
cwd: expect.any(String),
windowsHide: true,
env: expect.objectContaining({ CODEX_HOME: '/home/tester/.codex' })
})
)
const spawnEnv = spawnMock.mock.calls[0]?.[2]?.env as NodeJS.ProcessEnv
expect(spawnEnv.ORCA_HOST_ONLY_SECRET).toBeUndefined()
const shellCommand = spawnMock.mock.calls[0]?.[1]?.[5] as string
expect(shellCommand).toContain('getent passwd')
expect(shellCommand).toContain('exec "$_orca_wsl_shell" -ilc')
expect(shellCommand).toContain('/mnt/c/repo')
expect(shellCommand).toContain("'agent'")
expect(shellCommand).toContain('--mode')
expect(shellCommand).toContain('fast')
})
})
it('routes Windows batch-script agent commands through cmd.exe', async () => {
const originalComSpec = process.env.ComSpec
process.env.ComSpec = 'C:\\Windows\\System32\\cmd.exe'
try {
await withPlatform('win32', async () => {
const listeners = new Map<string, (value: unknown) => void>()
const child = {
pid: 123,
kill: vi.fn(),
stdout: { on: vi.fn((event, callback) => listeners.set(`stdout:${event}`, callback)) },
stderr: { on: vi.fn((event, callback) => listeners.set(`stderr:${event}`, callback)) },
stdin: { end: vi.fn() },
on: vi.fn((event, callback) => listeners.set(event, callback))
}
spawnMock.mockReturnValue(child as never)
const pending = generateCommitMessageFromContext(
{
branch: 'main',
stagedSummary: 'M\tREADME.md',
stagedPatch: '+hello'
},
{
agentId: 'custom',
model: '',
customAgentCommand: 'C:/tools/agent.cmd'
},
{
kind: 'local',
cwd: 'C:\\repo'
}
)
listeners.get('stdout:data')?.(Buffer.from('Update README\n'))
listeners.get('close')?.(0)
await expect(pending).resolves.toMatchObject({
success: true,
message: 'Update README'
})
expect(spawnMock).toHaveBeenCalledWith(
'C:\\Windows\\System32\\cmd.exe',
['/d', '/c', 'C:/tools/agent.cmd'],
expect.objectContaining({
cwd: 'C:\\repo',
windowsHide: true
})
)
})
} finally {
if (originalComSpec === undefined) {
delete process.env.ComSpec
} else {
process.env.ComSpec = originalComSpec
}
}
})
it('rejects unsafe argv prompts for Windows batch-script agent commands', async () => {
await withPlatform('win32', async () => {
const result = await generateCommitMessageFromContext(
{
branch: 'main',
stagedSummary: 'M\tREADME.md',
stagedPatch: '+hello & goodbye'
},
{
agentId: 'custom',
model: '',
customAgentCommand: 'C:/tools/agent.cmd {prompt}'
},
{
kind: 'local',
cwd: 'C:\\repo'
}
)
expect(result).toEqual({
success: false,
error:
'C:/tools/agent.cmd cannot be run as a Windows batch command with the prompt in argv. Remove {prompt} so Orca sends the prompt on stdin.'
})
expect(spawnMock).not.toHaveBeenCalled()
})
})
})
Reference in New Issue View Git Blame Copy Permalink