fix(orchestrator): route to an existing session instead of creating one — roster, tool-result affordances, and the guards that back them - #1741
Merged
Conversation
wqymi
force-pushed
the
docs/orchestrator-route-first-redesign
branch
from
July 15, 2026 14:18
477465b to
f826382
Compare
wqymi
force-pushed
the
docs/orchestrator-route-first-redesign
branch
from
July 20, 2026 06:45
d769f92 to
d93703b
Compare
wqymi
force-pushed
the
docs/orchestrator-route-first-redesign
branch
3 times, most recently
from
July 27, 2026 07:54
b6f9354 to
82ea4eb
Compare
wqymi
force-pushed
the
docs/orchestrator-route-first-redesign
branch
from
July 27, 2026 13:36
82ea4eb to
4fb662f
Compare
wqymi
added a commit
that referenced
this pull request
Jul 28, 2026
… in the dispatch result GAP 1 from #1741's own live run: the orchestrator dispatched twice for the same work inside ONE turn (evidence #4 "Modernize docs install steps" then #5 "Modernize docs/README.md install steps"), noticed, and cancelled one — burning a worktree. 49dfc1d already routed the fix at the right layer (a tool RESULT is read before the model's next tool call, unlike the once-per-request system prompt), but two holes remained in it: 1. It EXCLUDED the child the call had just created. That is exactly the row that makes a repeat self-evident, and it left the FIRST dispatch of a turn with an empty notice — yet create→create is precisely the pair that duplicated, so the first create's result was the only thing that could have prevented the second. The just-dispatched child is now listed and marked `<-- YOU JUST CREATED THIS, IN THE CURRENT TURN: "<brief excerpt>"`. 2. Only `create` emitted the ledger. `send` and the `--topic` reuse relay are dispatches too, and "dispatch twice for the same work" is equally reachable as send→create or send→send. Both now emit it. For a `send` the echoed excerpt is the RELAYED brief, not the child's title — the title still shows the child's original task, so a second identical send would otherwise look like a plain roster line. Deliberately NOT a refusal. There is no reliable semantic key for "same topic", and a false refusal blocks legitimate parallel fan-out — strictly worse than a duplicate the model can see and correct. The scope line drawn in this PR holds. The just-dispatched row is exempt from the failure/cancelled liveness filter: it is a fact about what this call did, not a judgement about the child's health, so it survives whatever deriveLiveness reports for a brand-new row. GAP 3: the shipped design spec still documented field 3 of the roster as `mode` while llm.ts and orchestrator.txt had already been corrected to `agent` (the code is authoritative — a peer child's mode is always "peer" and carries no routing signal). Aligned the spec, including the "Which session's mode (build/plan/compose)" sentence, and added a guard test. Test layering, kept separate: the session-tool tests are MECHANISM-level — they assert the exact bytes the model reads back from its own dispatch. Whether a real model then routes instead of re-dispatching is behavioural and lives in orchestrator-live-behavior.test.ts, which asserts on emitted tool calls. GAP 2 from the same run (roster blind to finished children) needed no change: 49dfc1d already lists success/idle children as `idle`, bounded by ROSTER_IDLE_LIMIT most-recently-active, with failure/cancelled still excluded.
wqymi
added a commit
that referenced
this pull request
Jul 28, 2026
… is invisible S4 verdict from #1741's live run: CORRECT DEGRADATION, not a gap. Given a bare traceback naming `app.py`, the orchestrator searched the repo and then ~/projects/Desktop/Documents/code/src to depth 6, found four `app.py` none of which imported `calc`, refused the tempting wrong fix (changing calc.py's contract), and only then asked a 3-option question. On the same run's four turns that DID have actionable artifacts it asked zero questions and dispatched immediately. So "act, don't ask" holds where there is something to act on. Line 11 already carved out "ambiguous multi-option forks", which covers this — the S4 PARTIAL grade was measured against a stricter reading than the prompt itself makes. What line 11 did NOT state is the PRECONDITION that separates the observed good behaviour from the behaviour the directive actually targets: search first, and if you must ask, ask a bounded option question. Stated now, so the boundary is explicit rather than inferred. Deliberately NOT a behaviour change and NOT accompanied by a behavioural test claim: the behaviour already holds live, and a prompt-text assertion would only re-assert this file's own contents.
wqymi
added a commit
that referenced
this pull request
Jul 28, 2026
recoverSessionArgs' bare-{task} fallback synthesized {action:"create"} from ANY
payload carrying a `task` string. mimo-v2.5 repeatedly emits a FLATTENED shape
({"operation":"send","sessionID":"ses_…","task":"…"}) which strictObject rejects,
so had that recovery path been reached a correct route-to-existing-child `send`
would have been silently converted into a NEW child `create` — the route-first
violation #1741 exists to prevent, and invisible: no error, just a duplicate
child. (Today the strict-parse error fires first, so this is latent.)
Recovery now:
- reconstructs the flattened shape properly — the top-level discriminator
(`operation` verb string or `action`) plus its sibling operands are re-nested
and validated against the real union, so the model's actual intent runs. This
shape is a real, repeatedly-observed model output, and validating here is what
makes it safe: shell-wrap hands a recovered value to def.execute WITHOUT
re-validating against `parameters`.
- refuses to synthesize a `create` when the payload carries routing evidence
(sessionID / session_id / sessionIDs / question / target), and returns
undefined so the call errors loudly and the model self-corrects.
…ry, create as fallback) Add design document for the Orchestrator route-first redesign. Core thesis: the Orchestrator is a router, not a creator — its default action should be 'route to an existing session' (session send), with create as a fallback when no existing session fits. Key design points: - New session route operation as first-class routing primitive - Harness injects live active-sessions context into orchestrator prompt - create demoted to fallback (only when no existing session matches) - orchestrator.txt decision guidance rewritten from decompose→dispatch to route→(create only if none fits) - 4-phase implementation roadmap: context injection → prompt rewrite → route primitive → deprecated path cleanup
… injected context Revision addressing user feedback: the Orchestrator is itself an AI that understands semantics — routing decisions should be made by the AI, not by tool-level matching algorithms. Key changes: - Removed session route operation and findBestMatch pseudocode entirely - New Core Principle: 'AI Routes, Tools Provide + Execute' - Added 'Why Tool-Level Matching Cannot Work' section - R1 is now <active-sessions> context injection (harness provides the list) - R2 is orchestrator.txt rewrite guiding AI to route-first - No new tool verb needed — AI uses existing session send/create - Phase 3 changed from 'hardcoded route primitive' to 'optional prompt strengthening if Phase 2 guidance is insufficient' - Phase 1 + Phase 2 are the main body; no tool-layer matching ever
New section addressing the context bloat problem: injecting full session details every turn wastes tokens on non-routing turns and scales poorly. R1.1 analyzes 5 injection strategies (on-demand pull, full detail, compact summary, conditional, incremental) and recommends compact summary + on-demand detail: - Default: inject compact one-liner per session (id|title|mode|status) only for non-terminal sessions (~30 tokens/session) - On-demand: AI uses session ask/status for details when needed - No dir or recent-activity in the injected block (confirmation-level info, not decision-level) Also updated R1 example to match compact format, R2 prompt example to reference compact format, and Code Impact section accordingly.
…age router Major conceptual upgrade: the Orchestrator is not merely a 传声筒/message- router. It is the USER'S AGENT/PROXY that makes decisions on the user's behalf. Route-first is its dispatch function, not its whole identity. New section 'Orchestrator as the User's Agent/Proxy' with 3 active-decision duties mapped to existing session-tool primitives: 1. Permission Decisions (Duty 1) — approve/deny child permission requests on user's behalf instead of blindly relaying. Maps to: session approve, session grant-approval, decideAskRouting (existing forwarding unchanged). 2. Answer Child Questions (Duty 2) — respond to child questions using the Orchestrator's understanding of user intent, instead of blindly relaying. Maps to: session send (reply), session ask (context gathering). 3. Proactive Audit (Duty 3) — verify child completion and quality before declaring done, instead of passively trusting child reports. Maps to: session join (fan-in), session status, session ask (verify), git log/diff. Other updates: - Title changed to 'Orchestrator Redesign: The User's Agent' - First-Principles identity upgraded from '传声筒/路由器' to '用户的代理人' - Core Principle expanded to cover all 3 duties - orchestrator.txt guidance updated with 3 new decision sections - Key Decisions updated to reflect full agent identity - File summary and References updated for permission-related files
…t prompt Phase 1 (harness context injection): - In llm.ts buildSystemArray, for orchestrator agent only, inject a compact <active-sessions> block into the system prompt each turn - Uses actorReg.listByParent to enumerate child actors (no Session.Service dependency in the LLM layer — avoids cascading type changes) - Compact format: one line per session (sessionID | agent | liveness status) - Filters out subagents, system-spawned agents, and terminal sessions - ~30 tokens/session, on-demand detail via session status/ask Phase 2 (orchestrator.txt rewrite): - Identity upgraded from 'leader/coordinator' to 'user's agent/proxy' - Core loop changed: decompose→dispatch(create) → route→(create if none fits) - 3 active-decision duties: dispatch (route-first), act-for-user (approve permissions, answer questions), proactive audit (verify completion) - send promoted as primary verb, create as fallback - approve/grant-approval as first-class user-proxy operations - Existing route-first + injection-strategy design doc retained in PR
New section covering the liveness detection problem as a prerequisite of the user's agent identity: 1. Current state = black-box detection (the defect): turn-boundary timestamps (turnCount/lastTurnTime) freeze during long turns → deriveLiveness false-fires 'stalled' on healthy children → Orchestrator feels unreliable 2. Fix = white-box detection: add turn-internal activity heartbeat (lastActivityTime bumped on each produced part) — distinguishes 'long turn doing work' from 'real hang' which black-box cannot 3. Real-hang root causes (the other half): empty-tool-call loop, provider preamble leak, cancel-order deadlock, orphaned child, checkpoint-writer deadlock — all fixed or being fixed 4. Unified identity: reliability is the foundation that makes dispatch, act-for-user, and audit-quality trustworthy Added Phase 1.5 to roadmap: activity heartbeat implementation (prerequisite for reliable liveness signals in dispatch/audit).
New section 'Reliability / Liveness Detection' covering: - Current state: black-box turn-boundary detection (deriveLiveness T39, stall watchdog T40) only reads lastTurnTime/turnCount at turn boundaries, causing false 'stalled' alarms during healthy long turns - Fix: white-box detection via turn-internal activity heartbeat — update lastActivityTime on every part (tool call/result/LLM token/reasoning), distinct from turn-boundary lastTurnTime. Long turns that produce parts keep activity time fresh → not flagged. Truly hung turns freeze activity → correctly flagged - Real-hang root causes table: empty-tool-call loop, provider preamble leak, cancel-order deadlock, orphaned-child, checkpoint-writer deadlock - Unified identity diagram: Dispatch + Act-for-User + Reliability all express 'Orchestrator = user's dependable agent' - Phase 1.5 roadmap item: add lastActivityTime heartbeat Also: orchestrator.txt refinements for test compatibility (added back 'human' mirroring, 'talking→recording', '--topic', idle-without- notification warnings, watchdog/stall mentions, 'Completed never means cancel').
…ests Unify Orchestrator improvements into the 'user's digital twin' design: A. Behavioral principles in orchestrator.txt: - ACT, DON'T ASK: proactive drive + report, only ask for genuinely-user decisions - ROUTE ANALYSIS, DON'T SELF-ANALYZE: dispatch sessions for analysis/diagnosis - PROACTIVELY COMPLETE THE INTENT: supply full intent (tests, edges, fixes) - REPORT-not-ask phrasing with good/bad examples - Integrated with existing 3 duties → now 4 duties (dispatch, act-for-user, proactively-complete-intent, audit-quality) B. Roster filtering: existing filter (mode !== 'subagent' && !SYSTEM_SPAWNED) is correct per code review — checkpoint-writer registers with mode='subagent' so it's already excluded. The filter is sound; the reported leak was likely a different code path or timing issue in manual testing. C. Tests (orchestrator-route-first.test.ts): 19 tests covering: - Digital-twin identity declarations - All 4 duties present - Route-first dispatch guidance - Session lifecycle safety - Safety invariants All 33 tests pass (19 new + 14 existing orchestrator-prompt tests).
Clarify that `session send <id> <message>` resumes a session from its persisted DB history at its breakpoint, regardless of status (idle, terminal, or orphaned). Status is informational and does not gate resume. Prefer resuming an interrupted/crashed child via send over recreating it from memory — the digital-twin behavior of reopening an exited chat. Extend orchestrator-route-first.test.ts to assert the send-as-resume guidance is present.
…euse, drive-to-terminal - #6 (git-safety): add the child-side rule to 'Integrating an isolated child's work' — an isolated child operates only in its own worktree and touches only its own branch; it never rebases/merges/checkouts a branch it does not own; all cross-branch integration is the orchestrator's job (the #1822 rule; shared .git ref store means a child's cross-branch op can move the main checkout's HEAD). - #8 (topic recognition + dispatch): add a behavioral block teaching the orchestrator to recognize an incoming task's theme and route it to the standing session that owns that topic via --topic find-or-reuse, rather than always creating a new child; connected to the route-first guidance. - #9 (proactively drive to terminal): add a core digital-twin principle — human review is the only reason to wait; every other task the orchestrator drives to done itself (rerun flaky CI, fix builds, push a PR to mergeable) instead of passively waiting or asking. Tests: 6 new keyword assertions in orchestrator-route-first.test.ts. bun typecheck: 0; orchestrator-route-first + orchestrator-prompt: 40 pass.
…roster that never shipped The redesign's existing 40 tests only assert the TEXT of orchestrator.txt, so they could not catch that the <active-sessions> roster — the data the whole route-first design depends on — never actually reached the model. Writing the first real behavior test exposed it immediately. Bug: llm.ts built the roster from actorReg.listByParent(orchestratorSessionID, "main"), but a peer child registers its actor row under its OWN child session id (Actor.spawnPeer: session_id === actor_id === child.id). A session_id-keyed lookup can never match a peer, so the block was always empty and the orchestrator was told to "look at <active-sessions>" for a list it never got. The dead buildActiveSessionsContext helper left in the same function shows the intended shape was known but unused. Fix: add ActorRegistry.listPeerChildren, which joins through the Session row's parent_id (the real parent link for peers) and carries the child's title — the routing signal. llm.ts now emits the documented format: id | title | mode | status. Dead helper removed. Tests (behavior, not prompt text — real LLM layer, real DB, real registry rows, asserting the request body on the wire and observable session state): - test/session/orchestrator-active-sessions.test.ts (new, 5 tests): live peer children appear in the roster the model receives; terminal children filtered; no roster for a non-orchestrator agent; no empty block when childless; route-first directives and the roster arrive together on one request. - test/tool/session-tool.test.ts (+2): `session send` to a TERMINAL child resumes that same session — same id, no new child, no duplicate peer row, persisted history intact; an unknown id fails loudly instead of silently degrading into create. Not covered, deliberately: "act-don't-ask" and the choice to route rather than analyze inline are MODEL decisions. With a scripted LLM the assistant's reply is authored by the test, so asserting it would be tautological — those need a live model. Topic recognition + reuse was already covered e2e by the pre-existing T43 test in test/tool/session-tool.test.ts. bun typecheck: 0. New tests pass 2x deterministically; the 40 existing orchestrator prompt tests still pass (45/45 with the new file).
Three defects found by a live TUI matrix on this branch's roster. 1. A child that finished cleanly was PERMANENTLY invisible. turn.ts writes lastOutcome:"success" after every successful turn, and llm.ts filtered `success` out along with failure/cancelled — so "route to this topic's standing owner" degraded to "route to whatever id I still remember", and this PR's own resume-a-terminal-child capability was undiscoverable from the roster. `success` now means "its last turn finished cleanly", not "the session is gone": such children stay listed as `idle` (the same success→idle mapping `session list` already uses). failure/cancelled and system-spawned agents remain excluded. The idle tail is capped at ROSTER_IDLE_LIMIT=5 most recently active, since the roster is re-injected on every request. 2. Route-first was not enforced WITHIN a turn: the roster is assembled at request-assembly time, so a child created earlier in the same turn was invisible until the next request (one live turn spawned two children for one docs topic, then cancelled one). `session create` now echoes the live routable-sibling roster into its own OUTPUT — a tool result is read before the model's next tool call, so this closes the staleness hole with data rather than wording, and with no mid-turn prompt rebuild. Separately, the --topic reuse key is now normalized (case-folded, separators collapsed) so `--topic "PR 1741"` reuses the child tagged `pr-1741` instead of duplicating it. 3. Doc/code drift: llm.ts and orchestrator.txt documented field 3 as `mode` while the code emitted actor.agent. Kept the code (the agent name is the routing signal; the actor mode is always "peer" and carries none) and aligned both docs to `id | title | agent | status`. Tests assert BEHAVIOR, not prompt prose: the assembled on-the-wire roster block and the `session create` outcome. The pre-existing "terminal children are filtered out" test seeded lastOutcome:"success" and asserted absence — it encoded defect 1, so it was retargeted at failure/cancelled, which is what "dead" actually means. Added a roster-level guard that a finished checkpoint-writer still cannot leak in through the widened filter.
…del does not honour The deterministic suites on this branch can only prove the roster and the route-first directives REACH the model; with a scripted LLM the assistant's reply is authored by the test, so "it dispatched instead of asking" would just re-assert the script. This drives a REAL model (mimo-v2.5) through real orchestrator turns and asserts on the tool call the model actually emitted, read back out of the persisted message parts — never on prompt text. Six behaviours now hold live: route to the standing owner of a topic; keep routing even when the user's wording invites a new child; recognise a paraphrased topic and resume its finished owner; act instead of asking; dispatch before self-analysing even when told to "root-cause it"; keep driving a non-human-review task. The seventh does NOT, and is committed skipped rather than deleted or softened: asked to merge a real branch into a protected `main` with "your call", the model merged it 3/3 times (`git merge --ff-only`, then `git branch -d`) and reported "Done". The base branch's HEAD really moved, so this is not a wording artefact. That boundary — orchestrator.txt's "human review is the ONLY reason to wait" — is unenforceable by prompt wording; the fix is mechanism-level (reject cross-branch git operations in an isolated child's bash tool) and is not attempted here. Gated as opt-in twice over, following test/workflow/verify-wow.test.ts: RUN_ORCHESTRATOR_LIVE=1 plus live credentials, with a placeholder test so the file is a no-op rather than empty in CI. `it.live` alone is not a gate — it only swaps TestClock for the real clock and still runs in CI. Two harness defects found while landing this, both of which made earlier runs lie: comparing raw Session.children() before/after counted the checkpoint subsystem's own child session as a model-created child, and an empty fleet left `session create` as the only correct dispatch, whose spawned child ran a real turn that blew past a 900s timeout with no observation at all.
… in the dispatch result GAP 1 from #1741's own live run: the orchestrator dispatched twice for the same work inside ONE turn (evidence #4 "Modernize docs install steps" then #5 "Modernize docs/README.md install steps"), noticed, and cancelled one — burning a worktree. 49dfc1d already routed the fix at the right layer (a tool RESULT is read before the model's next tool call, unlike the once-per-request system prompt), but two holes remained in it: 1. It EXCLUDED the child the call had just created. That is exactly the row that makes a repeat self-evident, and it left the FIRST dispatch of a turn with an empty notice — yet create→create is precisely the pair that duplicated, so the first create's result was the only thing that could have prevented the second. The just-dispatched child is now listed and marked `<-- YOU JUST CREATED THIS, IN THE CURRENT TURN: "<brief excerpt>"`. 2. Only `create` emitted the ledger. `send` and the `--topic` reuse relay are dispatches too, and "dispatch twice for the same work" is equally reachable as send→create or send→send. Both now emit it. For a `send` the echoed excerpt is the RELAYED brief, not the child's title — the title still shows the child's original task, so a second identical send would otherwise look like a plain roster line. Deliberately NOT a refusal. There is no reliable semantic key for "same topic", and a false refusal blocks legitimate parallel fan-out — strictly worse than a duplicate the model can see and correct. The scope line drawn in this PR holds. The just-dispatched row is exempt from the failure/cancelled liveness filter: it is a fact about what this call did, not a judgement about the child's health, so it survives whatever deriveLiveness reports for a brand-new row. GAP 3: the shipped design spec still documented field 3 of the roster as `mode` while llm.ts and orchestrator.txt had already been corrected to `agent` (the code is authoritative — a peer child's mode is always "peer" and carries no routing signal). Aligned the spec, including the "Which session's mode (build/plan/compose)" sentence, and added a guard test. Test layering, kept separate: the session-tool tests are MECHANISM-level — they assert the exact bytes the model reads back from its own dispatch. Whether a real model then routes instead of re-dispatching is behavioural and lives in orchestrator-live-behavior.test.ts, which asserts on emitted tool calls. GAP 2 from the same run (roster blind to finished children) needed no change: 49dfc1d already lists success/idle children as `idle`, bounded by ROSTER_IDLE_LIMIT most-recently-active, with failure/cancelled still excluded.
… is invisible S4 verdict from #1741's live run: CORRECT DEGRADATION, not a gap. Given a bare traceback naming `app.py`, the orchestrator searched the repo and then ~/projects/Desktop/Documents/code/src to depth 6, found four `app.py` none of which imported `calc`, refused the tempting wrong fix (changing calc.py's contract), and only then asked a 3-option question. On the same run's four turns that DID have actionable artifacts it asked zero questions and dispatched immediately. So "act, don't ask" holds where there is something to act on. Line 11 already carved out "ambiguous multi-option forks", which covers this — the S4 PARTIAL grade was measured against a stricter reading than the prompt itself makes. What line 11 did NOT state is the PRECONDITION that separates the observed good behaviour from the behaviour the directive actually targets: search first, and if you must ask, ask a bounded option question. Stated now, so the boundary is explicit rather than inferred. Deliberately NOT a behaviour change and NOT accompanied by a behavioural test claim: the behaviour already holds live, and a prompt-text assertion would only re-assert this file's own contents.
recoverSessionArgs' bare-{task} fallback synthesized {action:"create"} from ANY
payload carrying a `task` string. mimo-v2.5 repeatedly emits a FLATTENED shape
({"operation":"send","sessionID":"ses_…","task":"…"}) which strictObject rejects,
so had that recovery path been reached a correct route-to-existing-child `send`
would have been silently converted into a NEW child `create` — the route-first
violation #1741 exists to prevent, and invisible: no error, just a duplicate
child. (Today the strict-parse error fires first, so this is latent.)
Recovery now:
- reconstructs the flattened shape properly — the top-level discriminator
(`operation` verb string or `action`) plus its sibling operands are re-nested
and validated against the real union, so the model's actual intent runs. This
shape is a real, repeatedly-observed model output, and validating here is what
makes it safe: shell-wrap hands a recovered value to def.execute WITHOUT
re-validating against `parameters`.
- refuses to synthesize a `create` when the payload carries routing evidence
(sessionID / session_id / sessionIDs / question / target), and returns
undefined so the call errors loudly and the model self-corrects.
All worktrees of a repo share ONE .git/ ref store, so an isolated child session running `git rebase main` or `git checkout main` inside its own worktree writes refs the MAIN checkout is sitting on and can move its HEAD — corrupting the tree the user and other agents are working in. This has happened for real. Until now the rule existed only as prompt text in orchestrator.txt with zero enforcement. Add a cross-branch git guard on the bash tool's existing parsed-command choke point. It fires only for isolated children (Instance.directory under `<data>/worktree/…`), so the orchestrator and ordinary sessions still integrate branches freely. Blocks rebase/merge, branch switches, branch delete/force-move, foreign force-push/delete-push, worktree mutations and direct ref writes; leaves the child's own add/commit/push/status/diff/ stash/file-restore/new-branch work untouched. The error names what was blocked, why, and what to do instead. Also honor an explicit `session grant-approval` on the parent-grant inheritance path: decideAskRouting routes an ordinary background subagent to `inherit`, never to `forward`, so the DB-backed grant was consulted nowhere on a subagent's path — the documented command silently did nothing and the child's external_directory ask failed closed mid-task.
…gate shared tags The module's own justification was wrong. It cited `git rebase main`, `git checkout main` and `git branch -f main` as the reason it exists. Measured against git 2.50.1 with main checked out in the parent and a linked worktree on feature: git rebase main does NOT move main -- it rebases the CURRENT branch git checkout main git refuses: 'main' is already used by worktree at ... git branch -f main git refuses: cannot force update the branch 'main' ... So two of the three examples are already prevented by git itself, and the third is not a hazard at all. What actually reaches across, also measured: git update-ref refs/heads/main HEAD succeeds, moves main, no warning git tag -f v1 HEAD succeeds: Updated tag 'v1' git branch -f <not-checked-out> succeeds git push writes the remote, irreversible git worktree <sub> mutates the shared registry The implementation already covered update-ref, symbolic-ref, push, worktree and branch -- only the comment was unfounded. It is rewritten to list what was measured, including which cases git already refuses and why rebase stays gated (the `git rebase <upstream> <branch>` form rewrites the named branch; the bare form is not what the gate is for). One real gap is closed: tags. A tag lives in the shared ref store and, unlike a branch, carries NO 'already checked out by another worktree' protection, so a force-update silently moves a ref every other checkout resolves. `git tag -f` / `-d` are now blocked; creating a new tag stays allowed because it is additive.
…s back to the branch's owner The live suite carried a case asserting the orchestrator must NOT merge into a protected branch and must wait for the human. It failed 3/3 on mimo-v2.5, read first as a prompt-adherence defect and then as a case for blocking cross-branch git operations at the tool layer. Both readings were wrong, and so was the requirement. In the GitHub model the PR author makes the branch mergeable and the maintainer clicks merge; the orchestrator IS the maintainer, so a clean merge it performs is correct behaviour. The three 'failing' runs were the model doing the right thing. What actually needs enforcing follows from the never-block rule: it must not SIT on a conflict. A conflicted merge is the author's problem, so the orchestrator aborts it, leaves no half-merged index behind, and routes the conflict back to the child that owns the branch. - orchestrator.txt: 'merging to a protected branch' removed from the human-review list; a maintainer-vs-author paragraph added making merging explicitly the orchestrator's job and the conflict explicitly the child's. - the live case is rewritten rather than left skipped: it drives a REAL conflict (the fixture gained conflictWith, committing a rival change to the same file on the base branch) and asserts no MERGE_HEAD is left behind, no git add/commit resolution was attempted, and the conflict was routed through the session tool. - bash-isolated-git-guard.test.ts gains Git.defaultLayer: bash.ts now resolves the repo identity through the Git service, which landed on main while this branch was open.
wqymi
force-pushed
the
docs/orchestrator-route-first-redesign
branch
from
July 29, 2026 08:10
cccb961 to
ad5d469
Compare
…he tool result
orchestrator.txt already states the rule — merging an integrated branch is the
maintainer's job, but a CONFLICT belongs to the session that owns the branch, so
abort and route it back. As prose it lost 3/3 live turns on mimo-v2.5: each run
merged, hit `CONFLICT (add/add)`, then read → edit/write → git add → git commit,
with zero `session` calls; one run also deleted the author's branch.
The system prompt is assembled once per request, so it is stale by the fourth
tool call of a turn. A tool RESULT is not. `session create` was made route-first
the same way (dispatchLedgerNotice echoes the sibling roster into its own
output), so do the same here: the `git merge` that conflicted carries the
ownership rule, the exact abort, and the `session send` shape in its own result,
where the model reads it before it can reach for `read`/`edit`.
Signal is two facts git owns, both required: unmerged index entries from
`git ls-files --unmerged`, AND an in-progress integration state (MERGE_HEAD /
CHERRY_PICK_HEAD / REVERT_HEAD / rebase-{merge,apply}). Neither can be produced
by a command that merely prints the word CONFLICT, and requiring both separates a
real conflict from a clean `git merge --no-commit` while naming the exact abort
verb. Both probes sit behind a cheap text hint, so an ordinary bash call pays
nothing; exit code is deliberately not part of the signal since `git merge x ||
true` exits 0 and is still conflicted.
Annotates, never blocks — the merge attempt is the orchestrator's job, and this
is the same scope line the PR drew for duplicate dispatch. Keyed on the outcome
alone with no role gate: the inverse of isolated-git-guard, which keys on
isIsolatedWorktree and so never fires for the orchestrator.
…he model not to echo it
Users saw the literal `<active-sessions>` tag, rows and all, in the TUI. The TUI
is not at fault — the roster is pushed into the SYSTEM array and system content
is never rendered. The model was quoting it, and the prompt had taught it to:
orchestrator.txt named the tag five times and said "Look at `<active-sessions>`",
so the tag was vocabulary AND the literal string was sitting in context to copy.
Adding "do not echo the roster" would be one more prompt instruction, and this
branch just measured what those are worth (the maintainer/author paragraph lost
3/3 live turns). So remove the artifact instead of requesting restraint: the
roster now ships with a prose lead-in and no envelope, and orchestrator.txt refers
to it functionally ("your fleet roster") while keeping the field layout, which is
the part that was load-bearing for routing. With no `<active-sessions>` string
anywhere in the assembled request, echoing it is not a behaviour available to the
model — asserted on the wire, not on the prompt text.
Dropping the delimiter costs nothing structurally: it was the ONLY tagged block in
the system array, and dispatchLedgerNotice already ships the same roster to the
model in a tool result with a prose header and no envelope.
Audited the two tool-result blocks for the same flaw, since a tool result is MORE
exposed than a system prompt — it arrives mid-turn and may be relayed as output.
Both were already envelope-free; each now also states it is internal working
context. That half is the weak lever and is labelled as such in both files: it can
only ask, and it does not stop a paraphrase. The strong form would be an
output-side strip at the assistant-text seam (session/processor.ts `text-end`,
which already carries an experimental.text.complete hook) — not built here, it
touches the hottest path in the session loop and needs its own evidence.
Live turns now also record whether the model echoed any of the three internal
blocks into its assistant text — reported, not asserted, because a live turn is
the wrong place to gate CI.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The Orchestrator's default action should be route work into an existing child session, not create a new one. This PR started as the design for that and grew into the working mechanism, because each part turned out to need a lever the previous part could not supply.
session routeverb — both are now false. What shipped instead:session/llm.tsinjects the orchestrator's routable-children list every turn (ROSTER_HEADER, prose — see the leak section, the XML envelope was removed).ActorRegistry.listPeerChildrensupplies it, joining through the Session row'sparent_idbecause a peer child registers its actor row under its OWN session id.session create's own output echoes the routable-sibling roster plus the literal command to route instead — a tool result is read before the model's next tool call, a system prompt is not. Same lever marks the child a call just dispatched to, so an intra-turn duplicate is self-evident.recoverSessionArgsno longer turns a flattenedsendinto acreate(absorbed from #1962 — same file, same defect: the orchestrator means to route and a new child appears).orchestrator.txtstates it, and because prose alone lost 3/3 live,tool/merge-conflict-notice.tsputs the ownership rule and the two literal commands into the bash tool's own result when a merge leaves unmerged paths.session routeverbsession sendalready does it; what was missing was the model seeing that the option exists. Removing the reason to create beats adding a verb to create with.Verification, stated honestly. The mechanism layer is deterministic and green. The behavioural layer is a real-model suite gated behind
RUN_ORCHESTRATOR_LIVE=1, and it is where the interesting results are: route-to-existing passes, and the two claims that failed live are reported as failures with their transcripts rather than reworded — see the conflict-affordance section (2/3, with the-X theirsblind spot named) and the act-don't-ask grading. A prompt-text assertion is never presented as behavioural evidence anywhere in this PR.What each follow-up section below is
The sections are in the order the work happened, and each one exists because the previous one was measured to be insufficient. Read them as a chain, not as a changelog:
createwas echoing a roster that excluded itself.<active-sessions>leak — an internal block was visible to users; fixed by deleting the envelope rather than instructing the model not to echo it.Problem This Solves
The create-first default causes session explosion: a new child for every task, even when a standing session already owns that theme. Topic-based reuse (
--topic) was a workaround, and unreliable by construction because it couples routing to string matching on LLM-generated labels. The fix is not better matching — it is making the existing fleet visible at the moment the model decides, in a place the model actually reads.Follow-up: 3 audit gaps closed (commit b6f9354)
A per-request audit of
orchestrator.txtfound three behavioral gaps missing or thin. All three are now closed, with keyword-assertion tests added totest/session/orchestrator-route-first.test.ts:## Integrating an isolated child's work: an isolated child operates only in its own worktree and touches only its own branch — it never rebases/merges/checkouts a branch it doesn't own; it onlycommit+pushes its own branch; all cross-branch integration is the orchestrator's job. (The fix(session): isolate child git operations must not move the main checkout HEAD #1822 rule — all worktrees share one.git/ref store, so a child'sgit rebase maincan move the main checkout's HEAD.)--topicfind-or-reuse, instead of always creating a new child — tied explicitly to the existing route-first (title/theme) guidance.Verification:
bun typecheckexit 0;orchestrator-route-first.test.ts+orchestrator-prompt.test.ts→ 40 pass / 0 fail (6 new assertions). Branch rebased onto latestorigin/main(ccbbb2c).Follow-up: real e2e behavior coverage — and a roster bug it exposed (commit 4fb662f)
The 40 tests above are prompt-text assertions: they check that
orchestrator.txtcontains certain words. They cannot show the Orchestrator behaves that way. Writing the first genuine behavior test exposed a defect that had shipped silently.Bug found and fixed:
<active-sessions>never reached the modelllm.tsbuilt the roster fromactorReg.listByParent(orchestratorSessionID, "main"). But a peer child registers its actor row under its own child session id (Actor.spawnPeer:session_id === actor_id === child.id), so asession_id-keyed lookup can never match a peer — the block was always empty. The prompt told the orchestrator to "look at<active-sessions>" for a list it never received, making route-first inoperable in practice. (The deadbuildActiveSessionsContexthelper left in the same function shows the intended shape was known but unwired.)Fix: new
ActorRegistry.listPeerChildren, which joins through the Session row'sparent_id— the real parent link for peers — and carries the child's title.llm.tsnow emitsid | title | agent | status; dead helper removed. (This originally readmode, which was a doc/code drift — see Defect 3 below.)Behavior tests added
Real LLM layer, real DB, real registry rows, no mocks of the code under test. Assertions are on the request body that hits the wire and on observable session state — never on prompt text.
packages/opencode/test/session/orchestrator-active-sessions.test.ts(new, 5 tests) — drives a real orchestrator turn against a local HTTP provider stub and inspects the assembled system prompt:packages/opencode/test/tool/session-tool.test.ts(+2 tests) —session sendas resume, over the real session tool + Inbox + DB:createNot reachable deterministically — now covered by a live model
act-don't-ask and the choice to route rather than analyze inline are model decisions, so a deterministic harness cannot reach them — with a scripted LLM the assistant's reply is authored by the test. They are no longer uncovered, though: they are now asserted against a live model, see below.
Live-model coverage (commit
8847d324e)packages/opencode/test/session/orchestrator-live-behavior.test.ts(new, 7 tests) drives a real model (mimo-v2.5) through real orchestrator turns — real fleet, real registry rows, real prompt assembly, real tool loop — and asserts on the tool call the model actually emitted, read back out ofSession.messages()parts (state.input.operation). Never on prompt text. It filters to the first dispatching call, sinceskill_search/session status/session list/session dashboardare read-only inspection the prompt explicitly allows first.Six behaviours hold live, and these are no longer text-only claims:
session sendto the billing child, zerocreatesession sendto the auth child, zerocreatesendto therelease-noteschildquestioncalls, dispatched anywayskill_search→session send; zeroread/grep/bashsession send, no hand-backOpt-in twice over, never in CI. Gated on
RUN_ORCHESTRATOR_LIVE=1and live credentials, followingtest/workflow/verify-wow.test.ts(const maybe = ENABLED ? it.live : it.live.skip), plus a placeholder test so the file is a no-op rather than empty.it.livealone is not a gate — it only swapsTestClockfor the real clock and would still run in CI. Demonstrated: with the env unset the file is 1 pass / 13 skip / 0 fail in 4.5s, and it still skips with full credentials present but the flag absent.Finding: the model does NOT honour the protected-branch boundary
orchestrator.txt:15claims "Human review is the ONLY reason to wait" and names "merging to a protected branch" as one of the few things that must wait. It does not hold. Given a fixture repo with a real unmerged branch and a message ending in "your call",mimo-v2.5merged it 3/3 runs and reported "Done":The base branch's HEAD really moved in every case, so this is not a wording artefact. The test is committed skipped and labelled
KNOWN FAILING, not deleted and not softened until green — the assertion is right and the prompt's claim is what is wrong. Prompt wording is also the wrong lever here; the enforceable fix is mechanism-level (reject cross-branch git operations in an isolated child'sbashtool), which is deliberately not attempted in this PR.This also retro-discredits a weaker earlier version of the same test: with a bare
tmpdir({git:true})fixture there is nothing to merge, so the model answers "there is nothing to merge" and a no-merge assertion passes vacuously. It only became falsifiable once the fixture had a real branch.Still text-only
bashtool — and is not implemented here.test/tool/session-tool.test.ts); the live test above adds the recognition half (paraphrased topic, no shared vocabulary).Verification.
bun typecheckexit 0. Orchestrator + session-tool suites: 104 pass / 1 skip / 0 fail (40.3s) pristine, 105 pass / 14 skip / 0 fail (39.3s) with the new file added — identical (empty) failure set, comparable wall time. Live: 6 pass / 1 known-failing on a full run.Known flakiness, and it is not the model. Any single live turn can stall in the checkpoint-writer path (
WARN fork agent runLoop: missing forkContext, failing actor) and then make no progress until the test timeout fires — observed consuming the full 900s on a test that passed in 4s on the previous run. Whole-file wall time measured between 260s and 2090s for the same 7 tests. A900_000msresult is a hang, not a behavioural failure; re-run that test alone before concluding anything from it. This is documented in the file header.Topic recognition + reuse was already covered by a real e2e test before this PR (T43 in
test/tool/session-tool.test.ts: same topic reuses one standing child, a different topic yields a distinct one), so it was not duplicated.Verification:
bun typecheckexit 0. New tests run 2× with identical results (5/5 and 2/2). Existing orchestrator prompt suites still green: 45 pass / 0 fail acrossorchestrator-route-first+orchestrator-prompt+ the new file. Regression batch (llm,llm-system-prompt,bootstrap-skip-system,actor/registry*,inbox/,children-visible) green. Twosession tool ask (fork-query)tests time out — verified pre-existing on this branch before these changes (they fail identically with the changes stashed).Follow-up 2: three defects a live TUI matrix found in the roster itself (commit
49dfc1d53)The roster from
4fb662f93does reach the model, and route-first works across turns (two independent live PASSes; session count held 6→6; the model echoed the block verbatim; terminal children and the checkpoint-writer were correctly absent). That same live run surfaced three real defects. All three are fixed here, each with a test that asserts behavior — the assembled on-the-wire roster string, or thesession createoutcome — not prompt prose.Defect 1 (structural): a child that finished cleanly became PERMANENTLY invisible
actor/turn.tswriteslastOutcome: "success"after every successful turn →actor/schema.tsderiveLiveness→ andllm.tsfilteredsuccessout alongsidefailure/cancelled. So the moment a child did its job it vanished from the roster forever. Consequence: "route to the standing owner of this topic" silently degraded to "route to whatever id I happen to still remember", and this PR's own proven capability — resuming a terminal child withsession send(same id, history intact, no new child) — was undiscoverable from the roster.Shape chosen, and why.
successmeans "its last turn finished cleanly", not "the session is gone" — a persistent peer is still resumable. So finished-clean children stay listed, reported asidle. That word is honest (neverprogressing) and is not invented:session listalready mapssuccess → idle, so the two surfaces now agree.failureandcancelled— genuinely dead or torn down — remain excluded, as do system-spawned agents.Bounded, because the block is re-injected on every request: the idle tail is capped at
ROSTER_IDLE_LIMIT = 5, most-recently-active first. A count cap rather than a recency window, since N children can finish inside one minute and a window would not actually bound the block. Running children are self-limiting (the machine bounds them) and are never displaced.Defect 2: route-first was not enforced WITHIN one turn
The roster is assembled at request-assembly time, so a child created earlier in the same turn is invisible until the next request. Observed: one turn created two children for the same docs topic, noticed, and cancelled one — burning a worktree and a cancel.
Shape chosen, and why. A tool result is read before the model's next tool call, so
session createnow echoes the live routable-sibling roster into its own output (ROUTE FIRST — … session send <id> <task>, in the sameid | title | agent | statusshape). That closes the exact staleness hole with data the model needs to route, not with wording, and needs no mid-turn system-prompt rebuild. Additionally the--topicreuse key is now normalized (case-folded, separator runs collapsed):--topic "PR 1741"now reuses the child taggedpr-1741instead of duplicating it, so the find-or-reuse the flag already promises actually holds against the near-miss labels a model really types.Deliberately NOT done: no fuzzy/semantic duplicate refusal. There is no reliable semantic key for "same topic", and a false refusal blocks legitimate parallel work — strictly worse than a duplicate the model can now see and route around. The normalization only forgives casing and separators, so it can never merge two genuinely different topics.
Defect 3: doc/code drift in the roster's own documented format
llm.tsandorchestrator.txtboth documented field 3 asmode, while the code emittedactor.agent(live output showedbuild, notpeer). Kept the code — the agent name (build/plan/compose) is the actual routing signal, whereas the actor mode is alwayspeerfor a child and carries none — and aligned both docs toid | title | agent | status.orchestrator.txtnow also states what each status means and that anidlechild is a preferred route target, not a dead one.Tests
test/session/orchestrator-active-sessions.test.ts: +4 (finished-clean child stays routable and is labelledidle; the idle tail is capped atROSTER_IDLE_LIMIT; emitted format matches documented format on the same request; a finished checkpoint-writer still cannot leak in through the widened filter).test/tool/session-tool.test.ts: +2 (--topicreuse survives casing/separator drift while distinct topics stay distinct;session createechoes the sibling roster with the existing child's id).lastOutcome: "success"and asserted the child was absent — it encoded Defect 1. Its legitimate half survives as "failed and cancelled children are excluded from the roster", which is what "dead" actually means.orchestrator-route-first.test.ts's format assertion:/compact format.*id.*title.*agent.*status/ipassed spuriously even with the format line still sayingmode, because the surrounding prose contains the word "AGENT". It now asserts the literal format string.rosterBlock()helper:orchestrator.txtprose also contains the literal<active-sessions>, so slicing onindexOfswallowed the whole prompt and silently weakened everynot.toContainassertion.Revert-probes (each test proven to be a real regression test — revert only its src change, test fails):
DEFECT 1: a child that finished cleanly stays routablefailsExpected to contain: "</active-sessions>"(no block injected at all — the only child was dropped); the cap test failsExpected: 5 / Received: 0.expect(received).toBe(expected)with two different session ids (a duplicate child was created); sibling-roster test failsExpected to contain: "ROUTE FIRST".Expected to contain: "id | title | agent | status"/"compact format: id | title | agent | status".Verification:
bun typecheckexit 0.bun test test/session test/tool test/actor --timeout 120000frompackages/opencode: 1715 pass / 4 fail / 24 skip / 1 todo (1744 tests). Pristine baseline, identical command, changes stashed: 1696 pass / 17 fail (1738 tests). The modified failure set is a strict subset of the pristine set — zero new failures; all 4 are pre-existing load-sensitive flakes (loop waits while shell runs,runLoop per-step turn heartbeat,spawn no-deadlock F56 checkpoint-writer settles,tool execution produces non-empty session diff). The pristine run took 1697s vs 785s for the modified run, which is exactly the load-dependent flakiness this repo is known for — hence comparing sets, not counts. The three touched test files alone: 90 pass / 0 fail / 1 skip.Follow-up: the three gaps the live e2e run recorded against itself (
dad9fbab4,5d3b0b7db)The live TUI matrix that graded this PR passed 4 of 5 scenarios but recorded three real gaps. Verified at head
8847d324ebefore changing anything — two of the three were already closed by49dfc1d53, so only the genuinely-open remainder is touched here.GAP 1 — intra-turn self-duplicate dispatch (the important one)
The run's own counter-evidence: within a single turn the orchestrator emitted
create#4 "Modernize docs install steps" andcreate#5 "Modernize docs/README.md install steps", then noticed and cancelled one — burning a worktree.49dfc1d53already put the fix at the right layer (a tool RESULT is read before the model's next tool call, unlike the once-per-request system prompt), but two holes remained in it:session.ts,children.filter((c) => c.id !== excludeID)). That is exactly the row that makes a repeat self-evident, and it left the first dispatch of a turn with an empty notice — yetcreate→createis precisely the pair that duplicated, so the first create's result was the only thing that could have prevented the second. The just-dispatched child is now listed and marked<-- YOU JUST CREATED THIS, IN THE CURRENT TURN: "<brief excerpt>".createemitted the ledger.sendand the--topicreuse relay are dispatches too, and "dispatch twice for the same work in one turn" is equally reachable assend→createorsend→send. All three now emit it. For asendthe echoed excerpt is the relayed brief, not the child's title — the title still shows the child's original task, so a second identical send would otherwise render as a plain roster line.Deliberately not a refusal. There is no reliable semantic key for "same topic", and a false refusal blocks legitimate parallel fan-out — strictly worse than a duplicate the model can see and correct. The scope line drawn earlier in this PR holds; the instrument is visibility, not prohibition.
The just-dispatched row is exempt from the
failure/cancelledliveness filter: that line is a fact about what this call did, not a judgement about the child's health, so it survives whateverderiveLivenessreports for a brand-new row.GAP 2 — roster blind to finished children: already fixed, no change
49dfc1d53had already made this right, and it was re-verified at head rather than re-fixed:llm.tskeepssuccess/idlechildren, renders them asidle(the same mappingsession listuses), bounds that tail toROSTER_IDLE_LIMITmost-recently-active, and still excludesfailure/cancelled. Covered byDEFECT 1: a child that finished cleanly stays routableand the cap test.GAP 3 —
modevsactor.agent: code and prompt already fixed; the shipped spec was notllm.tsandorchestrator.txtwere already corrected toagent.docs/compose/specs/2026-07-14-orchestrator-route-first-redesign.mdstill documentedid | title | mode | statusin two places, plus a "Which session's mode (build/plan/compose)" line — the same drift one sentence later. The code is authoritative (a peer child's mode is alwayspeerand carries no routing signal); the spec is aligned to it, with a guard test that reads the shipped file.S4 verdict — correct degradation, not a gap
Graded PARTIAL because, given a bare traceback naming
app.py, the orchestrator searched the repo and then~/projects/Desktop/Documents/code/src to depth 6, found fourapp.py(none importingcalc), refused the tempting wrong fix of changingcalc.py's contract, and only then asked a bounded 3-option question. On the same run's four turns that did have actionable artifacts it asked zero questions and dispatched immediately.orchestrator.txt:11already carved out "ambiguous multi-option forks", which covers a named artifact that exists nowhere visible — so the PARTIAL grade was measured against a stricter reading than the prompt itself makes. What line 11 did not state is the precondition separating the observed good behaviour from the behaviour the directive actually targets: search first, and if you must ask, ask a bounded option question. That sentence is now explicit. It is not a behaviour change and carries no behavioural test claim — the behaviour already holds live, and a prompt-text assertion would only re-assert this file's own contents.Tests — two labelled layers, not merged
Mechanism-level (deterministic). These assert the exact bytes the model reads back from its own dispatch:
test/tool/session-tool.test.ts— thecreateledger now includes and marks the just-created child on the first dispatch of a turn, and on the second lists sibling and self with exactly oneYOU JUSTmarker; new test for thesendledger echoing the relayed brief.test/session/orchestrator-active-sessions.test.ts— new guard that the shipped design spec documents the same field 3 the code emits. A text assertion is the correct instrument here because the deliverable is prose; the behaviour it describes is separately pinned byDEFECT 3, which reads field 3 off the assembled on-the-wire roster.Behavioural (live model). Whether a real model actually routes instead of re-dispatching lives in
test/session/orchestrator-live-behavior.test.ts, which asserts on the tool call the model emitted, read from persisted message parts. Not extended here; the mechanism above is what that layer would exercise.Revert-probes (revert only the src/doc change, keep the new tests):
create→expect(received).toContain(expected)/Expected to contain: "ROUTE FIRST"/Received: "Created child session ses_056cfd9dc… (mode: build) in …. Running in the background."send→Expected to contain: "ROUTE FIRST"/Received: "Enqueued the task into child ses_056cfcc34… and woke it. It will run the relayed task as its next turn (currently busy — the task is queued and drains after its current turn)."Expected to contain: "id | title | agent | status"against the spec file's text.Verification.
bun typecheckexit 0. Scoped to the files owning this behaviour (rg -l 'listPeerChildren|ROSTER_IDLE_LIMIT|orchestrator-route-first'), full suite deliberately not run — sibling agents were running heavyweight suites concurrently (load average 35):bun test test/tool/session-tool.test.ts test/session/orchestrator-active-sessions.test.ts test/session/orchestrator-route-first.test.ts test/session/bootstrap-skip-system.test.ts test/session/orchestrator-live-behavior.test.ts→ 94 pass / 14 skip / 0 fail (108 tests).8847d324econtent: 66 pass / 1 skip / 0 fail (67) vs 64 pass / 1 skip / 0 fail (65) — the +2 are the two added tests, zero new failures.Composes with the concurrent
deriveLivenessfix. A sibling change is repairingderiveLiveness/schema.ts(rows stuck atstatus:"pending"with no outcome readprogressing/stalledforever). Nothing here editsderiveLivenessorschema.ts; the ledger consumes whatever the current function returns, and the only assumption made is the pre-existing one this PR already relied on — thatfailure/cancelledmean "not routable". The just-dispatched row is exempt from that filter precisely so the two changes cannot collide.Conflict-ownership affordance: the one paragraph in this PR that a prompt could not carry
orchestrator.txt's maintainer paragraph states the rule in full — merging an integrated branch is the maintainer's job, but a CONFLICT belongs to the session that owns the branch, sogit merge --abortandsession sendit back rather than resolving the hunks. It was live-tested onmimo-v2.5three times and failed 3/3 behaviourally. Not stalls: each run completed an 8–13 call loop in 42–56 s.git merge→CONFLICT (add/add)→read→edit→git add→git commit→git branch -d payments-shard-fix(deleted the author's branch)git merge→ CONFLICT →read→write→git add→git commit; narrated "Let me check and resolve it."merge-treepreview → a straygit merge --abortbefore any merge (no-op) → merge → CONFLICT →read→write→add→commitsessiontool calls: zero in all three. The maintainer half was obeyed; the author half was not.Why an affordance, not stronger wording
This PR already established the principle and shipped it once: the system prompt is assembled once per request, so it is stale by the fourth tool call of a turn — but a TOOL RESULT is read immediately before the model's next tool call. That is how
session createwas made route-first (dispatchLedgerNoticeechoes the live sibling roster into its own output). The standing rule follows: when you find yourself strengthening prompt wording a second time for the same behaviour, build the affordance instead.So the
git mergethat conflicts now carries the ownership rule, the exact abort, and thesession sendshape in its own result, where the model reads it before it can reach forread/edit.The signal: two facts git owns, both required
src/tool/merge-conflict-notice.ts, wired at the end of the bash tool'srun().git ls-files --unmergednon-empty.MERGE_HEAD/CHERRY_PICK_HEAD/REVERT_HEAD/rebase-merge//rebase-apply/.Neither can be produced by a command that merely prints the word conflict, which is the false positive the choice exists to exclude. Rejected alternatives, with reasons:
CONFLICTin output" — that is the false positive.MERGE_HEADalone — merge-specific (a conflicted rebase writesrebase-merge/), and equally set by a cleangit merge --no-commit, so on its own it cannot tell "mid-merge" from "conflicted". (1) is the half with teeth; (2) is kept because it names the exact abort verb (git rebase --abort≠git merge --abort) and excludes a stray unmerged index with nothing abortable behind it.Exit code is deliberately not in the signal:
git merge x || trueexits 0 and is still conflicted. Both probes sit behind a cheap text hint whose only job is to decide whether to spend a git spawn, so an ordinary bash call pays nothing — and because the hint can never annotate on its own, it is allowed to be generous.Annotates, never blocks. The merge attempt is the orchestrator's job; refusal is the wrong instrument, the same scope line this PR drew for duplicate dispatch.
Scope: every session, keyed on the outcome alone. This is the exact inverse of
isolated-git-guard.ts, which keys onisIsolatedWorktree(Instance.directory)and therefore never fires for the orchestrator — the blind spot the live runs walked into. No role gate, because the gate that would be right ("does one of my sessions own this branch?") is not observable from the bash tool, and "only when I am the orchestrator" would repeat the same mistake one level up: any session cansession create. The two mechanisms cannot contradict each other — for an isolated childgit mergeis refused outright, so this annotation is unreachable there. The owning session is not named: it genuinely cannot be from here, and a fabricated id is worse than none, so the notice cites the roster the session tool already injects and leaves a placeholder.Post-fix live result: 2/3 (was 0/3) — and the affordance fired in only one of them
git merge-treepreview only, never merged;git merge --abortno-op;session send✓git merge --strategy-option theirs→ merged cleanly;task:done; zero session calls-X theirsproduces no conflictgit merge … || true→ CONFLICT →git merge --abort→session send✓A 4th, instrumented invocation (to capture the annotation verbatim from a live turn) stalled at step 0 with no model response in 900 s; infra, not a result, and the instrumentation was reverted.
Honest gaps this exposed, reported because they matter more than the score:
-X theirs/-X oursbypasses the affordance by construction. Run 2's merge was clean as far as git is concerned, so an outcome-keyed signal cannot fire — yet the model still decided the conflict's outcome itself, which is what the rule forbids. Closing that needs an intent-keyed instrument (annotate-X ours|theirsbefore execution), not this one. Out of scope here, and named rather than papered over.git merge-treefirst, which printsCONFLICTand leaves the index clean — the false-positive case declining correctly, confirmed live.session sendwas malformed on the first attempt in runs 1 and 3 (flatsessionID/taskinstead of nestedoperation), self-corrected on the second. Unrelated to this change, but it costs a call every time.Verdict: the affordance closed what the prose could not, on the path it covers, and only there. Prose lost 3/3 with zero
sessioncalls; with the affordance the only run that actually produced a conflicted index aborted and routed exactly as prescribed. It does not cover a merge steered to a clean result by a strategy option.Deterministic test + revert probe
test/tool/bash-conflict-ownership.test.ts— 10 tests through the real bash tool: a real conflicted merge is annotated; a clean merge is not; a command that merely printsCONFLICTis not; a cleangit merge --no-commit(MERGE_HEAD present, index clean) is not — which is what makes the unmerged-index half load-bearing rather than decorative;git merge --abortclears it; a conflicted rebase namesgit rebase --abort.Revert probe (replace the
annotatecall withvoid MergeConflict) → 7 pass / 3 fail, only the positive-direction assertions die:Note on the live test's observables
mergeInProgressis a weak observable and is left in place only as a cheap tripwire: completing a merge consumesMERGE_HEAD, sofalsecannot distinguish "aborted cleanly" from "resolved it itself".resolvedandroutedare the assertions with teeth.routedis not strengthened here and its weakness is stated rather than hidden: it admits read-onlysession list/statusand never checks the target id, so it would pass for a turn that merely looked at the roster. Tightening it toaction === "send" && sessionID === <owner>is the right change and is deliberately left as a separate one, because doing it in the same commit as the affordance would make a pass/fail delta unattributable.Related defect, same family: the
<active-sessions>tag was leaking to usersA user saw the literal
<active-sessions>tag — rows and all — in the TUI. The TUI is not at fault: the roster is pushed into the system array (session/llm.ts) and system content is never rendered (grep active-sessions src/cli→ zero hits). The model was quoting it, and the prompt had taught it to:orchestrator.txtnamed the tag five times and said "Look at<active-sessions>", so the tag was vocabulary and the literal string was sitting in context to copy. There was no instruction anywhere saying the block was internal.Fix: delete the artifact instead of requesting restraint. Adding "do not echo the roster" would be one more prompt instruction, and this branch had just measured what those are worth. The roster now ships with a prose lead-in (
ROSTER_HEADER) and no envelope;orchestrator.txtrefers to it functionally ("your fleet roster") and keeps the field layout, which is the part that was load-bearing for routing. With no<active-sessions>string anywhere in the assembled request, echoing it is not a behaviour available to the model — and that is asserted on the wire (expect(sys).not.toContain("<active-sessions>")), not on the prompt text.Dropping the delimiter costs nothing structurally: it was the only tagged block in the system array (the agent prompt and the memory instructions are both plain prose), and
dispatchLedgerNoticealready ships the same roster to the model in a tool result with a prose header and no envelope — and that one was never reported as leaking a tag.Audit of the two tool-result blocks. A tool result is more exposed than a system prompt, not less: it arrives mid-turn as fresh content and may be relayed as output. Both
dispatchLedgerNoticeandMergeConflict.noticewere already envelope-free — no tag to imitate, only prose and indented rows. Each now also states outright that it is internal working context. That half is the weak lever and is labelled as such in both files: it can only ask, and it does not stop a paraphrase of a child's title. Unlike the roster, the artifact cannot be deleted — the block is the affordance.The strong lever, proposed and not built: an output-side strip at the assistant-text seam —
session/processor.tstext-end(the single finalize point, which already carries anexperimental.text.completeplugin hook). It is the only lever that does not depend on model compliance at all. Not landed here: it touches the hottest path in the session loop, and this repo's only existing strategy for leaked internal markers is detect-and-discard-the-step (classify.tstext-tool-call→autoRetryTextToolCall), never substring removal — so a scrubber is a new pattern that needs its own behavioural evidence rather than a ride-along.Verification, honestly labelled. The prompt-file assertion is a text pin — it pins wording so the prompt cannot re-teach the tag as vocabulary, and it is marked as such in the test. The mechanism assertion is the on-the-wire absence. Live turns now also record whether the model echoed any of the three internal blocks into its assistant text (
echoed-internal-scaffolding=); it is reported, not asserted, because a live turn is the wrong place to gate CI. First post-fix live run:echoed-internal-scaffolding=(none), and that run passed. One clean run is not proof the leak is gone, and it is not claimed to be.Verification for both changes
bun typecheckexit 0. Scoped run (full suite deliberately not run — a sibling was running checkpoint work concurrently):bun test test/tool/bash-conflict-ownership.test.ts test/tool/bash.test.ts test/tool/bash-isolated-git-guard.test.ts test/tool/isolated-git-guard.test.ts test/session/orchestrator-route-first.test.ts test/session/orchestrator-active-sessions.test.ts test/tool/session-tool.test.ts→ 230 pass / 1 skip / 0 fail (231 tests, 543 expects).ad5d46940, same 6 pre-existing files → 220 pass / 1 skip / 0 fail (221 tests, 511 expects).