[brentkelly]

The board reached threads through a dozen scattered upstream calls (engine dispatches, domain event streams, provider event streams, read-model snapshots and raw SQL on upstream projection tables). The v2 merge would have had to rewrite all of them inside supervisorReactor.ts.

This adds board/ThreadGateway.ts, one board-owned Effect service in board vocabulary (commands, BoardThreadState reads, ThreadSignal events), and ThreadGatewayV1, which wraps today's v1 calls one for one. The supervisor reactor, decider context, todo tail, orphan sweep, board RPC, MCP board toolkit, worktreeRetire, PR gateway and usage-limit detector all go through it. Tests run on an in-memory fake; a contract suite runs against V1 on a real stack, and a boundary test enforces that only ThreadGatewayV1 imports orchestration/provider/persistence code or names upstream tables.

Part of T3O-55 (card T3O-58).

🤖 Generated with Claude Code

[brentkelly]

High-level summary

This PR puts every board→thread interaction behind a board-owned ThreadGateway service with one implementation, ThreadGatewayV1, that wraps the v1 engine, projections and provider stream. It re-points the supervisor reactor, the decider context, the todo tail, the orphan sweep, board RPC, the MCP board toolkit, worktreeRetire, the PR gateway and the usage-limit detector onto it. The harness now runs on an in-memory fake, and a boundary test holds the import/SQL line. The shape is good: the vocabulary really is the board's, the signal mapping is a faithful move of the old reactor branches, and the boundary test is a sharp instrument. Most of the wrapper is 1:1 as claimed, but not all of it. This round has three improvements and two nitpicks. The most important is that settle({auto: true}) now stamps snapshotSequence at dispatch time, which turns the engine's optimistic-concurrency guard on automatic settles into a no-op. projects() now materialises the whole command read model where the decider used to run a single cheap query. The in-memory fake's delete also leaves a thread readable as live, which contradicts the contract the harness is supposed to pin. None of these is an incident on its own, but the first is a behaviour change hidden inside a refactor that was meant to change no behaviour.

[brentkelly]

Warning

Improvement (1/3): auto-settle's snapshotSequence is read at dispatch time, which disables the engine's change-since-snapshot guard

OrchestrationEngine.dispatch refuses a thread.auto-settle when eventStore.hasEventAfter({ aggregateId: threadId, sequenceExclusive: command.snapshotSequence }), giving the error "changed before automatic settlement". The guard exists so that a settle decided against a stale view of the thread is refused. Before this PR, the release sweep took model.snapshotSequence from the read model it loaded at the top of the pass, before it read the shell and evaluated isAutoSettlementCandidate. The sequence therefore bounded the view the decision was made on.

settle({ auto: true }) now reads engine.latestSequence here, after the caller has already evaluated autoSettleCandidate in a separate threadState read (supervisorReactor.ts:767-776). Any thread event that lands between the candidacy read and the dispatch is now before the stamped sequence, so the guard can never fire. Picture a human sending a message that completes quickly, or a new activity row that would reset the inactivity window. The decider's session/pending-request checks still catch a thread that is visibly busy at dispatch, but the "changed since you looked" protection is gone. This is a behaviour change in a PR whose contract is a 1:1 wrap.

Suggested fix: keep the sequence bound to the view the decision was made on. The simplest option within the current interface is for V1's auto-settle to capture engine.latestSequence first, then re-read the shell and re-check isAutoSettlementCandidate, then dispatch with the captured sequence. Alternatively, carry an opaque stateRef on BoardThreadState and pass it back through ThreadSettleInput. Add a contract test in which a thread event lands after the candidacy read and the auto-settle is refused.

[brentkelly]

Fixed in c0ad107. settle({auto: true}) now reads the projection's getSnapshotSequence() first, then re-reads the shell and re-checks isAutoSettlementCandidate. It refuses a non-candidate, and otherwise dispatches with the sequence it captured first. Any event the thread logs after that sequence trips the engine's hasEventAfter guard again, as it does in upstream's ThreadSettlementReactor. Doing the check inside V1 keeps the interface free of a v1 sequence number. Test: ThreadGatewayV1.contract.test.ts › "an automatic settle refuses a thread that is no longer a candidate" snoozes a candidate thread and expects a refusal. It failed before this change (the settle landed) and passes now.

[brentkelly]

Adjudication (round 1): fix-upheld. Checked c0ad107. settle({auto}) now reads getSnapshotSequence() first, then re-reads the shell and re-checks isAutoSettlementCandidate, and dispatches with the sequence it captured first. A change after the re-read now trips the engine's hasEventAfter guard. A change between the caller's read and the re-read is caught by the re-check. The new contract test (a snoozed thread is refused) passes. It would have failed before, because without the re-check the engine accepts a settle on a snoozed thread.

[brentkelly]

Warning

Improvement (2/3): projects() materialises the full command read model, and the decider now pays for it on every create/set-project

getCommandReadModel() reads every project, thread, proposed plan, thread PR, session and latest turn in one transaction. projects() keeps only model.projects and throws the rest away. Before this PR, loadDecisionContext used boardUpstreamReads.listProjects, which was a single SELECT id, title, deleted_at FROM main.projection_projects. Now every board.card.create and board.card.set-project dispatch loads the whole read model inside the board engine's command path (BoardEngine.ts:114-122). That is the path plan approval uses to create child cards in a burst. board_create_card reads it twice: once in listProjects and once in the decider. worktreeRetire also went from one read-model load to two, because threadsInWorktree and projects each call getCommandReadModel() (worktreeRetire.ts:491-498). Performance is a stated non-negotiable for this repo, and on a mature install the thread set is the large part of that read.

Suggested fix: back projects() with a direct projects query (the old listProjectRows SQL or a projects-only snapshot read) instead of getCommandReadModel(). Optionally let threadsInWorktree filter in SQL on worktree_path, so retirement does not load every thread either.

[brentkelly]

Fixed in 1a232f5. projects() is now one SELECT on projection_projects (id, title, workspace_root, deleted_at, scripts_json). That is the old boardProjects query plus the two columns worktreeRetire's hooks need. The decider context and board_create_card no longer load the read model. worktreeRetire is back to one full read (threadsInWorktree), the same as before this PR. The contract suite now also asserts scripts.

[brentkelly]

Adjudication (round 1): fix-upheld. Checked 1a232f5. projects() is now a single projection_projects SELECT, and the read model is no longer hydrated. Scripts are decoded with fromJsonString(Array(ProjectScript)). The contract suite asserts scripts and passes.

[brentkelly]

Warning

Improvement (3/3): the fake's delete leaves the thread readable as alive, which contradicts the gateway contract

delete removes the entry from records but leaves states alone, so threadState(id) still returns the fixture's state after a delete: deleted: false, and alive: true if it was mid-turn. ThreadGatewayV1.threadState returns deleted: true, alive: false for a deleted thread, and the interface says "One thread, deleted ones included". The fake was introduced so that the harness tests pin the board's behaviour against the contract and survive the v2 merge. A fake that disagrees with V1 on a basic transition means a supervisor path that deletes a thread and then reads it back, such as card deletion, project change or spawn cleanup, is exercised against a state the real gateway never produces. Tests can then pass for the wrong reason.

Suggested fix: on delete, also replace the thread's state with the same deleted shape V1 returns (deleted: true, alive: false, sessionStatus: "none", …). Mirror that in records too (deleted: true instead of removal), so threadRecords matches V1, which keeps deleted rows.

[brentkelly]

Fixed in eb933d2. The fake's delete now keeps the record, marked deleted: true, and replaces the state with deletedThreadState(...). That builder is now exported from ThreadGateway.ts, and V1 uses the same one, so the two can't drift apart. exists (the send precondition) now treats a deleted thread as gone, matching the engine. All 93 board and MCP-board files pass (1,391 tests).

[brentkelly]

Adjudication (round 1): fix-upheld. Checked eb933d2. On delete, the fake now marks the record deleted and stores deletedThreadState(). That builder is shared with V1, so the two can't drift apart. exists also treats deleted threads as gone. The full board suite passes: 91 files, 1,302 tests.

[brentkelly]

Note

Nitpick (1/2): stale comment points at a pre-filter that no longer exists

The comment justifies board.card-step-recovered by comparison with "the volume that made thread.activity-appended need its own pre-filter above". That pre-filter moved into ThreadGatewayV1.observe, and this stream now carries only board events.

Suggested fix: drop the comparison, or point it at the gateway's filter in ThreadGatewayV1.ts.

[brentkelly]

Fixed in 349ed37. The comment now points at ThreadGatewayV1.observe, which is where thread activity gets filtered.

[brentkelly]

Adjudication (round 1): fix-upheld. The comment now points at ThreadGatewayV1.observe, where the thread-activity filtering actually happens.

[brentkelly]

Note

Nitpick (2/2): the spawn compensation is now a gateway contract, but it is untested

ThreadGatewayShape.spawn documents that "A thread whose first turn is refused is deleted again", and v2 will have to honour that. Neither the contract suite nor any other test covers this tapError → thread.delete path, which the build summary also acknowledges. The old reactor code was untested too, so this is no regression. But this is the moment the behaviour became an interface promise, so it is the cheapest time to pin it.

Suggested fix: add a ThreadGatewayV1 test with a stub OrchestrationEngineService whose dispatch refuses thread.turn.start. Assert that a thread.delete with the :cleanup command id follows and that spawn fails with ThreadCommandRefusedError.

[brentkelly]

Fixed in 349ed37. New test ThreadGatewayV1.test.ts › "a refused first turn deletes the thread it created and reports the refusal". It runs makeThreadGatewayV1 against a mocked engine that refuses thread.turn.start, and asserts the dispatch sequence create → :turn → :cleanup delete, plus a ThreadCommandRefusedError.

[brentkelly]

Adjudication (round 1): fix-upheld. The new ThreadGatewayV1.test.ts case runs a mocked engine that refuses thread.turn.start. It asserts the dispatch order create → :turn → :cleanup delete and a ThreadCommandRefusedError, and it passes.

[brentkelly]

High-level summary — round 2 (re-review at 349ed37)

This PR moves every board→thread interaction behind one board-owned ThreadGateway, with a ThreadGatewayV1 that wraps the v1 engine, projection reads, provider stream and settlement policy. It re-points the supervisor reactor, decider context, todo tail, MCP board tools, board RPC, PR gateway, usage-limit detector and worktree retirer onto that gateway, and runs the harness on an in-memory fake. I re-read the gateway, V1 and the re-pointed callers against board/t3o-55. Spawn, settle and refusal classification match the old dispatchLanded / dispatchOptional semantics. The thread-signal fan-in matches what the two old forked subscriptions delivered. The todo tail's cursor and catch-up logic is unchanged apart from vocabulary. All five round-1 fixes still hold at the line. The gateway-focused suites (contract, V1, boundary, todos, settle, worktreeRetire) pass locally: 67/67.

This round has no critical or improvement findings, only three nitpicks: a fake observe that is not a broadcast as the interface promises, a stale requirements comment in the MCP registration, and an orphan-todo sweep that is no longer a single statement. Nothing here blocks. Separately, GitHub reports the PR as CONFLICTING against board/t3o-55, since T3O-57 (#180) landed there. That is a sync job, not a review finding.

Status of round-1 findings

#	prior finding	status
r1-i1	🟡 Auto-settle snapshotSequence read at dispatch time disables the change-since-snapshot guard	✅ closed: sequence captured before the shell re-read and candidacy re-check (ThreadGatewayV1.ts:480-500)
r1-i2	🟡 projects() materialises the full command read model	✅ closed: a single projection_projects SELECT
r1-i3	🟡 Fake delete leaves the thread readable as alive	✅ closed: shared deletedThreadState, record kept as deleted
r1-n1	🟢 Stale comment about the removed thread.activity-appended pre-filter	✅ closed
r1-n2	🟢 Spawn compensation untested	✅ closed: ThreadGatewayV1.test.ts covers create → :turn refused → :cleanup

[brentkelly]

Note

Nitpick (1/3): the fake's observe is a shared queue, not a per-subscriber stream

ThreadGatewayShape.observe is documented as "Each access is its own subscription", and V1 honours that, since every run of the merged streamDomainEvents / streamEvents is a fresh PubSub subscription. The fake's Stream.fromQueue(signals) shares one queue across all runners, so a second observe consumer would split the signals between them. Each consumer would miss signals instead of every consumer seeing all of them. The reactor is the only consumer today, so nothing is wrong yet. But the fake is meant to be the contract the harness survives the v2 merge on, and a future second observer would get silently wrong test results.

Suggested fix: back emit with a PubSub plus a replay buffer (or PubSub.unbounded({ replay })), and make observe Stream.fromPubSub. That keeps both the "never lost before attach" buffering and per-subscriber delivery.

[brentkelly]

Fixed in the first of the three commits ending at b1d2848. The fake now backs observe with an unbounded PubSub that has an unbounded replay buffer, so every subscription gets every signal, and a late subscriber still sees signals emitted before it attached (the buffering the harness relies on). Test: threadGateway.testkit.test.ts › "delivers every signal to every subscription, including late ones". Two concurrent readers each take two signals. On the old Queue-backed fake the signals were split between them and the test hung until timeout. It passes now.

[brentkelly]

Note

Nitpick (2/3): the registration comment still lists ProjectionSnapshotQuery as a requirement

The board MCP handlers now take ThreadGateway here and no longer require ProjectionSnapshotQuery. The header comment in mcp/toolkits/board/registration.ts:8-9, which isn't in this diff, still names the old requirement set, which now contradicts the boundary rule this PR introduced.

Suggested fix: replace ProjectionSnapshotQuery with ThreadGateway in that file's requirements list.

[brentkelly]

Fixed in b1d2848. The header in mcp/toolkits/board/registration.ts now lists ThreadGateway where it said ProjectionSnapshotQuery.

[brentkelly]

Note

Nitpick (3/3): the orphan-todo sweep is no longer a single statement

Before this change, the sweep was one DELETE … WHERE thread_id NOT IN (SELECT … FROM main.projection_threads …), so the live set and the delete were evaluated together. Now the reactor reads listLiveThreadIds() and passes the snapshot to a separate DELETE (supervisorReactor.ts boot reconcile). A thread that is created, linked and posts its first plan update between those two calls would have its fresh todo row dropped. At boot that window is tiny, and the next turn.plan.updated rebuilds the row, so the only cost is a briefly missing todo list. This is an inherent cost of keeping board SQL off upstream tables, but it isn't stated anywhere.

Suggested fix: add a one-line note at the call site that the snapshot is best-effort and self-heals on the next plan update, or restrict the json_each arm to rows whose updated_at predates the snapshot read.

[brentkelly]

Fixed by bounding the delete, rather than only documenting that it self-heals. boardSweepThreadTodos(liveThreadIds, listedAt): the supervisor reads the clock before listLiveThreadIds(), and the live-set clause now applies only to rows with updated_at < listedAt. A row written after the list was taken belongs to a thread the list may have missed, so the sweep spares it. The link and card clauses are unchanged. Test: boardThreadTodos.test.ts › "AC 20: the boot sweep spares a row written after the live list was taken". It fails with the bound neutralised and passes with it, and the existing AC 20 test still passes.