diff --git a/src/cli/bg.test.ts b/src/cli/bg.test.ts index e98959f0d..5a1012722 100644 --- a/src/cli/bg.test.ts +++ b/src/cli/bg.test.ts @@ -6,6 +6,7 @@ import { describe, expect, it } from 'bun:test' import { buildBackgroundSessionLaunch, buildBackgroundChildProcessConfig, + buildBackgroundSessionDisplayCommand, confirmBackgroundSessionLaunch, followLogFile, killBackgroundSession, @@ -20,11 +21,19 @@ import { BACKGROUND_SESSION_ID_ENV, BACKGROUND_SESSION_LAUNCHER_PID_ENV, } from './bgFinalizer.js' +import { + BACKGROUND_PROCESS_MARKER_FLAG, + backgroundProcessMarkerToken, + generateBackgroundProcessMarker, +} from './bgRouting.js' import type { BackgroundSession, BackgroundSessionProcessIdentity, } from './bgRegistry.js' +const TEST_PROCESS_MARKER = 'a'.repeat(64) +const OTHER_PROCESS_MARKER = 'b'.repeat(64) + class TestOutput extends EventEmitter { chunks: Buffer[] = [] destroyed = false @@ -98,6 +107,57 @@ async function withTempFile( } describe('background session CLI parsing', () => { + it('generates a fresh bounded lower-case hex marker from 32 random bytes', () => { + const first = generateBackgroundProcessMarker(size => { + expect(size).toBe(32) + return new Uint8Array(size).fill(0x11) + }) + const second = generateBackgroundProcessMarker(size => + new Uint8Array(size).fill(0x22), + ) + + expect(first).toBe('11'.repeat(32)) + expect(second).toBe('22'.repeat(32)) + expect(second).not.toBe(first) + }) + + it('strips inherited marker options before -- but preserves prompt text after it', () => { + const inline = backgroundProcessMarkerToken(TEST_PROCESS_MARKER) + for (const inherited of [ + [inline], + [BACKGROUND_PROCESS_MARKER_FLAG, TEST_PROCESS_MARKER], + ]) { + const parsed = parseBackgroundInvocation([ + '--bg', + ...inherited, + '--print', + '--', + inline, + ]) + + expect(parsed.prompt).toBe(inline) + expect(parsed.childArgs).toEqual(['--print', '--', inline]) + } + }) + + it('preserves marker-looking required-option values', () => { + const markerLookingValue = backgroundProcessMarkerToken(TEST_PROCESS_MARKER) + const parsed = parseBackgroundInvocation([ + '--bg', + '--system-prompt', + markerLookingValue, + 'actual prompt', + ]) + + expect(parsed.prompt).toBe('actual prompt') + expect(parsed.childArgs).toEqual([ + '--system-prompt', + markerLookingValue, + '--print', + 'actual prompt', + ]) + }) + it('builds a print-mode child command and preserves provider/model flags', () => { const parsed = parseBackgroundInvocation([ '--provider', @@ -471,6 +531,7 @@ describe('background session CLI parsing', () => { sessionName: 'tests', stdoutLogPath: '/tmp/bg.out.log', backgroundSessionId: 'bg-tests', + processMarker: TEST_PROCESS_MARKER, launcherPid: 700, }) @@ -479,6 +540,7 @@ describe('background session CLI parsing', () => { '--max-old-space-size=8192', '--expose-gc', '/repo/bin/openclaude', + backgroundProcessMarkerToken(TEST_PROCESS_MARKER), '--print', 'fix failing tests', ]) @@ -503,6 +565,7 @@ describe('background session CLI parsing', () => { }, stdoutLogPath: '/tmp/bg.out.log', backgroundSessionId: 'bg-no-wrapper', + processMarker: TEST_PROCESS_MARKER, launcherPid: 701, }) @@ -522,6 +585,7 @@ describe('background session CLI parsing', () => { processEnv: {}, stdoutLogPath: '/tmp/bg.out.log', backgroundSessionId: 'bg-bun-owner', + processMarker: TEST_PROCESS_MARKER, launcherPid: 702, }) @@ -531,6 +595,107 @@ describe('background session CLI parsing', () => { expect(config.env[BACKGROUND_SESSION_LAUNCHER_PID_ENV]).toBe('702') }) + it('injects one fresh marker immediately after a spaced entrypoint', () => { + const inherited = backgroundProcessMarkerToken(OTHER_PROCESS_MARKER) + const promptMarker = backgroundProcessMarkerToken(OTHER_PROCESS_MARKER) + const config = buildBackgroundChildProcessConfig({ + execPath: 'C:\\Program Files\\nodejs\\node.exe', + execArgv: ['--expose-gc'], + entrypoint: 'C:\\repo path\\dist\\cli.mjs', + childArgs: [ + inherited, + '--provider', + 'openai', + '--model', + 'gpt-5', + '--session-id', + '550e8400-e29b-41d4-a716-446655440000', + '--from-pr', + '1642', + '--print', + '--', + promptMarker, + ], + processEnv: {}, + stdoutLogPath: 'C:\\logs path\\bg.out.log', + backgroundSessionId: 'bg-spaced-paths', + processMarker: TEST_PROCESS_MARKER, + launcherPid: 703, + }) + const markerToken = backgroundProcessMarkerToken(TEST_PROCESS_MARKER) + const entrypointIndex = config.args.indexOf('C:\\repo path\\dist\\cli.mjs') + + expect(config.args[entrypointIndex + 1]).toBe(markerToken) + expect(config.args.filter(arg => arg === markerToken)).toHaveLength(1) + expect(config.args.slice(0, config.args.indexOf('--'))).not.toContain( + inherited, + ) + expect(config.args.slice(config.args.indexOf('--'))).toEqual([ + '--', + promptMarker, + ]) + }) + + it('keeps marker-looking required-option values during defensive injection', () => { + const markerLookingValue = backgroundProcessMarkerToken(OTHER_PROCESS_MARKER) + const config = buildBackgroundChildProcessConfig({ + execPath: '/usr/bin/node', + execArgv: [], + entrypoint: '/repo/bin/openclaude', + childArgs: [ + '--system-prompt', + markerLookingValue, + '--print', + 'work', + ], + processEnv: {}, + stdoutLogPath: '/tmp/bg.out.log', + backgroundSessionId: 'bg-marker-looking-value', + processMarker: TEST_PROCESS_MARKER, + launcherPid: 704, + }) + + expect(config.args).toEqual([ + '--max-old-space-size=8192', + '--expose-gc', + '/repo/bin/openclaude', + backgroundProcessMarkerToken(TEST_PROCESS_MARKER), + '--system-prompt', + markerLookingValue, + '--print', + 'work', + ]) + }) + + it('omits only the internal marker from the displayed launch command', () => { + const markerToken = backgroundProcessMarkerToken(TEST_PROCESS_MARKER) + const promptMarker = backgroundProcessMarkerToken(OTHER_PROCESS_MARKER) + + expect( + buildBackgroundSessionDisplayCommand( + [ + 'node', + '/repo path/dist/cli.mjs', + markerToken, + '--provider', + 'openai', + '--print', + '--', + promptMarker, + ], + TEST_PROCESS_MARKER, + ), + ).toEqual([ + 'node', + '/repo path/dist/cli.mjs', + '--provider', + 'openai', + '--print', + '--', + promptMarker, + ]) + }) + it('escalates process-tree termination and waits for exit before returning', async () => { const signals: Array = [] let aliveChecks = 0 @@ -679,6 +844,17 @@ describe('background session process termination safety', () => { stdoutLogPath: '/tmp/stdout.log', stderrLogPath: '/tmp/stderr.log', } + const markedSession: BackgroundSession = { + ...session, + processMarker: TEST_PROCESS_MARKER, + command: [ + 'node', + 'openclaude', + backgroundProcessMarkerToken(TEST_PROCESS_MARKER), + '--session-id', + 'conversation-safety', + ], + } function identity( state: BackgroundSessionProcessIdentity['state'], @@ -716,6 +892,30 @@ describe('background session process termination safety', () => { ]) }) + it('signals a marked session only after its exact token is freshly verified', async () => { + const calls: string[] = [] + let aliveChecks = 0 + + await terminateBackgroundSessionProcessTree(markedSession, { + isProcessAlive: () => ++aliveChecks <= 2, + getProcessCommand: pid => { + calls.push(`verify:${pid}`) + return `node openclaude ${backgroundProcessMarkerToken(TEST_PROCESS_MARKER)} --session-id conversation-safety` + }, + killTree: async (pid, signal) => { + calls.push(`signal:${pid}:${signal}`) + }, + sleep: async () => {}, + termGraceMs: 1, + pollIntervalMs: 1, + }) + + expect(calls).toEqual([ + 'verify:4242', + 'signal:4242:SIGTERM', + ]) + }) + it('refuses a mismatched identity before SIGTERM and does not mark killed', async () => { const calls: string[] = [] @@ -744,12 +944,37 @@ describe('background session process termination safety', () => { } expect(String(refusal)).toContain('refused to signal an unverified process') + expect(String(refusal)).toContain( + 'This older background session could not be verified safely', + ) + expect(String(refusal)).toContain('terminate PID 4242 manually') expect(String(refusal)).not.toContain('private prompt value') expect(calls).toEqual([]) }) - it('does not verify or signal terminal records when a reused PID would match', async () => { - for (const status of ['stale', 'killed', 'exited', 'failed'] as const) { + it('does not signal or mark a marked session whose token mismatches', async () => { + const calls: string[] = [] + + await expect( + killBackgroundSession(markedSession, { + isProcessAlive: () => true, + getProcessCommand: () => + `node openclaude ${backgroundProcessMarkerToken(OTHER_PROCESS_MARKER)} --session-id conversation-safety`, + killTree: async (_pid, signal) => { + calls.push(`signal:${signal}`) + }, + markKilled: async selected => { + calls.push(`mark:${selected.id}`) + return { ...selected, status: 'killed' } + }, + }), + ).rejects.toThrow('refused to signal an unverified process') + + expect(calls).toEqual([]) + }) + + it('does not verify or signal authoritative terminal records when a reused PID would match', async () => { + for (const status of ['killed', 'exited', 'failed'] as const) { const calls: string[] = [] const killed = await killBackgroundSession( @@ -774,6 +999,110 @@ describe('background session process termination safety', () => { } }) + it('fails closed instead of marking a stale session whose identity mismatches', async () => { + const calls: string[] = [] + + await expect( + killBackgroundSession( + { ...markedSession, status: 'stale' }, + { + isProcessAlive: () => true, + getProcessCommand: () => + `node openclaude ${backgroundProcessMarkerToken(OTHER_PROCESS_MARKER)} --session-id conversation-safety`, + killTree: async (_pid, signal) => { + calls.push(`signal:${signal}`) + }, + markKilled: async selected => { + calls.push(`mark:${selected.id}`) + return { ...selected, status: 'killed' } + }, + }, + ), + ).rejects.toThrow('refused to signal an unverified process') + + expect(calls).toEqual([]) + }) + + it('refuses a matching legacy identity after the session was already stale', async () => { + const calls: string[] = [] + + await expect( + killBackgroundSession( + { ...session, status: 'stale' }, + { + isProcessAlive: () => true, + getProcessCommand: () => + 'node openclaude --resume conversation-safety', + killTree: async (_pid, signal) => { + calls.push(`signal:${signal}`) + }, + markKilled: async selected => { + calls.push(`mark:${selected.id}`) + return { ...selected, status: 'killed' } + }, + }, + ), + ).rejects.toThrow('PID ownership cannot be re-established safely') + + expect(calls).toEqual([]) + }) + + it('allows an exact marked identity to authorize a previously stale session', async () => { + const calls: string[] = [] + const states: BackgroundSessionProcessIdentity['state'][] = [ + 'matches', + 'matches', + ] + + const killed = await killBackgroundSession( + { ...markedSession, status: 'stale' }, + { + isProcessAlive: () => false, + verifySessionIdentity: () => { + const state = states.shift()! + calls.push(`verify:${state}`) + return identity(state) + }, + killTree: async (_pid, signal) => { + calls.push(`signal:${signal}`) + }, + markKilled: async selected => { + calls.push(`mark:${selected.id}`) + return { ...selected, status: 'killed' } + }, + }, + ) + + expect(killed.status).toBe('killed') + expect(calls).toEqual([ + 'verify:matches', + 'verify:matches', + 'signal:SIGTERM', + 'mark:bg-safety', + ]) + }) + + it('marks a stale session killed without signalling when its PID is gone', async () => { + const calls: string[] = [] + + const killed = await killBackgroundSession( + { ...session, status: 'stale' }, + { + isProcessAlive: () => false, + killTree: async (_pid, signal) => { + calls.push(`signal:${signal}`) + }, + markKilled: async selected => { + calls.push(`mark:${selected.id}`) + return { ...selected, status: 'killed' } + }, + }, + ) + + expect(killed.status).toBe('killed') + expect(calls).toEqual(['mark:bg-safety']) + }) + it('freshly verifies an unknown session that becomes readable before killing', async () => { const calls: string[] = [] const states: BackgroundSessionProcessIdentity['state'][] = [ @@ -866,6 +1195,39 @@ describe('background session process termination safety', () => { ]) }) + it('does not send SIGKILL when a marked token changes after SIGTERM', async () => { + const calls: string[] = [] + const commands = [ + `node openclaude ${backgroundProcessMarkerToken(TEST_PROCESS_MARKER)} --session-id conversation-safety`, + `node openclaude ${backgroundProcessMarkerToken(OTHER_PROCESS_MARKER)} --session-id conversation-safety`, + ] + + await expect( + terminateBackgroundSessionProcessTree(markedSession, { + isProcessAlive: () => true, + getProcessCommand: () => { + calls.push('verify') + return commands.shift()! + }, + killTree: async (_pid, signal) => { + calls.push(`signal:${signal}`) + }, + sleep: async () => { + calls.push('sleep') + }, + termGraceMs: 1, + pollIntervalMs: 1, + }), + ).rejects.toThrow('refused to signal an unverified process') + + expect(calls).toEqual([ + 'verify', + 'signal:SIGTERM', + 'sleep', + 'verify', + ]) + }) + it('fails closed when the live process identity is unreadable', async () => { const signals: Array = [] @@ -882,6 +1244,26 @@ describe('background session process termination safety', () => { expect(signals).toEqual([]) }) + it('does not mark a marked session killed when identity is unreadable', async () => { + const calls: string[] = [] + + await expect( + killBackgroundSession(markedSession, { + isProcessAlive: () => true, + getProcessCommand: () => null, + killTree: async (_pid, signal) => { + calls.push(`signal:${signal}`) + }, + markKilled: async selected => { + calls.push(`mark:${selected.id}`) + return { ...selected, status: 'killed' } + }, + }), + ).rejects.toThrow('refused to signal an unverified process') + + expect(calls).toEqual([]) + }) + it('refuses signalling when liveness becomes unreadable during command lookup', async () => { const calls: string[] = [] let probes = 0 diff --git a/src/cli/bg.ts b/src/cli/bg.ts index 877e981b1..c2a29c213 100644 --- a/src/cli/bg.ts +++ b/src/cli/bg.ts @@ -25,8 +25,11 @@ import { type BackgroundSessionProcessIdentityOptions, } from './bgRegistry.js' import { + backgroundProcessMarkerToken, BACKGROUND_SESSION_ID_ENV, BACKGROUND_SESSION_LAUNCHER_PID_ENV, + generateBackgroundProcessMarker, + stripBackgroundProcessMarkerArgs, } from './bgRouting.js' export type ParsedBackgroundInvocation = { @@ -56,6 +59,7 @@ export type BuildBackgroundChildProcessConfigInput = { sessionName?: string stdoutLogPath: string backgroundSessionId: string + processMarker: string launcherPid?: number } @@ -233,6 +237,7 @@ export function buildBackgroundChildProcessConfig( // launcher otherwise relaunches itself before finalizer ownership is checked. // Node-only heap flags are still supplied by safeNodeExecArgvForBackground. env[HEAP_RELAUNCHED_ENV] = '1' + const childArgs = stripBackgroundProcessMarkerArgs(input.childArgs) return { command: input.execPath, @@ -243,7 +248,8 @@ export function buildBackgroundChildProcessConfig( input.processEnv, ), input.entrypoint, - ...input.childArgs, + backgroundProcessMarkerToken(input.processMarker), + ...childArgs, ], env, } @@ -432,7 +438,7 @@ export async function buildBackgroundSessionLaunch( export function parseBackgroundInvocation( args: string[], ): ParsedBackgroundInvocation { - let childArgs = stripBackgroundFlag(args) + let childArgs = stripBackgroundProcessMarkerArgs(stripBackgroundFlag(args)) const name = findSessionName(childArgs)?.trim() || undefined const promptIndex = findPromptIndex(childArgs) const prompt = promptIndex === -1 ? undefined : childArgs[promptIndex] @@ -482,6 +488,21 @@ function formatCommand(command: string[]): string { .join(' ') } +export function buildBackgroundSessionDisplayCommand( + command: string[], + processMarker: string, +): string[] { + const markerToken = backgroundProcessMarkerToken(processMarker) + const delimiterIndex = command.indexOf('--') + const optionEnd = delimiterIndex === -1 ? command.length : delimiterIndex + const markerIndex = command.findIndex( + (arg, index) => index < optionEnd && arg === markerToken, + ) + return markerIndex === -1 + ? [...command] + : [...command.slice(0, markerIndex), ...command.slice(markerIndex + 1)] +} + function printSessionTable( sessions: Awaited>, ): void { @@ -839,8 +860,12 @@ function unverifiedProcessError( session: BackgroundSession, reason: string, ): Error { + const action = + session.processMarker === undefined + ? `This older background session could not be verified safely. Restart it to use stronger process identity, or terminate PID ${session.pid} manually after confirming ownership.` + : 'Re-run `openclaude ps` and retry after confirming the session identity.' return new Error( - `OpenClaude refused to signal an unverified process for background session ${session.id} (PID ${session.pid}): ${reason}. Re-run \`openclaude ps\` and retry after confirming the session identity.`, + `OpenClaude refused to signal an unverified process for background session ${session.id} (PID ${session.pid}): ${reason}. ${action}`, ) } @@ -921,10 +946,23 @@ export async function killBackgroundSession( (async (selected: BackgroundSession) => await markBackgroundSessionKilled(selected.id)) - if (isTerminalBackgroundSession(session)) return await markKilled(session) + if (session.status !== 'stale' && isTerminalBackgroundSession(session)) { + return await markKilled(session) + } const identity = verifySelectedBackgroundSessionIdentity(session, options) - if (authorizeBackgroundSessionSignal(session, identity) === 'matches') { + const authorization = authorizeBackgroundSessionSignal(session, identity) + if ( + authorization === 'matches' && + session.status === 'stale' && + session.processMarker === undefined + ) { + throw unverifiedProcessError( + session, + 'this older session was already stale, so its PID ownership cannot be re-established safely', + ) + } + if (authorization === 'matches') { await terminateBackgroundSessionProcessTree(session, options) } @@ -1032,6 +1070,7 @@ export async function handleBgFlag(args: string[]): Promise { } const id = backgroundSessionId() + const processMarker = generateBackgroundProcessMarker() const { childArgs, sessionId } = await buildBackgroundSessionLaunch( parsed.childArgs, randomUUID(), @@ -1053,6 +1092,7 @@ export async function handleBgFlag(args: string[]): Promise { sessionName: parsed.name, stdoutLogPath: logPaths.stdoutLogPath, backgroundSessionId: id, + processMarker, launcherPid: process.pid, }) @@ -1108,6 +1148,7 @@ export async function handleBgFlag(args: string[]): Promise { provider: findFlagValue(childArgs, '--provider'), model: findFlagValue(childArgs, '--model'), sessionId, + processMarker, stdoutLogPath: logPaths.stdoutLogPath, stderrLogPath: logPaths.stderrLogPath, logFilesPrecreated: true, @@ -1131,6 +1172,11 @@ export async function handleBgFlag(args: string[]): Promise { console.log(`Logs: ${session.stdoutLogPath}`) console.log(`Follow: openclaude logs ${session.id} -f`) console.log( - `Command: ${formatCommand([basename(childConfig.command), ...childConfig.args])}`, + `Command: ${formatCommand( + buildBackgroundSessionDisplayCommand([ + basename(childConfig.command), + ...childConfig.args, + ], processMarker), + )}`, ) } diff --git a/src/cli/bgFinalizer.test.ts b/src/cli/bgFinalizer.test.ts index a88bcdf96..7af3edae7 100644 --- a/src/cli/bgFinalizer.test.ts +++ b/src/cli/bgFinalizer.test.ts @@ -10,10 +10,16 @@ import { prepareBackgroundSessionFinalizer, } from './bgFinalizer.js' import { buildBackgroundChildProcessConfig } from './bg.js' +import { + BACKGROUND_PROCESS_MARKER_FLAG, + backgroundProcessMarkerToken, + isValidBackgroundProcessMarker, +} from './bgRouting.js' import { _setBackgroundSessionsRootForTesting, listBackgroundSessions, refreshBackgroundSessionStatuses, + verifyBackgroundSessionProcessIdentity, type BackgroundSession, } from './bgRegistry.js' @@ -343,6 +349,7 @@ describe('background session finalizer', () => { id: string, args: string[], ): Promise { + const processMarker = 'c'.repeat(64) const processEnv: NodeJS.ProcessEnv = { ...process.env, OPENCLAUDE_CONFIG_DIR: configDir, @@ -356,6 +363,7 @@ describe('background session finalizer', () => { processEnv, stdoutLogPath: join(configDir, `${id}.out.log`), backgroundSessionId: id, + processMarker, launcherPid: process.pid, }) const child = spawn(childConfig.command, childConfig.args, { @@ -367,7 +375,11 @@ describe('background session finalizer', () => { await mkdir(join(sessionsRoot, 'sessions'), { recursive: true }) await writeFile( join(sessionsRoot, 'sessions', `${id}.json`), - JSON.stringify(ownedSession(id, child.pid)), + JSON.stringify({ + ...ownedSession(id, child.pid), + processMarker, + command: [childConfig.command, ...childConfig.args], + }), ) const [code] = (await once(child, 'exit')) as [number] return code @@ -484,11 +496,73 @@ describe('background session finalizer', () => { exitCode: expectation.exitCode, terminalReason: 'exit_code', }) + expect(isValidBackgroundProcessMarker(session.processMarker)).toBe(true) + expect(session.command).toContain( + backgroundProcessMarkerToken(session.processMarker!), + ) + expect(stdout).not.toContain(BACKGROUND_PROCESS_MARKER_FLAG) expect(await Bun.file(session.stdoutLogPath).exists()).toBe(true) expect(await Bun.file(session.stderrLogPath).exists()).toBe(true) }) } + it.skipIf(process.platform === 'win32')( + 'recognizes a live built marked child through the production command probe', + async () => { + const id = 'bg-built-live-marker' + const processMarker = 'e'.repeat(64) + const processEnv: NodeJS.ProcessEnv = { + ...process.env, + OPENCLAUDE_CONFIG_DIR: configDir, + } + delete processEnv.OPENCLAUDE_DISABLE_CLI_ENTRYPOINT_AUTO_RUN + const childConfig = buildBackgroundChildProcessConfig({ + execPath: 'node', + execArgv: [], + entrypoint: installedLauncherPath, + childArgs: ['--version'], + processEnv, + stdoutLogPath: join(configDir, `${id}.out.log`), + backgroundSessionId: id, + processMarker, + launcherPid: process.pid, + }) + const child = spawn(childConfig.command, childConfig.args, { + env: childConfig.env, + stdio: ['ignore', 'ignore', 'ignore'], + }) + if (!child.pid) throw new Error('built CLI fixture did not start') + const exit = once(child, 'exit') + let forceTimer: ReturnType | undefined + + try { + expect(child.kill('SIGSTOP')).toBe(true) + const session: BackgroundSession = { + ...ownedSession(id, child.pid), + processMarker, + command: [childConfig.command, ...childConfig.args], + } + await mkdir(join(sessionsRoot, 'sessions'), { recursive: true }) + await writeFile( + join(sessionsRoot, 'sessions', `${id}.json`), + JSON.stringify(session), + ) + + expect(verifyBackgroundSessionProcessIdentity(session).state).toBe( + 'matches', + ) + expect(await refreshBackgroundSessionStatuses()).toMatchObject([ + { id, status: 'running', processMarker }, + ]) + } finally { + child.kill('SIGCONT') + forceTimer = setTimeout(() => child.kill('SIGKILL'), 5_000) + await exit + clearTimeout(forceTimer) + } + }, + ) + it('keeps the installed launcher PID stable and shows outcomes truthfully in ps', async () => { expect(await runBuiltCliSession('bg-built-success', ['--version'])).toBe(0) expect( @@ -520,6 +594,12 @@ describe('background session finalizer', () => { expect(stderr).toBe('') expect(stdout).toMatch(/bg-built-success\s+exited/) expect(stdout).toMatch(/bg-built-failure\s+failed/) + for (const session of await listBackgroundSessions()) { + expect(session.processMarker).toBe('c'.repeat(64)) + expect(session.command).toContain( + backgroundProcessMarkerToken(session.processMarker!), + ) + } }) it.skipIf(process.platform === 'win32')( diff --git a/src/cli/bgRegistry.test.ts b/src/cli/bgRegistry.test.ts index 4f267a1a8..90983089a 100644 --- a/src/cli/bgRegistry.test.ts +++ b/src/cli/bgRegistry.test.ts @@ -17,6 +17,15 @@ import { type BackgroundSession, } from './bgRegistry.js' +const TEST_PROCESS_MARKER = 'a'.repeat(64) +const OTHER_PROCESS_MARKER = 'b'.repeat(64) +// Keep this test-only constructor available on the pre-marker base so the +// marker-authoritative regression can prove red there. Production identity +// constructs its expected token independently, so token-format drift still +// fails the marked-match tests below. +const backgroundProcessMarkerToken = (marker: string) => + `--openclaude-bg-session-marker=${marker}` + const { recordBackgroundSessionNaturalTermination, recordBackgroundSessionNaturalTerminationSync, @@ -82,10 +91,16 @@ describe('background session registry', () => { name: 'auth-refactor', pid: 12345, cwd: '/repo', - command: ['openclaude', '--print', 'refactor auth'], + command: [ + 'openclaude', + backgroundProcessMarkerToken(TEST_PROCESS_MARKER), + '--print', + 'refactor auth', + ], provider: 'openai', model: 'gpt-5', sessionId: 'conversation-1', + processMarker: TEST_PROCESS_MARKER, now: new Date('2026-06-15T08:00:00.000Z'), }) @@ -98,9 +113,15 @@ describe('background session registry', () => { provider: 'openai', model: 'gpt-5', sessionId: 'conversation-1', + processMarker: TEST_PROCESS_MARKER, startedAt: '2026-06-15T08:00:00.000Z', updatedAt: '2026-06-15T08:00:00.000Z', - command: ['openclaude', '--print', 'refactor auth'], + command: [ + 'openclaude', + backgroundProcessMarkerToken(TEST_PROCESS_MARKER), + '--print', + 'refactor auth', + ], }) expect(session.stdoutLogPath).toBe( join(configDir, 'bg-sessions', 'logs', 'bg-test-1.out.log'), @@ -111,6 +132,87 @@ describe('background session registry', () => { const sessions = await listBackgroundSessions() expect(sessions.map(s => s.id)).toEqual(['bg-test-1']) + expect(sessions[0]?.processMarker).toBe(TEST_PROCESS_MARKER) + }) + + it('loads legacy session metadata without a process marker', async () => { + await mkdir(join(configDir, 'bg-sessions', 'sessions'), { + recursive: true, + }) + await writeFile( + join(configDir, 'bg-sessions', 'sessions', 'bg-legacy.json'), + JSON.stringify({ + id: 'bg-legacy', + pid: 123, + cwd: '/repo', + status: 'running', + sessionId: 'conversation-legacy', + startedAt: '2026-06-15T08:00:00.000Z', + updatedAt: '2026-06-15T08:00:00.000Z', + command: ['openclaude', '--print', 'work'], + stdoutLogPath: '/tmp/stdout.log', + stderrLogPath: '/tmp/stderr.log', + }), + ) + + const [session] = await listBackgroundSessions() + expect(session?.id).toBe('bg-legacy') + expect(session?.processMarker).toBeUndefined() + }) + + it('rejects malformed process markers on creation', async () => { + await expect( + createBackgroundSession({ + id: 'bg-invalid-marker', + pid: 123, + cwd: '/repo', + command: ['openclaude', '--print', 'work'], + sessionId: 'conversation-invalid-marker', + processMarker: 'not-valid', + }), + ).rejects.toThrow('Invalid background process marker') + + expect(await listBackgroundSessions()).toEqual([]) + }) + + it('ignores metadata containing malformed process markers', async () => { + await mkdir(join(configDir, 'bg-sessions', 'sessions'), { + recursive: true, + }) + const invalidMarkers = [ + '', + 'a'.repeat(63), + 'a'.repeat(65), + 'A'.repeat(64), + `${'a'.repeat(63)} `, + `${'a'.repeat(63)}/`, + `${'a'.repeat(63)};`, + `${'a'.repeat(63)}\u0000`, + ] + + await Promise.all( + invalidMarkers.map(async (processMarker, index) => { + const id = `bg-invalid-marker-${index}` + await writeFile( + join(configDir, 'bg-sessions', 'sessions', `${id}.json`), + JSON.stringify({ + id, + pid: 123 + index, + cwd: '/repo', + status: 'running', + sessionId: `conversation-${index}`, + processMarker, + startedAt: '2026-06-15T08:00:00.000Z', + updatedAt: '2026-06-15T08:00:00.000Z', + command: ['openclaude', '--print', 'work'], + stdoutLogPath: '/tmp/stdout.log', + stderrLogPath: '/tmp/stderr.log', + }), + ) + }), + ) + + expect(await listBackgroundSessions()).toEqual([]) }) it('resolves exact live names before session id prefixes', async () => { @@ -1130,6 +1232,108 @@ describe('background session registry', () => { ) }) + it('keeps a marked Windows session running when its exact early token matches', async () => { + const markerToken = backgroundProcessMarkerToken(TEST_PROCESS_MARKER) + await createBackgroundSession({ + id: 'bg-marked-windows', + name: 'marked-windows', + pid: 333, + cwd: 'C:\\repo path', + command: [ + 'C:\\Program Files\\nodejs\\node.exe', + 'C:\\repo path\\dist\\cli.mjs', + markerToken, + '--session-id', + 'conversation-marked', + '--print', + 'work', + ], + sessionId: 'conversation-marked', + processMarker: TEST_PROCESS_MARKER, + now: new Date('2026-06-15T08:00:00.000Z'), + }) + + const refreshed = await refreshBackgroundSessionStatuses({ + isProcessAlive: () => true, + getProcessCommand: () => + `"C:\\Program Files\\nodejs\\node.exe" "C:\\repo path\\dist\\cli.mjs" ${markerToken} --session-id conversation-marked --print work`, + now: new Date('2026-06-15T08:05:00.000Z'), + }) + + expect(refreshed[0]).toMatchObject({ + id: 'bg-marked-windows', + status: 'running', + updatedAt: '2026-06-15T08:00:00.000Z', + }) + await expect( + createBackgroundSession({ + id: 'bg-marked-windows-contender', + name: 'marked-windows', + pid: 334, + cwd: '/repo', + command: ['openclaude', '--print', 'contender'], + sessionId: 'conversation-contender', + }), + ).rejects.toThrow('already exists') + }) + + it('marks a marked session stale when the same session id has a different marker', async () => { + const markerToken = backgroundProcessMarkerToken(TEST_PROCESS_MARKER) + await createBackgroundSession({ + id: 'bg-marked-wrong-marker', + pid: 333, + cwd: '/repo', + command: [ + 'node', + '/repo/openclaude', + markerToken, + '--session-id', + 'conversation-marked', + ], + sessionId: 'conversation-marked', + processMarker: TEST_PROCESS_MARKER, + now: new Date('2026-06-15T08:00:00.000Z'), + }) + + const refreshed = await refreshBackgroundSessionStatuses({ + isProcessAlive: () => true, + getProcessCommand: () => + `node /repo/openclaude ${backgroundProcessMarkerToken(OTHER_PROCESS_MARKER)} --session-id conversation-marked`, + now: new Date('2026-06-15T08:05:00.000Z'), + }) + + expect(refreshed[0]).toMatchObject({ + id: 'bg-marked-wrong-marker', + status: 'stale', + updatedAt: '2026-06-15T08:05:00.000Z', + }) + }) + + it('keeps a marked session unknown when its live command is unreadable', async () => { + const markerToken = backgroundProcessMarkerToken(TEST_PROCESS_MARKER) + await createBackgroundSession({ + id: 'bg-marked-unreadable', + pid: 333, + cwd: '/repo', + command: ['node', '/repo/openclaude', markerToken, '--print', 'work'], + sessionId: 'conversation-marked', + processMarker: TEST_PROCESS_MARKER, + now: new Date('2026-06-15T08:00:00.000Z'), + }) + + const refreshed = await refreshBackgroundSessionStatuses({ + isProcessAlive: () => true, + getProcessCommand: () => null, + now: new Date('2026-06-15T08:05:00.000Z'), + }) + + expect(refreshed[0]).toMatchObject({ + id: 'bg-marked-unreadable', + status: 'unknown', + updatedAt: '2026-06-15T08:05:00.000Z', + }) + }) + it('keeps running sessions fresh when their process identity still matches', async () => { await createBackgroundSession({ id: 'bg-running', @@ -1331,6 +1535,20 @@ describe('isBackgroundSessionProcessAlive process identity', () => { stdoutLogPath: '/tmp/stdout.log', stderrLogPath: '/tmp/stderr.log', } + const markerToken = backgroundProcessMarkerToken(TEST_PROCESS_MARKER) + const markedSession: BackgroundSession = { + ...session, + processMarker: TEST_PROCESS_MARKER, + command: [ + '/opt/Open Claude/node', + '/repo path/dist/cli.mjs', + markerToken, + '--session-id', + session.sessionId, + '--print', + 'work', + ], + } it('does not treat a reused PID whose command merely contains the arg as alive (#1770)', () => { // The live process at this PID is unrelated: its final token "16420" only @@ -1360,6 +1578,100 @@ describe('isBackgroundSessionProcessAlive process identity', () => { expect(alive).toBe(true) }) + it('does not fall back to the session id when marked process identity is missing', () => { + const result = verifyBackgroundSessionProcessIdentity(markedSession, { + isProcessAlive: () => true, + getProcessCommand: () => + '/opt/Open Claude/node /repo path/dist/cli.mjs --session-id conversation-identity --print work', + }) + + expect(result.state).toBe('mismatch') + }) + + it('matches a marked Unix command with spaced executable and entrypoint paths', () => { + const result = verifyBackgroundSessionProcessIdentity(markedSession, { + isProcessAlive: () => true, + getProcessCommand: () => + `/opt/Open Claude/node /repo path/dist/cli.mjs ${markerToken} --session-id conversation-identity --print work`, + }) + + expect(result.state).toBe('matches') + }) + + it('does not accept a missing, wrong, or shifted marker for a marked session', () => { + const prefix = '/opt/Open Claude/node /repo path/dist/cli.mjs' + const commands = [ + `${prefix} --print work --session-id conversation-identity`, + `${prefix} ${backgroundProcessMarkerToken(OTHER_PROCESS_MARKER)} --session-id conversation-identity --print work`, + `${prefix} --print ${markerToken} --session-id conversation-identity work`, + ] + + expect( + commands.map( + command => + verifyBackgroundSessionProcessIdentity(markedSession, { + isProcessAlive: () => true, + getProcessCommand: () => command, + }).state, + ), + ).toEqual(['mismatch', 'mismatch', 'mismatch']) + }) + + it('does not match marker prefix, suffix, or prompt-only collisions', () => { + const prefix = '/opt/Open Claude/node /repo path/dist/cli.mjs' + const commands = [ + `${prefix} prefix-${markerToken} --session-id conversation-identity`, + `${prefix} ${markerToken}-suffix --session-id conversation-identity`, + `${prefix} --print -- ${markerToken}`, + ] + + expect( + commands.map( + command => + verifyBackgroundSessionProcessIdentity(markedSession, { + isProcessAlive: () => true, + getProcessCommand: () => command, + }).state, + ), + ).toEqual(['mismatch', 'mismatch', 'mismatch']) + }) + + it('distinguishes truncated marked identity from a shorter unrelated command', () => { + const prefix = '/opt/Open Claude/node /repo path/dist/cli.mjs' + const withinMarker = markerToken.slice(0, -8) + const states = [ + verifyBackgroundSessionProcessIdentity(markedSession, { + isProcessAlive: () => true, + getProcessCommand: () => prefix, + }).state, + verifyBackgroundSessionProcessIdentity(markedSession, { + isProcessAlive: () => true, + getProcessCommand: () => `${prefix} ${withinMarker}`, + }).state, + verifyBackgroundSessionProcessIdentity(markedSession, { + isProcessAlive: () => true, + getProcessCommand: () => 'unrelated short command', + }).state, + ] + + expect(states).toEqual(['unreadable', 'unreadable', 'mismatch']) + }) + + it('treats a marked session whose stored command omits its marker as unreadable', () => { + const result = verifyBackgroundSessionProcessIdentity( + { + ...markedSession, + command: ['node', 'openclaude', '--print', 'work'], + }, + { + isProcessAlive: () => true, + getProcessCommand: () => 'node openclaude --print work', + }, + ) + + expect(result.state).toBe('unreadable') + }) + it('does not match the session id as a substring of a larger token (#1770)', () => { // A short id must not match an unrelated live command that merely contains // it inside a longer token — the same reused-PID collision class as the diff --git a/src/cli/bgRegistry.ts b/src/cli/bgRegistry.ts index ac65cea3d..ae9782d0b 100644 --- a/src/cli/bgRegistry.ts +++ b/src/cli/bgRegistry.ts @@ -27,6 +27,10 @@ import { isProcessRunning, } from '../utils/genericProcessUtils.js' import { jsonParse, jsonStringify } from '../utils/slowOperations.js' +import { + backgroundProcessMarkerToken, + isValidBackgroundProcessMarker, +} from './bgRouting.js' export type BackgroundSessionStatus = | 'running' @@ -45,6 +49,7 @@ export type BackgroundSession = { provider?: string model?: string sessionId: string + processMarker?: string startedAt: string updatedAt: string command: string[] @@ -85,6 +90,7 @@ export type CreateBackgroundSessionInput = { provider?: string model?: string sessionId: string + processMarker?: string now?: Date stdoutLogPath?: string stderrLogPath?: string @@ -426,6 +432,8 @@ function isBackgroundSession( typeof candidate.provider === 'string') && (candidate.model === undefined || typeof candidate.model === 'string') && typeof candidate.sessionId === 'string' && + (candidate.processMarker === undefined || + isValidBackgroundProcessMarker(candidate.processMarker)) && typeof candidate.startedAt === 'string' && typeof candidate.updatedAt === 'string' && isStringArray(candidate.command) && @@ -682,6 +690,12 @@ export async function createBackgroundSession( if (!Number.isInteger(input.pid) || input.pid <= 0) { throw new Error(`Invalid background session pid: ${input.pid}`) } + if ( + input.processMarker !== undefined && + !isValidBackgroundProcessMarker(input.processMarker) + ) { + throw new Error('Invalid background process marker') + } await assertBackgroundSessionNameAvailable(input.name) const timestamp = iso(input.now) const logPaths = getBackgroundSessionLogPaths(input.id) @@ -694,6 +708,9 @@ export async function createBackgroundSession( ...(input.provider ? { provider: input.provider } : {}), ...(input.model ? { model: input.model } : {}), sessionId: input.sessionId, + ...(input.processMarker + ? { processMarker: input.processMarker } + : {}), startedAt: timestamp, updatedAt: timestamp, command: input.command, @@ -897,6 +914,35 @@ function commandLineMatchesBackgroundSession( return commandLineContainsArgs(commandLine, session.command) } +function markedCommandLineIdentity( + commandLine: string, + session: BackgroundSession, + processMarker: string, +): 'matches' | 'mismatch' | 'unreadable' { + const markerToken = backgroundProcessMarkerToken(processMarker) + const storedTokens = session.command.flatMap(tokenizeCommandLine) + const expectedIndex = storedTokens.indexOf(markerToken) + if (expectedIndex === -1) return 'unreadable' + + const liveTokens = tokenizeCommandLine(commandLine) + const comparablePrefixLength = Math.min(expectedIndex, liveTokens.length) + for (let index = 0; index < comparablePrefixLength; index += 1) { + if (liveTokens[index] !== storedTokens[index]) return 'mismatch' + } + + if (liveTokens.length <= expectedIndex) return 'unreadable' + const candidate = liveTokens[expectedIndex]! + if (candidate === markerToken) return 'matches' + if ( + expectedIndex === liveTokens.length - 1 && + candidate.length > 0 && + markerToken.startsWith(candidate) + ) { + return 'unreadable' + } + return 'mismatch' +} + export function verifyBackgroundSessionProcessIdentity( session: BackgroundSession, options?: BackgroundSessionProcessIdentityOptions, @@ -928,6 +974,11 @@ export function verifyBackgroundSessionProcessIdentity( if (command == null || command.trim() === '') { return result('unreadable') } + if (session.processMarker !== undefined) { + return result( + markedCommandLineIdentity(command, session, session.processMarker), + ) + } return result( commandLineMatchesBackgroundSession(command, session) ? 'matches' diff --git a/src/cli/bgRouting.ts b/src/cli/bgRouting.ts index a3494f8a3..53b5e1e5b 100644 --- a/src/cli/bgRouting.ts +++ b/src/cli/bgRouting.ts @@ -1,4 +1,55 @@ +import { randomBytes } from 'node:crypto' + export const BACKGROUND_SESSION_ID_ENV = 'OPENCLAUDE_INTERNAL_BACKGROUND_SESSION_ID' export const BACKGROUND_SESSION_LAUNCHER_PID_ENV = 'OPENCLAUDE_INTERNAL_BACKGROUND_LAUNCHER_PID' + +export const BACKGROUND_PROCESS_MARKER_FLAG = + '--openclaude-bg-session-marker' + +const BACKGROUND_PROCESS_MARKER_BYTES = 32 +const BACKGROUND_PROCESS_MARKER_RE = /^[a-f0-9]{64}$/ + +export function isValidBackgroundProcessMarker(value: unknown): value is string { + return ( + typeof value === 'string' && BACKGROUND_PROCESS_MARKER_RE.test(value) + ) +} + +export function generateBackgroundProcessMarker( + getRandomBytes: (size: number) => Uint8Array = randomBytes, +): string { + const bytes = getRandomBytes(BACKGROUND_PROCESS_MARKER_BYTES) + if (bytes.byteLength !== BACKGROUND_PROCESS_MARKER_BYTES) { + throw new Error( + 'Background process marker entropy source returned the wrong length', + ) + } + return Buffer.from(bytes).toString('hex') +} + +export function backgroundProcessMarkerToken(marker: string): string { + if (!isValidBackgroundProcessMarker(marker)) { + throw new Error('Invalid background process marker') + } + return `${BACKGROUND_PROCESS_MARKER_FLAG}=${marker}` +} + +export function stripBackgroundProcessMarkerArgs(args: string[]): string[] { + const first = args[0] + const inlinePrefix = `${BACKGROUND_PROCESS_MARKER_FLAG}=` + if ( + first?.startsWith(inlinePrefix) && + isValidBackgroundProcessMarker(first.slice(inlinePrefix.length)) + ) { + return args.slice(1) + } + if ( + first === BACKGROUND_PROCESS_MARKER_FLAG && + isValidBackgroundProcessMarker(args[1]) + ) { + return args.slice(2) + } + return [...args] +} diff --git a/src/main.tsx b/src/main.tsx index 6b563fd00..70176514e 100644 --- a/src/main.tsx +++ b/src/main.tsx @@ -49,6 +49,7 @@ import type { ToolInputJSONSchema } from './Tool.js'; import { createSyntheticOutputTool, isSyntheticOutputToolEnabled } from './tools/SyntheticOutputTool/SyntheticOutputTool.js'; import { registerTaskReportCommand } from './cli/commands/taskReport.js'; import { registerAimlapiCommand } from './cli/aimlapiCommand.js'; +import { BACKGROUND_PROCESS_MARKER_FLAG, isValidBackgroundProcessMarker } from './cli/bgRouting.js'; import { getTools } from './tools.js'; import { canUserConfigureAdvisor, getInitialAdvisorSetting, isAdvisorEnabled, isValidAdvisorModel, modelSupportsAdvisor } from './utils/advisor.js'; import { isAgentSwarmsEnabled } from './utils/agentSwarmsEnabled.js'; @@ -3644,6 +3645,13 @@ async function run(): Promise { } }).version(`${MACRO.DISPLAY_VERSION ?? MACRO.VERSION} (OpenClaude)`, '-v, --version', 'Output the version number'); + program.addOption(new Option(`${BACKGROUND_PROCESS_MARKER_FLAG} `, 'Internal background process identity marker').argParser(value => { + if (!isValidBackgroundProcessMarker(value)) { + throw new InvalidArgumentError('Invalid background process marker'); + } + return value; + }).hideHelp()); + // Worktree flags program.option('-w, --worktree [name]', 'Create a new git worktree for this session (optionally specify a name)'); program.option('--tmux', 'Create a tmux session for the worktree (requires --worktree). Uses iTerm2 native panes when available; use --tmux=classic for traditional tmux.');