Skip to content

fix(orchestration): accept worker reports without the dispatch capability - #23982

Merged
Jinwoo-H merged 7 commits into
mainfrom
worker-reports-without-capability
Sep 30, 2026
Merged

Jinwoo-H merged 7 commits into
mainfrom
worker-reports-without-capability

Conversation

@Jinwoo-H

@Jinwoo-H Jinwoo-H commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 13 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​329 $\color{#cf222e}{\Huge{\mathbf{−}}}$​276 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​53
Prod 17 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​255 $\color{#cf222e}{\Huge{\mathbf{−}}}$​207 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​48

ELI5

When a coordinator hands a task to a worker agent, Orca pastes a secret token into the worker's prompt, and the worker has to repeat it on every "I'm done", heartbeat, escalation, and question. The token only lives in the agent's conversation. It disappears when the conversation is compacted, never shows up in a re-rendered dispatch-show --preamble, and gets revoked by a stop that didn't actually stop anything. When that happens, the worker's "I'm done" is rejected forever and the task hangs.

This PR stops the Orca host from checking that token. It still checks everything it checked before (the report comes from the exact process Orca started in the worker's pane), and adds one new check that keeps the one real protection the token gave: a coordinator or another worker can't report as this worker.

What Changed

Before → after, as a coordinator and its workers see it:

Situation Before After
Worker runs worker_done / ask copied from dispatch-show --preamble (which never contains the token) Rejected: "The Dispatch capability is missing" Accepted
Worker's conversation was compacted and it retypes the command without --dispatch-capability Rejected, task hangs Accepted
Worker still passes the old flag (old preamble, old CLI) Accepted Accepted; the flag is ignored
Coordinator runs send --type worker_done --from <worker> from its own terminal Rejected only because it lacked the token Refused with consumer_fenced ("This terminal is a Run coordinator and cannot report as …"), nothing recorded
Another worker reports as this worker Rejected (no token) Refused with consumer_fenced
worker-stop on a tab Orca didn't open (stop ends as stop_unknown), then the worker reports Rejected: "capability is revoked"; task stuck blocked Accepted and settled; worker moves to succeeded/failed
Worker reports while a stop is actually in progress (stopping) Rejected (token revoked) Still refused, now with an explicit reason (dispatch_inactive: "is stopping; its worker cannot report until worker-stop settles")
A stale process left in the worker's pane (new incarnation) reports or asks Rejected Still rejected, now worker_identity_changed from send and ask, local and remote (local send said sender_not_assignee before)
ask from a pane whose process changed Accepted if there was no token (no process check at all) Rejected: ask now has the same process check as send

Mechanism:

  1. No token check. send-point-to-point.ts, ask-methods.ts, send-remote.ts and ask-remote.ts no longer read the token or capability_hash / capability_revoked_at; the existing exact-process check (isDispatchProcessCurrent locally, isRemoteAttachmentProcessCurrent on a connected server) decides for every row, including rows minted before the upgrade. verifyDispatchCapability and verifyRemoteAttachmentAuthority had no other callers and are deleted.
  2. Caller-pane fence (lifecycle-caller-fence.ts). Every CLI request already carries the calling terminal's ORCA_PANE_KEY / ORCA_TERMINAL_HANDLE (since fix(orchestration): preserve active workers across updates #11271). For worker_done, heartbeat, escalation, decision_gate and ask, the host resolves that pane to a live terminal on this host. It refuses only when that terminal is a different pane than --from and is a different orchestration party: a Run's coordinator or the worker of another active dispatch. The worker's own terminal passes even when its --from handle went stale. Anything that names no live pane here (no env, a stale handle after a remint, another Orca's pane, a scrubbed env) counts as absent, and a live pane that is not a party (an agent-teams teammate, the user's shell) passes. Accepted limit: a tmux server keeps the env of the pane that started it, so a worker inside a tmux server that was started from a live coordinator or worker pane is refused. Session callers are skipped because the session binding already fences them. assertCallerHandleMatchesEvidence is unchanged, so coordinator verbs keep today's behavior.
  3. One worker-state rule, local and remote (orchestration/worker-report-admission.ts, which also holds the settleable-state lists the settlement paths read). stopping refuses (dispatch_inactive); a replaced process refuses (worker_identity_changed); stop_unknown and start_unknown accept, because an unproven stop or start is "unverifiable", not "exited" (docs/reference/ssh-execution-boundary.md). Settlement follows: settleWorkerReport treats stop_unknown like start_unknown (the settlement half of fix(orchestration): keep a wrongly-stopped worker's dispatch capability #21713, with a new stop_unknown → ready lifecycle edge), and a connected server's relay settle accepts start_unknown and stop_unknown as well as ready.

Unchanged on purpose: Orca still mints the token and prints the flag in new preambles (that is a follow-up PR), worker-stop still refuses to close tabs Orca doesn't own, and capability_revoked_at is still written.

Why

The token's only property is "the caller has read this conversation", which is exactly what breaks on compaction, truncation, re-rendering and secret scanners. The process check already covers a stale process. What the token additionally stopped in practice was a different agent, usually the coordinator, running the worker's command; the caller-pane fence keeps that without anything an agent has to remember.

Alternatives considered:

  • Deliver the token through the environment. Can't work: dispatches (dispatch --inject, worker-start --terminal, retries) are created after the agent process exists, and a process's environment is fixed at spawn.
  • Verify the per-launch token (ORCA_AGENT_LAUNCH_TOKEN) with hook attestation. Never rejects more than the plain pane check above, and returns nothing for hookless agents, agents before their first hook, reconnected SSH panes and structured workers. It adds a dependency, not protection.
  • Trust --from alone. Lets the coordinator report as the worker, recorded as the worker's own report.
  • OS peer credentials on the socket. Different on every OS, and SSH/WSL requests arrive through a relay or interop with no useful peer process.
  • Keep the token for remote workers only. Same bugs for the longest-lived workers, and two authorization models.
  • Reject whenever the env names any other live pane. Would refuse shared agent daemons and agent-teams teammates started from the worker's own pane; limiting the refusal to coordinator/worker panes cannot break those.

What we accept (fail-open, by design):

  • The fence trusts the caller's env. A coordinator or other worker whose ORCA_PANE_KEY / ORCA_TERMINAL_HANDLE is unset, stale, or names no live pane here is not fenced, and its report is then judged only by the process check on --from.
  • A Dispatch with no recorded process incarnation (an assignee with no stable pane, as before this PR) is authorized by handle ownership alone. That path predates this PR and is unchanged.

Security notes: a same-user process that fakes its env can still report as a worker, but it could already read the token from the worker's terminal (terminal reads are not redacted) or type into that terminal. The SSH relay CLI channel is reachable only by the same remote user (credential file 0600, socket under the user's own relay directory), and connected servers don't call orchestration.send on the Run home (the home pulls relay items), so the token was never a boundary there either.

Mixed versions

  • New host, old CLI: the CLI keeps sending the flag, which is ignored; it also sends the pane env the fence reads (every CLI since fix(orchestration): preserve active workers across updates #11271; the two days of CLIs before that fall through to the process check).
  • Old host, new CLI: unchanged, since this PR doesn't touch the CLI.
  • New host, dispatch minted before the upgrade: the hash is ignored; the process check and the fence apply.
  • Downgrade: an old host sees the hash it minted and the worker still has the token.
  • Connected servers in mixed versions: authorization is local to the host running the worker, so each host is self-consistent.
  • No new wire fields. Rejection text and codes change: a refused coordinator report is now a thrown consumer_fenced instead of a recorded dispatch_capability_invalid rejection, and the error code dispatch_capability_invalid remains defined for old hosts.

Linked Issue

Part of STA-8833 (item 1: the per-dispatch capability). Items 2 (Codex tab title) and 3 (unacknowledged-delivery warnings) are out of scope.

Fixes #21688, #21534, #20565, #16604, #15222

Refs #23656 (item 1 only), #17632 (the token stops being printed in a follow-up PR), #12769, #15124 (recovery through dispatch-show --preamble now works, but a truncated preamble still lacks the dispatch id), #16903, #13364 (a rejected worker_done still leaves the worker at ready; see #15265).

Visual Proof

N/A — orchestration authorization and state-machine change with no UI. The end-to-end CLI transcript below is the behavioral proof.

Testing

  • I manually tested these changes locally
  • Automated tests added/updated, or explained why not below

End to end, before and after (macOS, headless isolated runtime built from main at a0abcb6818b and from this branch; the worker is a fake Codex started in a tab, and every worker command runs inside the worker's own terminal with its real env; the coordinator's commands are typed into the coordinator's terminal):

Step main This PR
Coordinator runs send --type worker_done --from <worker> dispatch_capability_invalid (missing) consumer_fenced, task stays dispatched
Worker runs ask copied from dispatch-show --preamble dispatch_capability_invalid (missing) accepted
Worker runs the injected worker_done without the flag dispatch_capability_invalid, task dispatched completed
Worker runs worker_done copied from dispatch-show --preamble dispatch_capability_invalid accepted (duplicate settle)
worker-stop on a user-opened tab, then the injected worker_done stop → stop_unknown; report → "capability is revoked", task blocked stop → stop_unknown; report → completed, worker succeeded

Automated: new worker-report-authority.test.ts (15 cases, including the worker's own terminal with a stale --from handle, which fails without the same-dispatch check): coordinator and other-worker panes are fenced for both send and ask with nothing recorded; no env, own pane with a stale handle, a foreign pane, a stale handle, and a live non-party pane are all accepted; a stop in flight refuses reports and questions; succeeded and failed reports after stop_unknown settle; a stale process after stop_unknown is refused; remote stop_unknown and start_unknown reports settle, and a remote stop in flight refuses. The fence cases fail with the fence disabled and the stop_unknown success case fails without the new lifecycle edge. ask.test.ts gains the process check; existing tests that used the token as an oracle now assert dispatch state.

Commands run:

  • ./node_modules/.bin/vitest run --config config/vitest.config.ts src/main/runtime src/main/ssh src/main/ipc src/cli src/shared — 28,609 passed; the 2 failures are fish-shell tests that depend on the local fish install and import nothing changed here
  • pnpm run typecheck:tsc:node, pnpm run typecheck:tsc:cli, pnpm tc:node — clean
  • oxlint on changed files, pnpm run audit:anti-slop, pnpm run check:code-quality:changed — 0 findings

Windows ("windows 2", Git Bash, worktree at 3a5443552e, which includes this change): vitest run over src/main/runtime/orchestration, .../rpc/methods/orchestration/messaging, orchestration-task-list-failure-reason and orchestration-mutation-recovery — 144 files pass. orchestration-mutation-recovery.test.ts fails 3 cases with Windows argument quoting ("orca-dev" "orchestration" …); main's copy of that file fails the same 3 cases on the same machine, because the test doesn't pin process.platform.

Not tested: a live SSH/connected-server worker end to end. The remote path is covered by the DB/RPC tests above; the change has no platform-specific code.

Review

Agent skill upstream boundary

  • Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

Security, cross-platform, SSH, mobile, compatibility and performance are covered above. The fence costs two in-memory lookups per lifecycle message, plus a few indexed queries only when the caller's env names a live pane other than --from.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

…ability

Worker lifecycle reports and questions no longer depend on the per-dispatch
capability token that lives only in the agent's conversation. The host now:

- ignores capability_hash/capability_revoked_at for authorization on every
  row and checks the exact worker process instead (ask gains that check);
- refuses a report whose calling terminal is provably another orchestration
  party (a Run coordinator or another Dispatch's worker), treating env that
  names no live pane here as absent;
- applies one worker-state rule locally and remotely: a stop in flight
  refuses, while stop_unknown and start_unknown accept and settle.

Minting and printing the flag are unchanged, so an older host and older
preambles keep working.
@greptile-apps

greptile-apps Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 3/5

[High risk] Removes dispatch capability verification from worker reports.

The PR does not appear safe to merge while a caller without usable pane evidence can report as a worker.

Findings

  1. P1 Security Coordinator can impersonate worker reports ▶

Summary

This PR accepts worker lifecycle reports and questions without a dispatch capability, using process identity and a caller-pane fence instead. It also permits reports from workers in unproven start or stop states to settle their dispatches.

Reviews (7) · Last reviewed commit: "Merge remote-tracking branch 'origin/mai..."

Comment on lines +25 to +30
if (
!callerHandle ||
!callerPaneKey ||
(args.fromPaneKey && isEquivalentPaneKey(callerPaneKey, args.fromPaneKey))
) {
return

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 security Coordinator can impersonate worker reports

A coordinator can run orchestration send --from <worker> --type worker_done with its pane environment unset or stale. This fence then lets the request through because it cannot identify the coordinator’s pane. The later checks look up the pane and process for the declared worker handle, not the calling process, so the coordinator’s report can settle the worker’s task without the worker reporting.

How this was verified: The CLI accepts an explicit --from without pane environment, and the host checks lifecycle authority using that declared handle after the fence returns.

@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 32e426fb-3c8a-42a0-bc56-3652b74e89e3

📥 Commits

Reviewing files that changed from the base of the PR and between 2cf98e2 and 56d18dd.

📒 Files selected for processing (2)
  • src/main/runtime/rpc/methods/orchestration/messaging/send.test.ts
  • src/main/runtime/rpc/methods/orchestration/messaging/worker-report-authority.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/main/runtime/rpc/methods/orchestration/messaging/worker-report-authority.test.ts

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


📝 Walkthrough

Walkthrough

Messaging routes replace dispatch-capability authorization with caller fencing and worker-state and process-identity checks. Local and remote settlement paths accept applicable unknown worker states. Successful local reports can return unproven workers to ready.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 56d18

The updated tests align with process-based worker authority and settlement after unproven stops. No merge-blocking issue is established; normal checks remain appropriate.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 56d18

The replacement checks can accept a report in another worker’s name when caller identity is missing or stale. This threatens task-result integrity and can carry forged reports to a connected run home. The demonstrated scope requires access to the runtime and knowledge of a live worker handle; unauthenticated access or broader privilege escalation was not established.

Retained concerns

  • High · security · inferred: Capability-free compatibility calls can impersonate another active worker when caller evidence is absent or stale. The fence passes those cases, and subsequent process checks inspect the worker selected by from rather than independently authenticating the invoker. A caller with runtime access and a known live worker handle can consequently attempt forged completion, heartbeat, or question operations, including reports relayed to the worker’s run home. This is not limited to legacy dispatches without a process incarnation.
Security review details

Security Blast Radius

  • inferred — The identified path targets active dispatches whose worker handles can be named by a caller with runtime access. Repetition can affect multiple discoverable workers rather than only one task. For remote attachments, the consequence can propagate to their owning run homes through existing relay authority. Arbitrary tenant, host, secret-store, or environment access was not established.

Security Findings and Attack Paths

  • inferred — A compatibility caller supplies a victim worker’s from handle and omits caller evidence. The fence returns without rejection; the runtime then resolves the victim’s pane and current process. Those values can satisfy report admission and reach task settlement or question creation without the former dispatch-scoped capability. Tests confirm missing evidence is accepted, but a full adversarial transport invocation was not executed.

Trust Boundaries and Controls

  • observed — Known coordinator and sibling-worker caller panes are fenced before effects. Mismatched worker processes and stopping workers are also refused. These controls constrain stale-worker and identified cross-party calls, but do not authenticate an unresolved compatibility caller.

Resilience and Maintainability Implications

  • observed — Remote settlement retains atomic relay/attachment writes, contiguous home-import sequencing, and outcome-matching acknowledgment. These controls contain replay and partial-failure inconsistencies; they do not compensate for admitting a report under the wrong caller identity.

Hardening Proposals

  • proposed — Bind report authority to an authenticated invoking worker independently of from. Token-free recovery can use host-held caller attestation or another dispatch-bound credential outside conversation state; missing or stale identity should not authorize lifecycle mutation merely because the named worker is still running.
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR implements first-hand settlement from stop_unknown and start_unknown. It restores task and dispatch lifecycle state, clears last_error on successful re-entry, and rejects reports while `s… Retain local capability authority when a stop becomes unproven and the dispatch remains pending or dispatched. Revoke it after a proven stop or settlement, including the required racing-settlement behavior. Retain the remote `capability…
Docstring Coverage ⚠️ Warning Docstring coverage is 12.90% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 31 functions across 28 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely identifies the main change: worker reports can be accepted without the dispatch capability.
Description check ✅ Passed The description is detailed and follows the template. It explains the user impact, implementation, rationale, linked issues, testing, platform results, known failures, compatibility, security, and che…
Out of Scope Changes check ✅ Passed The process checks, caller-pane fence, worker-state admission, capability-free report and question paths, remote settlement changes, and related tests support the worker recovery and authority objecti…
Full details: Linked Issues check

Explanation

The PR implements first-hand settlement from stop_unknown and start_unknown. It restores task and dispatch lifecycle state, clears last_error on successful re-entry, and rejects reports while stopping. It does not meet the capability-state requirements in [#21688]. settleWorkerReportInTransaction still writes capability_revoked_at when it settles a report. beginRemoteAttachmentStop and failRemoteAttachment clear capability_hash before an unproven outcome, and no restoration occurs for an active stop_unknown attachment. The new tests cover report admission and settlement, but they do not verify local capability restoration or remote capability retention and clearing.

Resolution

Retain local capability authority when a stop becomes unproven and the dispatch remains pending or dispatched. Revoke it after a proven stop or settlement, including the required racing-settlement behavior. Retain the remote capability_hash for an unproven stop, clear it after a proven stop or settlement, and keep authority rejected while the state is stopping. Add automated assertions for these local and remote capability-state transitions. Preserve worker-abandon from stop_unknown.

✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR

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

❤️ Share

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

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes — initial review of the orchestration authorization refactor at 4dcc047.

  • Per-dispatch token check removed — verifyDispatchCapability and verifyRemoteAttachmentAuthority are deleted; send-point-to-point, ask-methods, send-remote, and ask-remote now authorize every row (including pre-upgrade rows) from the exact-process check alone. mintDispatchCapability/revokeDispatchCapability/capability_hash/capability_revoked_at are still written for the downgrade path.
  • Caller-pane fence added (lifecycle-caller-fence.ts) — refuses worker_done/heartbeat/escalation/decision_gate/ask with consumer_fenced only when the caller's env resolves to a live pane that is a different orchestration party (another Run coordinator or another Dispatch's assignee). Absent/scrubbed/foreign/stale env passes; session callers are skipped.
  • One worker-state rule (worker-report-admission.ts) — stopping refuses; stop_unknown/start_unknown accept as unverifiable; ask gains the process check that send already had.
  • stop_unknown settlement — settleWorkerReport reconnects stop_unknown like start_unknown, with a new worker stop_unknown → ready lifecycle edge and last_error: null on the reconnect.
  • Federation relay settle broadened — SETTLEABLE_REMOTE_ATTACHMENT_STATES (ready/start_unknown/stop_unknown) with a WHERE dispatch_id = ? AND state = ? optimistic guard.

I read the complete diff end to end, verified the callers of the removed/added symbols, and confirmed the three touched suites (worker-report-authority.test.ts 14, send.test.ts 34, ask.test.ts 18) pass. Two points I specifically probed and cleared: (1) the fence does not false-positive on a stale --from handle after a pane remint, because the runtime still resolves the retained pre-allocated handle to the current pane key; (2) no stop_unknown/start_unknown combination is left permanently unsettled in the current lifecycle. The residual same-user env-scrub bypass and the still-printed --dispatch-capability flag are both called out in the PR body as known/accepted or deferred, and the docs still describing the token belong to that deferred follow-up.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

…nt party

- One module owns the unproven/settleable worker states and the refusal rule; local send records it, ask and remote throw it. A stale process is worker_identity_changed on every path.
- The caller fence passes the worker's own terminal when its --from handle went stale.
- Document the shared-tmux-server limit; drop the dead dispatch_capability_invalid rejection member; tests assert dispatch state, not the capability column.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/main/runtime/rpc/methods/orchestration/messaging/worker-report-authority.test.ts (1)

124-133: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Rename the test to match its assertion.

The test title says the worker's own terminal is "not fenced". The test then expects worker_identity_changed rejection. The caller fence passes, but the process check rejects the report because term_worker no longer resolves. Rename the test to state both results. A future reader could otherwise read the rejection as a regression.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: de3959cc-4450-415b-8815-04711edcc7fc

📥 Commits

Reviewing files that changed from the base of the PR and between 244d8a2 and abfa3ef.

📒 Files selected for processing (22)
  • skill-guides/orchestration/references/worker-contract.md
  • src/cli/bundled-skill-guides.ts
  • src/main/runtime/orchestration/coordinator-dispatch-unobserved-prompt.test.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-task-dispatch-races.test.ts
  • src/main/runtime/orchestration/db/dispatch-context/worker-report-settlement.ts
  • src/main/runtime/orchestration/db/federation/federation-relay-enqueue.ts
  • src/main/runtime/orchestration/db/federation/federation-relay-item.ts
  • src/main/runtime/orchestration/lifecycle-reconciliation.test.ts
  • src/main/runtime/orchestration/lifecycle-reconciliation.ts
  • src/main/runtime/orchestration/worker-report-admission.ts
  • src/main/runtime/orchestration/worker-start-unobserved-prompt-settlement.test.ts
  • src/main/runtime/rpc/methods/orchestration/messaging/ask-methods.ts
  • src/main/runtime/rpc/methods/orchestration/messaging/ask-remote.ts
  • src/main/runtime/rpc/methods/orchestration/messaging/lifecycle-caller-fence.ts
  • src/main/runtime/rpc/methods/orchestration/messaging/send-point-to-point.ts
  • src/main/runtime/rpc/methods/orchestration/messaging/send-remote.ts
  • src/main/runtime/rpc/methods/orchestration/messaging/send.test.ts
  • src/main/runtime/rpc/methods/orchestration/messaging/worker-report-authority.test.ts
  • src/main/runtime/rpc/orchestration-runtime-update-settlement.test.ts
  • src/main/runtime/rpc/orchestration-task-dispatch-invariant.test.ts
💤 Files with no reviewable changes (6)
  • src/main/runtime/orchestration/lifecycle-reconciliation.ts
  • src/main/runtime/orchestration/db-task-dispatch-invariant.test.ts
  • src/main/runtime/orchestration/db-task-dispatch-races.test.ts
  • src/main/runtime/orchestration/coordinator-dispatch-unobserved-prompt.test.ts
  • src/main/runtime/orchestration/lifecycle-reconciliation.test.ts
  • src/main/runtime/rpc/orchestration-task-dispatch-invariant.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/main/runtime/orchestration/worker-start-unobserved-prompt-settlement.test.ts

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

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes — incremental delta since the prior pullfrog review at 4dcc047: 244d8a2 (fence message wording only) and abfa3ef (the consolidation below).

  • One worker-state module — worker-report-admission.ts moved up to orchestration/, now exporting UNPROVEN_WORKER_STATES (start_unknown|stop_unknown), SETTLEABLE_WORKER_STATES (ready|start_unknown|stop_unknown), isWorkerStateIn, and a single workerReportRefusal/assertWorkerCanReport. The duplicate SETTLEABLE_REMOTE_ATTACHMENT_STATES is gone; federation settle and ask/remote send now share the one rule.
  • Send-path authority collapsed into lifecycleRefusal — resolveLifecycleAuthority was rewritten to return LifecycleRefusal | null and delegate the stop/process decision to workerReportRefusal, dropping the last capability reads. Behavior is equivalent to 244d8a2 apart from the intended worker_identity_changed code/reason rename.
  • Fence covers the worker's own terminal — the caller fence early-returns when the caller's owned dispatch equals the one --from resolves to (new ownedDispatchId helper), so a stale --from handle is settled by the process check (worker_identity_changed) instead of being refused as consumer_fenced; only a different party is refused.
  • stop_unknown settles like start_unknown — new worker stop_unknown → ready lifecycle edge, settleWorkerReport reconnects stop_unknown through ready (last_error: null), and the relay settle accepts both unproven states with a WHERE ... state = ? optimistic guard.
  • Docs — the worker-contract reference (and generated bundle) now explains that send/ask returning consumer_fenced means the command ran from another party's terminal.

I read the full PR diff plus the incremental range-diff, verified the refactor is behavior-preserving against 244d8a2, confirmed no dangling references to the removed symbols, and ran the touched suites (7 files / 124 tests, including worker-report-authority 15, send 34, ask 18; plus 12 federation/settlement files / 62 tests) — all green. I traced the two disposition-changing questions with independent specialists: the fence's same-dispatch early return is closed for any recognized coordinator/other-worker party (the env-scrub bypass is pre-existing from 4dcc047 and already called out in the PR body, so not re-raised), and every reachable start_unknown/stop_unknown report now settles exactly once in the correct direction.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (1)

🟠 Major · Bind null-incarnation reports to an authenticated worker identity. · ask-methods.ts:78-82

src/main/runtime/rpc/methods/orchestration/messaging/ask-methods.ts:78-82
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

Bind null-incarnation reports to an authenticated worker identity.

A terminal can call orchestration.send with another worker’s from handle. When the dispatch has no process_incarnation, the current path treats that request as current. A forged worker_done report can therefore settle the worker’s task. orchestration.ask has the same bypass.

Require an exact process match or another independently authenticated worker identity at both admission points. Do not use the request’s from handle, pane key, or absent compatibility evidence as authority. Preserve unknown-start and unknown-stop recovery only when it supplies that independent identity; otherwise reject the report or question.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: daacb640-db2b-43db-aa6c-febb44102eec

📥 Commits

Reviewing files that changed from the base of the PR and between abfa3ef and 2cf98e2.

📒 Files selected for processing (2)
  • src/main/runtime/orchestration/db-task-dispatch-lifecycle-guards.test.ts
  • src/main/runtime/rpc/methods/orchestration/messaging/send.test.ts
💤 Files with no reviewable changes (1)
  • src/main/runtime/rpc/methods/orchestration/messaging/send.test.ts

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: a worker-stop that closed nothing still revokes the capability, muting a worker that is still running

1 participant