mirror of
https://github.com/stablyai/orca.git
synced 2026-10-22 08:03:37 +00:00
fix(agents): settle and surface the OMP terminal lifecycle (#14278)
OMP never emits agent_settled and its ctx.isIdle can stay false through completion, so the generated status extension's idle recheck never fired and the pane stuck at "working". Settle on OMP's own agent_end contract instead: willContinue === true keeps the pane active, anything else is terminal. Also forward the approval lifecycle OMP suppresses unless an extension listens for it, publish the `ask` questions payload as a live prompt, and stop a generic Pi frame from downgrading a pane the hook already identified as OMP. Co-authored-by: Vlad Rafeev <dandaka@users.noreply.github.com> Co-authored-by: outside.observer <devatnull@users.noreply.github.com>
This commit is contained in:
co-authored by
Vlad Rafeev
outside.observer
parent
9bb8836bb6
commit
a63e7f4cbb
@@ -0,0 +1,82 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import { createHarness, type Harness } from './agent-status-extension-test-harness'
|
||||
|
||||
function postedPayloads(harness: Harness): unknown[] {
|
||||
return harness.fetchMock.mock.calls.map(([_url, init]) => JSON.parse(String(init?.body)).payload)
|
||||
}
|
||||
|
||||
describe('OMP agent lifecycle reporting', () => {
|
||||
it.each([
|
||||
['no willContinue field', { kind: 'omp' as const }, {}],
|
||||
['willContinue false', { kind: 'omp' as const }, { willContinue: false }],
|
||||
['a runtime-routed OMP binary', { kind: 'pi' as const, title: 'omp' }, { willContinue: false }],
|
||||
['no event payload at all', { kind: 'omp' as const }, undefined]
|
||||
])(
|
||||
'posts a terminal agent_end for %s while isIdle stays false',
|
||||
async (_label, harnessArgs, event) => {
|
||||
const harness = createHarness(harnessArgs)
|
||||
|
||||
await harness.callHook('agent_end', event, { isIdle: () => false })
|
||||
|
||||
await vi.waitFor(() => expect(harness.fetchMock).toHaveBeenCalledTimes(1))
|
||||
expect(postedPayloads(harness)).toEqual([{ hook_event_name: 'agent_end' }])
|
||||
}
|
||||
)
|
||||
|
||||
it('keeps the pane working while agent_end schedules an automatic continuation', async () => {
|
||||
vi.useFakeTimers()
|
||||
try {
|
||||
const harness = createHarness({ kind: 'omp' })
|
||||
|
||||
await harness.callHook('agent_end', { willContinue: true }, { isIdle: () => false })
|
||||
await vi.advanceTimersByTimeAsync(1_000)
|
||||
expect(harness.fetchMock).not.toHaveBeenCalled()
|
||||
// No idle-recheck timer is left armed to settle the pane behind the continuation.
|
||||
expect(vi.getTimerCount()).toBe(0)
|
||||
|
||||
await harness.callHook('agent_start')
|
||||
await harness.callHook('agent_end', { willContinue: false }, { isIdle: () => false })
|
||||
await vi.advanceTimersByTimeAsync(0)
|
||||
expect(postedPayloads(harness)).toEqual([
|
||||
{ hook_event_name: 'agent_start' },
|
||||
{ hook_event_name: 'agent_end' }
|
||||
])
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
|
||||
it('forwards the approval lifecycle so a parked permission prompt is visible', async () => {
|
||||
const harness = createHarness({ kind: 'omp' })
|
||||
|
||||
await harness.callHook('tool_approval_requested', {
|
||||
toolName: 'bash',
|
||||
reason: 'tools.approval.bash: prompt',
|
||||
approvalMode: 'prompt'
|
||||
})
|
||||
await vi.waitFor(() => expect(harness.fetchMock).toHaveBeenCalledTimes(1))
|
||||
await harness.callHook('tool_approval_resolved', { toolName: 'bash', approved: true })
|
||||
|
||||
await vi.waitFor(() => expect(harness.fetchMock).toHaveBeenCalledTimes(2))
|
||||
expect(postedPayloads(harness)).toEqual([
|
||||
{
|
||||
hook_event_name: 'tool_approval_requested',
|
||||
tool_name: 'bash',
|
||||
reason: 'tools.approval.bash: prompt',
|
||||
approval_mode: 'prompt'
|
||||
},
|
||||
{ hook_event_name: 'tool_approval_resolved', tool_name: 'bash', approved: true }
|
||||
])
|
||||
})
|
||||
|
||||
it('leaves Pi and Prime runtimes out of the OMP-only approval lifecycle', async () => {
|
||||
const pi = createHarness({ kind: 'pi' })
|
||||
const prime = createHarness({ kind: 'prime-agent' })
|
||||
|
||||
await pi.callHook('tool_approval_requested', { toolName: 'bash' })
|
||||
|
||||
expect(pi.fetchMock).not.toHaveBeenCalled()
|
||||
expect(prime.handlers.tool_approval_requested).toBeUndefined()
|
||||
})
|
||||
})
|
||||
@@ -1,193 +1,6 @@
|
||||
import { runInNewContext } from 'node:vm'
|
||||
// TypeScript 7 is a native CLI; transpile tests still need the legacy JavaScript API.
|
||||
import ts from 'typescript-api'
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import { getPiAgentStatusExtensionSource } from './agent-status-extension-source'
|
||||
|
||||
type HookContext = {
|
||||
isIdle?: () => boolean
|
||||
sessionManager?: {
|
||||
getSessionId?: () => unknown
|
||||
getSessionFile?: () => unknown
|
||||
}
|
||||
}
|
||||
|
||||
type HookHandler = (event?: unknown, context?: HookContext) => Promise<void> | void
|
||||
|
||||
type FakeCurlChild = {
|
||||
on: ReturnType<typeof vi.fn>
|
||||
stdin: {
|
||||
on: ReturnType<typeof vi.fn>
|
||||
end: ReturnType<typeof vi.fn>
|
||||
}
|
||||
}
|
||||
|
||||
type Harness = {
|
||||
fetchMock: ReturnType<typeof vi.fn>
|
||||
spawnMock: ReturnType<typeof vi.fn>
|
||||
spawnedChildren: FakeCurlChild[]
|
||||
fsMock: {
|
||||
existsSync: ReturnType<typeof vi.fn>
|
||||
readFileSync: ReturnType<typeof vi.fn>
|
||||
statSync: ReturnType<typeof vi.fn>
|
||||
}
|
||||
handlers: Record<string, HookHandler>
|
||||
processEnv: Record<string, string | undefined>
|
||||
callHook: (name: string, event?: unknown, context?: HookContext) => Promise<void>
|
||||
// Re-invoke the extension factory in the same process (as Pi does on an
|
||||
// in-process extension reload), swapping in the freshly registered handlers.
|
||||
reload: () => void
|
||||
}
|
||||
|
||||
const BASE_ENV = {
|
||||
ORCA_PANE_KEY: 'pane-1',
|
||||
ORCA_AGENT_LAUNCH_TOKEN: 'launch-1',
|
||||
ORCA_TAB_ID: 'tab-1',
|
||||
ORCA_WORKTREE_ID: 'tree-1',
|
||||
ORCA_AGENT_HOOK_PORT: '4321',
|
||||
ORCA_AGENT_HOOK_TOKEN: 'token-1',
|
||||
ORCA_AGENT_HOOK_ENV: 'env-1',
|
||||
ORCA_AGENT_HOOK_VERSION: '1.2.3'
|
||||
} satisfies Record<string, string>
|
||||
|
||||
// Why: ownership keys on process.pid, so reload and child-process tests need
|
||||
// stable, distinct identities.
|
||||
const SELF_PID = 4242
|
||||
|
||||
function createHarness(args: {
|
||||
kind: 'pi' | 'omp' | 'prime-agent'
|
||||
env?: Record<string, string | undefined>
|
||||
pid?: number
|
||||
title?: string
|
||||
argv?: string[]
|
||||
existsSync?: (path: string) => boolean
|
||||
readFileSync?: (path: string, encoding: string) => string
|
||||
statSync?: (path: string) => { mtimeMs: number; size: number; ino: number }
|
||||
fetchImpl?: (...params: Parameters<typeof fetch>) => Promise<unknown>
|
||||
}): Harness {
|
||||
const fetchMock = vi.fn(
|
||||
args.fetchImpl ??
|
||||
(async () => ({
|
||||
ok: true
|
||||
}))
|
||||
)
|
||||
|
||||
const spawnedChildren: FakeCurlChild[] = []
|
||||
const spawnMock = vi.fn(() => {
|
||||
const child: FakeCurlChild = {
|
||||
on: vi.fn(),
|
||||
stdin: {
|
||||
on: vi.fn(),
|
||||
end: vi.fn()
|
||||
}
|
||||
}
|
||||
spawnedChildren.push(child)
|
||||
return child
|
||||
})
|
||||
|
||||
const fsMock = {
|
||||
existsSync: vi.fn(args.existsSync ?? (() => false)),
|
||||
statSync: vi.fn(
|
||||
args.statSync ??
|
||||
((path: string) => {
|
||||
throw Object.assign(new Error(`ENOENT: ${path}`), { code: 'ENOENT' })
|
||||
})
|
||||
),
|
||||
readFileSync: vi.fn(
|
||||
args.readFileSync ??
|
||||
((path: string) => {
|
||||
throw Object.assign(new Error(`ENOENT: ${path}`), { code: 'ENOENT' })
|
||||
})
|
||||
)
|
||||
}
|
||||
|
||||
const module = {
|
||||
exports: {} as { default?: (pi: { on: (name: string, handler: HookHandler) => void }) => void }
|
||||
}
|
||||
const requireMock = vi.fn((specifier: string) => {
|
||||
if (specifier === 'fs') {
|
||||
return fsMock
|
||||
}
|
||||
if (specifier === 'child_process') {
|
||||
return { spawn: spawnMock }
|
||||
}
|
||||
throw new Error(`unexpected require(${specifier})`)
|
||||
})
|
||||
|
||||
const processMock = {
|
||||
env: {
|
||||
...BASE_ENV,
|
||||
...(args.kind === 'prime-agent' ? { PRIME_AGENT_INTERNAL_DAEMON_WORKER: '1' } : {}),
|
||||
...args.env
|
||||
},
|
||||
pid: args.pid ?? SELF_PID,
|
||||
title: args.title ?? 'node',
|
||||
argv: args.argv ?? ['node', '/usr/bin/orca']
|
||||
}
|
||||
|
||||
const context = {
|
||||
module,
|
||||
exports: module.exports,
|
||||
require: requireMock,
|
||||
process: processMock,
|
||||
fetch: fetchMock,
|
||||
console: {
|
||||
warn: vi.fn(),
|
||||
error: vi.fn(),
|
||||
log: vi.fn()
|
||||
},
|
||||
Promise,
|
||||
Buffer,
|
||||
URL,
|
||||
AbortController,
|
||||
setTimeout,
|
||||
clearTimeout
|
||||
} as Record<string, unknown>
|
||||
context.globalThis = context
|
||||
|
||||
const source = getPiAgentStatusExtensionSource(args.kind)
|
||||
const output = ts.transpileModule(source, {
|
||||
compilerOptions: {
|
||||
module: ts.ModuleKind.CommonJS,
|
||||
target: ts.ScriptTarget.ES2020
|
||||
}
|
||||
}).outputText
|
||||
runInNewContext(output, context)
|
||||
|
||||
const register = module.exports.default
|
||||
if (!register) {
|
||||
throw new Error('expected default export from generated source')
|
||||
}
|
||||
|
||||
const handlers: Record<string, HookHandler> = {}
|
||||
const registerInto = (target: Record<string, HookHandler>): void => {
|
||||
register({
|
||||
on(name: string, handler: HookHandler) {
|
||||
target[name] = handler
|
||||
}
|
||||
})
|
||||
}
|
||||
registerInto(handlers)
|
||||
|
||||
return {
|
||||
fetchMock,
|
||||
spawnMock,
|
||||
spawnedChildren,
|
||||
fsMock,
|
||||
handlers,
|
||||
processEnv: processMock.env,
|
||||
callHook: async (name, event, hookContext) => {
|
||||
await handlers[name]?.(event, hookContext)
|
||||
},
|
||||
reload: () => {
|
||||
for (const key of Object.keys(handlers)) {
|
||||
delete handlers[key]
|
||||
}
|
||||
registerInto(handlers)
|
||||
}
|
||||
}
|
||||
}
|
||||
import { createHarness, SELF_PID } from './agent-status-extension-test-harness'
|
||||
|
||||
describe('getPiAgentStatusExtensionSource', () => {
|
||||
it('registers Prime hooks only in the event-emitting daemon worker', () => {
|
||||
@@ -826,10 +639,12 @@ describe('getPiAgentStatusExtensionSource', () => {
|
||||
}
|
||||
})
|
||||
|
||||
it('keeps reporting Pi-compatible agents once their agent_end handlers settle', async () => {
|
||||
// OMP is excluded on purpose: it settles on its own agent_end contract, covered in
|
||||
// agent-status-extension-omp-lifecycle.test.ts.
|
||||
it('keeps reporting idle-context agents once their agent_end handlers settle', async () => {
|
||||
vi.useFakeTimers()
|
||||
try {
|
||||
for (const kind of ['pi', 'omp', 'prime-agent'] as const) {
|
||||
for (const kind of ['pi', 'prime-agent'] as const) {
|
||||
const harness = createHarness({ kind })
|
||||
let idle = false
|
||||
const context = { isIdle: vi.fn(() => idle) }
|
||||
|
||||
@@ -0,0 +1,192 @@
|
||||
// Runs the generated status extension inside a VM sandbox with stubbed
|
||||
// fetch/fs/child_process, so specs assert what a real Pi/OMP process would POST.
|
||||
import { runInNewContext } from 'node:vm'
|
||||
// TypeScript 7 is a native CLI; transpile tests still need the legacy JavaScript API.
|
||||
import ts from 'typescript-api'
|
||||
import { vi } from 'vitest'
|
||||
|
||||
import { getPiAgentStatusExtensionSource } from './agent-status-extension-source'
|
||||
|
||||
export type HookContext = {
|
||||
isIdle?: () => boolean
|
||||
sessionManager?: {
|
||||
getSessionId?: () => unknown
|
||||
getSessionFile?: () => unknown
|
||||
}
|
||||
}
|
||||
|
||||
export type HookHandler = (event?: unknown, context?: HookContext) => Promise<void> | void
|
||||
|
||||
export type FakeCurlChild = {
|
||||
on: ReturnType<typeof vi.fn>
|
||||
stdin: {
|
||||
on: ReturnType<typeof vi.fn>
|
||||
end: ReturnType<typeof vi.fn>
|
||||
}
|
||||
}
|
||||
|
||||
export type Harness = {
|
||||
fetchMock: ReturnType<typeof vi.fn>
|
||||
spawnMock: ReturnType<typeof vi.fn>
|
||||
spawnedChildren: FakeCurlChild[]
|
||||
fsMock: {
|
||||
existsSync: ReturnType<typeof vi.fn>
|
||||
readFileSync: ReturnType<typeof vi.fn>
|
||||
statSync: ReturnType<typeof vi.fn>
|
||||
}
|
||||
handlers: Record<string, HookHandler>
|
||||
processEnv: Record<string, string | undefined>
|
||||
callHook: (name: string, event?: unknown, context?: HookContext) => Promise<void>
|
||||
// Re-invoke the extension factory in the same process (as Pi does on an
|
||||
// in-process extension reload), swapping in the freshly registered handlers.
|
||||
reload: () => void
|
||||
}
|
||||
|
||||
const BASE_ENV = {
|
||||
ORCA_PANE_KEY: 'pane-1',
|
||||
ORCA_AGENT_LAUNCH_TOKEN: 'launch-1',
|
||||
ORCA_TAB_ID: 'tab-1',
|
||||
ORCA_WORKTREE_ID: 'tree-1',
|
||||
ORCA_AGENT_HOOK_PORT: '4321',
|
||||
ORCA_AGENT_HOOK_TOKEN: 'token-1',
|
||||
ORCA_AGENT_HOOK_ENV: 'env-1',
|
||||
ORCA_AGENT_HOOK_VERSION: '1.2.3'
|
||||
} satisfies Record<string, string>
|
||||
|
||||
// Why: ownership keys on process.pid, so reload and child-process tests need
|
||||
// stable, distinct identities.
|
||||
export const SELF_PID = 4242
|
||||
|
||||
export function createHarness(args: {
|
||||
kind: 'pi' | 'omp' | 'prime-agent'
|
||||
env?: Record<string, string | undefined>
|
||||
pid?: number
|
||||
title?: string
|
||||
argv?: string[]
|
||||
existsSync?: (path: string) => boolean
|
||||
readFileSync?: (path: string, encoding: string) => string
|
||||
statSync?: (path: string) => { mtimeMs: number; size: number; ino: number }
|
||||
fetchImpl?: (...params: Parameters<typeof fetch>) => Promise<unknown>
|
||||
}): Harness {
|
||||
const fetchMock = vi.fn(
|
||||
args.fetchImpl ??
|
||||
(async () => ({
|
||||
ok: true
|
||||
}))
|
||||
)
|
||||
|
||||
const spawnedChildren: FakeCurlChild[] = []
|
||||
const spawnMock = vi.fn(() => {
|
||||
const child: FakeCurlChild = {
|
||||
on: vi.fn(),
|
||||
stdin: {
|
||||
on: vi.fn(),
|
||||
end: vi.fn()
|
||||
}
|
||||
}
|
||||
spawnedChildren.push(child)
|
||||
return child
|
||||
})
|
||||
|
||||
const fsMock = {
|
||||
existsSync: vi.fn(args.existsSync ?? (() => false)),
|
||||
statSync: vi.fn(
|
||||
args.statSync ??
|
||||
((path: string) => {
|
||||
throw Object.assign(new Error(`ENOENT: ${path}`), { code: 'ENOENT' })
|
||||
})
|
||||
),
|
||||
readFileSync: vi.fn(
|
||||
args.readFileSync ??
|
||||
((path: string) => {
|
||||
throw Object.assign(new Error(`ENOENT: ${path}`), { code: 'ENOENT' })
|
||||
})
|
||||
)
|
||||
}
|
||||
|
||||
const module = {
|
||||
exports: {} as { default?: (pi: { on: (name: string, handler: HookHandler) => void }) => void }
|
||||
}
|
||||
const requireMock = vi.fn((specifier: string) => {
|
||||
if (specifier === 'fs') {
|
||||
return fsMock
|
||||
}
|
||||
if (specifier === 'child_process') {
|
||||
return { spawn: spawnMock }
|
||||
}
|
||||
throw new Error(`unexpected require(${specifier})`)
|
||||
})
|
||||
|
||||
const processMock = {
|
||||
env: {
|
||||
...BASE_ENV,
|
||||
...(args.kind === 'prime-agent' ? { PRIME_AGENT_INTERNAL_DAEMON_WORKER: '1' } : {}),
|
||||
...args.env
|
||||
},
|
||||
pid: args.pid ?? SELF_PID,
|
||||
title: args.title ?? 'node',
|
||||
argv: args.argv ?? ['node', '/usr/bin/orca']
|
||||
}
|
||||
|
||||
const context = {
|
||||
module,
|
||||
exports: module.exports,
|
||||
require: requireMock,
|
||||
process: processMock,
|
||||
fetch: fetchMock,
|
||||
console: {
|
||||
warn: vi.fn(),
|
||||
error: vi.fn(),
|
||||
log: vi.fn()
|
||||
},
|
||||
Promise,
|
||||
Buffer,
|
||||
URL,
|
||||
AbortController,
|
||||
setTimeout,
|
||||
clearTimeout
|
||||
} as Record<string, unknown>
|
||||
context.globalThis = context
|
||||
|
||||
const source = getPiAgentStatusExtensionSource(args.kind)
|
||||
const output = ts.transpileModule(source, {
|
||||
compilerOptions: {
|
||||
module: ts.ModuleKind.CommonJS,
|
||||
target: ts.ScriptTarget.ES2020
|
||||
}
|
||||
}).outputText
|
||||
runInNewContext(output, context)
|
||||
|
||||
const register = module.exports.default
|
||||
if (!register) {
|
||||
throw new Error('expected default export from generated source')
|
||||
}
|
||||
|
||||
const handlers: Record<string, HookHandler> = {}
|
||||
const registerInto = (target: Record<string, HookHandler>): void => {
|
||||
register({
|
||||
on(name: string, handler: HookHandler) {
|
||||
target[name] = handler
|
||||
}
|
||||
})
|
||||
}
|
||||
registerInto(handlers)
|
||||
|
||||
return {
|
||||
fetchMock,
|
||||
spawnMock,
|
||||
spawnedChildren,
|
||||
fsMock,
|
||||
handlers,
|
||||
processEnv: processMock.env,
|
||||
callHook: async (name, event, hookContext) => {
|
||||
await handlers[name]?.(event, hookContext)
|
||||
},
|
||||
reload: () => {
|
||||
for (const key of Object.keys(handlers)) {
|
||||
delete handlers[key]
|
||||
}
|
||||
registerInto(handlers)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -30,6 +30,34 @@ export function getPiAgentStatusHandlerSourceLines(kind: PiAgentKind): string[]
|
||||
: []
|
||||
const ownerEnv = kind === 'prime-agent' ? 'ORCA_PRIME_AGENT_STATUS_OWNED' : 'ORCA_PI_STATUS_OWNED'
|
||||
|
||||
// Why: OMP suppresses its approval lifecycle unless an extension listens for it,
|
||||
// and it is the only signal that the run is parked on a permission prompt rather
|
||||
// than still working. Prime has no OMP runtime, so the handlers would be dead there.
|
||||
const approvalHandlers =
|
||||
kind === 'prime-agent'
|
||||
? []
|
||||
: [
|
||||
` pi.on('tool_approval_requested', (event${ctxParam}) => {`,
|
||||
...captureSessionMetadata,
|
||||
' if (!isOmpRuntime()) return',
|
||||
" post('tool_approval_requested', {",
|
||||
' tool_name: event.toolName,',
|
||||
' reason: event.reason,',
|
||||
' approval_mode: event.approvalMode,',
|
||||
' })',
|
||||
' })',
|
||||
'',
|
||||
` pi.on('tool_approval_resolved', (event${ctxParam}) => {`,
|
||||
...captureSessionMetadata,
|
||||
' if (!isOmpRuntime()) return',
|
||||
" post('tool_approval_resolved', {",
|
||||
' tool_name: event.toolName,',
|
||||
' approved: event.approved,',
|
||||
' })',
|
||||
' })',
|
||||
''
|
||||
]
|
||||
|
||||
return [
|
||||
'// Why: pi assistant messages carry content as an array of parts',
|
||||
"// ({ type: 'text', text } / tool_use / tool_result / reasoning). We only",
|
||||
@@ -102,6 +130,7 @@ export function getPiAgentStatusHandlerSourceLines(kind: PiAgentKind): string[]
|
||||
' })',
|
||||
' })',
|
||||
'',
|
||||
...approvalHandlers,
|
||||
" // Why: capture the assistant's final text on each completed message",
|
||||
' // so the dashboard preview reflects the most recent reply even before',
|
||||
' // agent_end fires. message_end is the right hook because pi guarantees',
|
||||
@@ -115,7 +144,9 @@ export function getPiAgentStatusHandlerSourceLines(kind: PiAgentKind): string[]
|
||||
' })',
|
||||
'',
|
||||
' // Why: modern Pi stays non-idle across retry/compaction/follow-up work,',
|
||||
' // while legacy Pi/OMP becomes idle after its final agent_end handlers.',
|
||||
' // while legacy Pi becomes idle after its final agent_end handlers. OMP has',
|
||||
' // neither signal — it never emits agent_settled and its ctx.isIdle can stay',
|
||||
' // false through completion — so it settles on its own agent_end contract below.',
|
||||
' const AGENT_END_IDLE_RECHECK_MS = 25',
|
||||
' const AGENT_END_IDLE_RECHECK_MAX_MS = 250',
|
||||
' let agentSettledSupported = false',
|
||||
@@ -167,8 +198,16 @@ export function getPiAgentStatusHandlerSourceLines(kind: PiAgentKind): string[]
|
||||
' postAgentEndOnce()',
|
||||
' })',
|
||||
'',
|
||||
" pi.on('agent_end', (_event, ctx) => {",
|
||||
" pi.on('agent_end', (event, ctx) => {",
|
||||
...captureSessionMetadata,
|
||||
' // Why: OMP marks a scheduled automatic continuation on the event itself;',
|
||||
' // any other value is terminal. Builds predating the flag settle as terminal',
|
||||
' // rather than sticking at "working".',
|
||||
' if (isOmpRuntime()) {',
|
||||
' clearPendingAgentEndCheck()',
|
||||
' if (event?.willContinue !== true) postAgentEndOnce()',
|
||||
' return',
|
||||
' }',
|
||||
' if (agentSettledSupported) return',
|
||||
" if (!ctx || typeof ctx.isIdle !== 'function') {",
|
||||
' postAgentEndOnce()',
|
||||
|
||||
@@ -234,6 +234,27 @@ describe('Pi-compatible title detection', () => {
|
||||
expect(resolveCompatibleAgentTypeForOwner('codex', 'omp')).toBe('codex')
|
||||
})
|
||||
|
||||
it('keeps a live OMP identity from being downgraded by a generic Pi owner', () => {
|
||||
const status = normalizeCompatibleAgentStatusEntryForOwner(
|
||||
{
|
||||
state: 'working',
|
||||
prompt: '',
|
||||
updatedAt: 1,
|
||||
stateStartedAt: 1,
|
||||
agentType: 'omp',
|
||||
paneKey: 'tab-1:leaf-1',
|
||||
terminalTitle: '\u280b OMP',
|
||||
stateHistory: []
|
||||
},
|
||||
'pi'
|
||||
)
|
||||
|
||||
expect(status.agentType).toBe('omp')
|
||||
expect(status.terminalTitle).toBe('\u280b OMP')
|
||||
expect(resolveCompatibleAgentTypeForOwner('omp', 'pi')).toBe('omp')
|
||||
expect(resolveCompatibleAgentTypeForOwner('pi', 'omp')).toBe('omp')
|
||||
})
|
||||
|
||||
it.each(['~/omp/working', 'omp-harness ready', '~/pi/working', 'pi-scratch ready'])(
|
||||
'does not classify path or hyphen false positive %s',
|
||||
(title) => {
|
||||
|
||||
@@ -801,7 +801,7 @@ describe('shared agent-hook-listener', () => {
|
||||
expect(tool?.payload.interactivePrompt).toBeUndefined()
|
||||
})
|
||||
|
||||
it('maps OMP ask to blocked without publishing a native prompt', () => {
|
||||
it('maps OMP ask to blocked and publishes its questions payload', () => {
|
||||
const tool = normalizeHookPayload(
|
||||
state,
|
||||
'omp',
|
||||
@@ -831,9 +831,62 @@ describe('shared agent-hook-listener', () => {
|
||||
agentType: 'omp',
|
||||
toolName: 'ask'
|
||||
})
|
||||
expect(tool?.payload.interactivePrompt).toBeUndefined()
|
||||
expect(tool?.payload.interactivePrompt).toBe(
|
||||
JSON.stringify({ questions: [{ question: 'Choose', options: ['x', 'y'] }] })
|
||||
)
|
||||
})
|
||||
|
||||
it('blocks an OMP pane on a tool approval request and clears it on resolution', () => {
|
||||
const requested = normalizeHookPayload(
|
||||
state,
|
||||
'omp',
|
||||
{
|
||||
paneKey: PANE_KEY,
|
||||
payload: {
|
||||
hook_event_name: 'tool_approval_requested',
|
||||
tool_name: 'bash',
|
||||
reason: 'tools.approval.bash: prompt',
|
||||
approval_mode: 'prompt'
|
||||
}
|
||||
},
|
||||
'production'
|
||||
)
|
||||
|
||||
expect(requested?.payload).toMatchObject({
|
||||
state: 'blocked',
|
||||
agentType: 'omp',
|
||||
toolName: 'bash',
|
||||
toolInput: 'tools.approval.bash: prompt'
|
||||
})
|
||||
|
||||
const resolved = normalizeHookPayload(
|
||||
state,
|
||||
'omp',
|
||||
{
|
||||
paneKey: PANE_KEY,
|
||||
payload: { hook_event_name: 'tool_approval_resolved', tool_name: 'bash', approved: true }
|
||||
},
|
||||
'production'
|
||||
)
|
||||
|
||||
expect(resolved?.payload).toMatchObject({ state: 'working', toolName: 'bash' })
|
||||
expect(resolved?.payload.toolInput).toBeUndefined()
|
||||
})
|
||||
|
||||
it.each(['tool_approval_requested', 'tool_approval_resolved'])(
|
||||
'ignores %s from Pi-compatible agents that do not emit it',
|
||||
(hookEventName) => {
|
||||
expect(
|
||||
normalizeHookPayload(
|
||||
state,
|
||||
'pi',
|
||||
{ paneKey: PANE_KEY, payload: { hook_event_name: hookEventName, tool_name: 'bash' } },
|
||||
'production'
|
||||
)
|
||||
).toBeNull()
|
||||
}
|
||||
)
|
||||
|
||||
it('captures Pi session ids on Pi-compatible status events', () => {
|
||||
const event = normalizeHookPayload(
|
||||
state,
|
||||
|
||||
@@ -902,6 +902,19 @@ function summarizeApprovalInput(toolInput: unknown): string {
|
||||
}
|
||||
}
|
||||
|
||||
/** The question tool's own input, verbatim, as the `{ questions }` envelope every client parses. */
|
||||
function serializeQuestionPrompt(toolInput: unknown): string | undefined {
|
||||
if (toolInput === undefined || toolInput === null) {
|
||||
return undefined
|
||||
}
|
||||
try {
|
||||
return JSON.stringify(toolInput)
|
||||
} catch {
|
||||
// Why: circular/unserializable input from a buggy agent — a missing live card beats throwing in the hook hot path.
|
||||
return undefined
|
||||
}
|
||||
}
|
||||
|
||||
/** Normalized JSON envelope for a pending prompt: AskUserQuestion → `{ questions }` (shape kept stable for back-compat); other tool on PermissionRequest → `{ approval }`; else undefined. */
|
||||
function deriveInteractivePrompt(
|
||||
toolName: string | undefined,
|
||||
@@ -918,12 +931,7 @@ function deriveInteractivePrompt(
|
||||
toolInput !== undefined &&
|
||||
toolInput !== null
|
||||
) {
|
||||
try {
|
||||
return JSON.stringify(toolInput)
|
||||
} catch {
|
||||
// Why: circular/unserializable input from a buggy agent — a missing live card beats throwing in the hook hot path.
|
||||
return undefined
|
||||
}
|
||||
return serializeQuestionPrompt(toolInput)
|
||||
}
|
||||
if (eventName === 'PermissionRequest' && typeof toolName === 'string' && toolName.length > 0) {
|
||||
try {
|
||||
@@ -2053,6 +2061,18 @@ function extractCopilotToolFields(
|
||||
return update
|
||||
}
|
||||
|
||||
/** The structured question tool: Pi names it `AskUserQuestion`, OMP names it `ask`, Prime has none.
|
||||
* One predicate so the blocked state and the published prompt payload can't disagree. */
|
||||
function isPiCompatibleAskTool(
|
||||
agentKind: 'pi' | 'omp' | 'prime-agent',
|
||||
toolName: string | undefined
|
||||
): boolean {
|
||||
if (agentKind === 'omp') {
|
||||
return toolName === 'ask'
|
||||
}
|
||||
return agentKind === 'pi' && isAskUserQuestionTool(toolName)
|
||||
}
|
||||
|
||||
function extractPiToolFields(
|
||||
eventName: unknown,
|
||||
hookPayload: Record<string, unknown>,
|
||||
@@ -2066,16 +2086,34 @@ function extractPiToolFields(
|
||||
const toolName = readString(hookPayload, 'tool_name')
|
||||
const rawToolInput = hookPayload.tool_input
|
||||
const toolInput = deriveToolInputPreview(toolName, rawToolInput)
|
||||
// Why: OMP shares this extractor; only derive interactivePrompt for Pi so OMP ask_user_question metadata stays unchanged.
|
||||
// Why: OMP's `ask` input is the same questions/options shape, so the live card can render
|
||||
// it directly instead of waiting for the transcript read to settle.
|
||||
const interactivePrompt =
|
||||
agentKind === 'pi' && (eventName === 'tool_call' || eventName === 'tool_execution_start')
|
||||
? deriveInteractivePrompt(toolName, rawToolInput, eventName)
|
||||
isPiCompatibleAskTool(agentKind, toolName) &&
|
||||
(eventName === 'tool_call' || eventName === 'tool_execution_start')
|
||||
? serializeQuestionPrompt(rawToolInput)
|
||||
: undefined
|
||||
return toolUpdate(
|
||||
{ toolName, toolInput, interactivePrompt },
|
||||
{ hasToolInputField: hasOwnField(hookPayload, 'tool_input') }
|
||||
)
|
||||
}
|
||||
if (
|
||||
agentKind === 'omp' &&
|
||||
(eventName === 'tool_approval_requested' || eventName === 'tool_approval_resolved')
|
||||
) {
|
||||
// Why: the approval is the pane's whole state right now, so name the awaited tool and show
|
||||
// the policy reason that triggered it; resolving clears both back to the running tool call.
|
||||
return toolUpdate(
|
||||
{
|
||||
toolName: readString(hookPayload, 'tool_name'),
|
||||
toolInput:
|
||||
eventName === 'tool_approval_requested' ? readString(hookPayload, 'reason') : undefined,
|
||||
interactivePrompt: undefined
|
||||
},
|
||||
{ hasToolInputField: true }
|
||||
)
|
||||
}
|
||||
if (eventName === 'message_end' && hookPayload.role === 'assistant') {
|
||||
const text = readString(hookPayload, 'text')
|
||||
if (text) {
|
||||
@@ -3942,22 +3980,27 @@ function normalizePiCompatibleEvent(
|
||||
// Why: gate on the event's own tool_name so a stale cached question can't re-enter blocked.
|
||||
const toolName = readString(hookPayload, 'tool_name')
|
||||
const isPiCompatibleAsk =
|
||||
((agentType === 'pi' && isAskUserQuestionTool(toolName)) ||
|
||||
(agentType === 'omp' && toolName === 'ask')) &&
|
||||
isPiCompatibleAskTool(agentType, toolName) &&
|
||||
(eventName === 'tool_call' || eventName === 'tool_execution_start')
|
||||
// Why: OMP parks on an approval selector without any further tool event, so the pane
|
||||
// reads as working until this arrives — and stays blocked until its resolution.
|
||||
const isOmpApprovalRequest = agentType === 'omp' && eventName === 'tool_approval_requested'
|
||||
const isOmpApprovalResolution = agentType === 'omp' && eventName === 'tool_approval_resolved'
|
||||
|
||||
const stateName = isPiCompatibleAsk
|
||||
? 'blocked'
|
||||
: eventName === 'before_agent_start' ||
|
||||
eventName === 'agent_start' ||
|
||||
eventName === 'tool_call' ||
|
||||
eventName === 'tool_execution_start' ||
|
||||
eventName === 'tool_execution_end' ||
|
||||
eventName === 'message_end'
|
||||
? 'working'
|
||||
: eventName === 'agent_end'
|
||||
? 'done'
|
||||
: null
|
||||
const stateName =
|
||||
isPiCompatibleAsk || isOmpApprovalRequest
|
||||
? 'blocked'
|
||||
: isOmpApprovalResolution ||
|
||||
eventName === 'before_agent_start' ||
|
||||
eventName === 'agent_start' ||
|
||||
eventName === 'tool_call' ||
|
||||
eventName === 'tool_execution_start' ||
|
||||
eventName === 'tool_execution_end' ||
|
||||
eventName === 'message_end'
|
||||
? 'working'
|
||||
: eventName === 'agent_end'
|
||||
? 'done'
|
||||
: null
|
||||
|
||||
if (!stateName) {
|
||||
return null
|
||||
|
||||
@@ -107,7 +107,9 @@ function hasIdleSuffix(title: string, sourceProfile: SyntheticAgentTitleProfile)
|
||||
|
||||
/**
|
||||
* Why: remote OMP surfaces may report Pi as the live status identity, while
|
||||
* launch ownership still identifies the user-selected agent.
|
||||
* launch ownership still identifies the user-selected agent. The owner wins,
|
||||
* except when the owner is the group's generic identity and the incoming frame
|
||||
* names a specific sibling — downgrading that makes the label oscillate.
|
||||
*/
|
||||
export function resolveCompatibleAgentTypeForOwner(
|
||||
incomingAgentType: AgentType | null | undefined,
|
||||
@@ -125,6 +127,9 @@ export function resolveCompatibleAgentTypeForOwner(
|
||||
) {
|
||||
return incomingAgentType
|
||||
}
|
||||
if (ownerProfile.titleIdentityFallback && !incomingProfile.titleIdentityFallback) {
|
||||
return incomingAgentType
|
||||
}
|
||||
return ownerAgentType as AgentType
|
||||
}
|
||||
|
||||
|
||||
@@ -5,6 +5,9 @@ export type SyntheticAgentTitleProfile = {
|
||||
permissionLabel: string
|
||||
idleLabel: string
|
||||
titleIdentityGroup?: string
|
||||
/** The group's generic member, whose frames every other member also emits. A specific
|
||||
* sibling identity outranks it so the pane label can't oscillate between the two. */
|
||||
titleIdentityFallback?: boolean
|
||||
synthesizeTerminalTitle?: boolean
|
||||
synthesizeWorkingTitle?: boolean
|
||||
}
|
||||
@@ -34,7 +37,8 @@ export const SYNTHETIC_AGENT_TITLE_PROFILES: Record<string, SyntheticAgentTitleP
|
||||
workingLabel: 'Pi',
|
||||
permissionLabel: 'Pi - action required',
|
||||
idleLabel: 'Pi ready',
|
||||
titleIdentityGroup: 'pi-compatible'
|
||||
titleIdentityGroup: 'pi-compatible',
|
||||
titleIdentityFallback: true
|
||||
},
|
||||
omp: {
|
||||
workingLabel: 'OMP',
|
||||
|
||||
Reference in New Issue
Block a user