fix(daemon): keep agent-completion detection alive on pre-v27 daemons (#10478)
* fix(daemon): keep agent-completion detection alive on pre-v27 daemons DaemonPtyAdapter.inspectProcess() threw terminal_liveness_unavailable when the connected daemon predated protocol v27. The intended provider-level fallback only fires when a provider lacks inspectProcess, so for the daemon adapter the throw propagated: agent-completion-coordinator swallowed it into consecutiveInspectionErrors and retried forever, killing process-exit completions and pending-title validation. Daemons intentionally survive app updates, so updating in place with agent terminals open routes those PTYs to a legacy adapter and permanently disables agent-finished notifications until the terminal is recreated. Compose the inspection client-side from getForegroundProcess, which v26 fully supports. No new wire traffic and no new daemon capability. * test(daemon): pin null-foreground semantics on the pre-v27 inspect fallback The legacy composition had no coverage for a null foreground, which is the one daemon response shape whose semantics diverge from v27: there inspectProcess goes through getAliveSession() and throws for a vanished session, while getForegroundProcess is deliberately null-not-throw. It is also the only shape that reaches a user-visible completion, so reading it as idle is a deliberate choice that should not change silently.
This commit is contained in:
parent
2653794c82
commit
6592c01592
|
|
@ -6,6 +6,7 @@ import { mkdtempSync, mkdirSync, rmSync, existsSync, readFileSync, writeFileSync
|
|||
import { DaemonClient } from './client'
|
||||
import { DaemonProtocolError } from './daemon-errors'
|
||||
import { DaemonPtyAdapter } from './daemon-pty-adapter'
|
||||
import { COMPLETION_PROCESS_INSPECTION_PROTOCOL_VERSION } from './daemon-protocol-version'
|
||||
import { DaemonServer } from './daemon-server'
|
||||
import { HeadlessEmulator } from './headless-emulator'
|
||||
import { getHistorySessionDirName } from './history-paths'
|
||||
|
|
@ -927,6 +928,95 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => {
|
|||
})
|
||||
})
|
||||
|
||||
describe('inspectProcess on pre-inspection daemon protocols', () => {
|
||||
// Why: daemons outlive an in-place app update, so a v26 daemon must still answer inspections
|
||||
// client-side; throwing here leaves agent-completion detection permanently retrying.
|
||||
type ClientInternals = {
|
||||
client: { request: ReturnType<typeof vi.fn>; disconnect: ReturnType<typeof vi.fn> }
|
||||
}
|
||||
|
||||
function createLegacyAdapter(request: ReturnType<typeof vi.fn>): DaemonPtyAdapter {
|
||||
const legacy = new DaemonPtyAdapter({
|
||||
socketPath,
|
||||
tokenPath,
|
||||
protocolVersion: COMPLETION_PROCESS_INSPECTION_PROTOCOL_VERSION - 1
|
||||
})
|
||||
;(legacy as unknown as ClientInternals).client = { request, disconnect: vi.fn() }
|
||||
return legacy
|
||||
}
|
||||
|
||||
it('composes a real inspection from the foreground call the daemon does support', async () => {
|
||||
const request = vi.fn(async () => ({ foregroundProcess: 'codex' }))
|
||||
const legacy = createLegacyAdapter(request)
|
||||
|
||||
expect(await legacy.inspectProcess('sess-a')).toEqual({
|
||||
foregroundProcess: 'codex',
|
||||
hasChildProcesses: true
|
||||
})
|
||||
// Why: the fallback must not depend on a capability the legacy daemon lacks.
|
||||
expect(request).toHaveBeenCalledWith('getForegroundProcess', { sessionId: 'sess-a' })
|
||||
expect(request).not.toHaveBeenCalledWith('inspectProcess', expect.anything())
|
||||
|
||||
legacy.dispose()
|
||||
})
|
||||
|
||||
it('reports an idle shell as having no child processes', async () => {
|
||||
const legacy = createLegacyAdapter(vi.fn(async () => ({ foregroundProcess: 'bash' })))
|
||||
|
||||
expect(await legacy.inspectProcess('sess-a')).toEqual({
|
||||
foregroundProcess: 'bash',
|
||||
hasChildProcesses: false
|
||||
})
|
||||
|
||||
legacy.dispose()
|
||||
})
|
||||
|
||||
it('reports a null foreground as idle, matching what the legacy daemon can report', async () => {
|
||||
// Why: pins the one response shape whose semantics differ from v27, where inspectProcess goes
|
||||
// through getAliveSession() and throws for a vanished session while getForegroundProcess stays
|
||||
// null-not-throw. Reading it as idle is deliberate — this is also the only shape that reaches a
|
||||
// user-visible completion — so the divergence must not change silently.
|
||||
const legacy = createLegacyAdapter(vi.fn(async () => ({ foregroundProcess: null })))
|
||||
|
||||
expect(await legacy.inspectProcess('sess-a')).toEqual({
|
||||
foregroundProcess: null,
|
||||
hasChildProcesses: false
|
||||
})
|
||||
|
||||
legacy.dispose()
|
||||
})
|
||||
|
||||
it('rejects rather than reading as idle when the daemon call fails', async () => {
|
||||
// Why: getForegroundProcess swallows errors into null; composing through it would turn a dead
|
||||
// socket into a false "agent exited" completion, the mirror of the bug this path fixes.
|
||||
const legacy = createLegacyAdapter(
|
||||
vi.fn(async () => {
|
||||
throw new Error('socket_closed')
|
||||
})
|
||||
)
|
||||
|
||||
await expect(legacy.inspectProcess('sess-a')).rejects.toThrow('socket_closed')
|
||||
|
||||
legacy.dispose()
|
||||
})
|
||||
|
||||
it('still delegates to the daemon once the protocol supports inspectProcess', async () => {
|
||||
const request = vi.fn(async () => ({ foregroundProcess: 'codex', hasChildProcesses: true }))
|
||||
const current = new DaemonPtyAdapter({
|
||||
socketPath,
|
||||
tokenPath,
|
||||
protocolVersion: COMPLETION_PROCESS_INSPECTION_PROTOCOL_VERSION
|
||||
})
|
||||
;(current as unknown as ClientInternals).client = { request, disconnect: vi.fn() }
|
||||
|
||||
await current.inspectProcess('sess-a')
|
||||
|
||||
expect(request).toHaveBeenCalledWith('inspectProcess', { sessionId: 'sess-a' })
|
||||
|
||||
current.dispose()
|
||||
})
|
||||
})
|
||||
|
||||
describe('serialize / revive', () => {
|
||||
it('serialize returns JSON', async () => {
|
||||
const { id } = await adapter.spawn({ cols: 80, rows: 24 })
|
||||
|
|
|
|||
|
|
@ -818,17 +818,30 @@ export class DaemonPtyAdapter implements IPtyProvider {
|
|||
// No flow control for daemon-backed terminals
|
||||
}
|
||||
|
||||
async hasChildProcesses(id: string): Promise<boolean> {
|
||||
const foregroundProcess = await this.getForegroundProcess(id)
|
||||
// Why: daemon-backed PTYs can host long-lived agents while detached; cleanup prompts must not treat them as idle shells.
|
||||
// Why: daemon-backed PTYs can host long-lived agents while detached; cleanup prompts must not treat them as idle shells.
|
||||
private hasChildProcessesFromForeground(foregroundProcess: string | null): boolean {
|
||||
return foregroundProcess !== null && !isShellProcess(foregroundProcess)
|
||||
}
|
||||
|
||||
async hasChildProcesses(id: string): Promise<boolean> {
|
||||
return this.hasChildProcessesFromForeground(await this.getForegroundProcess(id))
|
||||
}
|
||||
|
||||
async inspectProcess(
|
||||
id: string
|
||||
): Promise<{ foregroundProcess: string | null; hasChildProcesses: boolean }> {
|
||||
if (this.protocolVersion < COMPLETION_PROCESS_INSPECTION_PROTOCOL_VERSION) {
|
||||
throw new Error('terminal_liveness_unavailable')
|
||||
// Why: pre-v27 daemons survive an in-place app update; compose the inspection client-side from the
|
||||
// one call they do support instead of throwing, or completion detection stays dead until recreate.
|
||||
// Requests directly (not via getForegroundProcess) so a dead socket still rejects rather than
|
||||
// reading as an idle foreground and dispatching a false completion.
|
||||
const { foregroundProcess } = await this.client.request<{
|
||||
foregroundProcess: string | null
|
||||
}>('getForegroundProcess', { sessionId: id })
|
||||
return {
|
||||
foregroundProcess,
|
||||
hasChildProcesses: this.hasChildProcessesFromForeground(foregroundProcess)
|
||||
}
|
||||
}
|
||||
return this.client.request<{
|
||||
foregroundProcess: string | null
|
||||
|
|
|
|||
Loading…
Reference in New Issue