mirror of
https://github.com/stablyai/orca.git
synced 2026-10-17 00:03:08 +00:00
Validate ORCA_TERMINAL_HANDLE and fall back to active terminal if stale
Long-lived shells can retain a stale ORCA_TERMINAL_HANDLE environment variable after the runtime remints a pane handle. This can cause commands to bake obsolete terminal handles into coordinator preambles or tasks. - Check if the environment-provided handle is live via terminal.show before using it in dispatch, task-create, or run operations. - Fall back to resolving the active terminal/implicit sender if the environment handle is stale. - Map raw "no_active_terminal" errors to a helpful user-facing error message suggesting the use of the "--from" flag.
This commit is contained in:
@@ -12,6 +12,7 @@ vi.mock('../format', () => ({ printResult: vi.fn() }))
|
||||
vi.mock('../selectors', () => ({ getTerminalHandle: getTerminalHandleMock }))
|
||||
|
||||
import { ORCHESTRATION_HANDLERS } from './orchestration'
|
||||
import { RuntimeClientError } from '../runtime-client'
|
||||
|
||||
afterEach(() => {
|
||||
getTerminalHandleMock.mockReset()
|
||||
@@ -186,6 +187,237 @@ describe('orchestration send structured payload flags', () => {
|
||||
devMode: false
|
||||
})
|
||||
})
|
||||
|
||||
it('continues to use ORCA_TERMINAL_HANDLE as worker lifecycle sender authority', async () => {
|
||||
process.env.ORCA_TERMINAL_HANDLE = 'term_worker_env'
|
||||
|
||||
await invokeSend(
|
||||
new Map<string, string | boolean>([
|
||||
['to', 'term_coord'],
|
||||
['subject', 'done'],
|
||||
['type', 'worker_done']
|
||||
])
|
||||
)
|
||||
|
||||
expect(callMock).toHaveBeenCalledTimes(1)
|
||||
expect(callMock).toHaveBeenCalledWith('orchestration.send', {
|
||||
from: 'term_worker_env',
|
||||
to: 'term_coord',
|
||||
subject: 'done',
|
||||
body: undefined,
|
||||
type: 'worker_done',
|
||||
priority: undefined,
|
||||
threadId: undefined,
|
||||
payload: undefined,
|
||||
devMode: false
|
||||
})
|
||||
})
|
||||
|
||||
it('reports sender resolution failure instead of raw no_active_terminal', async () => {
|
||||
getTerminalHandleMock.mockRejectedValue(
|
||||
new RuntimeClientError('no_active_terminal', 'no_active_terminal')
|
||||
)
|
||||
|
||||
await expect(
|
||||
invokeSend(
|
||||
new Map<string, string | boolean>([
|
||||
['to', 'term_coord'],
|
||||
['subject', 'done'],
|
||||
['type', 'worker_done']
|
||||
])
|
||||
)
|
||||
).rejects.toMatchObject({
|
||||
code: 'no_active_sender_terminal',
|
||||
message: expect.stringContaining('Pass --from')
|
||||
})
|
||||
expect(callMock).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
describe('orchestration dispatch coordinator handle', () => {
|
||||
beforeEach(() => {
|
||||
callMock.mockReset()
|
||||
getTerminalHandleMock.mockReset()
|
||||
delete process.env.ORCA_TERMINAL_HANDLE
|
||||
})
|
||||
|
||||
const invokeDispatch = (flags: Map<string, string | boolean>) =>
|
||||
ORCHESTRATION_HANDLERS['orchestration dispatch']({
|
||||
flags,
|
||||
client: { call: callMock },
|
||||
cwd: '/tmp/repo',
|
||||
json: true
|
||||
} as never)
|
||||
|
||||
const invokeDispatchShow = (flags: Map<string, string | boolean>) =>
|
||||
ORCHESTRATION_HANDLERS['orchestration dispatch-show']({
|
||||
flags,
|
||||
client: { call: callMock },
|
||||
cwd: '/tmp/repo',
|
||||
json: true
|
||||
} as never)
|
||||
|
||||
it('falls back to active sender resolution when env handle is stale', async () => {
|
||||
process.env.ORCA_TERMINAL_HANDLE = 'term_stale_coord'
|
||||
callMock
|
||||
.mockRejectedValueOnce(
|
||||
new RuntimeClientError('terminal_handle_stale', 'terminal_handle_stale')
|
||||
)
|
||||
.mockResolvedValueOnce({
|
||||
result: { dispatch: { id: 'ctx_1', task_id: 'task_1', status: 'dispatched' } }
|
||||
})
|
||||
getTerminalHandleMock.mockResolvedValue('term_live_coord')
|
||||
|
||||
await invokeDispatch(
|
||||
new Map<string, string | boolean>([
|
||||
['task', 'task_1'],
|
||||
['to', 'term_worker'],
|
||||
['inject', true]
|
||||
])
|
||||
)
|
||||
|
||||
expect(callMock).toHaveBeenNthCalledWith(1, 'terminal.show', {
|
||||
terminal: 'term_stale_coord'
|
||||
})
|
||||
expect(getTerminalHandleMock).toHaveBeenCalled()
|
||||
expect(callMock).toHaveBeenNthCalledWith(2, 'orchestration.dispatch', {
|
||||
task: 'task_1',
|
||||
to: 'term_worker',
|
||||
from: 'term_live_coord',
|
||||
inject: true,
|
||||
dryRun: undefined,
|
||||
returnPreamble: undefined,
|
||||
devMode: false
|
||||
})
|
||||
})
|
||||
|
||||
it('uses a live coordinator handle for dispatch-show preamble previews', async () => {
|
||||
process.env.ORCA_TERMINAL_HANDLE = 'term_stale_coord'
|
||||
callMock
|
||||
.mockRejectedValueOnce(
|
||||
new RuntimeClientError('terminal_handle_stale', 'terminal_handle_stale')
|
||||
)
|
||||
.mockResolvedValueOnce({
|
||||
result: { dispatch: null, preamble: 'preamble' }
|
||||
})
|
||||
getTerminalHandleMock.mockResolvedValue('term_live_coord')
|
||||
|
||||
await invokeDispatchShow(
|
||||
new Map<string, string | boolean>([
|
||||
['task', 'task_1'],
|
||||
['preamble', true]
|
||||
])
|
||||
)
|
||||
|
||||
expect(callMock).toHaveBeenNthCalledWith(1, 'terminal.show', {
|
||||
terminal: 'term_stale_coord'
|
||||
})
|
||||
expect(callMock).toHaveBeenNthCalledWith(2, 'orchestration.dispatchShow', {
|
||||
task: 'task_1',
|
||||
preamble: true,
|
||||
from: 'term_live_coord',
|
||||
devMode: false
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('orchestration task-create caller handle', () => {
|
||||
beforeEach(() => {
|
||||
callMock.mockReset()
|
||||
getTerminalHandleMock.mockReset()
|
||||
delete process.env.ORCA_TERMINAL_HANDLE
|
||||
})
|
||||
|
||||
const invokeTaskCreate = (flags: Map<string, string | boolean>) =>
|
||||
ORCHESTRATION_HANDLERS['orchestration task-create']({
|
||||
flags,
|
||||
client: { call: callMock },
|
||||
cwd: '/tmp/repo',
|
||||
json: true
|
||||
} as never)
|
||||
|
||||
it('records a live env terminal handle as task creator', async () => {
|
||||
process.env.ORCA_TERMINAL_HANDLE = 'term_creator'
|
||||
callMock
|
||||
.mockResolvedValueOnce({ result: { terminal: { handle: 'term_creator' } } })
|
||||
.mockResolvedValueOnce({ result: { task: { id: 'task_1', status: 'ready' } } })
|
||||
|
||||
await invokeTaskCreate(new Map<string, string | boolean>([['spec', 'do work']]))
|
||||
|
||||
expect(callMock).toHaveBeenNthCalledWith(1, 'terminal.show', { terminal: 'term_creator' })
|
||||
expect(callMock).toHaveBeenNthCalledWith(2, 'orchestration.taskCreate', {
|
||||
spec: 'do work',
|
||||
taskTitle: undefined,
|
||||
displayName: undefined,
|
||||
deps: undefined,
|
||||
parent: undefined,
|
||||
callerTerminalHandle: 'term_creator'
|
||||
})
|
||||
})
|
||||
|
||||
it('does not persist a stale env terminal handle as task creator', async () => {
|
||||
process.env.ORCA_TERMINAL_HANDLE = 'term_stale'
|
||||
callMock
|
||||
.mockRejectedValueOnce(
|
||||
new RuntimeClientError('terminal_handle_stale', 'terminal_handle_stale')
|
||||
)
|
||||
.mockResolvedValueOnce({ result: { task: { id: 'task_1', status: 'ready' } } })
|
||||
getTerminalHandleMock.mockRejectedValue(
|
||||
new RuntimeClientError('no_active_terminal', 'no_active_terminal')
|
||||
)
|
||||
|
||||
await invokeTaskCreate(new Map<string, string | boolean>([['spec', 'do work']]))
|
||||
|
||||
expect(callMock).toHaveBeenNthCalledWith(1, 'terminal.show', { terminal: 'term_stale' })
|
||||
expect(callMock).toHaveBeenNthCalledWith(2, 'orchestration.taskCreate', {
|
||||
spec: 'do work',
|
||||
taskTitle: undefined,
|
||||
displayName: undefined,
|
||||
deps: undefined,
|
||||
parent: undefined,
|
||||
callerTerminalHandle: undefined
|
||||
})
|
||||
})
|
||||
|
||||
it('propagates unexpected active fallback failures after a stale env handle', async () => {
|
||||
process.env.ORCA_TERMINAL_HANDLE = 'term_stale'
|
||||
callMock.mockRejectedValueOnce(
|
||||
new RuntimeClientError('terminal_handle_stale', 'terminal_handle_stale')
|
||||
)
|
||||
getTerminalHandleMock.mockRejectedValue(
|
||||
new RuntimeClientError('runtime_unavailable', 'runtime_unavailable')
|
||||
)
|
||||
|
||||
await expect(
|
||||
invokeTaskCreate(new Map<string, string | boolean>([['spec', 'do work']]))
|
||||
).rejects.toMatchObject({
|
||||
code: 'runtime_unavailable'
|
||||
})
|
||||
|
||||
expect(callMock).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('falls back to the active terminal when a stale env handle has a live replacement', async () => {
|
||||
process.env.ORCA_TERMINAL_HANDLE = 'term_stale'
|
||||
callMock
|
||||
.mockRejectedValueOnce(
|
||||
new RuntimeClientError('terminal_handle_stale', 'terminal_handle_stale')
|
||||
)
|
||||
.mockResolvedValueOnce({ result: { task: { id: 'task_1', status: 'ready' } } })
|
||||
getTerminalHandleMock.mockResolvedValue('term_live')
|
||||
|
||||
await invokeTaskCreate(new Map<string, string | boolean>([['spec', 'do work']]))
|
||||
|
||||
expect(callMock).toHaveBeenNthCalledWith(1, 'terminal.show', { terminal: 'term_stale' })
|
||||
expect(callMock).toHaveBeenNthCalledWith(2, 'orchestration.taskCreate', {
|
||||
spec: 'do work',
|
||||
taskTitle: undefined,
|
||||
displayName: undefined,
|
||||
deps: undefined,
|
||||
parent: undefined,
|
||||
callerTerminalHandle: 'term_live'
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('orchestration timeout flag validation', () => {
|
||||
|
||||
@@ -127,7 +127,8 @@ async function resolveOrchestrationTerminalHandle(
|
||||
flags: Map<string, string | boolean>,
|
||||
cwd: string,
|
||||
client: Parameters<CommandHandler>[0]['client'],
|
||||
flagName: 'from' | 'terminal'
|
||||
flagName: 'from' | 'terminal',
|
||||
options: { validateEnvHandle?: boolean } = {}
|
||||
): Promise<string> {
|
||||
const explicit = getOptionalStringFlag(flags, flagName)
|
||||
if (explicit) {
|
||||
@@ -135,11 +136,107 @@ async function resolveOrchestrationTerminalHandle(
|
||||
}
|
||||
const envHandle = process.env.ORCA_TERMINAL_HANDLE
|
||||
if (envHandle && envHandle.length > 0) {
|
||||
if (flagName === 'from' && options.validateEnvHandle) {
|
||||
// Why: long-lived shells can retain an ORCA_TERMINAL_HANDLE after the
|
||||
// runtime remints the pane handle; do not bake that stale id into
|
||||
// coordinator preambles.
|
||||
const live = await isLiveTerminalHandle(envHandle, client)
|
||||
if (!live) {
|
||||
return await resolveImplicitOrchestrationSender(flags, cwd, client)
|
||||
}
|
||||
}
|
||||
return envHandle
|
||||
}
|
||||
if (flagName === 'from') {
|
||||
return await resolveImplicitOrchestrationSender(flags, cwd, client)
|
||||
}
|
||||
return await getTerminalHandle(flags, cwd, client)
|
||||
}
|
||||
|
||||
async function resolveTaskCreatorTerminalHandle(
|
||||
flags: Map<string, string | boolean>,
|
||||
cwd: string,
|
||||
client: Parameters<CommandHandler>[0]['client']
|
||||
): Promise<string | undefined> {
|
||||
const envHandle = process.env.ORCA_TERMINAL_HANDLE
|
||||
if (!envHandle || envHandle.length === 0) {
|
||||
return undefined
|
||||
}
|
||||
const live = await isLiveTerminalHandle(envHandle, client)
|
||||
if (live) {
|
||||
return envHandle
|
||||
}
|
||||
try {
|
||||
return await resolveImplicitOrchestrationSender(flags, cwd, client)
|
||||
} catch (err) {
|
||||
if (getClientErrorCode(err) !== 'no_active_sender_terminal') {
|
||||
throw err
|
||||
}
|
||||
return undefined
|
||||
}
|
||||
}
|
||||
|
||||
async function isLiveTerminalHandle(
|
||||
handle: string,
|
||||
client: Parameters<CommandHandler>[0]['client']
|
||||
): Promise<boolean> {
|
||||
try {
|
||||
await client.call('terminal.show', { terminal: handle })
|
||||
return true
|
||||
} catch (err) {
|
||||
if (isStaleTerminalIdentityError(err)) {
|
||||
return false
|
||||
}
|
||||
throw err
|
||||
}
|
||||
}
|
||||
|
||||
function getClientErrorCode(err: unknown): string | undefined {
|
||||
if (!err || typeof err !== 'object') {
|
||||
return undefined
|
||||
}
|
||||
const code = (err as { code?: unknown }).code
|
||||
return typeof code === 'string' ? code : undefined
|
||||
}
|
||||
|
||||
function isStaleTerminalIdentityError(err: unknown): boolean {
|
||||
const code = getClientErrorCode(err)
|
||||
return code === 'terminal_handle_stale' || code === 'terminal_gone'
|
||||
}
|
||||
|
||||
function isNoActiveTerminalError(err: unknown): boolean {
|
||||
return getClientErrorCode(err) === 'no_active_terminal'
|
||||
}
|
||||
|
||||
async function resolveCoordinatorTerminalHandle(
|
||||
flags: Map<string, string | boolean>,
|
||||
cwd: string,
|
||||
client: Parameters<CommandHandler>[0]['client']
|
||||
): Promise<string> {
|
||||
return await resolveOrchestrationTerminalHandle(flags, cwd, client, 'from', {
|
||||
validateEnvHandle: true
|
||||
})
|
||||
}
|
||||
|
||||
async function resolveImplicitOrchestrationSender(
|
||||
flags: Map<string, string | boolean>,
|
||||
cwd: string,
|
||||
client: Parameters<CommandHandler>[0]['client']
|
||||
): Promise<string> {
|
||||
try {
|
||||
return await getTerminalHandle(flags, cwd, client)
|
||||
} catch (err) {
|
||||
if (!isNoActiveTerminalError(err)) {
|
||||
throw err
|
||||
}
|
||||
throw new RuntimeClientError(
|
||||
'no_active_sender_terminal',
|
||||
'Could not determine the sender terminal for this orchestration command. ' +
|
||||
'Pass --from <terminal-handle> or run the command inside a live Orca terminal with ORCA_TERMINAL_HANDLE set.'
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
function isDevCliInvocation(): boolean {
|
||||
return process.env.ORCA_USER_DATA_PATH?.includes('orca-dev') ?? false
|
||||
}
|
||||
@@ -288,12 +385,8 @@ export const ORCHESTRATION_HANDLERS: Record<string, CommandHandler> = {
|
||||
})
|
||||
},
|
||||
|
||||
'orchestration task-create': async ({ flags, client, json }) => {
|
||||
const callerTerminalHandle =
|
||||
typeof process.env.ORCA_TERMINAL_HANDLE === 'string' &&
|
||||
process.env.ORCA_TERMINAL_HANDLE.length > 0
|
||||
? process.env.ORCA_TERMINAL_HANDLE
|
||||
: undefined
|
||||
'orchestration task-create': async ({ flags, client, cwd, json }) => {
|
||||
const callerTerminalHandle = await resolveTaskCreatorTerminalHandle(flags, cwd, client)
|
||||
const result = await client.call<{ task: { id: string; status: string } }>(
|
||||
'orchestration.taskCreate',
|
||||
{
|
||||
@@ -361,7 +454,7 @@ export const ORCHESTRATION_HANDLERS: Record<string, CommandHandler> = {
|
||||
},
|
||||
|
||||
'orchestration dispatch': async ({ flags, client, cwd, json }) => {
|
||||
const from = await resolveOrchestrationTerminalHandle(flags, cwd, client, 'from')
|
||||
const from = await resolveCoordinatorTerminalHandle(flags, cwd, client)
|
||||
const dryRun = flags.has('dry-run') ? true : undefined
|
||||
const returnPreamble = flags.has('return-preamble') ? true : undefined
|
||||
// Why: --to is only required for non-dry-run; the RPC handler re-enforces.
|
||||
@@ -437,7 +530,7 @@ export const ORCHESTRATION_HANDLERS: Record<string, CommandHandler> = {
|
||||
// Why: resolve --from when previewing so the preamble embeds a real
|
||||
// coordinator handle, matching what an actual dispatch would produce.
|
||||
const from = showPreamble
|
||||
? await resolveOrchestrationTerminalHandle(flags, cwd, client, 'from')
|
||||
? await resolveCoordinatorTerminalHandle(flags, cwd, client)
|
||||
: undefined
|
||||
const result = await client.call<{
|
||||
dispatch: { id: string; task_id: string; status: string } | null
|
||||
@@ -460,7 +553,7 @@ export const ORCHESTRATION_HANDLERS: Record<string, CommandHandler> = {
|
||||
},
|
||||
|
||||
'orchestration run': async ({ flags, client, cwd, json }) => {
|
||||
const from = await resolveOrchestrationTerminalHandle(flags, cwd, client, 'from')
|
||||
const from = await resolveCoordinatorTerminalHandle(flags, cwd, client)
|
||||
const result = await client.call<{
|
||||
runId: string
|
||||
status: string
|
||||
|
||||
Reference in New Issue
Block a user