[nwparker]

ELI5

orca orchestration ask --timeout-ms used to accept any number but quietly cap the wait at 30 minutes. That meant a script asking for 2 hours really waited only 30 minutes. Now the CLI stops right away with a clear error when the value is over 1,800,000 ms (30 minutes). It tells you to resume the same question after a timeout. Values from 1 to 1800000 behave exactly as before.

What changed

• src/cli/handlers/orchestration/question-handler.ts: --timeout-ms values above ORCHESTRATION_ASK_MAX_TIMEOUT_MS (1800000) are rejected with invalid_argument (exit 1). The check runs before identity resolution or any RPC, so nothing reaches the runtime and no question is created.
• src/cli/specs/orchestration.ts: the help note documents the range (1 to 1800000; default 600000).
• The orchestration skill's worker-contract.md gets a guide paragraph, and bundled-skill-guides.ts is regenerated.
• Boundary tests (1, 1800000, 1800001, MAX_SAFE_INTEGER) in handler and main() form, with JSON and text output.

Compatibility

• Server-side clamping is unchanged. Older CLIs and the wire format are unaffected.
• New CLI to older server: values at or below the cap pass through unchanged.

Verification (rebased onto main 28c7268, which includes #32168)

• pnpm test src/cli/handlers/orchestration.test.ts src/cli/index-orchestration.test.ts: 60/60 pass. pnpm test src/cli src/shared/orchestration: 192 files, 2197 pass, 9 skipped. pnpm tc passes after the rebase.
• oxlint and oxfmt are clean on the changed files. verify:bundled-skill-guides --check passes.
• I built the CLI (pnpm run build:cli) and ran it directly:
  • Local, --timeout-ms 1800001 and 9007199254740991: invalid_argument, runtimeId: null (no RPC was made), exit 1. Text mode prints the same message.
  • Local, --timeout-ms 1800000: reaches the runtime, which returns dispatch_inactive as expected because there is no Dispatch.
  • Paired Windows servers --environment awin (1.4.237) and --environment win-lowspec (1.4.205, older): 1800000 reaches the server (dispatch_inactive), and 1800001 is rejected locally (invalid_argument, runtimeId: null). Mixed-version behavior is as designed.
  • orca orchestration ask --help shows the new note.
• A repo grep for --timeout-ms usages in skill guides, docs and resources found a maximum of 900000, which is under the cap.

CI note

The earlier static analysis and typecheck failure came from main (max-lines in headless-terminal-tab-close.ts and a typecheck error in renderer-owned-tab-properties.test.ts). #32168 fixed both. This branch is now rebased on top of that fix, so the gated test, e2e and package jobs run again.

Remaining tradeoffs

• Intended behavior change: scripts that pass more than 30 minutes now fail fast with exit 1 instead of being silently capped. No in-repo callers do this.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: cead2728-eff7-4a23-975c-c7080f5f6376

📥 Commits

Reviewing files that changed from the base of the PR and between 007de80 and d55896f96bc601d4d4b967ebd5a3012c40d92706.

📒 Files selected for processing: 1

• src/cli/bundled-skill-guides.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.

📝 Walkthrough

The ask command now documents a default timeout of 600,000 ms and a maximum of 1,800,000 ms. The handler rejects larger values before making a request. Tests cover accepted timeout values, oversized values, and CLI output in JSON and text modes. The worker contract says to resume the same question after each timeout.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to d5589

No actionable merge-blocking risk was identified in this change.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Description check		The PR description follows the repository template closely. It covers ELI5, What changed, Why (via the Compatibility section and the ELI5 rationale), Verification, CI note, and remaining tradeoffs. It…	Add a Linked Issue entry in the form 'Fixes #30372' (or the correct issue number), because the template requires one for outside contributors. Add a Visual Proof entry. For this CLI-only change, write N/A and state that no UI or visual chan…

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: rejecting orchestration ask waits above the documented 30-minute maximum.
Linked Issues check		Issue #30372 is active and directly linked. question-handler.ts rejects --timeout-ms values above ORCHESTRATION_ASK_MAX_TIMEOUT_MS before terminal resolution or a runtime call. The guard applies…
Out of Scope Changes check		The production change implements the validation requested by issue #30372. The help text and worker-contract source and bundle implement the documentation request. The tests verify the new validation …
Docstring Coverage		No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 5…

Full details: Linked Issues check

Explanation

Issue #30372 is active and directly linked. question-handler.ts rejects --timeout-ms values above ORCHESTRATION_ASK_MAX_TIMEOUT_MS before terminal resolution or a runtime call. The guard applies to both --question and --resume. ask --help, worker-contract.md, and the bundled guide state the 10-minute default, 30-minute maximum, rejection behavior, and same-question resume behavior. Tests cover both request modes, the error, and the absence of a runtime call.

Full details: Out of Scope Changes check

Explanation

The production change implements the validation requested by issue #30372. The help text and worker-contract source and bundle implement the documentation request. The tests verify the new validation and preserve valid timeout behavior. No unrelated change is shown.

Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 5 files.

Full details: Description check

Explanation

The PR description follows the repository template closely. It covers ELI5, What changed, Why (via the Compatibility section and the ELI5 rationale), Verification, CI note, and remaining tradeoffs. It lists the files changed and the test evidence, and it states the behavior change for callers that pass more than 30 minutes. The Linked Issue section is missing: the template's Fixes # line is absent, although the PR objectives reference issue #30372. The Visual Proof section is also absent; the change is CLI-only, so it should say N/A with a reason. The Testing checklist and AI Disclosure section are not present. The Agent skill upstream boundary checkbox and the Checklist are not present either. The Testing section is substantive, but the template's manual-testing checkbox is not ticked.

Resolution

Add a Linked Issue entry in the form 'Fixes #30372' (or the correct issue number), because the template requires one for outside contributors. Add a Visual Proof entry. For this CLI-only change, write N/A and state that no UI or visual change exists; include the CLI help output as text evidence if desired. Add the Testing checklist with the boxes marked, and note the platforms tested (Local, Windows paired servers; no macOS, Linux, or SSH tests reported). Add the Agent skill upstream boundary checkbox, and state whether the change copies upstream skill-installer material. Add the AI Disclosure section if applicable, or leave it blank if the author is a StablyAI team member. Complete the Checklist section. Confirm that the Review and Notes sections were considered, including the security, cross-platform, SSH, and backwards-compatibility checks named in the template.

✨ Finishing Touches: 💡 1

🛠️ Fix failing CI checks: 💡

• Commit to this branch
• Create a new PR

📝 Generate docstrings

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}