Skip to content

feat: bound collaboration fan-out, wire the orchestrator's dispatch path, and surface goal cost at approve-time (P3 guards) - #267

Merged
saucam merged 2 commits into
mainfrom
feat/p3-guards-and-orchestrator-dispatch
Jul 28, 2026
Merged

feat: bound collaboration fan-out, wire the orchestrator's dispatch path, and surface goal cost at approve-time (P3 guards)#267
saucam merged 2 commits into
mainfrom
feat/p3-guards-and-orchestrator-dispatch

Conversation

@saucam

@saucam saucam commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

The two lightweight guards from docs/collaborative-session-design.md §11 P3 — plus the prerequisite the second one turned out to need, which was a real gap in the feature rather than scope creep.

Three commits, reviewable in order.


1. 740e9b2 — the tenant-wide live-children cap

MAX_COLLABORATION_CHILDREN caps ONE goal at 12. Nothing capped the number of goals. And role-children are long-lived send-driven sessions, not kind:"spawn" dispatch tasks — so dispatchActiveSpawnCount, the query behind dispatch.maxConcurrentWorkers, counts exactly zero of them. With rateLimit unlimited by design, ten collaborations meant 120 live autonomous agents bounded by nothing.

collaboration.maxLiveChildren (default 24 = two full-size fleets) closes it. It is a blast-radius bound, not a scheduler: it refuses a create that would cross the line and never queues, throttles, or kills anything running. A collaboration that exists but silently isn't working is harder to reason about than being told no, and the fix is one destroy away.

Two details that are decisions, not defaults:

  • The schema refuses any value between 1 and 11. A tenant cap below the per-goal cap would reject a single collaboration planChildren explicitly permits, leaving two bounds to contradict each other — the user told "max 12" by one and "over the limit" by the other.
  • An absent config means unlimited, not the default. loadConfig always populates 24; absent only happens for a hand-built config (a test, an embedder), and silently imposing a cap it never declared would be the surprising direction.

Checked before anything is built, so a refusal costs nothing to unwind.

2. 740e9b2 — the orchestrator's role-aware fleet surface

Design line 142 has always specified "a conductor-shaped Session whose fleet MCP surface gains role-aware delegation." It was never wired. #create passed fleet only for role:"conductor", while compileGoalPack told the orchestrator "Direct them with the fleet tools." It was instructed to use tools it did not have, and the coordination loop the blackboard exists to serve had no way to close.

This surfaced because guard 2 has no trigger without it: approve-time is the R3 gate on fleet_send, which an orchestrator could never reach.

The conductor's server could not be reused. It is ONE deliberate per-tenant privileged session; collaborations are user-created and many, so #buildFleetServer would let any orchestrator direct every session in the tenant. So: four tools scoped to the goal's own children, with the scoping in the deps rather than the tool descriptions — same doctrine as the blackboard service, so a second caller of those closures can't obtain an unscoped view by asking differently.

The omissions carry as much intent as the inclusions:

omitted why
fleet_spawn §2 fixes a goal's role bindings for its whole life, and the roster is what guard 1 counts — an orchestrator that could spawn would grow past a bound the owner set
fleet_find / fleet_recall / fleet_summary they query memory tenant-wide and use listSessions() only to label results, never bound them; and raw episode recall over its own children is a second, unscoped channel for exactly what the blackboard mediates (§4)
machine_map machine-wide repo topology is a conductor concern, not a goal's

The deps also pass no memory, so the excluded tools fail closed even if a future edit adds one back to the set — two independent reasons for them not to work.

fleet_tasks is filtered to the goal's own dispatches (new optional createdBy on dispatchListForTenant, filtered in SQL so limit still returns limit of the caller's own rows). The tenant board carries every other session's targets and result digests; it was the one place the scoping would otherwise have leaked.

Attribution is orchestrator:<goalId> — keyed to the goal, not to any per-boot identity, so it survives a restart the way the blackboard's authorSub does. The constitution now names the four tools it has and states spawn is absent, with a test tying the prose to ORCHESTRATOR_FLEET_TOOLS.

3. 0089d53 — the cost roll-up, on every approval surface

Cost on a dashboard is trivia. Cost on the button that authorizes more work is a control.

#collaborationCostRollup sums SessionUsage across the orchestrator and every live child, computed per request rather than stored — the child set changes over a goal's life (teardown, restart), so a counter would drift in both directions.

Three properties are the point:

  • Send-class only. On every Bash/Edit prompt the number becomes wallpaper and stops being read — that's how a cost display fails in practice.
  • Never fatal. A throw degrades to "no cost shown," never a failed approval. An approval path wedgeable by a cost display is worse than no display.
  • Same number, same words, everywhere. formatCollaborationCost lives in @codeoid/core and all three surfaces call it — web ApprovalBar, Telegram, TUI. Telegram matters most: it's where an owner approves from a phone, the surface least able to check spend elsewhere.

Additive on the wire, so the Rust codeoid-ui ignores the field until it chooses to render it. No lockstep.

Deliberately out of scope: no spend enforcement. The design puts "per-collaboration token budget" in P6 governance, and visibility before enforcement is the right order — you want real numbers in front of you before choosing a cap.


Verification

suite before after
daemon 2081 2149
core 136 140
web 370 370 (unchanged)

biome clean; web eslint exactly 30 warnings / 0 errors, byte-identical to main.

Note: the 0089d53 commit message says "2135 to 2148" — off by one, written before the built-server introspection test was added. The table above is the verified count. Left as-is rather than force-pushed: rewriting history to fix a stale number in prose would cancel a valid green CI run for no gain.

Eighteen mutations, each caught by exactly the tests that should catch it — cap never enforced, cap counted per-goal, cap leaking across tenants, refusing after building, schema floor removed, unscoped listSessions, spawn allowed, send target unchecked, interrupt unscoped, task board unfiltered, memory handed over, full tool set exposed, roll-up ignoring the goal/tenant filter, roll-up for non-collaborations, orchestrator omitted from the session count, send-class gate dropped, throw guard dropped, pick() ignoring the filter.

Five of those found defects or gaps rather than confirming coverage, which is the part worth reading:

  1. createdBy was silently clobbered by the conductor's own enqueue closure — tasks were misattributed and the fleet_tasks filter was defeated, handing the orchestrator the whole tenant board back. The as Parameters<...> cast used to force the field through was the tell; going direct to the dispatcher removed both.
  2. The memory assertion passed under a mutation that handed this.#memory straight to the orchestrator, because the shared harness injects no engine at all. It now sets a stub and asserts differentially against the conductor's deps — "the orchestrator doesn't get X" proves nothing unless the same run shows X was available to give.
  3. The send-class gate was untested — the first mutation pass caught nothing. Now driven through a real Session + MockSessionProvider.
  4. The throw guard was untested — same pass, same silence. Now covered.
  5. The tool-set test asserted only the exported constant, which would have passed while pick() silently ignored it. It now introspects the BUILT server's _registeredTools.

Live-verified over a real socket at a cap of 12: six collaborations fit, the seventh is refused with rate_limited and a message naming the count, the cap and the config key, no orphan orchestrator is left behind, and destroying a goal frees capacity.

Not in this PR

The rest of P3 — the dispatch barrier/join over a dispatch group, the panel phase kind, and orchestrator synthesis. Those are the N-way-join primitive and are substantial on their own; they now land on a fleet that is bounded, dispatchable, and cost-visible. Non-Claude orchestrators remain #245.

🤖 Generated with Claude Code

@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@saucam

saucam commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

pr-shepherd: CI is green at 0089d53 — no fixes were needed, nothing was pushed.

check result
daemon (lint / typecheck / test:coverage / build) SUCCESS
web (lint / typecheck / vitest / build) SUCCESS
Socket Security: Project Report SUCCESS
Socket Security: Pull Request Alerts SUCCESS

Reproduced locally as a cross-check on the same tree: biome clean (346 files), tsc --noEmit clean across root + protocol + core, and 2149 pass / 12 skip / 0 fail over src/tests src/daemon packages/protocol/src packages/core/src. The three error: lines in that output (network down, db gone, bad listener) are deliberate fixtures in push.test.ts / messages.test.ts, not failures.

The two flakiness candidates flagged at hand-off both passed on the first CI attempt with no retry — src/tests/collaboration.test.ts (second SessionManager over the same tmp sqlite + transcript dir) and src/tests/fleet-approval-gate.test.ts (until() 2s deadline). No settle-time or deadline changes were made, so nothing was loosened.

Branch is 1 commit behind main (b35aa3e, README-only). I deliberately did not rebase/force-push: that commit touches no file this PR touches, the PR is MERGEABLE, and pull_request CI checks out the head-into-base merge ref, so the green above already reflects the combined tree. Rebasing would only have cancelled a live, valid run. Rebase at merge time if you want linear history.

No review feedback to action: gemini-code-assist[bot] left only the sunset notice ("all code review activity has officially ceased"), and there are no inline review comments. Slack review request suppressed via --no-slack.

Merging is a human decision — over to you.

@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

saucam added 2 commits July 28, 2026 22:43
…atch path its constitution promised

Two of §11 P3's guards, plus the prerequisite the second one turned out to need.

## The tenant-wide live-children cap

`MAX_COLLABORATION_CHILDREN` caps ONE goal at 12. Nothing capped the number
of goals — and role-children are long-lived `send`-driven sessions, not
`kind:"spawn"` dispatch tasks, so `dispatchActiveSpawnCount` (the query
behind `dispatch.maxConcurrentWorkers`) counts exactly zero of them. With
`rateLimit` unlimited by design, ten collaborations meant 120 live
autonomous agents bounded by nothing.

`collaboration.maxLiveChildren` closes that. It is a blast-radius bound,
not a scheduler: it refuses a create that would cross the line and never
queues, throttles, or kills anything already running — a collaboration
that exists but silently isn't working is harder to reason about than
being told no, and the fix is one destroy away.

The schema refuses any value between 1 and 11. A tenant cap below the
per-goal cap would reject a single collaboration that `planChildren`
explicitly permits, leaving two bounds to contradict each other with the
user told "max 12" by one and "over the limit" by the other. Default 24 =
two full-size fleets. An ABSENT config means unlimited rather than the
default, so a hand-built config (a test, an embedder) never silently
acquires a cap it did not declare.

Checked before anything is built, so a refusal costs nothing to unwind.

## The orchestrator's role-aware fleet surface

Design line 142 has always called for "a conductor-shaped Session whose
fleet MCP surface gains role-aware delegation." It was never wired.
`#create` passed `fleet` only for `role:"conductor"`, while
`compileGoalPack` told the orchestrator "Direct them with the fleet
tools." It was instructed to use tools it did not have, and the
coordination loop the blackboard was built to serve had no way to close.

This came up because the second guard — a cost roll-up "surfaced at
approve-time" — has no trigger without it: approve-time IS the R3 gate on
`fleet_send`, which an orchestrator could never reach.

The conductor's server could not be reused. It is ONE deliberate
per-tenant privileged session; collaborations are user-created and many,
so handing an orchestrator `#buildFleetServer` would let any of them
direct every session in the tenant. So: four tools, scoped to its own
children, with the scoping in the DEPS rather than the tool descriptions —
same doctrine as the blackboard service, so a second caller of those
closures cannot get an unscoped view by asking differently.

The omissions carry as much intent as the inclusions:

- **no `fleet_spawn`** — §2 fixes a goal's role bindings for its whole
  life, and the roster is what the cap above counts, so an orchestrator
  that could spawn would grow past a bound the owner set.
- **no `fleet_find`/`fleet_recall`/`fleet_summary`** — these query memory
  tenant-wide and use `listSessions()` only to LABEL results, never to
  bound them, so an orchestrator holding them could recall episodes from
  outside its goal. They would also undercut §4: raw episode recall over
  its own children is a second, unscoped channel for exactly the material
  the blackboard mediates. The deps pass no `memory` either, so they fail
  closed even if a future edit adds one back to the set.
- **no `machine_map`** — machine-wide repo topology is a conductor concern.

`fleet_tasks` is filtered to the goal's own dispatches (a new optional
`createdBy` on `dispatchListForTenant`, filtered in SQL so `limit` still
returns `limit` of the caller's own rows). The tenant board carries every
other session's targets and result digests, and it was the one place the
scoping would otherwise have leaked.

Attribution is `orchestrator:<goalId>` — keyed to the goal, not to any
per-boot identity, so it survives a restart the way the blackboard's
`authorSub` does. The enqueue path calls the dispatcher DIRECTLY rather
than through the conductor's closure, which stamps its own `createdBy` and
would both misattribute the task and defeat that filter.

The constitution now names the four tools it actually has and states that
spawn is absent, with a test tying the prose to
`ORCHESTRATOR_FLEET_TOOLS`. An orchestrator told to "use the fleet tools"
burns a turn discovering which exist; one told it has spawn burns a turn
discovering it doesn't.

## Verification

Daemon suite 2081 to 2135. Twelve mutations, each caught by exactly the
tests that should catch it: cap never enforced, cap counted per-goal
instead of across goals, cap leaking across tenants, refusing after
building, schema floor removed, unscoped `listSessions`, spawn allowed,
send target unchecked, interrupt unscoped, task board unfiltered, memory
handed over, and the full conductor tool set exposed.

Two of those exposed real defects rather than confirming coverage. The
`createdBy` override was being silently clobbered by the conductor's own
closure — the cast used to force it through was the smell, and going
direct to the dispatcher removed both. And the memory assertion passed
under a mutation that handed `this.#memory` straight to the orchestrator,
because the shared harness injects no engine at all; it now sets a stub
and asserts differentially against the conductor's deps, since "the
orchestrator doesn't get X" proves nothing unless the same run shows X was
available to give.
…n every surface

The second §11 P3 guard: "a per-collaboration cost roll-up surfaced at
approve-time". Approve-time is the R3 gate on a send-class fleet dispatch,
which is why this needed the orchestrator's dispatch surface (previous
commit) before it had anywhere to land.

Cost on a dashboard is trivia. Cost on the button that authorizes more
work is a control — so it goes exactly there, and nowhere else.

## Rolled up live, never stored

`#collaborationCostRollup` sums `SessionUsage` across the orchestrator and
every live role-child. Summed from the session map on each request rather
than kept as a running total: the child set changes over a goal's life —
children are torn down, a restart rebuilds them — so a counter would drift
in both directions. It is read once per approval, not per turn.

## Three properties that are the whole point

- **Send-class only.** Attached to a dispatch approval and no other. On
  every Bash/Edit prompt the number becomes wallpaper and stops being read,
  which is how a cost display fails in practice rather than in theory.
- **Never fatal.** The callback is wrapped: a throw degrades to "no cost
  shown", never a failed approval. An approval path wedgeable by a cost
  display is strictly worse than no display.
- **Same number, same words, everywhere.** `formatCollaborationCost` lives
  in `@codeoid/core` and all three approval surfaces call it — web
  `ApprovalBar`, Telegram, and the TUI renderer. Telegram matters most
  here: it is where an owner approves from a phone, the surface least able
  to go check spend somewhere else. Three local formatters would drift, and
  a cost that reads differently depending on where you approve from is
  worse than none.

Additive on the wire (`ToolWaitingConfirmationState.collaborationCost`), so
the Rust codeoid-ui ignores it until it chooses to render it. No lockstep.

## Deliberately NOT in scope

No spend enforcement. The design puts "per-collaboration token budget" in
P6 governance, and visibility before enforcement is the right order — you
want real numbers in front of you before choosing a cap. A limit guessed at
now would be gold-plating.

## Verification

Daemon 2135 to 2148, web 370 unchanged, core 136 to 140. Six mutations,
each caught by exactly the tests that should catch it: roll-up ignoring the
goal/tenant filter, returning totals for non-collaborations, omitting the
orchestrator from the session count, dropping the send-class gate, dropping
the throw guard, and `pick()` ignoring the tool filter.

Two of those started as gaps rather than confirmations. The send-class gate
and the throw guard were both initially untested — the first mutation pass
caught nothing on either, which is what prompted driving them through a
real Session + MockSessionProvider in `fleet-approval-gate.test.ts` instead
of asserting the roll-up in isolation. And the tool-set test originally
asserted only the exported constant, which would have passed while `pick()`
silently ignored it; it now introspects the BUILT server's registered tools.

Live-verified over a real socket at a cap of 12: six collaborations fit,
the seventh is refused with `rate_limited` and a message naming the count,
the cap and the config key, no orphan orchestrator is left behind, and
destroying a goal frees capacity.
@saucam
saucam force-pushed the feat/p3-guards-and-orchestrator-dispatch branch from 0089d53 to 4efb019 Compare July 28, 2026 14:46
@saucam

saucam commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

pr-shepherd: re-confirmed green after the rebase onto b35aa3e. No fixes were needed, nothing was pushed.

Run 30370005942 ran against head 4efb019 (event pull_request), not a cached result from the pre-rebase 0089d53:

check conclusion head SHA
daemon (lint / typecheck / test:coverage / build) SUCCESS 4efb019
web (lint / typecheck / vitest / build) SUCCESS 4efb019
Socket Security: Project Report SUCCESS 4efb019
Socket Security: Pull Request Alerts SUCCESS 4efb019

Branch hygiene: 2 ahead / 0 behind origin/main, single author, both subjects feat:-prefixed. No second rebase was performed — the branch was already current, and a force-push would only have cancelled a valid run.

All P3 guard invariants survived the rebase intact: schema floor rejecting maxLiveChildren 1-11; cap checked before the orchestrator is built; ORCHESTRATOR_FLEET_TOOLS exactly the four read/send tools; orchestrator deps carrying no memory engine, with the differential assertion against a daemon that has one; the createdBy-filtered fleet_tasks board; the send-class-only gate plus non-fatal try/catch on the cost roll-up; and the test introspecting the built server's _registeredTools.

mergeStateStatus is CLEAN and reviewDecision is APPROVED — nothing is blocking. Merging remains a human decision.

@saucam
saucam merged commit 8e709ec into main Jul 28, 2026
4 checks passed
saucam added a commit that referenced this pull request Jul 29, 2026
…hree barrier defects an audit found

A full audit of the collaborative-sessions feature turned up one Critical and
two High defects. All four fixes here; the Critical one also repairs
already-merged behaviour from #267.

## Critical — an orchestrator never received its own dispatch results

`deliverEvents` resolved its recipient with `#conductorFor()` — the tenant's
CONDUCTOR — for every event. But a collaboration orchestrator queues its own
tasks (`fleet_send` since #267, `fleet_panel` here) and they are attributed
`orchestrator:<goalId>`.

With no conductor on the daemon — the common case, since one is created only
on an explicit `session.create` with `role:"conductor"`, never at boot — the
host returned false and the events were retried forever. Reproduced: a joined
panel left `pending: 1 ["group_done"]` after a tick, permanently. With a
conductor present it was wrong differently: the panel's result landed in the
CONDUCTOR's session and the orchestrator still never learned its panel had
joined.

So the entire §7 barrier→synthesis loop was unreachable. Everything upstream
worked; the last mile went to the wrong session.

Events now route by `task.createdBy`: an orchestrator-attributed task delivers
to that goal session, everything else to the conductor as before. Attribution
is parsed through `goalIdFromCreatedBy`, paired with the
`orchestratorCreatedBy` that writes it, because this bug WAS the drift between
an inline template string on one side and a `startsWith` on the other.

`deliverEvents` now returns the ids it delivered rather than a boolean. With
several possible recipients, "one is mid-turn" is a normal state, and
all-or-nothing would either stall an idle recipient or re-deliver its events
later as duplicates. Events whose collaboration no longer exists are retired
with an audit line instead of being retried forever.

## High — the barrier swallowed the "member is wedged" notice

`#emitEvent` is also how a `waiting_approval` wedge is surfaced, and
`#barrierAbsorb` swallowed it: the group was not terminal, so the notice was
absorbed and never written. A panel member that exhausted its tool budget went
unreported and the panel hung to lease expiry — and that notice is the one
message whose entire purpose is to reach a human.

The barrier now absorbs only THIS member's terminal completion. Keying on the
member's own status rather than on the event type keeps the rule in one place:
a non-terminal task has not completed, so it is not the barrier's business.

## High — two live tasks targeting one session evicted each other

`#watched` was `Map<sessionId, taskId>`. Every key used to be a freshly created
spawn worker, unique by construction — but a grouped send watches a
PRE-EXISTING session, and the same role-child can be the target of two live
dispatches (a second panel, or a plain `fleet_send` alongside a running one).
The second registration silently evicted the first, whose task then sat in
`running` until the lease expired, hanging its barrier for the whole lease.

Now `Map<sessionId, Set<taskId>>` behind `#watch`/`#unwatch`/`tasksForSession`,
with every watcher completing on the shared transition and every watcher's
lease renewed.

## Medium — duplicate-delivery window on a grouped send

`dispatchMarkRunning` now runs BEFORE `sendToSession`. `running` is already a
reclaimable state so ordering it first costs nothing, whereas marking after
left a window where a crash between delivery and the marker made the retry
re-send — the reviewer working its brief twice with the digest describing only
the second pass.

## Why the suite could not see any of this

`dispatcher.test.ts` drives a FakeHost whose `deliverEvents` always accepts, so
every barrier test passed against events that were undeliverable in production.
A fake that rubber-stamps the last mile cannot see a last-mile bug. The new
routing tests run against the REAL host in `dispatch-host.test.ts`.

Daemon suite 2166 to 2172. Six mutations caught by exactly the right tests:
conductor-only routing, all-or-nothing event marking, the watched collision,
the absorbed wedge notice, plus the earlier barrier set.

The mark-running ordering is NOT covered: it is a two-statement crash window
with no seam to inject a crash, so it rests on argument rather than a test.

Mutation testing also caught a flaw in these very tests. The first draft
asserted `pending().length === 0`, which a RETIRED event satisfies just as well
as a delivered one — so reverting to conductor-only routing passed. They now
assert a completed turn on the specific recipient session, since draining a
queue is not delivering to anybody.
saucam added a commit that referenced this pull request Jul 29, 2026
…268)

* feat: the dispatch barrier — N members, one joined result (P3 panel)

The one genuinely new dispatch primitive in the collaborative-session design
(§7 step 3). Today's dispatcher does per-task event injection; a review
panel needs an N-way join, because synthesis has to see every verdict at
once. N completions arriving one at a time give the orchestrator N chances
to synthesize from partial input — a panel silently degraded into a race.

## The barrier

`dispatch_tasks` gains `group_id` + `group_ordinal` (both additive; NULL
means standalone, which is every task queued before panels existed and
keeps its current behaviour exactly). Members are inserted in ONE
IMMEDIATE transaction: a crash mid-fan-out would otherwise leave a group
smaller than the panel the owner approved, and the barrier would fire on a
quorum nobody asked for.

`#barrierAbsorb` swallows each member's individual completion; the member
that observes the LAST terminal transition emits one merged event.

It joins on **all-terminal, not all-done**. Waiting for success would hang
a goal on its weakest member, and §7 is explicit that disagreement is shown
rather than hidden — a failed reviewer belongs in the merged digest as a
failure, not dropped and not blocking its peers forever.

The merge is mechanical only: collect, label, order. §7 forbids a silent
auto-vote, so there is no tallying here — the judgement is the
orchestrator's synthesis step or a human's. `group_ordinal` is stored
rather than derived because every member shares one `created_at`, so a
timestamp sort falls back to the random UUID and the "member N of 3" labels
would shuffle between reads.

## What the barrier forced: grouped sends join on WORK, not delivery

This is the part that isn't obvious. An ungrouped `send` completes the
moment the prompt is handed over — `dispatchComplete("delivered to
session …")`. A barrier over that fires the instant all N briefs are
delivered, before a single reviewer has read anything. The panel would join
on nothing.

So a GROUPED send is now watched to completion like a spawn: mark running,
watch the target, finish on its idle transition. Ungrouped sends are
untouched — nobody is joining on those. That change drags in three of the
dispatcher's crash-safety invariants, and each one fails silently:

1. **`#finishWorkerTask` destroyed the session unconditionally** on its
   done path. Grouped sends are the FIRST sends ever routed through there,
   and a send's target is a long-lived ROLE-CHILD — so every joining panel
   would have dismantled its own fleet. Now guarded on `kind === "spawn"`,
   with a contrast test proving a spawned worker is still destroyed so the
   guard can't be over-applied into a leak.
2. **Re-delivery is not idempotent.** After a restart the task is requeued
   and re-executed; blindly re-sending hands the reviewer its brief twice.
   `workerSessionId` being set is the marker that delivery already
   happened, so `#startGroupedSend` re-WATCHES instead.
3. **The turn may have finished while the daemon was down.** Re-watching an
   already-idle target waits for a transition that never comes, until the
   lease expires and burns an attempt. So current status decides: still
   working → watch; already settled → finish now; gone → fail
   non-retryably, and the barrier joins with that member marked failed.

## fleet_panel

Send-class by construction, which is what puts it behind the R3 approval
gate and — for a collaborative session — carries the goal's cost roll-up
into that prompt for free. Granted to the orchestrator, scoped in the DEPS
like its siblings: every target must be one of its own children, checked
where a second caller of that closure cannot route around it.

Unknown or foreign targets fail the WHOLE fan-out rather than running a
smaller panel. The owner approved a panel of N, and quietly reaching a
2-of-3 panel is exactly the silent degradation §7 warns about.

The orchestrator's constitution now states both halves of the contract:
prefer a panel when the answers belong together and do not synthesize
before the join arrives; then read each role's artifact, merge, and SHOW
disagreement rather than resolving it silently.

## Verification

Daemon suite 2149 to 2163. Six mutations, each caught by exactly the tests
that should catch it: the destroy guard removed (a joined panel tears down
its role-children), grouped sends completing on delivery, the barrier
firing on the first terminal member, waiting for all-DONE instead of
all-terminal, a restart re-sending the brief, and the panel scoping
removed.

One test caught a false claim in my own comment: `dispatchGroupMembers`
documented fan-out order while ordering by `created_at` — identical across
a group, so it fell back to random UUID order. That is what `group_ordinal`
exists for.

Not in this PR: the pipeline `panel` phase kind (§8 item 4). That is engine
work in `pipeline/` rather than dispatcher work, and it now has a barrier to
build on.

* fix: create the dispatch-group index AFTER the additive ALTER, not inside CREATE TABLE

On an existing database `CREATE TABLE IF NOT EXISTS` is a no-op, so a partial
index declared inside that block ran against a table without the column and
SQLite failed the whole migration: "no such column: group_id". The daemon then
refused to open a database it had been using — every existing install would
have been bricked on upgrade.

Every unit test builds a FRESH database, where CREATE declares every column and
the ALTER path never runs, so the entire suite was structurally blind to it. A
live upgrade probe (old binary writes the DB, new binary opens it) caught it.

`dispatch-store.test.ts` now reproduces the upgrade path by dropping the
columns from a built database and reopening it, so the additive-migration path
is covered by tests rather than by remembering to run a probe. Mutation-checked:
reinstating the original ordering fails all three.

* fix: route dispatch events to the session that dispatched them, and three barrier defects an audit found

A full audit of the collaborative-sessions feature turned up one Critical and
two High defects. All four fixes here; the Critical one also repairs
already-merged behaviour from #267.

## Critical — an orchestrator never received its own dispatch results

`deliverEvents` resolved its recipient with `#conductorFor()` — the tenant's
CONDUCTOR — for every event. But a collaboration orchestrator queues its own
tasks (`fleet_send` since #267, `fleet_panel` here) and they are attributed
`orchestrator:<goalId>`.

With no conductor on the daemon — the common case, since one is created only
on an explicit `session.create` with `role:"conductor"`, never at boot — the
host returned false and the events were retried forever. Reproduced: a joined
panel left `pending: 1 ["group_done"]` after a tick, permanently. With a
conductor present it was wrong differently: the panel's result landed in the
CONDUCTOR's session and the orchestrator still never learned its panel had
joined.

So the entire §7 barrier→synthesis loop was unreachable. Everything upstream
worked; the last mile went to the wrong session.

Events now route by `task.createdBy`: an orchestrator-attributed task delivers
to that goal session, everything else to the conductor as before. Attribution
is parsed through `goalIdFromCreatedBy`, paired with the
`orchestratorCreatedBy` that writes it, because this bug WAS the drift between
an inline template string on one side and a `startsWith` on the other.

`deliverEvents` now returns the ids it delivered rather than a boolean. With
several possible recipients, "one is mid-turn" is a normal state, and
all-or-nothing would either stall an idle recipient or re-deliver its events
later as duplicates. Events whose collaboration no longer exists are retired
with an audit line instead of being retried forever.

## High — the barrier swallowed the "member is wedged" notice

`#emitEvent` is also how a `waiting_approval` wedge is surfaced, and
`#barrierAbsorb` swallowed it: the group was not terminal, so the notice was
absorbed and never written. A panel member that exhausted its tool budget went
unreported and the panel hung to lease expiry — and that notice is the one
message whose entire purpose is to reach a human.

The barrier now absorbs only THIS member's terminal completion. Keying on the
member's own status rather than on the event type keeps the rule in one place:
a non-terminal task has not completed, so it is not the barrier's business.

## High — two live tasks targeting one session evicted each other

`#watched` was `Map<sessionId, taskId>`. Every key used to be a freshly created
spawn worker, unique by construction — but a grouped send watches a
PRE-EXISTING session, and the same role-child can be the target of two live
dispatches (a second panel, or a plain `fleet_send` alongside a running one).
The second registration silently evicted the first, whose task then sat in
`running` until the lease expired, hanging its barrier for the whole lease.

Now `Map<sessionId, Set<taskId>>` behind `#watch`/`#unwatch`/`tasksForSession`,
with every watcher completing on the shared transition and every watcher's
lease renewed.

## Medium — duplicate-delivery window on a grouped send

`dispatchMarkRunning` now runs BEFORE `sendToSession`. `running` is already a
reclaimable state so ordering it first costs nothing, whereas marking after
left a window where a crash between delivery and the marker made the retry
re-send — the reviewer working its brief twice with the digest describing only
the second pass.

## Why the suite could not see any of this

`dispatcher.test.ts` drives a FakeHost whose `deliverEvents` always accepts, so
every barrier test passed against events that were undeliverable in production.
A fake that rubber-stamps the last mile cannot see a last-mile bug. The new
routing tests run against the REAL host in `dispatch-host.test.ts`.

Daemon suite 2166 to 2172. Six mutations caught by exactly the right tests:
conductor-only routing, all-or-nothing event marking, the watched collision,
the absorbed wedge notice, plus the earlier barrier set.

The mark-running ordering is NOT covered: it is a two-statement crash window
with no seam to inject a crash, so it rests on argument rather than a test.

Mutation testing also caught a flaw in these very tests. The first draft
asserted `pending().length === 0`, which a RETIRED event satisfies just as well
as a delivered one — so reverting to conductor-only routing passed. They now
assert a completed turn on the specific recipient session, since draining a
queue is not delivering to anybody.
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.

2 participants