Skip to content

fix(agent-org): prevent Coordinator takeover and false delivery #995

Description

@ShiboSheng

Dependency and goal

Stack strictly on #763 / PR #991 at b96bd8b196e2c7fed6bcbcd05fd7261e3bf5e706, and land before #764. The Agent Org redesign rollout gate remains disabled by default until this fix and the later Group/projection gates pass.

This issue owns one safety correction: a Coordinator may orchestrate formal work, but must not poll, interrupt, cancel, take over, perform, and then self-certify the same work. It freezes four inseparable backend boundaries:

  1. Coordinator orchestration capability versus Member execution capability.
  2. Event-driven Coordinator waiting versus Provider/tool polling.
  3. Safe cancel/reassign handoff versus concurrent old/new side effects.
  4. Verified delivery evidence versus free-text completion claims.

The normative contract is the local design document docs/architecture/agent-org-long-lived-team-session-design.md, especially invariants 44–49 and §25.11A. Design remains the highest authority.

Evidence and root cause

The real-provider Team session Build Local Texas Holdem Game exposed the gap after PR8:

  • Coordinator performed 15 file-mutation tool calls while Implementer performed 25, so orchestration and execution were not actually separated.
  • Coordinator polled Task state 17 times with task_list, 5 times with task_get, and used 8 shell sleeps in a short interval instead of ending its Turn and waiting for durable events.
  • Coordinator cancelled all four formal Tasks while an Implementer was still producing workspace side effects, then continued the work itself.
  • Every observed Task cancellation was authored by Coordinator. The old TaskExecution continued writing after the durable Task had become Cancelled.
  • The run could remain at Finalizing or produce a success-like summary even though the formal board had no Completed Task/TaskOutput closure and all Tasks had been cancelled.

These are not prompt-only defects:

  • Coordinator is forbidden from owning a formal Task, but current tool assembly can still inherit ordinary SDE file/shell/browser capabilities.
  • The design previously rejected stale Task lifecycle/TaskOutput after cancel/reassign but explicitly allowed old workspace/external side effects to continue.
  • There is no hard rule that ends a Coordinator Turn after dispatch when no new work_revision or durable event exists.
  • Quiescence/Idle and a free-text completion call do not prove that the requested deliverable exists.

State after merge

  • A persisted coordinator Turn derives one CoordinatorOrchestration allowlist: bounded read, Task graph admin, Plan Approval, formal coordination, and completion request only.
  • Coordinator cannot write/move/delete files, run shell/process/tests/builds, operate browser/Computer Use, mutate external systems, use a generic delegate, own TaskExecution, or write TaskOutput.
  • After dispatch, a Coordinator Turn with no new durable event/revision terminates as waiting_for_org_event. Task, TaskOutput, Inbox, Plan, or user commits may coalesce into one new Root wake.
  • Cancelling or reassigning an in-progress Task atomically creates an execution handoff. A replacement remains Pending and cannot dispatch until the exact old runtime lease and Turn-owned processes are quiescent.
  • Only a backend-validated completion certificate may produce delivered. Idle, all-terminal Tasks, all-cancelled Tasks, Coordinator prose, and UI inference are not completion authority.

Implementation steps and authoritative entry points

1. Coordinator work profile

  • Derive CoordinatorOrchestration only from the persisted Agent Org Turn kind; do not persist a second role/capability flag.
  • Build a direct allowlist, not a denylist layered over the ordinary SDE registry.
  • Allow bounded transcript/Task/TaskOutput/Plan/Artifact/code reads, graph admin, Plan Approval, formal coordination, user reply, and completion request.
  • Exclude file mutation, arbitrary shell/process, test/build/deploy, browser/Computer Use, external mutation, generic delegate/subagent, TaskOutput write, and Task owner lifecycle.
  • Enforce the same decision in schema assembly, execute-time actor resolution, and the lowest Store/adapter boundary. Production Tauri, restart/recovery, debug/test entry points, and direct Tool construction use the same resolver.
  • Do not add a “read-only shell” classifier; arbitrary shell cannot be proven side-effect free from command text.

2. Event-driven Coordinator waiting

  • Persist the Coordinator trigger event and observed work_revision.
  • Permit one authoritative revision-keyed observation per trigger. Re-reading the same revision returns typed coordinator_no_new_work_facts guidance without repeated database queries.
  • When no committed fact changed, a no-progress breaker terminates the Turn with waiting_for_org_event.
  • Do not expose sleep, await-output, periodic Task polling, delayed self-message, per-Team timer, or periodic Provider wake.
  • Task/TaskOutput/Inbox/Plan event bursts coalesce to at most one Root wake. Working Watchdog remains a bounded repair safety net, not a model polling engine.

3. Safe Task cancel/reassign handoff

  • In one IMMEDIATE transaction, freeze the old in-progress Task, write structured cancel reason, create the Pending replacement when applicable, create its blocked assignment reference, and persist one TaskExecutionHandoffReceipt bound to old Task/owner/session/Turn/runtime lease/turn generation.
  • Receipt states are requested | yielding | released | timeout | unknown | failed; the receipt is not a new Task status.
  • Commit the fence before requesting Provider/process yield.
  • Every workspace-mutating TaskExecution tool revalidates Task/owner/Turn/lease/handoff immediately before its side effect.
  • Long-running processes are Turn-owned and stop as a process group.
  • Only after Provider terminal, exact release_if_current, zero Turn-owned processes, and no unknown external call may the receipt become Released and kick the existing replacement assignment/FIFO.
  • Record a 5-second SLO miss and enter durable Timeout/Unknown at 10 seconds. Timeout/Unknown keeps replacement blocked and waits for an explicit user resolution command; it never opens a second writer lane.
  • Restart resumes the same stop/wait receipt. It does not replay old or replacement side effects.

4. Verified completion

  • Treat a Coordinator completion call as a candidate, not authority.
  • In one IMMEDIATE transaction validate current Team, activation generation, work revision, all formal Turn/Inbox/recovery/handoff blockers, and the Task resolution closure.
  • A Task is satisfied only by Completed + TaskOutput, by a replaces_task_id chain ending in Completed + TaskOutput, or by a scoped user_scope_removed cancellation that cites the exact current-Team user EventStore event.
  • Every Task in the current work episode enters the closure by default. Do not add a model-controlled optional flag.
  • Persist one idempotent RunCompletionCertificate containing outcome, generation/revision, evidence Task/TaskOutput ids, bounded resolution links, validator version, and time.
  • delivered requires a complete closure and no blocker. Whole-episode user abandonment/Archive is cancelled; unresolved Failed/Unknown/needs-attention work is failed.
  • All-cancelled/all-failed/missing-output/stale-evidence/pending-handoff requests cannot become Delivered.
  • The final Coordinator summary references the committed certificate. Ordering remains certificate → assistant EventStore commit → Coordinator Turn terminal → Quiescence/Idle.

5. Wire, UI, observation, and recovery

  • Project Coordinator waiting_for_org_event separately from typing/running.
  • Project handoff waiting_for_previous_execution | released | timeout | unknown separately from Task status; do not optimistically show replacement Pending as running.
  • Show Delivered only with a certificate and bounded evidence. Cancelled/Failed/needs-attention must not use success copy.
  • PR8S only forbids uncertified success and projects certificate-backed outcomes. The unique FinalSummaryReceipt, bounded Finalizing lifecycle, terminal failure, and Retry belong to PR8F.
  • Record no-content metrics for tool denial, observation count, no-progress terminal, event-to-wake coalescing, old/new active workspace writers, handoff timing/outcome, certificate validation, and final commit ordering.

Scope boundaries

  • No Coordinator Task ownership or Coordinator TaskExecution.
  • No second Member runtime, scheduler, dispatcher, or workspace writer lane.
  • No new Task draining, revoked, waiting, waiting_for_user, or superseded state.
  • No rollback of side effects already committed before cancellation. Unknown non-idempotent outcomes remain visible and are not replayed automatically.
  • No arbitrary external exactly-once protocol.
  • No polling/background recovery loop.
  • No Group mention or peer Inbox source; feat(agent-org): [9/10] add multi-target Group ingress and linked Member Inbox #764 still owns those adapters.
  • Direct intervention, Team Pause/Archive, user Stop, and Task cancel keep distinct typed intents and receipts.
  • Transcript auto-scroll/typing-layout defects, repeated PR8 Return projection, and Planner plan-history visibility remain separately owned defects unless their producing invariant is directly changed here.

Failure scenarios that must be rejected or remain blocked

  • Coordinator receives a work tool through ordinary SDE fallback, stale registry cache, restart, direct Tool construction, or debug/test entry point.
  • Coordinator polls the same revision, sleeps, awaits output, self-wakes, or remains an active Provider with no new durable fact.
  • Cancel/replacement/handoff commits only partially.
  • Replacement starts before the old lease/process is released.
  • A stale old tool call produces a new side effect after the fence.
  • A stale release callback removes a newer runtime.
  • Timeout/Unknown automatically releases replacement or replays a non-idempotent side effect. “Keep stopped” cancels only the blocked replacement, does not restore the old Task, and leaves other Tasks running.
  • All Tasks Cancelled/Failed, missing TaskOutput, stale evidence, incomplete replacement closure, or pending handoff still produces Delivered.
  • Free text, Team Idle, all-terminal Tasks, or frontend state becomes success authority.
  • Ordinary SDE Send/Queue/Force Send/Stop/Resume/Compaction enters Agent Org tool/handoff/finality logic.

Acceptance

Deterministic owning-boundary tests

  • Coordinator schema/execute/adapter/Store allow-deny matrix, including restart, direct construction, and debug/test parity.
  • Trigger/revision observation idempotency, no-progress terminal, event burst coalescing, crash windows, and five-minute zero-wake/query/process checks.
  • Cancel/complete/reassign/Pause/Archive races; exact lease/generation CAS; process-group teardown; stale callback; timeout/unknown and explicit user resolution.
  • Active workspace-mutating TaskExecution count is always at most one across old/replacement owners.
  • Completion certificate all-valid/all-cancelled/all-failed/missing-output/stale-evidence/replacement/user-scope-removal matrices, response-loss replay, and EventStore/terminal ordering.
  • Canonical DDL/reset only while this rollout-gated data has no external compatibility promise; no ALTER TABLE or legacy probe unless preservation is explicitly requested.
  • Ordinary SDE has zero Agent Org context, receipt, query, listener, timer, or tool-profile change.

Real provider and packaged Tauri

Use a fresh isolated Team and a real provider to reproduce a realistic version of Build Local Texas Holdem Game:

  1. User asks Planner, Implementer, and Tester to build and verify a functioning local application.
  2. Coordinator creates and assigns formal Tasks, then becomes event-waiting; it never edits files, runs commands/tests, controls a browser, polls Tasks, or takes over.
  3. Implementer modifies the fixture and Tester runs the real app/tests. Tool trajectories and filesystem evidence belong to those Member TaskExecutions.
  4. While Implementer is deliberately slow, confirm five minutes of no Coordinator Provider/query/sleep activity until a durable progress/TaskOutput/Inbox event arrives.
  5. Cancel/reassign one in-progress Task through the real UI. The replacement stays Pending until the old runtime/process is released, and old/new active workspace writers never overlap.
  6. Exercise Timeout/Unknown resolution with visible Computer Use actions; debug endpoints may seed the isolated fault or read evidence only.
  7. Prove all-cancelled work cannot show Delivered; then complete a valid replacement closure with real TaskOutputs and prove exactly one certificate-backed delivery.
  8. Use Computer Use for every visible Send, Stop, Pause, Resume, Archive, Delete, confirmation, Session switch, and handoff-resolution action in the packaged Tauri App.
  9. Record database receipts, exact EventStore ordering, tool-schema/trajectory evidence, provider/runtime counts, process cleanup, screenshots, and ⌘5 request evidence.

Real-provider unavailability, quota exhaustion, or rate limiting is Blocked, not Pass.

Effort

Category P50 P90
Production 7,500 review lines 13,000 review lines
Tests / E2E / measurement 8,500 15,500
Old-path removal / locale / audit 1,500 3,000
Total 17,500 31,500
Substantive files about 95 about 145
Additional locale files up to 13 up to 13
Audit/evidence documents about 5 about 8

At 12,250 review lines (70% of P50), recheck scope and actual diff. If the forecast exceeds 31,500 review lines or 145 substantive files, sync Design/Issue and report before continuing. Above 47,250 review lines (P90 × 1.5), re-split the Impact Spike. If the work requires Group sources, a second runtime, a new Task state, external transaction compensation, more than three unplanned production files / 200 production lines, or total growth above 20%, stop and update the design and issue before implementation. Do not reduce authority, race, real-provider, packaged-app, SDE, or performance evidence to fit the estimate.

Plain-language contract

The Coordinator is the project manager, not a backup engineer. On a Timeout/Unknown handoff, Keep stopped cancels only that replacement while the rest of the Team continues. It can read the work, assign it, and verify real Member outputs. After assigning work it waits for an event instead of repeatedly asking whether the Member is done. If work must be reassigned, the old worker is safely stopped before the new worker starts. The Team may say “Delivered” only when the backend can point to completed Tasks and their actual outputs.

Metadata

Metadata

Assignees

Labels

Effort: HighagentAgent runtime, behavior, memory, providers, or orchestrationbugSomething isn't working

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions