Repository navigation
feat(orchestration): make multi-agent workflows durable - #16904
Conversation
|
Too many files changed for review (573 files, 100 file limit). |
c962a47 to
1ec0720
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change adds durable lifecycle transition receipts and attempt-observation facts to orchestration persistence. It adds lifecycle guards, outcome projection, worker attention, terminal recovery tracking, federated read and release flows, capability negotiation, worker-list pagination, and remote transcript reads. Terminal prompts now support queued delivery, observation, and idempotent retry. Worker-start accepts inline task specifications. Mailbox delivery supports cold-parked terminals. Orchestration guides now bundle conditional reference documents. Tests cover migration, lifecycle atomicity, federation behavior, prompt delivery, projections, recovery, and end-to-end parked-terminal delivery. Merge Risk: 🟠 High · up to This change introduces durable mailbox delivery, worker recovery, federated observation, and lifecycle management, but the current implementation can still duplicate or strand messages, leave workers or dispatches in incorrect states, and return misleading recovery or fleet results. The PR is not merge-ready until the concrete delivery, lifecycle, replay, release, and observation issues are fixed or explicitly accepted by the owners. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Out of Scope Changes checkExplanation The PR includes substantial functionality beyond the requirements in [ Resolution Split unrelated functionality into separate pull requests, or link the additional issues that define these requirements. Keep this pull request focused on durable dispatch observations, prompt-delivery idempotency, mailbox delivery, and the recovery behavior required to support them. Full details: Docstring CoverageExplanation Docstring coverage is 6.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 45 functions across 48 files. (196 skipped: 14 unsupported, 2 too large, 180 over the file limit.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
a9d51da to
9923fef
Compare
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (10)
src/main/runtime/rpc/methods/orchestration-local-worker-start.ts (1)
88-88: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCompute the readiness timeout once.
params.timeoutMs ?? 60_000appears three times. Hoist it into one constant and reuse it forstartOptions,waitForTerminal, and the response.Also applies to: 189-192, 254-254
src/main/runtime/rpc/methods/orchestration-worker-output.ts (1)
47-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the remote connection once.
The condition
session.connectionId && !isWslHookRelayConnectionId(session.connectionId)is evaluated twice. Bind it once so the provider lookup and the fail-closed guard cannot drift apart.♻️ Proposed refactor
- const remoteFilesystemProvider = - session.connectionId && !isWslHookRelayConnectionId(session.connectionId) - ? getSshFilesystemProvider(session.connectionId) - : undefined - if ( - session.connectionId && - !isWslHookRelayConnectionId(session.connectionId) && - !remoteFilesystemProvider - ) { - return fallbackOrThrow(args, 'remote_capability_unavailable') - } + const remoteConnectionId = + session.connectionId && !isWslHookRelayConnectionId(session.connectionId) + ? session.connectionId + : null + const remoteFilesystemProvider = remoteConnectionId + ? getSshFilesystemProvider(remoteConnectionId) + : undefined + if (remoteConnectionId && !remoteFilesystemProvider) { + return fallbackOrThrow(args, 'remote_capability_unavailable') + }src/main/runtime/orchestration/worker-output-archive.test.ts (1)
17-34: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the unused transcript fixture.
Test 1 mocks
readWorkerTranscript, and test 2 never reads a file. The tmpdir andsession.jsonlcreated at Lines 24-26 are never used, sodirectory,transcriptPath, and thermcleanup can be removed. Thedescribetitle also says "WSL routing" while the suite covers SSH routing too.src/main/runtime/orchestration/db/worker-terminal/worker-terminal-listing.ts (2)
69-84: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated
terminalStateCASE expression.The same 10-line SQL CASE now exists in
listWorkerTerminalResourcesandcountWorkerTerminalResources, and it must stay in lockstep withderiveWorkerTerminalListState. Three copies drift independently, and a listing/count mismatch would produce a wrong total for the same filter. Move the expression to one exported constant in this module and use it in both places.♻️ Suggested shape
+const TERMINAL_STATE_SQL = `CASE + WHEN r.id IS NULL THEN CASE WHEN COALESCE(w.agent_terminal_handle, d.assignee_handle) IS NOT NULL THEN 'retained' ELSE NULL END + WHEN r.release_state = 'released' THEN 'released' + ... +END`Also applies to: 220-234
132-140: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider selecting the resource columns in the main query.
The main query already joins
worker_terminal_resources r. The second query re-reads the same rows by dispatch id. Selectingr.*(aliased) in the first query removes one round trip and theINplaceholder list. This is optional; keep the two-query form if column-name collisions are the reason.src/main/runtime/orchestration/db/attempt-observation-types.ts (1)
109-109: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove the
PtyLivenessVerdictimport to the top of the file.The import sits after every declaration. TypeScript hoists it, so behavior is unchanged, but the file reads as if
AttemptLivenessObservationon Line 39 has no source.src/shared/orchestration-fleet-status-index.ts (1)
69-73: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDrop
StatusIdentityand the cast.
AgentStatusIpcPayloadnow declaresprocessIncarnation,endpointId, andendpointIncarnation(src/shared/agent-status-ipc-payload.ts, lines 30-34). The local widening type and theas StatusIdentitycast are redundant, and they will mask a future change to those field types.♻️ Proposed cleanup
-type StatusIdentity = AgentStatusIpcPayload & { - processIncarnation?: string - endpointId?: string - endpointIncarnation?: string -} -- const candidate = status as StatusIdentity + const candidate = statusAlso applies to: 97-97
src/main/runtime/rpc/methods/orchestration-federated-worker-start.ts (1)
35-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShare one
--depsparser between the local and federated start paths.Both modules define identical
parseTaskDepsimplementations for the same contract. Export one parser and import it in both modules to prevent syntax and error-message drift.src/shared/orchestration-worker-output.ts (1)
50-53: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winConsider a named clipping vocabulary instead of
string[].
clippingis a wire field with a closed vocabulary. Producers emitmessage_limit,archive_message_limit,message_limit_or_scan_window, andterminal_fallback. Astring[]type allows silent drift between producers and consumers. The file already uses this pattern forORCHESTRATION_WORKER_READ_FALLBACK_REASONS.♻️ Proposed typed vocabulary
+export const ORCHESTRATION_WORKER_READ_CLIPPING_REASONS = [ + 'message_limit', + 'archive_message_limit', + 'message_limit_or_scan_window', + 'terminal_fallback' +] as const +export type OrchestrationWorkerReadClipping = + (typeof ORCHESTRATION_WORKER_READ_CLIPPING_REASONS)[number]sourceExact?: boolean contentComplete?: boolean - clipping?: string[] + clipping?: OrchestrationWorkerReadClipping[]Also applies to: 71-74
src/main/runtime/orchestration/db/lifecycle-transition-boundary.test.ts (1)
8-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winScan the directory instead of a hardcoded file list.
The guard only covers eight named files. A new module that writes
stateorstatusdirectly is not scanned, so the boundary regression can reappear unnoticed. Glob the orchestration DB tree and exclude the lifecycle transition module itself. This also removes the need to update the list with every new writer.Note also that the bounded
[\s\S]{0,180}?gap misses anUPDATEstatement with a longer clause beforeSET.♻️ Proposed directory scan
- const root = resolve(__dirname) - const files = [ - 'worker-dispatch/worker-dispatch-outcome.ts', - 'worker-dispatch/worker-dispatch-abandon.ts', - 'worker-dispatch/worker-dispatch-stop.ts', - 'worker-dispatch/federated-worker-start-reconcile.ts', - 'dispatch-context/dispatch-completion.ts', - 'dispatch-context/task-dispatch-reconciliation.ts', - 'decision-gates/decision-gate-store.ts', - '../context-only-dispatch-release.ts' - ] + const root = resolve(__dirname, '..') + const files = globSync('**/*.ts', { cwd: root }).filter( + (file) => !file.endsWith('.test.ts') && !file.endsWith('db/lifecycle-transition.ts') + )
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d01b7c79-0141-4048-a87e-0712543597bd
📥 Commits
Reviewing files that changed from the base of the PR and between 39ac32f and a9d51da24242e57b8ca34d92b239c553a5f89982.
📒 Files selected for processing (176)
config/scripts/generate-bundled-skill-guides.mjsconfig/scripts/generate-bundled-skill-guides.test.mjsconfig/scripts/orchestration-skill-guidance.test.mjsresources/skills/current-manifest.jsonresources/skills/snapshot-registry.jsonskill-guides/orca-cli.mdskill-guides/orchestration.mdskill-guides/orchestration/references/coordinator-loop.mdskill-guides/orchestration/references/legacy-contract-migration.mdskill-guides/orchestration/references/low-level-topology.mdskill-guides/orchestration/references/messaging-and-gates.mdskill-guides/orchestration/references/placement-and-remote.mdskill-guides/orchestration/references/recovery-and-cleanup.mdskill-guides/orchestration/references/worker-contract.mdskills/orchestration/SKILL.mdsrc/cli/bundled-skill-guides.tssrc/cli/handlers/orchestration-worker-cli.test.tssrc/cli/handlers/orchestration/worker-launch-handler.tssrc/cli/handlers/orchestration/worker-terminal-handlers.tssrc/cli/handlers/terminal.test.tssrc/cli/handlers/terminal.tssrc/cli/index.test.tssrc/cli/root-help-text-secondary.tssrc/cli/runtime/client.tssrc/cli/specs/core.tssrc/cli/specs/orchestration-worker-specs.tssrc/cli/specs/terminal-send.tssrc/cli/terminal-format.tssrc/main/runtime/agent-prompt-submission-runtime.test.tssrc/main/runtime/agent-prompt-submission-verification.tssrc/main/runtime/orca-runtime.test.tssrc/main/runtime/orca-runtime.tssrc/main/runtime/orchestration-mailbox-notification-consistency.test.tssrc/main/runtime/orchestration/context-only-dispatch-release.tssrc/main/runtime/orchestration/coordinator-runtime-contract.tssrc/main/runtime/orchestration/coordinator-task-dispatch.tssrc/main/runtime/orchestration/db-task-dispatch-invariant.test.tssrc/main/runtime/orchestration/db-task-dispatch-lifecycle-guards.test.tssrc/main/runtime/orchestration/db-task-dispatch-races.test.tssrc/main/runtime/orchestration/db.tssrc/main/runtime/orchestration/db/attach-orchestration-db-methods.tssrc/main/runtime/orchestration/db/attempt-observation-store.tssrc/main/runtime/orchestration/db/attempt-observation-types.tssrc/main/runtime/orchestration/db/attempt-outcome-projection.test.tssrc/main/runtime/orchestration/db/attempt-outcome-projection.tssrc/main/runtime/orchestration/db/contract-constants.tssrc/main/runtime/orchestration/db/decision-gate-lifecycle.test.tssrc/main/runtime/orchestration/db/decision-gates/decision-gate-store.tssrc/main/runtime/orchestration/db/dispatch-context/dispatch-completion.tssrc/main/runtime/orchestration/db/dispatch-context/dispatch-context-store.tssrc/main/runtime/orchestration/db/dispatch-context/task-dispatch-reconciliation.tssrc/main/runtime/orchestration/db/dispatch-context/worker-report-settlement.tssrc/main/runtime/orchestration/db/dispatch-depth.tssrc/main/runtime/orchestration/db/dispatch-row-writer.tssrc/main/runtime/orchestration/db/federation/federated-dispatch-store.tssrc/main/runtime/orchestration/db/lifecycle-transition-boundary.test.tssrc/main/runtime/orchestration/db/lifecycle-transition.test.tssrc/main/runtime/orchestration/db/lifecycle-transition.tssrc/main/runtime/orchestration/db/orchestration-db-methods.tssrc/main/runtime/orchestration/db/reset/orchestration-reset.tssrc/main/runtime/orchestration/db/schema/create-core-tables-sql.tssrc/main/runtime/orchestration/db/schema/create-graph-tables-sql.tssrc/main/runtime/orchestration/db/schema/migrate-v13-v30.tssrc/main/runtime/orchestration/db/tasks/task-status-transition.tssrc/main/runtime/orchestration/db/tasks/task-store.tssrc/main/runtime/orchestration/db/worker-dispatch/federated-worker-start-reconcile.tssrc/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-abandon.tssrc/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-authority.tssrc/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-outcome.tssrc/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-stage.tssrc/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-start.tssrc/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-stop.tssrc/main/runtime/orchestration/db/worker-dispatch/worker-terminal-recovery.tssrc/main/runtime/orchestration/db/worker-terminal/worker-terminal-attention-query.tssrc/main/runtime/orchestration/db/worker-terminal/worker-terminal-listing.tssrc/main/runtime/orchestration/db/worker-terminal/worker-terminal-resource-store.tssrc/main/runtime/orchestration/db/worker-terminal/worker-terminal-user-takeover.tssrc/main/runtime/orchestration/federation-sync.test.tssrc/main/runtime/orchestration/federation-sync.tssrc/main/runtime/orchestration/lifecycle-reconciliation.test.tssrc/main/runtime/orchestration/lifecycle-reconciliation.tssrc/main/runtime/orchestration/mailbox-owner.tssrc/main/runtime/orchestration/mailbox-pointer-batch.tssrc/main/runtime/orchestration/mailbox-pointer-delivery-contract.tssrc/main/runtime/orchestration/mailbox-pointer-delivery.tssrc/main/runtime/orchestration/mailbox-pointer-submit.test.tssrc/main/runtime/orchestration/mailbox-pointer-submit.tssrc/main/runtime/orchestration/orchestration-peer-capability-cache.test.tssrc/main/runtime/orchestration/orchestration-peer-capability-cache.tssrc/main/runtime/orchestration/orchestration-reset-db.test.tssrc/main/runtime/orchestration/orchestration-schema-version-skew.tssrc/main/runtime/orchestration/orchestration-version-skew-migration.test.tssrc/main/runtime/orchestration/orchestration-worker-dispatch-db.test.tssrc/main/runtime/orchestration/preamble.test.tssrc/main/runtime/orchestration/r1-identity-migration.test.tssrc/main/runtime/orchestration/types.tssrc/main/runtime/orchestration/worker-attention-context.tssrc/main/runtime/orchestration/worker-output-archive.test.tssrc/main/runtime/orchestration/worker-output-archive.tssrc/main/runtime/orchestration/worker-provider-session.test.tssrc/main/runtime/orchestration/worker-provider-session.tssrc/main/runtime/orchestration/worker-report-observation.tssrc/main/runtime/orchestration/worker-start-unobserved-prompt-settlement.test.tssrc/main/runtime/orchestration/worker-terminal-ownership.tssrc/main/runtime/orchestration/worker-transcript-read.tssrc/main/runtime/orchestration/worker-transcript-remote-read.test.tssrc/main/runtime/orchestration/worker-transcript-remote-read.tssrc/main/runtime/rpc/core.tssrc/main/runtime/rpc/dispatcher-caller-fingerprint.tssrc/main/runtime/rpc/dispatcher.tssrc/main/runtime/rpc/methods/orchestration-composed-workers.test.tssrc/main/runtime/rpc/methods/orchestration-federated-fleet-snapshot.tssrc/main/runtime/rpc/methods/orchestration-federated-worker-read.tssrc/main/runtime/rpc/methods/orchestration-federated-worker-release-host.tssrc/main/runtime/rpc/methods/orchestration-federated-worker-release.tssrc/main/runtime/rpc/methods/orchestration-federated-worker-start-receipts.tssrc/main/runtime/rpc/methods/orchestration-federated-worker-start.tssrc/main/runtime/rpc/methods/orchestration-federation-control.tssrc/main/runtime/rpc/methods/orchestration-federation-liveness-verdict.test.tssrc/main/runtime/rpc/methods/orchestration-federation-output.test.tssrc/main/runtime/rpc/methods/orchestration-federation.test.tssrc/main/runtime/rpc/methods/orchestration-federation.tssrc/main/runtime/rpc/methods/orchestration-local-worker-start.tssrc/main/runtime/rpc/methods/orchestration-runs.test.tssrc/main/runtime/rpc/methods/orchestration-tasks-dispatch.test.tssrc/main/runtime/rpc/methods/orchestration-worker-archive-read.tssrc/main/runtime/rpc/methods/orchestration-worker-control.tssrc/main/runtime/rpc/methods/orchestration-worker-launch-preferences.test.tssrc/main/runtime/rpc/methods/orchestration-worker-legacy-federated-read.tssrc/main/runtime/rpc/methods/orchestration-worker-list-cursor.tssrc/main/runtime/rpc/methods/orchestration-worker-list-method.tssrc/main/runtime/rpc/methods/orchestration-worker-observation.test.tssrc/main/runtime/rpc/methods/orchestration-worker-observation.tssrc/main/runtime/rpc/methods/orchestration-worker-output.test.tssrc/main/runtime/rpc/methods/orchestration-worker-output.tssrc/main/runtime/rpc/methods/orchestration-worker-release-archive.test.tssrc/main/runtime/rpc/methods/orchestration-worker-release-completion.tssrc/main/runtime/rpc/methods/orchestration-worker-release-inventory.test.tssrc/main/runtime/rpc/methods/orchestration-worker-release-liveness-verdict.test.tssrc/main/runtime/rpc/methods/orchestration-worker-release-recovery.test.tssrc/main/runtime/rpc/methods/orchestration-worker-release-schemas.tssrc/main/runtime/rpc/methods/orchestration-worker-release-test-harness.tssrc/main/runtime/rpc/methods/orchestration-worker-release.test.tssrc/main/runtime/rpc/methods/orchestration-worker-release.tssrc/main/runtime/rpc/methods/orchestration-worker-start-prompt-contract.test.tssrc/main/runtime/rpc/methods/orchestration-worker-start-schema.tssrc/main/runtime/rpc/methods/orchestration-worker-terminal-resource-presentation.tssrc/main/runtime/rpc/methods/orchestration-workers-recovery.test.tssrc/main/runtime/rpc/methods/orchestration-workers.tssrc/main/runtime/rpc/methods/orchestration.tssrc/main/runtime/rpc/methods/terminal/terminal-prompt-replay-observation.tssrc/main/runtime/rpc/methods/terminal/terminal-send-method.tssrc/main/runtime/rpc/methods/terminal/unary-schemas.tssrc/main/runtime/rpc/orchestration-commit-notify-characterization.test.tssrc/main/runtime/rpc/orchestration-legacy-compatibility-dispatcher.test.tssrc/main/runtime/rpc/orchestration-mutation-executor.test.tssrc/main/runtime/rpc/orchestration-mutation-executor.tssrc/main/runtime/rpc/terminal-prompt-delivery-receipt.test.tssrc/main/sqlite/sync-database.test.tssrc/main/sqlite/sync-database.tssrc/renderer/src/store/slices/agent-status-runtime-orchestration.test.tssrc/renderer/src/store/slices/agent-status.tssrc/shared/agent-status-ipc-payload.tssrc/shared/agent-status-types.tssrc/shared/orchestration-fleet-attention.test.tssrc/shared/orchestration-fleet-attention.tssrc/shared/orchestration-fleet-projection.test.tssrc/shared/orchestration-fleet-projection.tssrc/shared/orchestration-fleet-status-index.tssrc/shared/orchestration-fleet-worker-projection.tssrc/shared/orchestration-rpc-contract.tssrc/shared/orchestration-worker-output.tssrc/shared/protocol-version.tssrc/shared/runtime-terminal-contracts.tssrc/shared/runtime-types.tstests/e2e/orchestration-idle-mail-delivery.spec.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.
| if (observation.status !== 'exited') { | ||
| try { | ||
| const close = await runtime.closeTerminal(observation.terminal.handle) | ||
| if (!close.ptyKilled) { | ||
| return { | ||
| dispatchId: attachment.dispatch_id, | ||
| state: 'release_unknown', | ||
| processAction: 'closed_agent_terminal', | ||
| lastError: describeUnconfirmedAgentStop(close), | ||
| output | ||
| } | ||
| } | ||
| } catch (error) { | ||
| return { | ||
| dispatchId: attachment.dispatch_id, | ||
| state: 'release_unknown', | ||
| processAction: 'none', | ||
| lastError: error instanceof Error ? error.message : String(error), | ||
| output | ||
| } | ||
| } | ||
| } | ||
| runtime.getOrchestrationDb().recordRemoteAttachmentStage({ | ||
| dispatchId: attachment.dispatch_id, | ||
| stage: 'released' | ||
| }) | ||
| return { | ||
| dispatchId: attachment.dispatch_id, | ||
| state: 'released', | ||
| processAction: | ||
| observation.status === 'exited' ? 'closed_exited_terminal' : 'closed_agent_terminal', | ||
| output | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
# Check how consumers interpret processAction and whether exited terminals are closed elsewhere.
rg -n 'closed_exited_terminal|processAction' --type=ts -C3Repository: stablyai/orca
Length of output: 151
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- repository conventions ---'
find /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4 -maxdepth 2 -type f -name '*.md' -print
printf '%s\n' '--- reviewed method ---'
cat -n src/main/runtime/rpc/methods/orchestration-federated-worker-release-host.ts
printf '%s\n' '--- direct references ---'
rg -n -C3 'closed_exited_terminal|processAction|releaseRemote|recordRemoteAttachmentStage|closeTerminal' src/main/runtime src --type ts
printf '%s\n' '--- SSH boundary ---'
cat -n docs/reference/ssh-execution-boundary.mdRepository: stablyai/orca
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- convention file list ---'
find /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4 -maxdepth 2 -type f -name '*.md' -print
printf '%s\n' '--- release method ---'
cat -n src/main/runtime/rpc/methods/orchestration-federated-worker-release-host.ts
printf '%s\n' '--- exact action references outside tests ---'
rg -n -C4 --glob '!**/*.test.ts' --glob '!**/*.spec.ts' \
'closed_exited_terminal|closed_agent_terminal|release_unknown|processAction' src
printf '%s\n' '--- closeTerminal and attachment-stage references near orchestration code ---'
rg -n -C3 --glob '!**/*.test.ts' --glob '!**/*.spec.ts' \
'closeTerminal|recordRemoteAttachmentStage|observation.status' src/main/runtime/rpc src/main/runtimeRepository: stablyai/orca
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- release caller and related contract ---'
rg -n -C6 --glob '!**/*.test.ts' --glob '!**/*.spec.ts' \
'releaseRemoteAttachment|orchestration-federated-worker-release-host|Remote.*Release|processAction' src/main
printf '%s\n' '--- bounded runtime-rpc learning ---'
cat /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4/learnings/src-main-runtime-rpc.md
printf '%s\n' '--- bounded source convention ---'
cat /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4/conventions/src.mdRepository: stablyai/orca
Length of output: 50369
Close exited terminals before marking them released. When observation.status === 'exited', runtime.closeTerminal(...) is skipped, but the method records stage: 'released' and returns processAction: 'closed_exited_terminal'. The federated caller forwards this action in the release receipt. Close the terminal before recording released, or return a non-released state when cleanup is not complete.
| const cursorIndex = args.cursor | ||
| ? args.workers.findIndex((worker) => worker.dispatchId === args.cursor) | ||
| : -1 | ||
| const start = cursorIndex >= 0 ? cursorIndex + 1 : 0 | ||
| const rows = args.workers.slice(start, start + limit) | ||
| const statusIndex = createFleetStatusIndex(args.statuses, rows) | ||
| const now = args.now ?? Date.now() | ||
| const workers = rows.map((worker) => | ||
| projectOrchestrationFleetWorker(worker, statusForFleetWorker(worker, statusIndex), now) | ||
| ) | ||
| const hasMore = start + workers.length < args.workers.length |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
An unknown cursor silently restarts pagination at page 1.
findIndex returns -1 when the cursor dispatch id is no longer in workers, for example after that dispatch is released between page requests. start then becomes 0 and the caller receives page 1 again with hasMore: true. A client that pages until hasMore is false can repeat forever. Reject the unknown cursor, or report it in the page metadata.
🐛 Proposed fix
- const cursorIndex = args.cursor
- ? args.workers.findIndex((worker) => worker.dispatchId === args.cursor)
- : -1
- const start = cursorIndex >= 0 ? cursorIndex + 1 : 0
+ let start = 0
+ if (args.cursor) {
+ const cursorIndex = args.workers.findIndex((worker) => worker.dispatchId === args.cursor)
+ if (cursorIndex < 0) {
+ throw new Error(`Unknown fleet page cursor ${args.cursor}.`)
+ }
+ start = cursorIndex + 1
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const cursorIndex = args.cursor | |
| ? args.workers.findIndex((worker) => worker.dispatchId === args.cursor) | |
| : -1 | |
| const start = cursorIndex >= 0 ? cursorIndex + 1 : 0 | |
| const rows = args.workers.slice(start, start + limit) | |
| const statusIndex = createFleetStatusIndex(args.statuses, rows) | |
| const now = args.now ?? Date.now() | |
| const workers = rows.map((worker) => | |
| projectOrchestrationFleetWorker(worker, statusForFleetWorker(worker, statusIndex), now) | |
| ) | |
| const hasMore = start + workers.length < args.workers.length | |
| let start = 0 | |
| if (args.cursor) { | |
| const cursorIndex = args.workers.findIndex((worker) => worker.dispatchId === args.cursor) | |
| if (cursorIndex < 0) { | |
| throw new Error(`Unknown fleet page cursor ${args.cursor}.`) | |
| } | |
| start = cursorIndex + 1 | |
| } | |
| const rows = args.workers.slice(start, start + limit) | |
| const statusIndex = createFleetStatusIndex(args.statuses, rows) | |
| const now = args.now ?? Date.now() | |
| const workers = rows.map((worker) => | |
| projectOrchestrationFleetWorker(worker, statusForFleetWorker(worker, statusIndex), now) | |
| ) | |
| const hasMore = start + workers.length < args.workers.length |
9923fef to
04031f5
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (7)
src/main/runtime/orchestration/db/attempt-observation-store.ts (1)
175-186: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe active-sibling predicate is duplicated across two new modules. Both queries decide
activeSiblingforprojectAttemptOutcomewith the same status and worker-state lists. If one list changes, the single-dispatch projection and the batched attention projection disagree abouttaskOutcome.
src/main/runtime/orchestration/db/attempt-observation-store.ts#L175-L186: move this predicate into one exported SQL fragment and use it here.src/main/runtime/orchestration/db/worker-terminal/worker-terminal-attention-query.ts#L43-L49: use the same shared fragment in theactive_siblingsubquery.src/main/runtime/orchestration/db/worker-terminal/worker-terminal-attention-query.ts (1)
104-116: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
OrchestrationErrorfor the not-found case.
recordAttemptObservationandgetAttemptOutcomeProjectionthrowOrchestrationError('dispatch_not_found', ...)for a missing dispatch. Line 113 throws a plainError, so callers that branch on the orchestration error code cannot classify it.src/main/runtime/orchestration/db/worker-terminal/worker-terminal-listing.ts (2)
152-160: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBind the dispatch ids with
json_eachinstead of one placeholder per row.
params.limitis optional, sorowsis unbounded. The placeholder list then grows with the row count and can exceed the SQLite bound-parameter limit.worker-terminal-attention-query.tsalready binds the same shape withIN (SELECT value FROM json_each(?)); reuse that pattern here.♻️ Proposed refactor
- : (this.db - .prepare( - `SELECT r.* FROM worker_terminal_resources r - WHERE r.owner_dispatch_id IN (${rows.map(() => '?').join(',')})` - ) - .all(...rows.map((row) => row.dispatch_id)) as WorkerTerminalResourceRow[]) + : (this.db + .prepare( + `SELECT r.* FROM worker_terminal_resources r + WHERE r.owner_dispatch_id IN (SELECT value FROM json_each(?))` + ) + .all(JSON.stringify(rows.map((row) => row.dispatch_id))) as WorkerTerminalResourceRow[])
87-102: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDefine the terminal-state CASE expression once and derive its state lists.
The duplicated CASE expression in
listWorkerTerminalResourcesandcountWorkerTerminalResourceshardcodes the current contents ofWORKER_RELEASABLE_STATESandWORKER_SETTLED_STATES. If those constants change, the list, count, andderiveWorkerTerminalListStatecan diverge. Extract one exported builder that renders the CASE from the shared constants, then call it from both functions.src/cli/handlers/terminal.ts (1)
114-133: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLocalize the new CLI strings. Use intent-named localization IDs with named placeholders for both sites.
src/cli/handlers/terminal.ts#L114-L133: replace the new validation and compatibility error text with localized messages.src/cli/help.ts#L111-L112: replace the new--include-remotehelp text with a localized message.Based on learnings: “For newly added user-facing strings in stablyai/orca TypeScript and TSX files, follow config/i18n-translation-source.md.”
Source: Learnings
tests/e2e/orchestration-idle-mail-delivery.spec.ts (1)
74-76: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse the helper's option type instead of redeclaring its shape.
tests/e2e/helpers/orchestration-mail-pane-agent.tsalready declaresMailPaneAgentOptionswith the sametitleOnStdinfield. This spec redeclares that shape twice. Export the helper type and import it so the two definitions cannot drift.♻️ Proposed refactor
In
tests/e2e/helpers/orchestration-mail-pane-agent.ts:-type MailPaneAgentOptions = { +export type MailPaneAgentOptions = { titleOnStdin?: { needle: string; title: string } }In this spec:
- openAgentPane: (options?: { - titleOnStdin?: { needle: string; title: string } - }) => Promise<AgentPane> + openAgentPane: (options?: MailPaneAgentOptions) => Promise<AgentPane>- const openAgentPane = async (options?: { - titleOnStdin?: { needle: string; title: string } - }): Promise<AgentPane> => { + const openAgentPane = async (options?: MailPaneAgentOptions): Promise<AgentPane> => {As per coding guidelines: "check whether an existing implementation already does the job (or nearly does). Extend or generalize it instead of building a parallel version".
Also applies to: 133-135
Source: Coding guidelines
src/main/runtime/orca-runtime.ts (1)
20524-20611: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueClarify the correlation cap scope.
AGENT_PROMPT_CORRELATION_LIMIT_PER_PTYis named per PTY, butregisterAgentPromptRequestprunes with the prefix${ptyId}\0${generation}\0. The cap therefore applies per PTY generation.advancePtyLifecycleGenerationclears prior generations, so the practical bound holds. Rename the constant or the prefix to match the intent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e545dbe9-79dd-4b9e-8216-eebc9a032668
📥 Commits
Reviewing files that changed from the base of the PR and between a9d51da24242e57b8ca34d92b239c553a5f89982 and 04031f5.
📒 Files selected for processing (42)
src/cli/args.tssrc/cli/handlers/orchestration-worker-cli.test.tssrc/cli/handlers/orchestration/worker-terminal-handlers.tssrc/cli/handlers/terminal.test.tssrc/cli/handlers/terminal.tssrc/cli/help.tssrc/cli/specs/orchestration-worker-specs.tssrc/cli/terminal-format.tssrc/main/runtime/agent-prompt-submission-runtime.test.tssrc/main/runtime/agent-prompt-submission-verification.test.tssrc/main/runtime/agent-prompt-submission-verification.tssrc/main/runtime/orca-runtime.test.tssrc/main/runtime/orca-runtime.tssrc/main/runtime/orchestration-mailbox-notification-consistency.test.tssrc/main/runtime/orchestration/db/attempt-observation-store.tssrc/main/runtime/orchestration/db/worker-terminal/worker-terminal-attention-query.tssrc/main/runtime/orchestration/db/worker-terminal/worker-terminal-listing.tssrc/main/runtime/orchestration/mailbox-pointer-delivery-contract.tssrc/main/runtime/orchestration/mailbox-pointer-delivery.tssrc/main/runtime/orchestration/mailbox-pointer-submit.test.tssrc/main/runtime/orchestration/mailbox-pointer-submit.tssrc/main/runtime/orchestration/orchestration-peer-capability-cache.test.tssrc/main/runtime/orchestration/orchestration-peer-capability-cache.tssrc/main/runtime/orchestration/worker-attention-context.tssrc/main/runtime/orchestration/worker-output-archive.tssrc/main/runtime/pty-inventory-liveness-verdict.test.tssrc/main/runtime/rpc/methods/orchestration-federated-fleet-snapshot.test.tssrc/main/runtime/rpc/methods/orchestration-federated-fleet-snapshot.tssrc/main/runtime/rpc/methods/orchestration-federated-worker-release-host.tssrc/main/runtime/rpc/methods/orchestration-federated-worker-release.tssrc/main/runtime/rpc/methods/orchestration-federation-attachment-observation.tssrc/main/runtime/rpc/methods/orchestration-federation-control.tssrc/main/runtime/rpc/methods/orchestration-federation-liveness-verdict.test.tssrc/main/runtime/rpc/methods/orchestration-federation-output.test.tssrc/main/runtime/rpc/methods/orchestration-worker-archive-read.tssrc/main/runtime/rpc/methods/orchestration-worker-list-cursor.tssrc/main/runtime/rpc/methods/orchestration-worker-list-method.tssrc/main/runtime/rpc/methods/orchestration-worker-list-pagination.test.tssrc/main/runtime/rpc/methods/orchestration-worker-release-completion.tssrc/shared/protocol-version.tstests/e2e/helpers/orchestration-mail-pane-agent.tstests/e2e/orchestration-idle-mail-delivery.spec.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.
04031f5 to
e36c235
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/runtime/orchestration/db/worker-terminal/worker-terminal-listing.ts (1)
95-110: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the terminal-state CASE expression into one shared constant.
Lines 97-107 and lines 252-262 contain the same 11-line CASE expression. It re-implements
deriveWorkerTerminalListStatefromsrc/main/runtime/orchestration/worker-terminal-ownership.tsin SQL, including the inlined contents ofWORKER_RELEASABLE_STATESandWORKER_SETTLED_STATES. If either constant changes, both SQL copies drift silently. The filter and the count then disagree with theterminalStatevalue that line 185 derives in TypeScript.Define the expression once in this module and build the state lists from the exported constants so a change in one place propagates.
Reuse of the existing implementation is required by the coding guidelines. As per coding guidelines: "Before writing new logic at any scale ... check whether an existing implementation already does the job (or nearly does). Extend or generalize it instead of building a parallel version".
♻️ Sketch: single source for the SQL expression
+const TERMINAL_STATE_SQL = `CASE + WHEN r.id IS NULL THEN CASE WHEN COALESCE(w.agent_terminal_handle, d.assignee_handle) IS NOT NULL THEN 'retained' ELSE NULL END + WHEN r.release_state = 'released' THEN 'released' + WHEN r.release_state = 'unknown' THEN 'release_unknown' + WHEN r.release_state IN ('requested', 'releasing') THEN 'release_pending' + WHEN r.ownership_state <> 'owned' OR r.release_state = 'retained' THEN 'retained' + WHEN COALESCE(w.state, 'unsupervised') <> 'unsupervised' AND COALESCE(w.state, '') IN (${sqlList(WORKER_RELEASABLE_STATES)}) THEN 'reclaimable' + WHEN COALESCE(w.state, 'unsupervised') <> 'unsupervised' AND COALESCE(w.state, '') IN (${sqlList(WORKER_SETTLED_STATES)}) THEN 'retained' + WHEN COALESCE(w.state, 'unsupervised') <> 'unsupervised' THEN 'active' + ELSE NULL +END`Then use
TERMINAL_STATE_SQLin bothlistWorkerTerminalResourcesandcountWorkerTerminalResources.Also applies to: 251-265
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a718f070-f9a7-4dd4-a2b6-df362b71c5b0
📥 Commits
Reviewing files that changed from the base of the PR and between 04031f5 and e36c235d1e9e842c10d4639007c3f723fc8a256a.
📒 Files selected for processing (14)
resources/skills/current-manifest.jsonresources/skills/snapshot-registry.jsonskill-stubs/orchestration.mdskills/orchestration/SKILL.mdsrc/main/runtime/orchestration/db/worker-terminal/worker-terminal-listing.tssrc/main/runtime/orchestration/orchestration-peer-capability-cache.test.tssrc/main/runtime/orchestration/orchestration-peer-capability-cache.tssrc/main/runtime/rpc/methods/orchestration-federated-worker-release-host.tssrc/main/runtime/rpc/methods/orchestration-federation-control.tssrc/main/runtime/rpc/methods/orchestration-federation-output.test.tssrc/main/runtime/rpc/methods/orchestration-worker-list-cursor.tssrc/main/runtime/rpc/methods/orchestration-worker-list-method.tssrc/main/runtime/rpc/methods/orchestration-worker-list-pagination.test.tssrc/main/runtime/rpc/methods/orchestration-worker-list-snapshot-store.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- skills/orchestration/SKILL.md
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.
e36c235 to
31e3454
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
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: 79cec803-edf0-4587-8a5a-617f0bff04ca
📒 Files selected for processing (188)
config/scripts/generate-bundled-skill-guides.mjsconfig/scripts/generate-bundled-skill-guides.test.mjsconfig/scripts/orchestration-skill-guidance.test.mjsresources/skills/current-manifest.jsonresources/skills/snapshot-registry.jsonskill-guides/orca-cli.mdskill-guides/orchestration.mdskill-guides/orchestration/references/coordinator-loop.mdskill-guides/orchestration/references/legacy-contract-migration.mdskill-guides/orchestration/references/low-level-topology.mdskill-guides/orchestration/references/messaging-and-gates.mdskill-guides/orchestration/references/placement-and-remote.mdskill-guides/orchestration/references/recovery-and-cleanup.mdskill-guides/orchestration/references/worker-contract.mdskill-stubs/orchestration.mdskills/orchestration/SKILL.mdsrc/cli/args.tssrc/cli/bundled-skill-guides.tssrc/cli/handlers/orchestration-worker-cli.test.tssrc/cli/handlers/orchestration/worker-launch-handler.tssrc/cli/handlers/orchestration/worker-terminal-handlers.tssrc/cli/handlers/terminal.test.tssrc/cli/handlers/terminal.tssrc/cli/help.tssrc/cli/index.test.tssrc/cli/root-help-text-secondary.tssrc/cli/runtime/client.tssrc/cli/specs/core.tssrc/cli/specs/orchestration-worker-specs.tssrc/cli/specs/terminal-send.tssrc/cli/terminal-format.test.tssrc/cli/terminal-format.tssrc/main/runtime/agent-prompt-submission-runtime.test.tssrc/main/runtime/agent-prompt-submission-verification.test.tssrc/main/runtime/agent-prompt-submission-verification.tssrc/main/runtime/orca-runtime.test.tssrc/main/runtime/orca-runtime.tssrc/main/runtime/orchestration-mailbox-notification-consistency.test.tssrc/main/runtime/orchestration/context-only-dispatch-release.tssrc/main/runtime/orchestration/coordinator-runtime-contract.tssrc/main/runtime/orchestration/coordinator-task-dispatch.tssrc/main/runtime/orchestration/db-task-dispatch-invariant.test.tssrc/main/runtime/orchestration/db-task-dispatch-lifecycle-guards.test.tssrc/main/runtime/orchestration/db-task-dispatch-races.test.tssrc/main/runtime/orchestration/db.tssrc/main/runtime/orchestration/db/attach-orchestration-db-methods.tssrc/main/runtime/orchestration/db/attempt-observation-store.tssrc/main/runtime/orchestration/db/attempt-observation-types.tssrc/main/runtime/orchestration/db/attempt-outcome-projection.test.tssrc/main/runtime/orchestration/db/attempt-outcome-projection.tssrc/main/runtime/orchestration/db/contract-constants.tssrc/main/runtime/orchestration/db/decision-gate-lifecycle.test.tssrc/main/runtime/orchestration/db/decision-gates/decision-gate-store.tssrc/main/runtime/orchestration/db/dispatch-context/dispatch-completion.tssrc/main/runtime/orchestration/db/dispatch-context/dispatch-context-store.tssrc/main/runtime/orchestration/db/dispatch-context/task-dispatch-reconciliation.tssrc/main/runtime/orchestration/db/dispatch-context/worker-report-settlement.tssrc/main/runtime/orchestration/db/dispatch-depth.tssrc/main/runtime/orchestration/db/dispatch-row-writer.tssrc/main/runtime/orchestration/db/federation/federated-dispatch-store.tssrc/main/runtime/orchestration/db/lifecycle-transition-boundary.test.tssrc/main/runtime/orchestration/db/lifecycle-transition.test.tssrc/main/runtime/orchestration/db/lifecycle-transition.tssrc/main/runtime/orchestration/db/orchestration-db-methods.tssrc/main/runtime/orchestration/db/reset/orchestration-reset.tssrc/main/runtime/orchestration/db/schema/create-core-tables-sql.tssrc/main/runtime/orchestration/db/schema/create-graph-tables-sql.tssrc/main/runtime/orchestration/db/schema/migrate-v13-v30.tssrc/main/runtime/orchestration/db/tasks/task-status-transition.tssrc/main/runtime/orchestration/db/tasks/task-store.tssrc/main/runtime/orchestration/db/worker-dispatch/federated-worker-start-reconcile.tssrc/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-abandon.tssrc/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-authority.tssrc/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-outcome.tssrc/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-stage.tssrc/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-start.tssrc/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-stop.tssrc/main/runtime/orchestration/db/worker-dispatch/worker-terminal-recovery.tssrc/main/runtime/orchestration/db/worker-terminal/worker-terminal-attention-query.tssrc/main/runtime/orchestration/db/worker-terminal/worker-terminal-listing.tssrc/main/runtime/orchestration/db/worker-terminal/worker-terminal-resource-store.tssrc/main/runtime/orchestration/db/worker-terminal/worker-terminal-user-takeover.tssrc/main/runtime/orchestration/federation-sync.test.tssrc/main/runtime/orchestration/federation-sync.tssrc/main/runtime/orchestration/lifecycle-reconciliation.test.tssrc/main/runtime/orchestration/lifecycle-reconciliation.tssrc/main/runtime/orchestration/mailbox-owner.tssrc/main/runtime/orchestration/mailbox-pointer-batch.tssrc/main/runtime/orchestration/mailbox-pointer-delivery-contract.tssrc/main/runtime/orchestration/mailbox-pointer-delivery.tssrc/main/runtime/orchestration/mailbox-pointer-submit.test.tssrc/main/runtime/orchestration/mailbox-pointer-submit.tssrc/main/runtime/orchestration/orchestration-peer-capability-cache.test.tssrc/main/runtime/orchestration/orchestration-peer-capability-cache.tssrc/main/runtime/orchestration/orchestration-reset-db.test.tssrc/main/runtime/orchestration/orchestration-schema-version-skew.tssrc/main/runtime/orchestration/orchestration-version-skew-migration.test.tssrc/main/runtime/orchestration/orchestration-worker-dispatch-db.test.tssrc/main/runtime/orchestration/preamble.test.tssrc/main/runtime/orchestration/r1-identity-migration.test.tssrc/main/runtime/orchestration/types.tssrc/main/runtime/orchestration/worker-attention-context.tssrc/main/runtime/orchestration/worker-output-archive.test.tssrc/main/runtime/orchestration/worker-output-archive.tssrc/main/runtime/orchestration/worker-provider-session.test.tssrc/main/runtime/orchestration/worker-provider-session.tssrc/main/runtime/orchestration/worker-report-observation.tssrc/main/runtime/orchestration/worker-start-unobserved-prompt-settlement.test.tssrc/main/runtime/orchestration/worker-terminal-ownership.tssrc/main/runtime/orchestration/worker-transcript-read.tssrc/main/runtime/orchestration/worker-transcript-remote-read.test.tssrc/main/runtime/orchestration/worker-transcript-remote-read.tssrc/main/runtime/pty-inventory-liveness-verdict.test.tssrc/main/runtime/rpc/core.tssrc/main/runtime/rpc/dispatcher-caller-fingerprint.tssrc/main/runtime/rpc/dispatcher.tssrc/main/runtime/rpc/methods/orchestration-composed-workers.test.tssrc/main/runtime/rpc/methods/orchestration-federated-fleet-snapshot.test.tssrc/main/runtime/rpc/methods/orchestration-federated-fleet-snapshot.tssrc/main/runtime/rpc/methods/orchestration-federated-worker-read.tssrc/main/runtime/rpc/methods/orchestration-federated-worker-release-host.tssrc/main/runtime/rpc/methods/orchestration-federated-worker-release.tssrc/main/runtime/rpc/methods/orchestration-federated-worker-start-receipts.tssrc/main/runtime/rpc/methods/orchestration-federated-worker-start.tssrc/main/runtime/rpc/methods/orchestration-federation-attachment-observation.tssrc/main/runtime/rpc/methods/orchestration-federation-control.tssrc/main/runtime/rpc/methods/orchestration-federation-liveness-verdict.test.tssrc/main/runtime/rpc/methods/orchestration-federation-output.test.tssrc/main/runtime/rpc/methods/orchestration-federation-release-recovery-scenarios.tssrc/main/runtime/rpc/methods/orchestration-federation.test.tssrc/main/runtime/rpc/methods/orchestration-federation.tssrc/main/runtime/rpc/methods/orchestration-local-worker-start.tssrc/main/runtime/rpc/methods/orchestration-runs.test.tssrc/main/runtime/rpc/methods/orchestration-tasks-dispatch.test.tssrc/main/runtime/rpc/methods/orchestration-worker-archive-read.tssrc/main/runtime/rpc/methods/orchestration-worker-control.tssrc/main/runtime/rpc/methods/orchestration-worker-launch-preferences.test.tssrc/main/runtime/rpc/methods/orchestration-worker-legacy-federated-read.tssrc/main/runtime/rpc/methods/orchestration-worker-list-cursor.tssrc/main/runtime/rpc/methods/orchestration-worker-list-method.tssrc/main/runtime/rpc/methods/orchestration-worker-list-pagination.test.tssrc/main/runtime/rpc/methods/orchestration-worker-list-snapshot-store.tssrc/main/runtime/rpc/methods/orchestration-worker-observation.test.tssrc/main/runtime/rpc/methods/orchestration-worker-observation.tssrc/main/runtime/rpc/methods/orchestration-worker-output.test.tssrc/main/runtime/rpc/methods/orchestration-worker-output.tssrc/main/runtime/rpc/methods/orchestration-worker-release-archive.test.tssrc/main/runtime/rpc/methods/orchestration-worker-release-completion.tssrc/main/runtime/rpc/methods/orchestration-worker-release-inventory.test.tssrc/main/runtime/rpc/methods/orchestration-worker-release-liveness-verdict.test.tssrc/main/runtime/rpc/methods/orchestration-worker-release-recovery.test.tssrc/main/runtime/rpc/methods/orchestration-worker-release-schemas.tssrc/main/runtime/rpc/methods/orchestration-worker-release-test-harness.tssrc/main/runtime/rpc/methods/orchestration-worker-release.test.tssrc/main/runtime/rpc/methods/orchestration-worker-release.tssrc/main/runtime/rpc/methods/orchestration-worker-start-prompt-contract.test.tssrc/main/runtime/rpc/methods/orchestration-worker-start-schema.tssrc/main/runtime/rpc/methods/orchestration-worker-terminal-resource-presentation.tssrc/main/runtime/rpc/methods/orchestration-workers-recovery.test.tssrc/main/runtime/rpc/methods/orchestration-workers.tssrc/main/runtime/rpc/methods/orchestration.tssrc/main/runtime/rpc/methods/terminal/terminal-prompt-replay-observation.tssrc/main/runtime/rpc/methods/terminal/terminal-send-method.tssrc/main/runtime/rpc/methods/terminal/unary-schemas.tssrc/main/runtime/rpc/orchestration-commit-notify-characterization.test.tssrc/main/runtime/rpc/orchestration-legacy-compatibility-dispatcher.test.tssrc/main/runtime/rpc/orchestration-mutation-executor.test.tssrc/main/runtime/rpc/orchestration-mutation-executor.tssrc/main/runtime/rpc/terminal-prompt-delivery-receipt.test.tssrc/main/sqlite/sync-database.test.tssrc/main/sqlite/sync-database.tssrc/renderer/src/store/slices/agent-status-runtime-orchestration.test.tssrc/renderer/src/store/slices/agent-status.tssrc/shared/agent-status-ipc-payload.tssrc/shared/agent-status-types.tssrc/shared/orchestration-fleet-attention.test.tssrc/shared/orchestration-fleet-attention.tssrc/shared/orchestration-fleet-projection.test.tssrc/shared/orchestration-fleet-projection.tssrc/shared/orchestration-fleet-status-index.tssrc/shared/orchestration-fleet-worker-projection.tssrc/shared/orchestration-rpc-contract.tssrc/shared/orchestration-worker-output.tssrc/shared/protocol-version.tssrc/shared/runtime-terminal-contracts.tssrc/shared/runtime-types.tstests/e2e/helpers/orchestration-mail-pane-agent.tstests/e2e/orchestration-idle-mail-delivery.spec.ts
🚧 Files skipped from review as they are similar to previous changes (167)
- src/main/runtime/agent-prompt-submission-verification.ts
- src/main/runtime/orchestration/orchestration-reset-db.test.ts
- src/main/runtime/orchestration/types.ts
- src/cli/specs/orchestration-worker-specs.ts
- src/main/runtime/orchestration/db-task-dispatch-races.test.ts
- src/main/runtime/orchestration/db/reset/orchestration-reset.ts
- src/main/runtime/rpc/methods/orchestration-workers-recovery.test.ts
- src/cli/help.ts
- src/main/runtime/rpc/methods/orchestration-runs.test.ts
- skill-stubs/orchestration.md
- src/main/runtime/orchestration/db.ts
- src/main/sqlite/sync-database.ts
- skill-guides/orchestration/references/placement-and-remote.md
- src/main/runtime/rpc/dispatcher-caller-fingerprint.ts
- src/main/runtime/orchestration/coordinator-runtime-contract.ts
- src/shared/agent-status-ipc-payload.ts
- src/cli/args.ts
- src/main/runtime/rpc/methods/terminal/unary-schemas.ts
- src/shared/orchestration-fleet-status-index.ts
- src/main/runtime/orchestration/lifecycle-reconciliation.ts
- src/main/runtime/rpc/core.ts
- config/scripts/generate-bundled-skill-guides.test.mjs
- src/cli/runtime/client.ts
- src/main/runtime/orchestration/db/schema/create-graph-tables-sql.ts
- src/main/runtime/rpc/methods/terminal/terminal-prompt-replay-observation.ts
- src/renderer/src/store/slices/agent-status-runtime-orchestration.test.ts
- src/shared/orchestration-fleet-attention.test.ts
- skill-guides/orchestration/references/low-level-topology.md
- src/cli/specs/terminal-send.ts
- src/main/runtime/rpc/methods/orchestration-worker-legacy-federated-read.ts
- src/cli/handlers/terminal.test.ts
- src/main/runtime/orchestration/context-only-dispatch-release.ts
- src/main/runtime/orchestration/db/lifecycle-transition.test.ts
- src/main/runtime/agent-prompt-submission-verification.test.ts
- src/main/runtime/orchestration/db/worker-terminal/worker-terminal-attention-query.ts
- src/main/runtime/orchestration/db-task-dispatch-invariant.test.ts
- src/main/runtime/orchestration/db-task-dispatch-lifecycle-guards.test.ts
- src/main/runtime/orchestration/db/lifecycle-transition-boundary.test.ts
- tests/e2e/helpers/orchestration-mail-pane-agent.ts
- src/main/runtime/rpc/methods/orchestration-worker-terminal-resource-presentation.ts
- src/main/runtime/orchestration/db/decision-gates/decision-gate-store.ts
- src/shared/agent-status-types.ts
- src/main/runtime/orchestration/db/dispatch-context/task-dispatch-reconciliation.ts
- src/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-stage.ts
- src/main/runtime/rpc/methods/orchestration-worker-release-schemas.ts
- resources/skills/snapshot-registry.json
- src/main/runtime/orchestration/db/decision-gate-lifecycle.test.ts
- src/cli/root-help-text-secondary.ts
- src/main/runtime/rpc/orchestration-mutation-executor.test.ts
- skill-guides/orca-cli.md
- src/main/runtime/rpc/orchestration-legacy-compatibility-dispatcher.test.ts
- src/main/runtime/orchestration/db/dispatch-depth.ts
- src/main/runtime/rpc/methods/orchestration-worker-release-liveness-verdict.test.ts
- src/main/runtime/orchestration/db/schema/create-core-tables-sql.ts
- skills/orchestration/SKILL.md
- src/main/runtime/rpc/methods/orchestration-worker-observation.test.ts
- src/shared/runtime-types.ts
- src/cli/specs/core.ts
- src/main/runtime/pty-inventory-liveness-verdict.test.ts
- src/main/runtime/rpc/methods/orchestration-worker-start-schema.ts
- src/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-outcome.ts
- src/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-abandon.ts
- src/main/runtime/orchestration/lifecycle-reconciliation.test.ts
- src/main/runtime/rpc/methods/orchestration-worker-launch-preferences.test.ts
- src/main/runtime/orchestration/worker-provider-session.test.ts
- src/main/runtime/orchestration/db/orchestration-db-methods.ts
- src/main/runtime/orchestration/worker-report-observation.ts
- src/main/runtime/orchestration/worker-transcript-read.ts
- src/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-authority.ts
- src/main/runtime/orchestration/db/dispatch-row-writer.ts
- src/shared/orchestration-rpc-contract.ts
- src/main/runtime/orchestration/db/contract-constants.ts
- src/main/runtime/orchestration/db/tasks/task-status-transition.ts
- src/main/runtime/rpc/methods/orchestration-worker-release-inventory.test.ts
- src/main/runtime/orchestration/db/worker-terminal/worker-terminal-user-takeover.ts
- src/main/runtime/rpc/methods/orchestration-tasks-dispatch.test.ts
- src/main/sqlite/sync-database.test.ts
- src/shared/orchestration-fleet-projection.ts
- src/main/runtime/rpc/methods/orchestration-federation.ts
- src/main/runtime/orchestration/mailbox-pointer-delivery-contract.ts
- src/main/runtime/orchestration/db/dispatch-context/dispatch-context-store.ts
- src/main/runtime/orchestration/db/attempt-observation-store.ts
- src/main/runtime/rpc/methods/orchestration-worker-release.ts
- src/main/runtime/rpc/methods/orchestration-federated-worker-release.ts
- src/main/runtime/rpc/methods/orchestration-federated-worker-start.ts
- src/main/runtime/orchestration/mailbox-pointer-batch.ts
- src/cli/handlers/orchestration/worker-launch-handler.ts
- src/main/runtime/orchestration/db/lifecycle-transition.ts
- src/main/runtime/orchestration/worker-attention-context.ts
- src/main/runtime/rpc/dispatcher.ts
- src/main/runtime/orchestration/db/tasks/task-store.ts
- src/main/runtime/rpc/methods/orchestration-federation-attachment-observation.ts
- src/cli/terminal-format.ts
- src/main/runtime/orchestration/federation-sync.test.ts
- src/main/runtime/rpc/methods/orchestration-worker-release.test.ts
- src/main/runtime/orchestration/coordinator-task-dispatch.ts
- src/shared/orchestration-fleet-worker-projection.ts
- src/main/runtime/rpc/methods/orchestration-federated-worker-start-receipts.ts
- src/main/runtime/rpc/orchestration-commit-notify-characterization.test.ts
- src/main/runtime/orchestration/worker-provider-session.ts
- src/main/runtime/rpc/methods/orchestration-composed-workers.test.ts
- resources/skills/current-manifest.json
- src/main/runtime/orchestration/db/worker-terminal/worker-terminal-resource-store.ts
- src/main/runtime/orchestration/orchestration-worker-dispatch-db.test.ts
- src/main/runtime/orchestration/db/worker-dispatch/worker-terminal-recovery.ts
- src/cli/handlers/orchestration/worker-terminal-handlers.ts
- src/main/runtime/orchestration/orchestration-version-skew-migration.test.ts
- src/cli/handlers/orchestration-worker-cli.test.ts
- src/main/runtime/rpc/methods/orchestration-worker-list-cursor.ts
- src/cli/handlers/terminal.ts
- src/main/runtime/orchestration/worker-transcript-remote-read.ts
- src/main/runtime/orchestration/db/worker-terminal/worker-terminal-listing.ts
- skill-guides/orchestration.md
- src/main/runtime/rpc/methods/orchestration-federation.test.ts
- src/cli/index.test.ts
- skill-guides/orchestration/references/legacy-contract-migration.md
- src/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-stop.ts
- src/main/runtime/orchestration/worker-transcript-remote-read.test.ts
- src/main/runtime/rpc/orchestration-mutation-executor.ts
- src/main/runtime/orchestration/federation-sync.ts
- skill-guides/orchestration/references/recovery-and-cleanup.md
- src/main/runtime/orchestration/worker-start-unobserved-prompt-settlement.test.ts
- src/main/runtime/orchestration/worker-terminal-ownership.ts
- config/scripts/generate-bundled-skill-guides.mjs
- src/main/runtime/rpc/terminal-prompt-delivery-receipt.test.ts
- src/main/runtime/orchestration/worker-output-archive.ts
- src/shared/runtime-terminal-contracts.ts
- src/main/runtime/rpc/methods/orchestration-local-worker-start.ts
- src/main/runtime/rpc/methods/orchestration-federated-worker-read.ts
- src/main/runtime/rpc/methods/orchestration-worker-control.ts
- src/main/runtime/rpc/methods/orchestration-worker-archive-read.ts
- src/main/runtime/orchestration/db/attempt-outcome-projection.ts
- src/main/runtime/orchestration/db/attach-orchestration-db-methods.ts
- src/main/runtime/rpc/methods/orchestration-federated-fleet-snapshot.ts
- src/main/runtime/rpc/methods/orchestration-worker-output.test.ts
- src/main/runtime/agent-prompt-submission-runtime.test.ts
- src/main/runtime/rpc/methods/orchestration-worker-release-recovery.test.ts
- src/renderer/src/store/slices/agent-status.ts
- src/main/runtime/orchestration/db/federation/federated-dispatch-store.ts
- src/main/runtime/rpc/methods/terminal/terminal-send-method.ts
- src/main/runtime/rpc/methods/orchestration-federated-fleet-snapshot.test.ts
- src/main/runtime/orchestration/db/dispatch-context/worker-report-settlement.ts
- src/main/runtime/orchestration/db/attempt-outcome-projection.test.ts
- src/main/runtime/orchestration/mailbox-owner.ts
- src/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-start.ts
- src/main/runtime/orchestration/db/attempt-observation-types.ts
- src/main/runtime/rpc/methods/orchestration-federated-worker-release-host.ts
- src/shared/orchestration-fleet-attention.ts
- src/main/runtime/rpc/methods/orchestration-worker-output.ts
- config/scripts/orchestration-skill-guidance.test.mjs
- src/main/runtime/orca-runtime.test.ts
- src/main/runtime/rpc/methods/orchestration-workers.ts
- src/main/runtime/orca-runtime.ts
- src/main/runtime/rpc/methods/orchestration-worker-observation.ts
- src/main/runtime/rpc/methods/orchestration-worker-start-prompt-contract.test.ts
- src/main/runtime/orchestration/mailbox-pointer-submit.ts
- src/main/runtime/orchestration/db/worker-dispatch/federated-worker-start-reconcile.ts
- src/main/runtime/orchestration/orchestration-peer-capability-cache.test.ts
- src/main/runtime/orchestration/mailbox-pointer-delivery.ts
- src/main/runtime/orchestration/mailbox-pointer-submit.test.ts
- src/main/runtime/orchestration-mailbox-notification-consistency.test.ts
- src/main/runtime/rpc/methods/orchestration-federation-liveness-verdict.test.ts
- src/main/runtime/rpc/methods/orchestration.ts
- src/main/runtime/rpc/methods/orchestration-worker-release-completion.ts
- src/main/runtime/orchestration/orchestration-peer-capability-cache.ts
- tests/e2e/orchestration-idle-mail-delivery.spec.ts
- src/main/runtime/rpc/methods/orchestration-federation-output.test.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review.
| const count = db.countWorkerTerminalResources({ | ||
| runId: params.run, | ||
| snapshot: args.snapshot, | ||
| terminalState: state | ||
| }) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Keep counts bound to the version-3 snapshot. args.snapshot is undefined for version-3 cursors, so page.total is frozen but counts changes with current inventory; count the retained filteredSnapshot.dispatchIds instead.
31e3454 to
64dfab2
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (2)
src/cli/handlers/terminal.ts (1)
111-126: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject value-less
--retry-requestand--wait-submit.
parseArgsstorestruewhen a value token is absent (src/cli/args.ts:89-133).getOptionalStringFlagandgetOptionalPositiveIntegerFlagboth discard that boolean (src/cli/flags.ts:23-29, 56-68).orca terminal send --text x --enter --retry-requesttherefore sends a prompt with no retry identity, which is the non-idempotent send this flag exists to prevent.--wait-submitwithout a value is dropped in the same way.🐛 Proposed fix
const promptCandidate = !!text && enter && !interrupt + if (flags.get('retry-request') === true || flags.get('wait-submit') === true) { + throw new RuntimeClientError( + 'invalid_argument', + '--retry-request and --wait-submit require a value; no input was sent.' + ) + } const retryRequest = getOptionalStringFlag(flags, 'retry-request')src/main/runtime/orca-runtime.ts (1)
36508-36511: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winGuard
makePaneKeyfor non-UUID leaf ids in the parked branch.Line 36511 passes
stagedLeaf.leafIdstraight tomakePaneKey.makePaneKeythrows for a legacy numeric leaf id, and every other call site in this file guards withisTerminalLeafIdfirst (for example lines 9063, 11518, 14761, and 34570). A throw here escapesresolveSubmitTarget, so the pointer submit fails and the messages are redriven without delivery. Guard the pane-key comparison instead of throwing.🛠️ Proposed guard
const pty = this.ptysById.get(ptyId) const leaf = parked ? pty?.connected && pty.tabId === stagedLeaf.tabId && - pty.paneKey === makePaneKey(stagedLeaf.tabId, stagedLeaf.leafId) + isTerminalLeafId(stagedLeaf.leafId) && + pty.paneKey === makePaneKey(stagedLeaf.tabId, stagedLeaf.leafId)
🧹 Nitpick comments (2)
src/main/runtime/orchestration/db/federation/remote-dispatch-attachment-release.ts (1)
20-20: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUse
runLifecycleWriteTransactionfor this lifecycle write.The current direct callers do not show an existing transaction, but this method can be called from one. Raw
BEGIN IMMEDIATEthen fails, andROLLBACKcan undo the caller's transaction. Use the helper and remove the per-branchCOMMITcalls so nested calls use savepoints.src/main/runtime/orchestration/mailbox-pointer-resume.ts (1)
17-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTrim the unused parameters from the resume signature.
The body uses only
deps,leaf, andmessages.state,mailboxHandle,enterDelayMs,leafKey,settle, andredriveare never read. The signature suggests that this function can settle flights and redrive mailboxes, which it does not do.♻️ Proposed signature reduction
export function resumePendingOrchestrationMailboxPointer< TWaiter extends OrchestrationMessageWaiter >(args: { deps: PointerDeliveryDependencies<TWaiter> - state: OrchestrationMailboxPointerState leaf: OrchestrationMailboxLeaf - mailboxHandle: string messages: readonly OrchestrationMailboxPointerMessage[] - enterDelayMs: number - leafKey: string - settle: (ptyId: string, flight: OrchestrationMailboxDeliveryFlight) => void - redrive: (mailboxHandle: string, force?: boolean) => void }): boolean {Then drop the matching arguments in
src/main/runtime/orchestration/mailbox-pointer-delivery.tslines 91-101 and remove the now-unusedmailbox-pointer-statetype import.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a9da1afd-2ecb-407f-a510-fde7224681c1
📒 Files selected for processing (245)
config/reliability-gates.jsoncconfig/scripts/generate-bundled-skill-guides.mjsconfig/scripts/generate-bundled-skill-guides.test.mjsconfig/scripts/orchestration-skill-guidance.test.mjsresources/skills/current-manifest.jsonresources/skills/snapshot-registry.jsonskill-guides/orca-cli.mdskill-guides/orchestration.mdskill-guides/orchestration/references/coordinator-loop.mdskill-guides/orchestration/references/legacy-contract-migration.mdskill-guides/orchestration/references/low-level-topology.mdskill-guides/orchestration/references/messaging-and-gates.mdskill-guides/orchestration/references/placement-and-remote.mdskill-guides/orchestration/references/recovery-and-cleanup.mdskill-guides/orchestration/references/worker-contract.mdskill-stubs/orchestration.mdskills/orchestration/SKILL.mdsrc/cli/args.tssrc/cli/bundled-skill-guides.tssrc/cli/handlers/orchestration-module-boundaries.test.tssrc/cli/handlers/orchestration-worker-cli.test.tssrc/cli/handlers/orchestration/worker-launch-handler.tssrc/cli/handlers/orchestration/worker-output.test.tssrc/cli/handlers/orchestration/worker-output.tssrc/cli/handlers/orchestration/worker-terminal-handlers.tssrc/cli/handlers/terminal.test.tssrc/cli/handlers/terminal.tssrc/cli/help.tssrc/cli/index.test.tssrc/cli/root-help-text-secondary.tssrc/cli/runtime/client-recovery.test.tssrc/cli/runtime/client.tssrc/cli/runtime/terminal-prompt-mutation-recovery.tssrc/cli/specs/core.tssrc/cli/specs/orchestration-worker-specs.tssrc/cli/specs/terminal-send.tssrc/cli/terminal-format.test.tssrc/cli/terminal-format.tssrc/main/index.tssrc/main/native-chat/host-readable-transcript-path.test.tssrc/main/native-chat/host-readable-transcript-path.tssrc/main/native-chat/session-file-resolver-wsl.test.tssrc/main/native-chat/session-file-resolver.tssrc/main/runtime/agent-prompt-receipt-correlation.test.tssrc/main/runtime/agent-prompt-submission-runtime.test.tssrc/main/runtime/agent-prompt-submission-verification.test.tssrc/main/runtime/agent-prompt-submission-verification.tssrc/main/runtime/orca-runtime.test.tssrc/main/runtime/orca-runtime.tssrc/main/runtime/orchestration-mailbox-cold-park-idle.test.tssrc/main/runtime/orchestration-mailbox-crash-recovery.test.tssrc/main/runtime/orchestration-mailbox-notification-consistency.test.tssrc/main/runtime/orchestration-mailbox-notification-test-harness.tssrc/main/runtime/orchestration-mailbox-pointer-cli-command.test.tssrc/main/runtime/orchestration-mailbox-transport-settlement.test.tssrc/main/runtime/orchestration-message-delivery-identity.test.tssrc/main/runtime/orchestration/cli-command.test.tssrc/main/runtime/orchestration/cli-command.tssrc/main/runtime/orchestration/context-only-dispatch-release.tssrc/main/runtime/orchestration/coordinator-runtime-contract.tssrc/main/runtime/orchestration/coordinator-task-dispatch.tssrc/main/runtime/orchestration/db-task-dispatch-invariant.test.tssrc/main/runtime/orchestration/db-task-dispatch-lifecycle-guards.test.tssrc/main/runtime/orchestration/db-task-dispatch-races.test.tssrc/main/runtime/orchestration/db-undelivered-mailboxes.test.tssrc/main/runtime/orchestration/db.tssrc/main/runtime/orchestration/db/attach-orchestration-db-methods.tssrc/main/runtime/orchestration/db/attempt-observation-store.tssrc/main/runtime/orchestration/db/attempt-observation-types.tssrc/main/runtime/orchestration/db/attempt-outcome-projection.test.tssrc/main/runtime/orchestration/db/attempt-outcome-projection.tssrc/main/runtime/orchestration/db/contract-constants.tssrc/main/runtime/orchestration/db/decision-gate-lifecycle.test.tssrc/main/runtime/orchestration/db/decision-gates/decision-gate-store.tssrc/main/runtime/orchestration/db/dispatch-context/dispatch-completion.tssrc/main/runtime/orchestration/db/dispatch-context/dispatch-context-store.tssrc/main/runtime/orchestration/db/dispatch-context/task-dispatch-reconciliation.tssrc/main/runtime/orchestration/db/dispatch-context/worker-report-settlement.tssrc/main/runtime/orchestration/db/dispatch-depth.tssrc/main/runtime/orchestration/db/dispatch-row-writer.tssrc/main/runtime/orchestration/db/federation/federated-dispatch-store.tssrc/main/runtime/orchestration/db/federation/remote-dispatch-attachment-authority.tssrc/main/runtime/orchestration/db/federation/remote-dispatch-attachment-release.tssrc/main/runtime/orchestration/db/lifecycle-transition-boundary.test.tssrc/main/runtime/orchestration/db/lifecycle-transition.test.tssrc/main/runtime/orchestration/db/lifecycle-transition.tssrc/main/runtime/orchestration/db/lifecycle-write-transaction-runner.tssrc/main/runtime/orchestration/db/messages/mailbox-pointer-enter-state.tssrc/main/runtime/orchestration/db/messages/message-inbox.tssrc/main/runtime/orchestration/db/messages/message-insert.tssrc/main/runtime/orchestration/db/mutation-receipts/mutation-receipt-store.tssrc/main/runtime/orchestration/db/orchestration-db-methods.tssrc/main/runtime/orchestration/db/reset/orchestration-reset.tssrc/main/runtime/orchestration/db/schema/create-core-tables-sql.tssrc/main/runtime/orchestration/db/schema/create-graph-tables-sql.tssrc/main/runtime/orchestration/db/schema/migrate-mailbox-pointer-enter-v33.tssrc/main/runtime/orchestration/db/schema/migrate-v13-v30.tssrc/main/runtime/orchestration/db/schema/migrate.tssrc/main/runtime/orchestration/db/schema/schema-column-probes.tssrc/main/runtime/orchestration/db/tasks/task-status-transition.tssrc/main/runtime/orchestration/db/tasks/task-store.tssrc/main/runtime/orchestration/db/worker-dispatch/federated-worker-start-reconcile.tssrc/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-abandon.tssrc/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-authority.tssrc/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-outcome.tssrc/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-stage.tssrc/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-start.tssrc/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-stop.tssrc/main/runtime/orchestration/db/worker-dispatch/worker-terminal-recovery.tssrc/main/runtime/orchestration/db/worker-terminal/worker-terminal-attention-query.tssrc/main/runtime/orchestration/db/worker-terminal/worker-terminal-inventory-counts.tssrc/main/runtime/orchestration/db/worker-terminal/worker-terminal-listing.tssrc/main/runtime/orchestration/db/worker-terminal/worker-terminal-resource-store.tssrc/main/runtime/orchestration/db/worker-terminal/worker-terminal-transfer.tssrc/main/runtime/orchestration/db/worker-terminal/worker-terminal-user-takeover.tssrc/main/runtime/orchestration/environment-transport.tssrc/main/runtime/orchestration/federation-sync.test.tssrc/main/runtime/orchestration/federation-sync.tssrc/main/runtime/orchestration/formatter.test.tssrc/main/runtime/orchestration/formatter.tssrc/main/runtime/orchestration/lifecycle-reconciliation.test.tssrc/main/runtime/orchestration/lifecycle-reconciliation.tssrc/main/runtime/orchestration/mailbox-owner.tssrc/main/runtime/orchestration/mailbox-pointer-batch.tssrc/main/runtime/orchestration/mailbox-pointer-delivery-contract.tssrc/main/runtime/orchestration/mailbox-pointer-delivery.tssrc/main/runtime/orchestration/mailbox-pointer-resume.tssrc/main/runtime/orchestration/mailbox-pointer-stage.tssrc/main/runtime/orchestration/mailbox-pointer-state.tssrc/main/runtime/orchestration/mailbox-pointer-submit.test.tssrc/main/runtime/orchestration/mailbox-pointer-submit.tssrc/main/runtime/orchestration/message-batch-atomicity.test.tssrc/main/runtime/orchestration/orchestration-peer-capability-cache.test.tssrc/main/runtime/orchestration/orchestration-peer-capability-cache.tssrc/main/runtime/orchestration/orchestration-reset-db.test.tssrc/main/runtime/orchestration/orchestration-schema-version-skew.tssrc/main/runtime/orchestration/orchestration-version-skew-migration.test.tssrc/main/runtime/orchestration/orchestration-worker-dispatch-db.test.tssrc/main/runtime/orchestration/preamble.test.tssrc/main/runtime/orchestration/r1-identity-migration.test.tssrc/main/runtime/orchestration/types.tssrc/main/runtime/orchestration/worker-attention-context.tssrc/main/runtime/orchestration/worker-output-archive.test.tssrc/main/runtime/orchestration/worker-output-archive.tssrc/main/runtime/orchestration/worker-provider-session.test.tssrc/main/runtime/orchestration/worker-provider-session.tssrc/main/runtime/orchestration/worker-report-observation.tssrc/main/runtime/orchestration/worker-start-unobserved-prompt-settlement.test.tssrc/main/runtime/orchestration/worker-terminal-ownership.tssrc/main/runtime/orchestration/worker-terminal-release-reconciliation.tssrc/main/runtime/orchestration/worker-transcript-payload.test.tssrc/main/runtime/orchestration/worker-transcript-payload.tssrc/main/runtime/orchestration/worker-transcript-read.tssrc/main/runtime/orchestration/worker-transcript-remote-read.test.tssrc/main/runtime/orchestration/worker-transcript-remote-read.tssrc/main/runtime/pty-inventory-liveness-verdict.test.tssrc/main/runtime/rpc/core.tssrc/main/runtime/rpc/dispatcher-caller-fingerprint.tssrc/main/runtime/rpc/dispatcher-unary-method-invocation.tssrc/main/runtime/rpc/dispatcher.tssrc/main/runtime/rpc/methods/orchestration-composed-workers.test.tssrc/main/runtime/rpc/methods/orchestration-federated-fleet-snapshot.test.tssrc/main/runtime/rpc/methods/orchestration-federated-fleet-snapshot.tssrc/main/runtime/rpc/methods/orchestration-federated-release-safety.test.tssrc/main/runtime/rpc/methods/orchestration-federated-transport-safety.test.tssrc/main/runtime/rpc/methods/orchestration-federated-worker-read.tssrc/main/runtime/rpc/methods/orchestration-federated-worker-release-host.tssrc/main/runtime/rpc/methods/orchestration-federated-worker-release.tssrc/main/runtime/rpc/methods/orchestration-federated-worker-start-receipt.test.tssrc/main/runtime/rpc/methods/orchestration-federated-worker-start-receipts.tssrc/main/runtime/rpc/methods/orchestration-federated-worker-start.tssrc/main/runtime/rpc/methods/orchestration-federation-attachment-observation.tssrc/main/runtime/rpc/methods/orchestration-federation-control.tssrc/main/runtime/rpc/methods/orchestration-federation-liveness-verdict.test.tssrc/main/runtime/rpc/methods/orchestration-federation-output.test.tssrc/main/runtime/rpc/methods/orchestration-federation-release-recovery-scenarios.tssrc/main/runtime/rpc/methods/orchestration-federation.test.tssrc/main/runtime/rpc/methods/orchestration-federation.tssrc/main/runtime/rpc/methods/orchestration-local-worker-start.tssrc/main/runtime/rpc/methods/orchestration-runs.test.tssrc/main/runtime/rpc/methods/orchestration-tasks-dispatch.test.tssrc/main/runtime/rpc/methods/orchestration-worker-archive-read.tssrc/main/runtime/rpc/methods/orchestration-worker-control.tssrc/main/runtime/rpc/methods/orchestration-worker-launch-preferences.test.tssrc/main/runtime/rpc/methods/orchestration-worker-legacy-federated-read.tssrc/main/runtime/rpc/methods/orchestration-worker-list-cursor.tssrc/main/runtime/rpc/methods/orchestration-worker-list-method.tssrc/main/runtime/rpc/methods/orchestration-worker-list-pagination.test.tssrc/main/runtime/rpc/methods/orchestration-worker-list-projection.tssrc/main/runtime/rpc/methods/orchestration-worker-list-snapshot-store.tssrc/main/runtime/rpc/methods/orchestration-worker-observation.test.tssrc/main/runtime/rpc/methods/orchestration-worker-observation.tssrc/main/runtime/rpc/methods/orchestration-worker-output.test.tssrc/main/runtime/rpc/methods/orchestration-worker-output.tssrc/main/runtime/rpc/methods/orchestration-worker-release-archive.test.tssrc/main/runtime/rpc/methods/orchestration-worker-release-completion.tssrc/main/runtime/rpc/methods/orchestration-worker-release-inventory.test.tssrc/main/runtime/rpc/methods/orchestration-worker-release-liveness-verdict.test.tssrc/main/runtime/rpc/methods/orchestration-worker-release-recovery.test.tssrc/main/runtime/rpc/methods/orchestration-worker-release-schemas.tssrc/main/runtime/rpc/methods/orchestration-worker-release-test-harness.tssrc/main/runtime/rpc/methods/orchestration-worker-release.test.tssrc/main/runtime/rpc/methods/orchestration-worker-release.tssrc/main/runtime/rpc/methods/orchestration-worker-start-prompt-contract.test.tssrc/main/runtime/rpc/methods/orchestration-worker-start-schema.tssrc/main/runtime/rpc/methods/orchestration-worker-stop.tssrc/main/runtime/rpc/methods/orchestration-worker-terminal-resource-presentation.tssrc/main/runtime/rpc/methods/orchestration-workers-recovery.test.tssrc/main/runtime/rpc/methods/orchestration-workers.tssrc/main/runtime/rpc/methods/orchestration.tssrc/main/runtime/rpc/methods/terminal/terminal-prompt-replay-observation.tssrc/main/runtime/rpc/methods/terminal/terminal-send-method.tssrc/main/runtime/rpc/methods/terminal/unary-schemas.tssrc/main/runtime/rpc/orchestration-commit-notify-characterization.test.tssrc/main/runtime/rpc/orchestration-legacy-compatibility-dispatcher.test.tssrc/main/runtime/rpc/orchestration-mutation-executor.test.tssrc/main/runtime/rpc/orchestration-mutation-executor.tssrc/main/runtime/rpc/orchestration-mutation-receipt.tssrc/main/runtime/rpc/terminal-prompt-delivery-receipt.test.tssrc/main/runtime/terminal-send-stale-leaf-liveness.test.tssrc/main/sqlite/sync-database.test.tssrc/main/sqlite/sync-database.tssrc/renderer/src/store/slices/agent-status-runtime-orchestration.test.tssrc/renderer/src/store/slices/agent-status.tssrc/shared/agent-status-ipc-payload.tssrc/shared/agent-status-types.tssrc/shared/orchestration-fleet-attention.test.tssrc/shared/orchestration-fleet-attention.tssrc/shared/orchestration-fleet-projection.test.tssrc/shared/orchestration-fleet-projection.tssrc/shared/orchestration-fleet-status-index.tssrc/shared/orchestration-fleet-worker-projection.tssrc/shared/orchestration-rpc-contract.tssrc/shared/orchestration-worker-output.tssrc/shared/pane-agent-identity-inventory.test.tssrc/shared/protocol-version.tssrc/shared/runtime-terminal-contracts.tssrc/shared/runtime-types.tstests/e2e/helpers/orchestration-mail-pane-agent.tstests/e2e/helpers/orchestration-mail-store.tstests/e2e/orchestration-idle-mail-delivery.spec.tstests/e2e/orchestration-idle-mail-restore.spec.tstests/e2e/orchestration-worker-terminal-visibility.spec.tstests/e2e/terminal-send-agent-prompt-submit.spec.tstests/tools/repro-terminal-send-submit.mjs
🚧 Files skipped from review as they are similar to previous changes (133)
- src/main/runtime/rpc/methods/orchestration-worker-launch-preferences.test.ts
- src/shared/orchestration-rpc-contract.ts
- src/main/sqlite/sync-database.ts
- resources/skills/current-manifest.json
- src/main/runtime/rpc/methods/terminal/unary-schemas.ts
- src/main/runtime/rpc/methods/terminal/terminal-prompt-replay-observation.ts
- src/main/runtime/rpc/methods/orchestration-worker-release-liveness-verdict.test.ts
- src/renderer/src/store/slices/agent-status-runtime-orchestration.test.ts
- src/main/runtime/orchestration/db/dispatch-depth.ts
- skill-guides/orca-cli.md
- src/shared/runtime-types.ts
- src/main/runtime/rpc/methods/orchestration-worker-list-cursor.ts
- src/main/runtime/rpc/methods/orchestration-workers-recovery.test.ts
- src/main/runtime/rpc/methods/orchestration-worker-start-schema.ts
- src/main/runtime/rpc/dispatcher-caller-fingerprint.ts
- src/shared/agent-status-ipc-payload.ts
- src/cli/root-help-text-secondary.ts
- src/main/runtime/rpc/methods/orchestration-federated-worker-read.ts
- src/renderer/src/store/slices/agent-status.ts
- src/main/runtime/orchestration/db/worker-terminal/worker-terminal-user-takeover.ts
- src/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-abandon.ts
- src/main/runtime/rpc/orchestration-legacy-compatibility-dispatcher.test.ts
- src/main/runtime/rpc/methods/orchestration-federation-attachment-observation.ts
- src/main/runtime/orchestration/mailbox-pointer-delivery-contract.ts
- src/main/runtime/orchestration/worker-attention-context.ts
- src/shared/orchestration-fleet-attention.test.ts
- src/main/runtime/orchestration/db/decision-gate-lifecycle.test.ts
- src/shared/agent-status-types.ts
- src/main/runtime/rpc/methods/orchestration-worker-release-schemas.ts
- src/main/runtime/orchestration/mailbox-pointer-batch.ts
- src/main/runtime/orchestration/db/worker-terminal/worker-terminal-attention-query.ts
- src/shared/runtime-terminal-contracts.ts
- src/main/runtime/rpc/methods/orchestration-runs.test.ts
- src/main/runtime/orchestration/db/schema/create-graph-tables-sql.ts
- src/main/runtime/orchestration/db/federation/federated-dispatch-store.ts
- src/main/runtime/orchestration/db/lifecycle-transition-boundary.test.ts
- src/main/runtime/orchestration/db/dispatch-context/task-dispatch-reconciliation.ts
- src/main/runtime/orchestration/worker-start-unobserved-prompt-settlement.test.ts
- src/main/runtime/orchestration/db/worker-dispatch/federated-worker-start-reconcile.ts
- src/cli/specs/orchestration-worker-specs.ts
- skill-guides/orchestration/references/placement-and-remote.md
- src/main/runtime/rpc/methods/terminal/terminal-send-method.ts
- config/scripts/orchestration-skill-guidance.test.mjs
- src/main/runtime/orchestration/orchestration-version-skew-migration.test.ts
- skill-guides/orchestration/references/recovery-and-cleanup.md
- src/main/runtime/rpc/methods/orchestration-worker-release-inventory.test.ts
- skill-guides/orchestration/references/low-level-topology.md
- src/main/runtime/rpc/methods/orchestration-composed-workers.test.ts
- src/main/runtime/orchestration/orchestration-peer-capability-cache.test.ts
- src/shared/orchestration-fleet-worker-projection.ts
- src/main/runtime/orchestration/worker-terminal-ownership.ts
- src/cli/args.ts
- src/main/runtime/orchestration/orchestration-worker-dispatch-db.test.ts
- src/main/runtime/orchestration/worker-provider-session.test.ts
- src/main/sqlite/sync-database.test.ts
- src/main/runtime/orchestration/coordinator-runtime-contract.ts
- src/main/runtime/orchestration/db/reset/orchestration-reset.ts
- src/main/runtime/rpc/methods/orchestration-worker-list-method.ts
- src/cli/handlers/orchestration/worker-launch-handler.ts
- resources/skills/snapshot-registry.json
- src/main/runtime/rpc/methods/orchestration-tasks-dispatch.test.ts
- src/shared/orchestration-fleet-projection.ts
- src/main/runtime/orchestration/db/dispatch-row-writer.ts
- src/main/runtime/orchestration/db.ts
- src/main/runtime/orchestration/federation-sync.test.ts
- src/main/runtime/orchestration/coordinator-task-dispatch.ts
- src/main/runtime/rpc/methods/orchestration-worker-output.ts
- src/main/runtime/orchestration/db/dispatch-context/dispatch-context-store.ts
- src/cli/specs/terminal-send.ts
- src/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-authority.ts
- src/main/runtime/orchestration/lifecycle-reconciliation.test.ts
- src/main/runtime/orchestration/db/tasks/task-store.ts
- src/main/runtime/rpc/methods/orchestration-worker-control.ts
- skill-guides/orchestration/references/legacy-contract-migration.md
- src/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-outcome.ts
- src/main/runtime/orchestration/db/worker-terminal/worker-terminal-resource-store.ts
- src/main/runtime/rpc/methods/orchestration-federated-worker-start-receipts.ts
- src/main/runtime/rpc/methods/orchestration-worker-archive-read.ts
- src/main/runtime/orchestration/worker-report-observation.ts
- src/main/runtime/rpc/methods/orchestration-local-worker-start.ts
- src/cli/specs/core.ts
- src/shared/orchestration-fleet-attention.ts
- tests/e2e/helpers/orchestration-mail-pane-agent.ts
- skill-guides/orchestration.md
- src/main/runtime/orchestration/db/decision-gates/decision-gate-store.ts
- src/main/runtime/orchestration/db/lifecycle-transition.test.ts
- src/main/runtime/orchestration/worker-transcript-remote-read.ts
- src/main/runtime/rpc/methods/orchestration-worker-list-snapshot-store.ts
- src/shared/orchestration-fleet-status-index.ts
- src/main/runtime/orchestration/mailbox-owner.ts
- src/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-stop.ts
- skill-stubs/orchestration.md
- src/cli/terminal-format.ts
- src/main/runtime/orchestration/db-task-dispatch-invariant.test.ts
- src/main/runtime/orchestration/db/attempt-observation-types.ts
- src/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-stage.ts
- src/main/runtime/rpc/methods/orchestration-workers.ts
- src/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-start.ts
- src/main/runtime/orchestration/lifecycle-reconciliation.ts
- config/scripts/generate-bundled-skill-guides.test.mjs
- src/main/runtime/orchestration/worker-output-archive.test.ts
- src/main/runtime/orchestration/db/worker-dispatch/worker-terminal-recovery.ts
- src/main/runtime/rpc/terminal-prompt-delivery-receipt.test.ts
- src/main/runtime/orchestration/worker-provider-session.ts
- config/scripts/generate-bundled-skill-guides.mjs
- src/main/runtime/rpc/methods/orchestration-federation.test.ts
- src/main/runtime/orchestration/orchestration-peer-capability-cache.ts
- src/main/runtime/orchestration/db/attempt-outcome-projection.ts
- src/main/runtime/orchestration/db/attempt-outcome-projection.test.ts
- src/main/runtime/rpc/methods/orchestration-worker-release.ts
- skills/orchestration/SKILL.md
- src/main/runtime/rpc/methods/orchestration-worker-release-test-harness.ts
- src/main/runtime/orchestration/db-task-dispatch-lifecycle-guards.test.ts
- src/cli/terminal-format.test.ts
- src/main/runtime/orchestration/db/tasks/task-status-transition.ts
- src/main/runtime/agent-prompt-submission-verification.ts
- src/main/runtime/rpc/methods/orchestration-federation-control.ts
- src/main/runtime/orchestration/db/attempt-observation-store.ts
- src/shared/protocol-version.ts
- src/main/runtime/rpc/methods/orchestration-federated-worker-release.ts
- src/main/runtime/orchestration/context-only-dispatch-release.ts
- src/main/runtime/rpc/methods/orchestration-worker-release-completion.ts
- src/main/runtime/rpc/methods/orchestration-worker-start-prompt-contract.test.ts
- src/main/runtime/rpc/methods/orchestration-federated-fleet-snapshot.ts
- src/main/runtime/rpc/methods/orchestration-federation-output.test.ts
- src/main/runtime/orchestration/db/dispatch-context/worker-report-settlement.ts
- src/main/runtime/orchestration/db/dispatch-context/dispatch-completion.ts
- src/main/runtime/orchestration/orchestration-schema-version-skew.ts
- src/main/runtime/rpc/methods/orchestration-worker-observation.test.ts
- tests/e2e/orchestration-idle-mail-delivery.spec.ts
- src/main/runtime/rpc/orchestration-mutation-executor.ts
- src/main/runtime/orchestration/orchestration-reset-db.test.ts
- src/shared/orchestration-fleet-projection.test.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.
| const stagedIds = new Set(ids) | ||
| let changed = 0 | ||
| for (const message of this.messages) { | ||
| if (stagedIds.has(message.id) && message.read === 0) { | ||
| message.pointer_enter_pending = 1 | ||
| message.pointer_pty_id = target.ptyId | ||
| message.pointer_process_incarnation = target.processIncarnation | ||
| changed += 1 | ||
| } | ||
| } | ||
| return changed === ids.length |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Preserve an existing pointer reservation.
Line 1245 overwrites an unread message that is already pointer-pending. A second target can replace the original ptyId and processIncarnation, so its later release cannot match or clear the reservation. Require pointer_enter_pending === 0 before staging.
From the #15190 map: what this PR closes, and a Windows QA offerI really like the direction here — durable receipts instead of keystroke inference is the right foundation. I've been tracking every stage of the orchestration loop in #15190, and this single PR moves more of that map than everything else combined since it was opened. Measured against the reports filed from my sessions:
If a Windows QA pass would help, this machine reproduced most of the failures above (Windows 11, multi-orchestrator, Claude + Codex + Grok lanes) and I'd be glad to rerun those measured scenarios on this branch — just say the word and whether to wait for the rebase. Three notes from reading the diff, none overlapping the existing review threads: 1. A worker whose prompt never started a turn now looks healthy in the fleet. With 2. The coordinator inject drops the receipt the PR standardizes. 3. The v33 partial index can't be used by its own queries. |
`native-smoke (windows-latest)` fails ~4.5% of runs on `preserves a multiline argument through the compiled remote launcher` with "Test timed out in 15000ms" — on unrelated PRs, for reasons that have nothing to do with them. Across 176 sampled attempts it is the only red that job produced, and it hit seven different PRs in two days: #16900, #16904, #16915, #16955 (twice), #16979, #17014, #17085. The test is six process creations: powershell.exe forks csc.exe, then the freshly compiled orca.exe forks node.exe, twice. Hosted Windows runners periodically slow process creation down, and this test amplifies that far harder than anything else in the job. Comparing the 80 attempts where it ran under 3s against the 12 where it ran over 12s, its own median goes 2198ms -> 15917ms (7.2x) while the same file's powershell-only test moves 556 -> 686ms (1.2x), the cmd.exe and Git Bash process tests in the neighbouring file move 1.4x, and the other 35 files put together move 1.5x. Measured across those 176 attempts: 1881ms to 35438ms, p50 4264ms, correlation +0.881 with the job's total Vitest duration. 8 of 176 (4.5%) exceeded the 15s cap; 2 of 176 (1.1%) also exceeded the shared 30s testTimeout, so deleting the override and inheriting the config is not enough on its own. 60s clears all 176 with 1.7x headroom on the worst. This is slow, not hung. Every body here is synchronous spawnSync, so Vitest cannot interrupt one — the timer fires only after the body returns and the reported duration is real elapsed time. That is why a failure reads `× ... 22464ms` under `Test timed out in 15000ms`. The work finished; the stopwatch was short. Seven reruns at one identical head measured 2053 / 4680 / 5551 / 8732 / 13506 / 14868 / 21937ms — the last of those would have been red on code that had not changed. The 15s came from #8897, which raised this test off Vitest's built-in 5s default because the job then ran bare `pnpm vitest run`. #8909 landed 3h27m later and pointed the job at config/vitest.config.ts, which is the real fix for that. The constant stayed behind and has been the binding budget ever since.
34ada8d to
4a32d07
Compare
`native-smoke (windows-latest)` fails ~4.5% of runs on `preserves a multiline argument through the compiled remote launcher` with "Test timed out in 15000ms" — on unrelated PRs, for reasons that have nothing to do with them. Across 176 sampled attempts it is the only red that job produced, and it hit seven different PRs in two days: #16900, #16904, #16915, #16955 (twice), #16979, #17014, #17085. The test is six process creations: powershell.exe forks csc.exe, then the freshly compiled orca.exe forks node.exe, twice. Hosted Windows runners periodically slow process creation down, and this test amplifies that far harder than anything else in the job. Comparing the 80 attempts where it ran under 3s against the 12 where it ran over 12s, its own median goes 2198ms -> 15917ms (7.2x) while the same file's powershell-only test moves 556 -> 686ms (1.2x), the cmd.exe and Git Bash process tests in the neighbouring file move 1.4x, and the other 35 files put together move 1.5x. Measured across those 176 attempts: 1881ms to 35438ms, p50 4264ms, correlation +0.881 with the job's total Vitest duration. 8 of 176 (4.5%) exceeded the 15s cap; 2 of 176 (1.1%) also exceeded the shared 30s testTimeout, so deleting the override and inheriting the config is not enough on its own. 60s clears all 176 with 1.7x headroom on the worst. This is slow, not hung. Every body here is synchronous spawnSync, so Vitest cannot interrupt one — the timer fires only after the body returns and the reported duration is real elapsed time. That is why a failure reads `× ... 22464ms` under `Test timed out in 15000ms`. The work finished; the stopwatch was short. Seven reruns at one identical head measured 2053 / 4680 / 5551 / 8732 / 13506 / 14868 / 21937ms — the last of those would have been red on code that had not changed. The 15s came from #8897, which raised this test off Vitest's built-in 5s default because the job then ran bare `pnpm vitest run`. #8909 landed 3h27m later and pointed the job at config/vitest.config.ts, which is the real fix for that. The constant stayed behind and has been the binding budget ever since.
|
Review audit against head Fixed concrete findings:
Intentionally not changed:
All changes are pushed to the PR branch. |
bae6453 to
d4e6fb4
Compare
|
Rebased onto current |
ELI5
Orca now treats orchestration like a durable control plane instead of inferring success from terminal keystrokes. Agents can tell whether a prompt was accepted or a turn started, replay an ambiguous request without sending twice, and recover coordinator mail after a crash. Completed workers can be inspected, released, or retained, and their panes no longer auto-resume as if the work were still running.
What changed
run-create/use/current/show/listare the row without routing plumbing (home_database,coordinator_pane_key) and without the duplicatebindingobject.terminal sendreceipts are honest and idempotent.input_acceptedandturn_startedare the only stages;--wait-submitobserves without resending;--retry-request <uuid>replays the exact request against the same process incarnation. A transport timeout keeps the retry ID; only a different runtime answering strips it. Value-less or non-UUID--retry-requestis rejected on the CLI and the SSH shim.dispatch:mailboxes.lifecycle-transition.ts) with a table-driven test over every caller edge. Task reopen/overturn stays in the public contract. A PTY exit duringworker-stopis the stop succeeding, not a failure.worker-start(--speccreates Task + attempt in one call),worker-show,worker-read(provider transcript first, bounded terminal fallback with a typed reason, local/WSL/SSH),worker-stop,worker-abandon,worker-release,worker-retain,worker-list(rowid-fenced pagination, fleet liveness,attention, literalnextAction).decideWorkerTerminalRelease): only anownedresource can be settled, the archive is mandatory where reachable, and an owner whose process is proven exited can always get out ofretainedviaarchive_status: unavailable. User-taken-over, external, and transferred panes stay retained.live/unverifiable/exitedonly, from execution-host evidence. Fleet projection reads the evidence clock, not the relay delivery clock. A host-certified exit outranks the worker's settled state.unverifiablenever authorizes stop, abandon, retry, or release, in code or in the guide.method_not_foundso every shipped host keeps transcript-first output; exited remote workers are closed before being reported closed; epoch fencing holds across peer restart, downgrade, and pairing rotation; no per-second forced capability probe.lifecycle_transition_receiptsledger and five never-read v31 identity columns.dispatch:<id>mailboxes get a real consumer generation ondispatch_contextsandremote_dispatch_attachments, bumped and fenced in the same transaction on every re-attach (manual inject, worker-start, federated attach). A stale worker whose Dispatch moved to another process now getsconsumer_fencedinstead of silently acking the new worker's Delivery. Run mailboxes already worked this way.dispatch_contextsrecords its creator (creator_handle,creator_pane_key), so a coordinator's context-only self-dispatch is bookkeeping rather than a nesting parent; before this, one self-dispatch made every laterworker-startfrom that coordinator fail the depth cap. Pre-v37 rows keep counting (fails closed).checkfrom a process whose pane no longer holds the Dispatch, or whose last Attempt was abandoned/failed and moved to another terminal, getsconsumer_fencedinstead of an empty inbox that reads as "no mail yet".--peek/--allstay readable. A paneless caller still getsstable_pane_requiredwith the rebind recovery.process_exitedstage whose termination reason isunknown(a stop that was issued but never observed) projectsunverifiable, notexited. Federatedworker-showcarries the execution host's verdict and host kind instead of a local guess. A live, ready worker with nothing pending hasnextAction: nonerather than pointing at theworker-showthat produced it.workerShowkeepsdispatch.task_idnext totaskIdfor shipped CLIs.ask --jsonuses the standard{ok, result}envelope like every sibling verb.worker-start --spec, namesworker-listfor completion accounting, documents--retry-request/request-show/--wait-submit, and requires positive evidence before any stall action. The other seven guides get the same treatment in skills: rewrite and trim the seven non-orchestration guides (do not merge without Jinwoo) #18724, split out so this PR stays orchestration-only.rpc/methods/orchestration-*(126 flat files) regrouped intoorchestration/{worker,federation,messaging,runs,gates}/.Why
User reports showed the same boundary failures: false
agent_prompt_stalledcausing duplicate sends (#15180), coordinators unable to trust screen scrapes, cold-parked terminals receiving a pointer without the submit, settled workers accumulating as live tabs and auto-resuming after restart, and no way to tell a stalled worker from a working one.Linked issues
Fixes #15180. Fixes #17935 (orchestration skill description is 866 characters; a guard now caps every bundled skill at 1,024). Supersedes #17651 (fence folded in). Advances #16660, #16522, #14907, #13047.
Review record
This PR was reviewed adversarially after revival: eight independent lenses (lifecycle, mailbox, send, worker, federation, transcript, complexity, live ergonomics), each required to prove findings with a failing test. That produced 16 proven blockers, all fixed with red-then-green regression tests, followed by two re-review rounds and a third fix wave that caught 3 regressions introduced by the fixes and 7 fixes that missed their target; all closed. A final pass (five lenses incl. a live built-runtime smoke, then a re-review of the fix wave) found and fixed seven more, chiefly the stale-worker mailbox steal, the self-dispatch depth wedge, and the unproven-exit certification. Three independent Codex (gpt-6-astra) passes followed: the first found nothing new, the second found and fixed 3 defects (task-status reachability, WSL-local host classification, peer-capability epoch), the third found and fixed 6 (production PTY controller never installed settled writes, ambiguous in-flight pointer failures allowed duplicate replay, SSH/relay deadlines cut off a valid
--wait-submit, stop-vs-exit race during inspection, and two release-recovery paths for vanished or exited terminals). The full record (findings, proof tests, triage, declines with reasons) is archived outside the repo.Rework after the live smoke. A first live cross-host run on the shipped adhoc build (this Mac, a paired Windows host on the same build, a paired Mac on 1.4.195, and an SSH host) found a P1: a running local worker read
unverifiable/missing_statusbecause the fleet snapshot rows lacked the terminal handle the matcher keyed on. A 59-row failure table over every bug fixed during review showed the same two classes recurring: a fact dropped in transit through optional fields, and two authorities for one fact. Two blind designs (Opus, Codex) converged on the same mechanisms, and the scoped tranches landed here with red-then-green seam tests from the real producer to the real consumer, faults injected only at the transport or hook-ingest boundary:WriteSettlement(accepted | refused{reason} | unverifiable{reason, bytesHandedToTransport}) from the SSH multiplexer through daemon client, providers, controller, to pointer staging. No boolean, no rejection-as-third-state. The two silent degrades that fabricated a handoff are deleted; a provider that cannot settle refuses before any effect. Pointer text and Enter share the contract; a partial flush isunverifiable, neverrefused.binding: worker | pane | unresolved{reason},clock: observed | delivery) minted once at ingest, so a hook row captured on one process incarnation can never bind to a later dispatch on the same pane. The matcher's!worker.paneKey ||defaults are gone. One host-scope parser replaces two.capability_unsupportedfrom an old peer is no longer relabelledhost_unavailable; a producer census test asserts every agent-status consumer path projects a pane-only hook row aslive.Two ergonomics defects the second live run surfaced on a real database are fixed here too: a pre-v3 dispatch already marked
completedprojected asoutcome_unknown/requiresAction: trueforever (three copies of the outcome ladder disagreed on legacy rows; now one resolver, legacycompletedreadssucceededwith nothing to act on, legacyfailedstays actionable on the failure), and an unscopedworker-listenumerated the entire database (now defaults to the Run bound to the calling terminal,--runoverrides, and the receipt's additivescopefield says which).A third live round on the shipped adhoc build of
b082443e1f(same four hosts) plus an unscripted run in the user's own prompt style (a plain Claude Code shell,/orchestration, three workers, zero errors, bound-Run default confirmed) found two more branch defects, fixed with red-then-green tests: a worker freshly started on a paired server projectedunverifiable/host_indeterminatewithrequiresActionfor ~3 minutes, including after its ownworker_done, because the host's federation observation returnedmissing_liveness_verdictfor any PTY the liveness register had not yet swept (the host now reads a connected pane it owns locally aslive; disconnected or SSH-scoped panes stayunverifiable); and six pre-v3 completed rows still carried aninputcategory because settling through the task-status path orfailDispatchnever closed the Dispatch's pending question threads (both paths close them now, and schema v38 closes threads already pending on settled rows). The guide'sworker-startexamples now show--model sonnet, since an omitted model inherits the launcher's default.A Codex adversarial pass on the tranche diff found one real design hole (identity minted at read time instead of ingest, now closed) and two daemon settlement paths that threw instead of settling (fixed). Two
@ts-nocheckruntime mixins on these paths were extracted into checked modules; the repo-wide@ts-nocheckcount is unchanged at 171.Deletions during review: ~1,900 lines (write-only ledger, unread columns, dead v1 archive path, test harnesses shipped in prod, duplicated liveness and state-machine copies, self-capability checks that were compile-time true).
Testing
pnpm typecheck:tsc:node|cli|webcleanpnpm run check:code-quality:changed0 findings;check:react-doctor:changed0pnpm verify:bundled-skill-guides,verify:skill-bundle-manifestpnpm teston the integrated head: 72,332 pass / 292 skipped; the only failures were three non-PR files (two zsh live-shell suites hit a node-pty spawn-helper ENOENT while a concurrent native rebuild ran, 44/44 in isolation;release-checkout.unit.test.tsis a known 30 s load timeout that passes in isolation onorigin/maintoo).terminal-send-agent-prompt-submit:198, each shown failing identically on main (main's E2E workflow is red on its last 40 runs). The terminal-send spec is root-caused and fixed separately in fix(e2e): stop deleting a pre-created agent's setup report #18707. The Windows hook-service flake (test(cursor): widen Windows hook spawn budget to fix ETIMEDOUT flake #17721) and the federation load flake did not recur.pnpm exec vitest runover the skill gate files plussrc/cli,config/scripts,src/main/skillspass; live smoke on the built CLI ofskills get orchestrationand--full(7 references).orca-dev serve, isolated profile): canonical loop, stop, release, archive read, retry rejection, stale-handle check, SIGKILL-and-replay all verified with receipts0d465e7931(this Mac and a paired Windows host on the build, a paired Mac left on 1.4.195, an SSH host): local, paired-new, paired-old and SSH loops all settle; running workers readliveon every host andexitedafter release; the old peer readscapability_unsupportedand refuses release honestly. Injected 10 s relay stall with a send in flight: delivered exactly once after recovery, zero duplicates. Every liveness field across 104 receipts is onlylive/unverifiable/exited.b082443e1f(same hosts): every loop settles; 942 of 948 legacy completed rows read settled withrequiresAction: falsebefore the question-thread fix and all of them after;worker-listscope readsbound/flag/allcorrectly; 122 JSON receipts carry onlylive/unverifiable/exited. Unscripted prompt-style run: clean.2da076d4e9(this Mac and the paired Windows host, both updated): a freshly started Windows worker readsliveon the first fleet poll and on all 20 that follow, with nohost_indeterminateat any point, andexitedafter release; all 948 legacy completed rows readrequiresAction: falsewithnextAction: noneafter schema v38; every verdict across 60 receipts islive/unverifiable/exited.Notes
method_not_found-negotiated methods; one new Electron-only IPC channel (agentStatus:legacyWorkerTerminalResumeFence) never crosses the wire.unverifiable; the execution host stays authoritative.targetIdnow projects host idsshinstead of an empty string (remote-wire-compatibility rule 3, old clients decode the same field). A fleet pane key without a terminal handle is nowunidentifiablerather than matched by pane key alone.terminal createon a reconnecting SSH host reports an opaqueNo PTY provider for connection;terminal listreportsorphaned:falseandterminal closereportsptyKilled:truefor a pane whose relay is gone (orchestration's own projection readsunverifiablecorrectly at the same moment).worker-listmaterializes every dispatch row per call; a positive "agent absent" signal distinct from PTY liveness is a product decision left open (a headless fake agent never reacheslive, so itsnextActionstaysinspect); a context-only self-dispatch still lists asrole: workerinworker-list;dispatchtask-not-found / task-not-ready / inject-rejected still surface asruntime_error; task and inbox receipts still expose raw row columns. Deferred skill product decisions live on skills: rewrite and trim the seven non-orchestration guides (do not merge without Jinwoo) #18724.