Skip to content
Open
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
2 changes: 1 addition & 1 deletion cli/src/api/apiMachine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export { normalizeWindowsDriveRoot } from './machinePathPolicy'

type MachineRpcHandlers = {
spawnSession: (options: SpawnSessionOptions) => Promise<SpawnSessionResult>
stopSession: (sessionId: string) => Promise<'stopped' | 'already_gone' | 'still_alive'>
stopSession: (sessionId: string) => Promise<'stopped' | 'already_gone' | 'still_alive' | 'unknown'>
requestShutdown: () => void
}

Expand Down
4 changes: 2 additions & 2 deletions cli/src/runner/controlClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,9 @@ export async function listRunnerSessions(): Promise<any[]> {
return result.children || [];
}

export async function stopRunnerSession(sessionId: string): Promise<'stopped' | 'already_gone' | 'still_alive'> {
export async function stopRunnerSession(sessionId: string): Promise<'stopped' | 'already_gone' | 'still_alive' | 'unknown'> {
const result = await runnerPost('/stop-session', { sessionId });
return result.status === 'stopped' || result.status === 'already_gone' || result.status === 'still_alive'
return result.status === 'stopped' || result.status === 'already_gone' || result.status === 'still_alive' || result.status === 'unknown'
? result.status
: 'still_alive';
}
Expand Down
4 changes: 2 additions & 2 deletions cli/src/runner/controlServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export function startRunnerControlServer({
onHappySessionWebhook
}: {
getChildren: () => TrackedSession[];
stopSession: (sessionId: string) => Promise<'stopped' | 'already_gone' | 'still_alive'>;
stopSession: (sessionId: string) => Promise<'stopped' | 'already_gone' | 'still_alive' | 'unknown'>;
spawnSession: (options: SpawnSessionOptions) => Promise<SpawnSessionResult>;
requestShutdown: () => void;
onHappySessionWebhook: (sessionId: string, metadata: Metadata) => void;
Expand Down Expand Up @@ -91,7 +91,7 @@ export function startRunnerControlServer({
}),
response: {
200: z.object({
status: z.enum(['stopped', 'already_gone', 'still_alive'])
status: z.enum(['stopped', 'already_gone', 'still_alive', 'unknown'])
})
}
}
Expand Down
11 changes: 9 additions & 2 deletions cli/src/runner/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -937,7 +937,7 @@ export async function startRunner(options: { workspaceRoots?: string[] } = {}):
}

// Stop a session by sessionId or PID fallback
const stopSession = async (sessionId: string): Promise<'stopped' | 'already_gone' | 'still_alive'> => {
const stopSession = async (sessionId: string): Promise<'stopped' | 'already_gone' | 'still_alive' | 'unknown'> => {
logger.debug(`[RUNNER RUN] Attempting to stop session ${sessionId}`);

// Try to find by sessionId first
Expand Down Expand Up @@ -1046,8 +1046,15 @@ export async function startRunner(options: { workspaceRoots?: string[] } = {}):
logger.debug(`[RUNNER RUN] Session ${sessionId} was previously observed exited`);
return 'already_gone';
}
// Distinct from 'still_alive': no PID matched this id anywhere (not
// tracked, not persisted, no verified-exit tombstone), so there is
// nothing here to confirm as alive OR dead. Callers that already have
// concrete knowledge of this session (e.g. one they just spawned) should
// treat this the same as 'still_alive' defensively; callers reconciling
// an old/stale row this runner instance never knew about should not be
// blocked forever by a status that never actually confirmed a live process.
logger.debug(`[RUNNER RUN] Session ${sessionId} not found without verified exit`);
return 'still_alive';
return 'unknown';
};

// Handle child process exit
Expand Down
5 changes: 4 additions & 1 deletion cli/src/runner/runner.integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@
it('should spawn & stop a session via HTTP (not testing RPC route, but similar enough)', async () => {
const response = await spawnTrackedSession('/tmp', 'spawned-test-456');

expect(response).toHaveProperty('success', true);

Check failure on line 206 in cli/src/runner/runner.integration.test.ts

View workflow job for this annotation

GitHub Actions / integration

src/runner/runner.integration.test.ts > Runner Integration Tests > should spawn & stop a session via HTTP (not testing RPC route, but similar enough)

AssertionError: expected { Object (error) } to have property "success" with value true - Expected: true + Received: undefined ❯ src/runner/runner.integration.test.ts:206:22
expect(response).toHaveProperty('sessionId');

// Verify session is tracked
Expand All @@ -219,7 +219,10 @@
expect(spawnedSession.happySessionId).toBeDefined();
expect(await stopRunnerSession(spawnedSession.happySessionId)).toBe('stopped');
expect(await stopRunnerSession(spawnedSession.happySessionId)).toBe('already_gone');
expect(await stopRunnerSession('unknown-session-id')).toBe('still_alive');
// Distinct from 'still_alive': no PID matched this id and there is no
// verified-exit tombstone, so the runner has no basis to call it either
// alive or dead (see cli/src/runner/run.ts's stopSession fallback).
expect(await stopRunnerSession('unknown-session-id')).toBe('unknown');
});

it.skipIf(process.env.HAPI_RUN_STRESS_TESTS !== 'true')(
Expand Down Expand Up @@ -280,7 +283,7 @@

// List all sessions
const sessions = await listRunnerSessions();
expect(sessions).toHaveLength(2);

Check failure on line 286 in cli/src/runner/runner.integration.test.ts

View workflow job for this annotation

GitHub Actions / integration

src/runner/runner.integration.test.ts > Runner Integration Tests > should track both runner-spawned and terminal sessions

AssertionError: expected [ { …(3) } ] to have a length of 2 but got 1 - Expected + Received - 2 + 1 ❯ src/runner/runner.integration.test.ts:286:22

// Verify we have one of each type
const terminalSession = sessions.find(
Expand Down Expand Up @@ -315,7 +318,7 @@
// Verify webhook was processed (session ID updated)
const sessions = await listRunnerSessions();
const session = sessions.find((s: any) => s.happySessionId === spawnResponse.sessionId);
expect(session).toBeDefined();

Check failure on line 321 in cli/src/runner/runner.integration.test.ts

View workflow job for this annotation

GitHub Actions / integration

src/runner/runner.integration.test.ts > Runner Integration Tests > should update session metadata when webhook is called

AssertionError: expected undefined to be defined ❯ src/runner/runner.integration.test.ts:321:21

// Clean up
await stopRunnerSession(spawnResponse.sessionId);
Expand Down Expand Up @@ -362,7 +365,7 @@

// All should succeed
results.forEach(res => {
expect(res.success).toBe(true);

Check failure on line 368 in cli/src/runner/runner.integration.test.ts

View workflow job for this annotation

GitHub Actions / integration

src/runner/runner.integration.test.ts > Runner Integration Tests > should handle concurrent session operations

AssertionError: expected undefined to be true // Object.is equality - Expected: true + Received: undefined ❯ src/runner/runner.integration.test.ts:368:27 ❯ src/runner/runner.integration.test.ts:367:13
expect(res.sessionId).toBeDefined();
});

Expand Down
4 changes: 2 additions & 2 deletions hub/src/sync/rpcGateway.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,10 @@ export class RpcGateway {
await this.sessionRpc(sessionId, RPC_METHODS.KillSession, {})
}

async stopRunnerSession(machineId: string, sessionId: string): Promise<'stopped' | 'already_gone' | 'still_alive'> {
async stopRunnerSession(machineId: string, sessionId: string): Promise<'stopped' | 'already_gone' | 'still_alive' | 'unknown'> {
const result = await this.machineRpc(machineId, RPC_METHODS.StopSession, { sessionId })
const status = result && typeof result === 'object' ? (result as { status?: unknown }).status : undefined
if (status === 'stopped' || status === 'already_gone' || status === 'still_alive') return status
if (status === 'stopped' || status === 'already_gone' || status === 'still_alive' || status === 'unknown') return status
throw new Error('Unexpected stop-session response')
}

Expand Down
89 changes: 78 additions & 11 deletions hub/src/sync/syncEngine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1567,7 +1567,13 @@ export class SyncEngine {
): Promise<void> {
if (spawnAttempted) {
const status = await this.rpcGateway.stopRunnerSession(machineId, childId)
if (status === 'still_alive') {
// Treat 'unknown' (no PID matched, no verified-exit tombstone) the
// same as 'still_alive' here: this child was supposedly just
// spawned this turn, so the runner not recognizing it at all is
// itself a sign something is wrong, not proof it's gone. Unlike
// archiveSession's stale-row fallback, there is no long-lived-row
// case here that needs unblocking.
if (status === 'still_alive' || status === 'unknown') {
throw new Error('Fork child termination was not confirmed')
}
}
Expand Down Expand Up @@ -1673,6 +1679,67 @@ export class SyncEngine {
await this.rpcGateway.killSession(sessionId)
} catch (error) {
if (error instanceof RpcTargetMissingError) {
// `killSession` addresses the CLI's own session-scoped socket
// registration (`${sessionId}:KillSession`). That registration
// can go stale independently of the actual runner child
// process — e.g. SessionCache.mergeSessions rotates the
// hub's canonical session id on resume without the CLI
// re-registering under it, or the socket is mid-reconnect —
// so a missing target does NOT by itself prove the process
// is dead. Before trusting that, ask the runner directly via
// the machine-level StopSession RPC, which resolves the
// child by PID and checks both the requested and confirmed
// session ids (see `stopSession` in cli/src/runner/run.ts),
// so it survives the exact id mismatch that just defeated
// `killSession`. Without this check, a live orphaned child
// gets marked `archived` and loses all runner-side
// supervision while continuing to run.
const machineId = this.sessionCache.getSession(sessionId)?.metadata?.machineId
if (machineId) {
let status: 'stopped' | 'already_gone' | 'still_alive' | 'unknown'
try {
status = await this.rpcGateway.stopRunnerSession(machineId, sessionId)
} catch (stopError) {
// `MachineCache.active` is a 45s heartbeat-derived
// heuristic (machineCache.ts's expireInactive), NOT the
// same signal as "this machine's RPC target is
// registered" — a socket only deregisters on actual
// disconnect (rpcRegistry.unregisterAll). A delayed
// heartbeat can leave `active` false while the socket,
// and this exact RPC, are still perfectly reachable.
// So don't pre-check `active` at all — just attempt the
// call and let its own failure mode tell us what
// happened: `RpcTargetMissingError` means the machine
// genuinely has no RPC target (nothing stronger to
// check than the original error), while any other
// failure (ack timeout, protocol error) means the
// machine IS reachable but this particular call didn't
// resolve — which does NOT mean the process is gone.
// Coercing that ambiguous case to "already gone" would
// silently archive a session whose runner just didn't
// answer in time — the exact bug this fallback exists
// to prevent, one RPC layer down.
status = stopError instanceof RpcTargetMissingError ? 'already_gone' : 'still_alive'
}
// `'unknown'` means the runner has no PID and no verified-exit
// tombstone for this id at all (cli/src/runner/run.ts's
// stopSession fail-closed default) — it is NOT confirmation
// that a process is running, but it is also NOT confirmation
// that it's gone: terminal-started sessions are tracked only
// in-memory (pidToTrackedSession), never persisted, so a
// still-alive terminal session can return 'unknown' right
// after this exact runner process restarts and loses that
// in-memory tracking — indistinguishable, from this RPC
// alone, from a session this runner generation never knew
// about at all. Given that ambiguity, treat it the same as
// `'still_alive'` — conservative, matching every other
// stopRunnerSession call site in this file — rather than
// risk silently archiving a session that's actually still
// running.
if (status === 'still_alive' || status === 'unknown') {
throw new Error('Session process is still running and could not be stopped')
}
}
this.sessionCache.markSessionArchivedFromHub(sessionId, 'Archived from hub (CLI unreachable)')
} else {
throw error
Expand Down Expand Up @@ -2774,7 +2841,7 @@ export class SyncEngine {
if (session.active || operation?.state !== 'reserved' || !machineId) return false
try {
const status = await this.rpcGateway.stopRunnerSession(machineId, session.id)
if (status === 'still_alive') return false
if (status === 'still_alive' || status === 'unknown') return false
return this.abortOpenCodeClearSession(
session.id, namespace, operation.replacementSessionId, 'reserved', true
).type === 'success'
Expand Down Expand Up @@ -3061,7 +3128,7 @@ export class SyncEngine {
const readyResult = await this.waitForSessionReady(spawnResult.sessionId)
if (readyResult !== 'ready') {
if (resumedStartingMode === 'pty' && readyResult === 'timeout') {
let status: 'stopped' | 'already_gone' | 'still_alive'
let status: 'stopped' | 'already_gone' | 'still_alive' | 'unknown'
try {
status = await this.rpcGateway.stopRunnerSession(
targetMachine.id,
Expand Down Expand Up @@ -3579,7 +3646,7 @@ export class SyncEngine {
machineId,
startedAt: Date.now(),
})
let status: 'stopped' | 'already_gone' | 'still_alive'
let status: 'stopped' | 'already_gone' | 'still_alive' | 'unknown'
try {
status = await this.rpcGateway.stopRunnerSession(machineId, sessionId)
} catch {
Expand All @@ -3589,7 +3656,7 @@ export class SyncEngine {
await new Promise((resolve) => setTimeout(resolve, 0))
const session = this.sessionCache.refreshSession(sessionId) ?? this.sessionCache.getSession(sessionId)
const attemptClearedByEnd = session?.metadata?.piResumeAttempt === undefined
if (status === 'still_alive') {
if (status === 'still_alive' || status === 'unknown') {
if (attemptClearedByEnd) return true
return false
}
Expand All @@ -3613,7 +3680,7 @@ export class SyncEngine {
startedAt: Date.now(),
childSessionId: sessionId,
})
let status: 'stopped' | 'already_gone' | 'still_alive'
let status: 'stopped' | 'already_gone' | 'still_alive' | 'unknown'
try {
status = await this.rpcGateway.stopRunnerSession(machineId, sessionId)
} catch {
Expand All @@ -3624,7 +3691,7 @@ export class SyncEngine {
const session = this.sessionCache.refreshSession(sessionId) ?? this.sessionCache.getSession(sessionId)
const original = this.sessionCache.refreshSession(originalSessionId) ?? this.sessionCache.getSession(originalSessionId)
const attemptClearedByEnd = original?.metadata?.piResumeAttempt === undefined
if (status === 'still_alive' && !attemptClearedByEnd) {
if ((status === 'still_alive' || status === 'unknown') && !attemptClearedByEnd) {
await this.writePiResumeAttempt(originalSessionId, namespace, {
...existingAttempt,
state: 'quarantined',
Expand Down Expand Up @@ -3742,13 +3809,13 @@ export class SyncEngine {
private async reconcilePersistedPtyResumeAttempt(session: Session): Promise<boolean> {
const attempt = session.metadata?.ptyResumeAttempt
if (!attempt) return true
let status: 'stopped' | 'already_gone' | 'still_alive'
let status: 'stopped' | 'already_gone' | 'still_alive' | 'unknown'
try {
status = await this.rpcGateway.stopRunnerSession(attempt.machineId, session.id)
} catch {
return false
}
if (status === 'still_alive') return false
if (status === 'still_alive' || status === 'unknown') return false

const current = this.sessionCache.getSession(session.id)
if (current?.active) {
Expand All @@ -3768,13 +3835,13 @@ export class SyncEngine {
const attempt = session.metadata?.piResumeAttempt
if (!attempt) return true
const childSessionId = attempt.childSessionId ?? session.id
let status: 'stopped' | 'already_gone' | 'still_alive'
let status: 'stopped' | 'already_gone' | 'still_alive' | 'unknown'
try {
status = await this.rpcGateway.stopRunnerSession(attempt.machineId, childSessionId)
} catch {
return false
}
if (status === 'still_alive') return false
if (status === 'still_alive' || status === 'unknown') return false

const child = this.sessionCache.getSession(childSessionId)
if (child?.active) this.handleSessionEnd({ sid: childSessionId, time: Date.now(), reason: 'error' })
Expand Down
Loading
Loading