fix(orchestration): stop the sender-terminal refusal recommending another pane's handle (#21097)

* fix(orchestration): stop the sender-terminal refusal recommending another pane's handle

The structured-session guard told callers to pass `--from <terminal-handle>`, but the
explicit-flag branch returns before that guard runs — so following the advice succeeds,
against a handle that necessarily belongs to a different pane, and the next `check`
consumes that pane's unread mail.

Both refusals now say what is actually true: no handle names a structured chat session,
and a caller that does have one should pass its own.

Also pins ORCA_STRUCTURED_SESSION in the gate CLI test, which until now decided which
refusal it exercised from ambient environment.

* fix(orchestration): route the lifecycle-send refusal to the structured message

`orchestration send --type worker_done|heartbeat` refuses in the send handler before
`resolveOrchestrationTerminalHandle` runs, so the structured guard never saw the case a
structured session hits most: the canonical worker lifecycle report. That caller was still
told to pass `--from` with "your own terminal's handle" — which it does not have, so any
handle it picked would belong to another pane.

`throwNoActiveSenderTerminal` now derives which refusal fits instead of each call site
deciding: marker set AND no handle means no identity exists, so the structured refusal
applies. A stale `ORCA_TERMINAL_HANDLE` is deliberately excluded — that caller does have an
identity, it just went stale, and keeps the advice to re-run under a live one.

Also corrects the guidance itself (`--agent` is a `worktree create` flag; `terminal create`
has no such flag), aligns the SSH fallback wording with its local twin, and pins
ORCA_STRUCTURED_SESSION in the send tests, which until now decided which refusal they
exercised from ambient environment.
This commit is contained in:
Brennan Benson
2026-09-18 08:15:12 -07:00
committed by GitHub
parent 66e0847398
commit a85e580e51
4 changed files with 66 additions and 8 deletions
@@ -34,6 +34,9 @@ import { okFixture, queueFixtures } from '../test-fixtures'
const originalTerminalHandle = process.env.ORCA_TERMINAL_HANDLE
const originalPaneKey = process.env.ORCA_PANE_KEY
// Why: a structured-session marker inherited from the runner diverts these cases to the
// structured refusal, so which branch they exercise would depend on who ran them.
const originalStructuredSession = process.env.ORCA_STRUCTURED_SESSION
const restoreEnv = (name: string, value: string | undefined): void => {
if (value === undefined) {
@@ -54,6 +57,7 @@ describe('orchestration gate commands carry caller identity', () => {
errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
delete process.env.ORCA_TERMINAL_HANDLE
delete process.env.ORCA_PANE_KEY
delete process.env.ORCA_STRUCTURED_SESSION
process.exitCode = 0
})
@@ -62,6 +66,7 @@ describe('orchestration gate commands carry caller identity', () => {
errorSpy.mockRestore()
restoreEnv('ORCA_TERMINAL_HANDLE', originalTerminalHandle)
restoreEnv('ORCA_PANE_KEY', originalPaneKey)
restoreEnv('ORCA_STRUCTURED_SESSION', originalStructuredSession)
process.exitCode = 0
})
@@ -192,7 +197,7 @@ describe('orchestration gate commands carry caller identity', () => {
expect(process.exitCode).toBe(1)
const stderr = errorSpy.mock.calls.map((call) => String(call[0])).join('\n')
expect(stderr).toContain('Pass --from <terminal-handle>')
expect(stderr).toContain("Pass --from with your own terminal's handle")
expect(callMock).not.toHaveBeenCalledWith('orchestration.gateCreate', expect.anything())
})
+33
View File
@@ -4,6 +4,9 @@ const callMock = vi.fn()
const getTerminalHandleMock = vi.hoisted(() => vi.fn())
const originalTerminalHandle = process.env.ORCA_TERMINAL_HANDLE
const originalPaneKey = process.env.ORCA_PANE_KEY
// Why: a structured-session marker inherited from the runner diverts these cases to the
// structured refusal, so which branch they exercise would depend on who ran them.
const originalStructuredSession = process.env.ORCA_STRUCTURED_SESSION
function lifecycleGroupRecipientError(type: 'worker_done' | 'heartbeat'): string {
return `${type} messages belong to one exact Dispatch and cannot target a group address.`
}
@@ -28,6 +31,11 @@ afterEach(() => {
} else {
process.env.ORCA_PANE_KEY = originalPaneKey
}
if (originalStructuredSession === undefined) {
delete process.env.ORCA_STRUCTURED_SESSION
} else {
process.env.ORCA_STRUCTURED_SESSION = originalStructuredSession
}
})
describe('orchestration send structured payload flags', () => {
@@ -36,6 +44,7 @@ describe('orchestration send structured payload flags', () => {
getTerminalHandleMock.mockReset()
delete process.env.ORCA_TERMINAL_HANDLE
delete process.env.ORCA_PANE_KEY
delete process.env.ORCA_STRUCTURED_SESSION
})
const invokeSend = (flags: Map<string, string | boolean>) =>
@@ -292,6 +301,29 @@ describe('orchestration send structured payload flags', () => {
expect(callMock).not.toHaveBeenCalled()
})
it('refuses a structured session without naming a handle it could pass', async () => {
process.env.ORCA_STRUCTURED_SESSION = '1'
getTerminalHandleMock.mockResolvedValue('term_sibling_pane')
// The refusal must not recommend --from: the explicit-flag branch returns before this guard,
// so the advice would succeed against a handle that necessarily belongs to another pane.
await expect(
invokeSend(
new Map<string, string | boolean>([
['to', 'term_coord'],
['subject', 'done'],
['type', 'worker_done'],
['outcome', 'succeeded']
])
)
).rejects.toMatchObject({
code: 'no_active_sender_terminal',
message: expect.not.stringContaining('Pass --from')
})
expect(getTerminalHandleMock).not.toHaveBeenCalled()
expect(callMock).not.toHaveBeenCalled()
})
it.each(['worker_done', 'heartbeat'] as const)(
'does not resolve an identity-less %s sender from the active terminal',
async (type) => {
@@ -327,6 +359,7 @@ describe('orchestration timeout flag validation', () => {
callMock.mockReset()
delete process.env.ORCA_TERMINAL_HANDLE
delete process.env.ORCA_PANE_KEY
delete process.env.ORCA_STRUCTURED_SESSION
})
const invokeCheck = (flags: Map<string, string | boolean>) =>
@@ -36,11 +36,7 @@ export async function resolveOrchestrationTerminalHandle(
// rightful worker never saw its mail. Refusing is the only honest answer: this child genuinely
// cannot infer its own identity.
if (isStructuredSessionWithoutIdentity()) {
throw new RuntimeClientError(
'no_active_sender_terminal',
`This chat session has no orchestration identity of its own, so --${flagName} cannot be inferred. ` +
`Pass --${flagName} <terminal-handle> explicitly; guessing would act on another pane's mailbox.`
)
throw structuredSessionRefusal(flagName)
}
if (flagName === 'from') {
return await resolveImplicitOrchestrationSender(flags, cwd, client)
@@ -188,10 +184,33 @@ async function resolveImplicitOrchestrationSender(
}
}
/**
* Why no flag is suggested: every caller reaches a refusal only after the explicit-flag branch has
* already returned, so `--from` advice would succeed — against a handle that necessarily belongs to
* another pane, whose unread mail the next `check` consumes.
*/
function structuredSessionRefusal(flagName: 'from' | 'terminal'): RuntimeClientError {
return new RuntimeClientError(
'no_active_sender_terminal',
`This chat session has no orchestration identity of its own, so --${flagName} cannot be inferred, ` +
`and no terminal handle names it — every live handle belongs to a different pane, and passing one ` +
`would consume that pane's mailbox. Drive a worker directly instead: create a worktree with ` +
`--agent to launch one in its first terminal, then use terminal send and terminal read.`
)
}
export function throwNoActiveSenderTerminal(): never {
// Lifecycle sends refuse here before the structured guard above ever runs, so this is the only
// place left that would tell an identity-less session to pass a handle it does not have. A stale
// ORCA_TERMINAL_HANDLE is a different case — that caller HAS an identity, so it keeps the advice
// to re-run under a live one.
if (isStructuredSessionWithoutIdentity() && !process.env.ORCA_TERMINAL_HANDLE) {
throw structuredSessionRefusal('from')
}
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.'
"Pass --from with your own terminal's handle — another pane's handle would act on its mailbox — " +
'or run the command inside a live Orca terminal with ORCA_TERMINAL_HANDLE set.'
)
}
@@ -27,7 +27,8 @@ export function resolveRemoteOrchestrationSender(
throw new RemoteCliArgumentError(
'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.'
"Pass --from with your own terminal's handle — another pane's handle would act on its mailbox — " +
'or run the command inside a live Orca terminal with ORCA_TERMINAL_HANDLE set.'
)
}
return explicit ?? envHandle ?? 'unknown'