Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
154 changes: 154 additions & 0 deletions src/cli/bgRegistry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ import { join } from 'node:path'
import {
_setBackgroundSessionsRootForTesting,
createBackgroundSession,
isBackgroundSessionProcessAlive,
isTerminalBackgroundSession,
listBackgroundSessions,
markBackgroundSessionKilled,
refreshBackgroundSessionStatuses,
resolveBackgroundSession,
type BackgroundSession,
} from './bgRegistry.js'

describe('background session registry', () => {
Expand Down Expand Up @@ -675,3 +677,155 @@ describe('background session registry', () => {
expect(await listBackgroundSessions()).toEqual([])
})
})

describe('isBackgroundSessionProcessAlive process identity', () => {
const session: BackgroundSession = {
id: 'bg-identity',
pid: 4242,
cwd: '/repo',
status: 'running',
startedAt: '2026-07-01T08:00:00.000Z',
updatedAt: '2026-07-01T08:00:00.000Z',
// sessionId deliberately absent from the command lines below so the stored
// launch invocation (command) is what has to match.
sessionId: 'conversation-identity',
command: ['node', 'openclaude', '1642'],
stdoutLogPath: '/tmp/stdout.log',
stderrLogPath: '/tmp/stderr.log',
}

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
// contains the stored selector "1642" as a substring. Ordered substring
// matching wrongly reported this session as alive, so `kill` could target
// the wrong process.
const alive = isBackgroundSessionProcessAlive(session, {
isProcessAlive: () => true,
getProcessCommand: () => 'node openclaude 16420 --serve',
})
expect(alive).toBe(false)
})

it('still recognizes the real process by exact command tokens', () => {
const alive = isBackgroundSessionProcessAlive(session, {
isProcessAlive: () => true,
getProcessCommand: () => 'node openclaude 1642 --serve',
})
expect(alive).toBe(true)
})

it('matches on the session id when it is present on the command line', () => {
const alive = isBackgroundSessionProcessAlive(session, {
isProcessAlive: () => true,
getProcessCommand: () => 'node openclaude conversation-identity',
})
expect(alive).toBe(true)
})

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
// command-arg path. Command args are absent from the live line so only the
// session-id branch can produce a match here.
const shortIdSession: BackgroundSession = {
...session,
sessionId: 'sess-1',
command: ['node', 'openclaude', 'unused-token'],
}
const alive = isBackgroundSessionProcessAlive(shortIdSession, {
isProcessAlive: () => true,
getProcessCommand: () => 'node openclaude sess-100 --serve',
})
expect(alive).toBe(false)
})

it('matches the session id only as a whole token', () => {
const shortIdSession: BackgroundSession = {
...session,
sessionId: 'sess-1',
command: ['node', 'openclaude', 'unused-token'],
}
const alive = isBackgroundSessionProcessAlive(shortIdSession, {
isProcessAlive: () => true,
getProcessCommand: () => 'node openclaude sess-1 --serve',
})
expect(alive).toBe(true)
})

it('matches a stored multi-word prompt arg across command tokens (#1770)', () => {
// A prompt like "refactor auth" is stored as a single argv entry but `ps`
// renders it as separate words; the matcher must span both. The session id
// is absent from the live line so the command args are what must match.
const promptSession: BackgroundSession = {
...session,
sessionId: 'conversation-absent',
command: ['node', 'openclaude', '--print', 'refactor auth'],
}
const alive = isBackgroundSessionProcessAlive(promptSession, {
isProcessAlive: () => true,
getProcessCommand: () =>
'node openclaude --print refactor auth --serve',
})
expect(alive).toBe(true)
})

it('matches a quoted Windows command line with a spaced exe path and prompt (#1770)', () => {
// Windows `Get-CimInstance ... CommandLine` returns the raw command line
// with quoted paths/prompts, so a whitespace split fuses quotes onto the
// edge tokens (`"C:\Program`, `node.exe"`, `"refactor`, `auth"`). The stored
// argv holds those values unquoted, so without quote trimming the contiguous
// run never matched and a live `--from-pr` resume (whose only identity path
// is the stored command) was wrongly marked stale.
const windowsSession: BackgroundSession = {
...session,
sessionId: 'conversation-absent',
command: [
'C:\\Program Files\\nodejs\\node.exe',
'C:\\repo\\dist\\cli.mjs',
'--from-pr',
'1642',
'--print',
'refactor auth',
],
}
const alive = isBackgroundSessionProcessAlive(windowsSession, {
isProcessAlive: () => true,
getProcessCommand: () =>
'"C:\\Program Files\\nodejs\\node.exe" C:\\repo\\dist\\cli.mjs --from-pr 1642 --print "refactor auth"',
})
expect(alive).toBe(true)
})

it('quote trimming does not reopen the substring collision (#1770)', () => {
// Trimming surrounding quotes must not degrade to substring matching: a
// quoted live token "16420" still only contains the stored selector "1642",
// so it must not satisfy the lookup.
const alive = isBackgroundSessionProcessAlive(session, {
isProcessAlive: () => true,
getProcessCommand: () => '"node" openclaude "16420" --serve',
})
expect(alive).toBe(false)
})

it('does not treat interspersed stored tokens as alive (#1770)', () => {
// The stored tokens all appear on the live command line but only as an
// ordered subsequence with unrelated tokens ("attacker", "extra") wedged
// between them, i.e. a different process at a reused PID. Requiring a
// contiguous whole-token run rejects this token-insertion collision; a
// subsequence match would wrongly report it alive and risk killing the
// wrong process.
const alive = isBackgroundSessionProcessAlive(session, {
isProcessAlive: () => true,
getProcessCommand: () => 'node attacker openclaude extra 1642 --serve',
})
expect(alive).toBe(false)
})

it('reports a dead process regardless of command line', () => {
const alive = isBackgroundSessionProcessAlive(session, {
isProcessAlive: () => false,
getProcessCommand: () => 'node openclaude 1642',
})
expect(alive).toBe(false)
})
})
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
58 changes: 51 additions & 7 deletions src/cli/bgRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -504,22 +504,66 @@ export async function refreshBackgroundSessionStatuses(options?: {

type BackgroundSessionProcessState = 'alive' | 'dead' | 'unknown'

// A spaced path or prompt is a single argv entry, but the raw command line
// quotes it, so a whitespace split fuses a quote onto the edge tokens. Windows
// `Get-CimInstance ... CommandLine` returns exactly this form — e.g.
// "C:\Program Files\nodejs\node.exe" ...\cli.mjs --from-pr 1642 --print "refactor auth"
// splits to `"C:\Program`, `Files\nodejs\node.exe"`, ..., `"refactor`, `auth"`.
// The stored argv holds those same values unquoted, so trim a single leading
// and/or trailing quote from each token before comparing. POSIX `ps` output is
// unquoted, making this a no-op there, and it never widens the token-boundary
// match below (a stripped token still has to equal the stored one). See #1770.
function tokenizeCommandLine(value: string): string[] {
return value
.split(/\s+/)
.map(token => token.replace(/^["']|["']$/g, ''))
.filter(token => token.length > 0)
}

function commandLineContainsArgs(commandLine: string, args: string[]): boolean {
if (args.length === 0) return false
let offset = 0
for (const arg of args) {
const index = commandLine.indexOf(arg, offset)
if (index === -1) return false
offset = index + arg.length
// Match the stored args against whole whitespace-delimited tokens, in order,
// rather than as a raw substring. Substring matching let a stored selector
// like "1642" satisfy a lookup against an unrelated live token "16420" (e.g. a
// reused PID whose command line merely contains those digits), so a dead
// session stayed classified as running. See #1770.
//
// A stored arg can itself contain whitespace — a prompt like "refactor auth"
// is a single argv entry but `ps` renders it as separate words — so expand
// each arg into its own tokens and require the flattened sequence to appear as
// one CONTIGUOUS run of whole command tokens. An ordered-subsequence match
// (skipping unrelated tokens between matches) would let a reused PID whose
// command line merely interleaves the stored tokens pass — e.g. stored
// ["node", "openclaude", "1642"] satisfied by "node attacker openclaude extra
// 1642 --serve" — reopening the same wrong-process `kill` risk for token
// insertion collisions. The real launch invocation appears as an unbroken run
// (only the interpreter path or trailing flags differ), so leading/trailing
// tokens are fine but interspersed ones are not.
const tokens = tokenizeCommandLine(commandLine)
const argTokens = args.flatMap(tokenizeCommandLine)
if (argTokens.length === 0) return false
if (argTokens.length > tokens.length) return false
for (let start = 0; start <= tokens.length - argTokens.length; start += 1) {
let matched = true
for (let offset = 0; offset < argTokens.length; offset += 1) {
if (tokens[start + offset] !== argTokens[offset]) {
matched = false
break
}
}
if (matched) return true
}
return true
return false
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

function commandLineMatchesBackgroundSession(
commandLine: string,
session: BackgroundSession,
): boolean {
if (commandLine.includes(session.sessionId)) return true
// Match the session id as a whole token, not a raw substring: an id like
// "sess-1" must not match an unrelated live command that merely contains
// "sess-100" (the same reused-PID collision this guard fixes for #1770).
if (commandLineContainsArgs(commandLine, [session.sessionId])) return true
// PR resume launches write to the resumed transcript id without carrying
// that id on argv, so use the stored launch invocation as the PID guard.
return commandLineContainsArgs(commandLine, session.command)
Expand Down