fix(orchestration): settle agent TUI before worker-start inject - #13571
fix(orchestration): settle agent TUI before worker-start inject#13571bbingz wants to merge 2 commits into
Conversation
worker-start already waited for tui-idle, but Claude can report idle during MCP boot before the composer accepts submit. Re-confirm idle after a short settle for Orca-created agents, and scale the paste→Enter delay with payload size so multi-KB preambles are not left unsubmitted. Fixes stablyai#13488
📝 WalkthroughWalkthroughThe runtime now calculates bracketed-paste submission delays from the platform and UTF-8 payload size, with a 5-second maximum. Worker orchestration now uses 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6db61997-37ef-4598-90dd-5aaba7eba433
📒 Files selected for processing (6)
src/main/runtime/orca-runtime.tssrc/main/runtime/rpc/methods/orchestration-worker-agent-readiness.test.tssrc/main/runtime/rpc/methods/orchestration-worker-agent-readiness.tssrc/main/runtime/rpc/methods/orchestration-workers.tssrc/shared/agent-prompt-injection.test.tssrc/shared/agent-prompt-injection.ts
stablyai#13488 preambles were ~8–10KB; bytes/20 still floored at 500ms on macOS. Use bytes/5 so those pastes wait ~1.6–2s before Enter, and fail closed when settle exhausts the caller readiness budget. Addresses review on stablyai#13571
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1211524d-c4d9-49fb-95ce-57d2286f8143
📒 Files selected for processing (4)
src/main/runtime/rpc/methods/orchestration-worker-agent-readiness.test.tssrc/main/runtime/rpc/methods/orchestration-worker-agent-readiness.tssrc/shared/agent-prompt-injection.test.tssrc/shared/agent-prompt-injection.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/runtime/rpc/methods/orchestration-worker-agent-readiness.ts
- src/shared/agent-prompt-injection.ts
- src/shared/agent-prompt-injection.test.ts
| vi.useFakeTimers() | ||
| vi.setSystemTime(0) | ||
|
|
||
| await expect( | ||
| waitForWorkerAgentTuiReady({ | ||
| runtime: { waitForTerminal }, | ||
| terminalHandle: 'term_worker', | ||
| timeoutMs: 1_000, | ||
| externalTerminal: false | ||
| }) | ||
| ).rejects.toThrow('timeout') | ||
|
|
||
| expect(waitForTerminal).toHaveBeenCalledTimes(1) | ||
| vi.useRealTimers() | ||
| }) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
rg -n "setTimeout|WORKER_AGENT_TUI_IDLE_SETTLE_MS|useFakeTimers|setSystemTime|runAllTimersAsync|advanceTimersToNextTimerAsync" src/main/runtime/rpc/methods/orchestration-worker-agent-readiness.ts src/main/runtime/rpc/methods/orchestration-worker-agent-readiness.test.tsRepository: stablyai/orca
Length of output: 894
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file sizes =="
wc -l src/main/runtime/rpc/methods/orchestration-worker-agent-readiness.ts src/main/runtime/rpc/methods/orchestration-worker-agent-readiness.test.ts
echo
echo "== implementation outline and relevant lines =="
ast-grep outline src/main/runtime/rpc/methods/orchestration-worker-agent-readiness.ts --view expanded || true
sed -n '1,130p' src/main/runtime/rpc/methods/orchestration-worker-agent-readiness.ts
echo
echo "== test relevant lines =="
sed -n '1,140p' src/main/runtime/rpc/methods/orchestration-worker-agent-readiness.test.ts
echo
echo "== deterministic timer behavior probe for fake timers =="
node - <<'JS'
let timers = []
const fakeSetTimeout = (cb, delay) => { timers.push({ cb, delay: Number(delay) }) }
setTimeout = fakeSetTimeout
fakeSetTimeout(() => {} , 1000)
const afterSetSystemTime = timers.length
const resolvedWithoutAdvancing = timers.length === afterSetSystemTime
timers.forEach(t => t.cb())
const resolvedAfterAdvancing = afterSetSystemTime > 0 && timers.length === afterSetSystemTime
console.log(JSON.stringify({ afterSetSystemTime, resolvedWithoutAdvancing, resolvedAfterAdvancing }))
JSRepository: stablyai/orca
Length of output: 5544
🌐 Web query:
Vitest vitest.dev API vi useFakeTimers setSystemTime runAllTimersAsync does setSystemTime run timers
💡 Result:
No, vi.setSystemTime does not run timers [1][2]. While vi.setSystemTime updates the system clock observed by date-related APIs (such as new Date, Date.now, performance.now, and hrtime), it does not trigger any timers scheduled via setTimeout or setInterval [1][3][2]. To execute timers in Vitest, you must use timer advancement methods such as: vi.runAllTimers or vi.runAllTimersAsync: These methods invoke all queued timers until the timer queue is empty [1][2]. vi.advanceTimersByTime(ms) or vi.advanceTimersByTimeAsync(ms): These advance the clock by a specified duration, triggering any timers that fall within that time window [4][3]. In summary, vi.setSystemTime is used to control time-based logic (like date formatting or expiration checks) [3][5], while timer execution is managed separately via the timer advancement API [5].
Citations:
- 1: https://github.com/vitest-dev/vitest/blob/main/packages/vitest/src/integrations/vi.ts
- 2: https://vitest.dev/api/vi
- 3: https://qaskills.sh/blog/vitest-fake-timers-date-testing-guide
- 4: https://vitest.dev/guide/mocking/timers
- 5: https://qaskills.sh/blog/vitest-mock-date-timezone-consistently
Advance the fake settle timer before asserting the timeout.
Set mock time to 0 after the first waitForTerminal returns: set the second waitForTerminal timestamp past the budget, schedule it with a fake timer, advance with await vi.runAllTimersAsync(), then assert rejects.toThrow('timeout'). Restore real timers in finally so assertion failures do not leave fake timers enabled.
3 participants
Summary
worker-start --worktree new-child --agentwaited for a singletui-idlethen paste+Enter.input_accepted.tui-idlebefore inject. External--terminalreuse is unchanged.Test plan
vitestagent-prompt-injection, worker-agent-readiness, workers-new-worktreeworker-start --worktree new-child --agent claudewith a setup hook + MCP-heavy agent; receiptreadyshould mean the turn actually startedFixes #13488