mirror of
https://github.com/stablyai/orca.git
synced 2026-10-30 16:47:34 +00:00
fix(cli): reject ask waits above the documented 30-minute maximum (#32044)
* fix(cli): reject orchestration ask waits above the documented maximum * fix(cli): keep ask timeout help ASCII and trim redundant cases Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -28,6 +28,11 @@ ORCA orchestration ask --from <worker_handle> --question "<question>" --options
|
||||
ORCA orchestration ask --from <worker_handle> --resume <message_id> --timeout-ms 600000
|
||||
```
|
||||
|
||||
Each wait defaults to 600000 ms (10 minutes). `--timeout-ms` accepts a positive
|
||||
integer up to 1800000 ms (30 minutes); larger values are rejected before creating
|
||||
or resuming a question. For decisions that take longer, resume the same question
|
||||
after each timeout.
|
||||
|
||||
A timeout or disconnect leaves the original question pending. Resume its
|
||||
message ID; do not create a duplicate question.
|
||||
|
||||
|
||||
File diff suppressed because one or more lines are too long
@@ -550,8 +550,8 @@ describe('orchestration timeout flag validation', () => {
|
||||
expect(getTerminalHandleMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it.each([String(Number.MAX_SAFE_INTEGER)])(
|
||||
'clamps a safe ask timeout %s before adding transport headroom',
|
||||
it.each(['1', '1800000'])(
|
||||
'accepts an ask timeout %s within the supported range with transport headroom',
|
||||
async (rawTimeout) => {
|
||||
process.env.ORCA_TERMINAL_HANDLE = 'term_worker'
|
||||
callMock.mockResolvedValue({
|
||||
@@ -569,12 +569,35 @@ describe('orchestration timeout flag validation', () => {
|
||||
|
||||
expect(callMock).toHaveBeenCalledWith(
|
||||
'orchestration.ask',
|
||||
expect.objectContaining({ timeoutMs: 1_800_000 }),
|
||||
{ timeoutMs: 1_805_000 }
|
||||
expect.objectContaining({ timeoutMs: Number(rawTimeout) }),
|
||||
{ timeoutMs: Number(rawTimeout) + 5_000 }
|
||||
)
|
||||
}
|
||||
)
|
||||
|
||||
it.each(['1800001', String(Number.MAX_SAFE_INTEGER)])(
|
||||
'rejects an oversized ask timeout %s before resolving identity or creating a question',
|
||||
async (timeoutMs) => {
|
||||
process.env.ORCA_TERMINAL_HANDLE = 'term_worker'
|
||||
callMock.mockResolvedValue({
|
||||
result: { answer: 'yes', messageId: 'msg_1', threadId: 'thread_1', timedOut: false }
|
||||
})
|
||||
await expect(
|
||||
invokeAsk(
|
||||
new Map<string, string | boolean>([
|
||||
['question', 'Proceed?'],
|
||||
['timeout-ms', timeoutMs]
|
||||
])
|
||||
)
|
||||
).rejects.toMatchObject({
|
||||
code: 'invalid_argument',
|
||||
message: expect.stringMatching(/--timeout-ms.*1800000.*30 minutes/)
|
||||
})
|
||||
expect(callMock).not.toHaveBeenCalled()
|
||||
expect(getTerminalHandleMock).not.toHaveBeenCalled()
|
||||
}
|
||||
)
|
||||
|
||||
it('keeps an omitted ask timeout out of the payload while using default headroom', async () => {
|
||||
process.env.ORCA_TERMINAL_HANDLE = 'term_worker'
|
||||
callMock.mockResolvedValue({
|
||||
|
||||
@@ -6,6 +6,7 @@ import { RuntimeClientError } from '../../runtime-client'
|
||||
import { resolveOrchestrationCliExecutable } from '../../runtime/orchestration-recovery-command'
|
||||
import {
|
||||
clampOrchestrationAskTimeoutMs,
|
||||
ORCHESTRATION_ASK_MAX_TIMEOUT_MS,
|
||||
resolveOrchestrationAskClientTimeoutMs
|
||||
} from '../../../shared/orchestration-ask-timeout'
|
||||
import type { LegacyCompatibilityResult } from '../../../shared/orchestration-check-output'
|
||||
@@ -21,6 +22,12 @@ import { resolveOrchestrationTerminalHandle } from './terminal-identity'
|
||||
export const ORCHESTRATION_QUESTION_HANDLER: Record<string, CommandHandler> = {
|
||||
'orchestration ask': async ({ flags, client, cwd, json }) => {
|
||||
const parsedTimeoutMs = getOptionalPositiveIntegerValueFlag(flags, 'timeout-ms')
|
||||
if (parsedTimeoutMs !== undefined && parsedTimeoutMs > ORCHESTRATION_ASK_MAX_TIMEOUT_MS) {
|
||||
throw new RuntimeClientError(
|
||||
'invalid_argument',
|
||||
`--timeout-ms must be at most ${ORCHESTRATION_ASK_MAX_TIMEOUT_MS} (30 minutes) per wait. For longer waits, resume the same question after a timeout.`
|
||||
)
|
||||
}
|
||||
const timeoutMs = clampOrchestrationAskTimeoutMs(parsedTimeoutMs)
|
||||
const from = await resolveOrchestrationTerminalHandle(flags, cwd, client, 'from')
|
||||
const question = getOptionalStringFlag(flags, 'question')
|
||||
|
||||
@@ -54,6 +54,53 @@ describe('orca cli worktree awareness', () => {
|
||||
spawnMock
|
||||
})
|
||||
|
||||
it.each([
|
||||
{ questionFlag: '--question', questionValue: 'Proceed?', json: true },
|
||||
{ questionFlag: '--resume', questionValue: 'msg_pending', json: false }
|
||||
])('reports oversized ask waits before any request: %o', async (testCase) => {
|
||||
process.env.ORCA_TERMINAL_HANDLE = 'term_worker'
|
||||
callMock.mockResolvedValue({
|
||||
id: 'req_ask',
|
||||
ok: true,
|
||||
result: { answer: 'yes', messageId: 'msg_1', threadId: 'msg_1', timedOut: false },
|
||||
_meta: { runtimeId: 'runtime-1' }
|
||||
})
|
||||
const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {})
|
||||
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
|
||||
|
||||
await main(
|
||||
[
|
||||
'orchestration',
|
||||
'ask',
|
||||
testCase.questionFlag,
|
||||
testCase.questionValue,
|
||||
'--timeout-ms',
|
||||
'14400000',
|
||||
...(testCase.json ? ['--json'] : [])
|
||||
],
|
||||
'/tmp/repo'
|
||||
)
|
||||
|
||||
expect(process.exitCode).toBe(1)
|
||||
expect(callMock).not.toHaveBeenCalled()
|
||||
if (testCase.json) {
|
||||
expect(logSpy).toHaveBeenCalledTimes(1)
|
||||
expect(JSON.parse(logSpy.mock.calls[0][0])).toMatchObject({
|
||||
ok: false,
|
||||
error: {
|
||||
code: 'invalid_argument',
|
||||
message: expect.stringMatching(/--timeout-ms.*1800000.*30 minutes/)
|
||||
}
|
||||
})
|
||||
expect(errorSpy).not.toHaveBeenCalled()
|
||||
} else {
|
||||
expect(logSpy).not.toHaveBeenCalled()
|
||||
expect(errorSpy).toHaveBeenCalledWith(
|
||||
expect.stringMatching(/--timeout-ms.*1800000.*30 minutes/)
|
||||
)
|
||||
}
|
||||
})
|
||||
|
||||
it('formats group orchestration sends in text mode', async () => {
|
||||
process.env.ORCA_TERMINAL_HANDLE = 'term_sender'
|
||||
callMock.mockResolvedValueOnce({
|
||||
|
||||
@@ -232,7 +232,7 @@ export const ORCHESTRATION_COMMAND_SPECS: CommandSpec[] = [
|
||||
identityFlagRoles: { from: 'caller' },
|
||||
notes: [
|
||||
'From an active Dispatch, a new question defaults to its owning Run mailbox.',
|
||||
'Timeout leaves the question pending; resume with the original message ID.'
|
||||
'--timeout-ms accepts 1 to 1800000 ms per wait (default 600000 = 10 minutes, max 30 minutes); larger values are rejected. Timeout leaves the question pending; resume with the original message ID.'
|
||||
]
|
||||
},
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user