Add headless dispatch arbiter

[m-aebrer]

Closes #394

Stage 2 builds on the routing guide and thinking metadata from stage 1 to add the fully headless, fail-closed Dispatch Arbiter before every subagent spawn, with typed observability across dreb's interfaces.

This is a stacked PR based on stage 1 (PR 405). Implementation plan posted as a comment below.

[m-aebrer]

Implementation Plan — Stage 2

Problem and approach

Stage 2 adds the actual Dispatch Arbiter: a fully headless background decision-maker that runs in the existing subagent background lifecycle after the parent requests a child but before any child process is spawned. It consumes the routing guide and thinking metadata delivered by stage 1, then may change only the selected agent type, canonical provider/model, and thinking level.

The implementation will stay close to the existing tab/session-title setter rather than introducing a general routing framework. A small DispatchArbiter helper will reuse the same proven shape: a bounded RollingContextBuffer, labeled message/tool activity, a direct completeSimple() call with an injected model registry and API key, an abort/timeout signal, and no tools. Unlike title generation, arbitration is control-path work: it has no parent-model fallback, is never fire-and-forget, and fails closed.

The launch path will split today's routing into three clear phases inside the shared executeSingle() path:

1. Normalize the immutable task/cwd and resolve the parent's current proposal to a concrete agent/model/thinking tuple using existing precedence and fallback behavior.
2. When enabled, invoke the headless arbiter once with bounded, scrubbed context and strictly validate its structured decision.
3. Apply the selected existing agent definition verbatim, exact-validate the final scoped model and thinking capability, then spawn the child.

Single and parallel tasks already converge on executeSingle(). Chain steps reach it only after {previous} substitution, so this placement gives one arbitration per actual spawn and lets each chain decision see the real substituted task.

External prior art reinforces the narrow design: RouteLLM treats routing as a quality/cost choice between adequate and stronger models, while official OpenAI and Anthropic guidance emphasizes schema-constrained outputs. Because dreb's generic provider layer does not expose one uniform native structured-output API, the host will accept only a strict JSON object, validate every field itself, optionally retry malformed output once, and never treat model text as executable policy.

Deliverables

1. Global-only arbiter configuration

Add subagentArbiter settings with enabled, model, thinking, and guidePath fields.

• Arbitration remains disabled by default.
• Runtime reads come only from the global settings object. Project settings cannot enable, disable, or reconfigure it.
• Enabling requires an exact resolvable arbiter model and a non-empty current session model scope. Missing, malformed, or unsupported configuration fails the affected launch before inference or spawn.
• The configured thinking level is capability-validated against the arbiter model.
• The standard guide path expands from ~/.dreb/agent/model-routing-guide.md; an explicit global path may replace it.
• Existing settings read/write and RPC settings surfaces expose the global configuration without allowing project shadowing.

2. Deterministic routing-guide loading and validation

Add a small parser/validator for the stage 1 guide contract.

• Read the guide once per arbitration attempt, apply a hard size bound, parse its YAML frontmatter, and use those same bytes for the model call so validation and inference cannot observe different files.
• Require schema version 1, valid metadata, unique canonical covered IDs, exactly one model section per covered ID, all required subsections, and the routing-safeguards section.
• Compare covered_model_ids and model headings exactly with the current live AgentSession.scopedModels canonical set. An empty scope, stale/missing model, duplicate, or extra model fails closed.
• Return bounded validated guide content/sections for prompt construction; do not silently repair malformed Markdown or substitute a different guide.

3. Fully headless Dispatch Arbiter

Create a mode-independent DispatchArbiter owned by AgentSession and feed it parent message_end and tool_execution_end events, mirroring the title setter's rolling-context pattern.

For each attempted dispatch it will build a structured, explicitly untrusted package containing:

• the immutable child task and cwd;
• the concrete proposed agent/model/thinking tuple;
• names, descriptions, effective tool sets, and model defaults for every agent definition available to the parent's subagent tool (never editable definitions or extra capabilities);
• canonical live candidate models and the validated guide;
• bounded first/latest user intent plus recent labeled assistant/tool activity from RollingContextBuffer;
• parent model and session title when present;
• repository basename, cwd, branch, and a bounded metadata-only git-status summary;
• parent/child lineage identifiers where available.

The task passed to the real child remains byte-for-byte unchanged. Before remote inference, the serialized arbiter package is secret-scrubbed with the existing built-in and configured extra patterns. Optional conversation and git context receive deterministic bounds and truncation markers; required task, guide, scope, and agent metadata are never silently dropped. If required content cannot fit the defined safety bounds, arbitration fails before inference.

The arbiter call will:

• resolve only the configured arbiter model, obtain its API key through ModelRegistry, and call completeSimple() directly with no tools;
• use the configured thinking level and one total timeout/abort path;
• require exactly { "agent": string, "model": "provider/model", "thinking": ThinkingLevel } after strict host parsing;
• permit at most one bounded retry for malformed structured output, with no fallback to the original proposal;
• never expose or persist the prompt, raw response, reasoning, or arbitrary generated text.

Injected completion and clock/timeout dependencies will keep all automated tests offline.

4. Fail-closed pre-spawn integration

Integrate arbitration in the common subagent execution path.

• Disabled mode is a true passthrough: no arbiter inference, guide read, event, or change to current child routing.
• Enabled mode runs exactly once before each single spawn, once per parallel item after the existing concurrency gate, and once per chain step after substitution.
• The final agent must be a key in the already-discovered agent map. Selecting it applies that definition's system prompt and filtered tools verbatim; the arbiter cannot rewrite either.
• The final model must exactly match a canonical model in the active live scope. No fuzzy matching, out-of-scope fallback, or provider collapsing is allowed.
• Final thinking is validated through the shared stage 1 capability validator.
• Task, cwd, substituted chain content, parent linkage, and all non-routing request fields remain immutable.
• Configuration, guide, scope, inference, timeout, parse, agent, model, thinking, auth/availability, and abort failures return an actionable failed SubagentResult and prove that spawn() was never called.

5. Typed arbitration observability and identity updates

Add one host-generated arbitration event for every enabled attempt, including unchanged decisions and failures.

The event will include an agent/operation ID, optional chain step, status, proposed tuple, nullable final tuple, changed-field list, and a bounded host error code/message. It will never contain model-authored explanations or raw inference data.

• AgentSession attaches the background agent ID, persists the safe event as a non-context custom JSONL entry, and emits it to listeners.
• A successful agent change updates the background registry atomically so later lifecycle state, TUI labels, RPC snapshots, and dashboard cards use the final agent identity rather than the requested one.
• The TUI displays concise host-formatted changed/unchanged/failure action information and refreshes background status.
• JSON and RPC receive the typed event through the existing event stream.
• Dashboard server/client state records the decision on the matching background agent and visibly renders the final route and changed fields, including chain-step decisions.
• background_agent_end and child lifecycle metadata continue to report what the spawned child actually used; arbitration metadata remains a separate pre-spawn record.

6. Documentation

Document:

• the global-only settings, disabled default, exact model/scope/guide requirements, and fail-closed behavior;
• the title-setter-style headless architecture and the precise context sent to the arbiter provider;
• secret scrubbing, bounds, immutable child fields, no-tools guarantee, and raw-output prohibition;
• routing precedence before and after arbitration;
• typed JSON/RPC/session/dashboard event shapes and UI behavior;
• the relationship between the stage 1 research skill's settings/argument scope and the arbiter's exact live runtime scope.

Files to create

• packages/coding-agent/src/core/dispatch-arbiter.ts — rolling context, prompt package, direct tool-less inference, strict decision parsing, validation, timeout, and safe event formatting
• packages/coding-agent/src/core/model-routing-guide.ts — bounded stage 1 guide parser, schema/section validation, and exact live-scope coverage check
• packages/coding-agent/test/dispatch-arbiter.test.ts — isolated headless inference, context, privacy, output, retry, and failure tests
• packages/coding-agent/test/subagent-arbiter.test.ts — all-mode pre-spawn integration and no-spawn-on-failure tests

Files to modify

• packages/coding-agent/src/core/settings-manager.ts — global-only arbiter settings types, validated access, and persistence methods
• packages/coding-agent/src/core/tools/subagent.ts — proposal/final routing phases, arbiter callback seam, all-mode integration, registry identity updates, and result failures
• packages/coding-agent/src/core/agent-session.ts — own/feed the arbiter, provide live scope/context, persist and emit typed arbitration events, update background identity
• packages/coding-agent/src/modes/interactive/interactive-mode.ts — render safe arbitration actions and refresh routed agent status
• packages/coding-agent/src/modes/rpc/rpc-types.ts and rpc-mode.ts — global arbiter settings snapshots/updates and typed event coverage
• packages/dashboard/src/shared/protocol.ts — arbitration metadata on background-agent DTOs
• packages/dashboard/src/server/runtime-pool.ts — project safe routing events into runtime snapshots
• packages/dashboard/src/client/state/reducer.ts — apply changed, unchanged, and failed decisions deterministically
• packages/dashboard/src/client/screens/session.tsx and screens/subagent.tsx — show final route and per-step action metadata
• packages/coding-agent/test/settings-manager.test.ts — global/project isolation and malformed-config behavior
• packages/coding-agent/test/rpc-settings-commands.test.ts — global settings API validation and persistence
• packages/coding-agent/test/agent-session-guardrails.test.ts — event persistence, safe parent delivery, and final identity behavior
• packages/coding-agent/test/rpc-background-agents.test.ts — typed arbitration relay and snapshot metadata
• packages/dashboard/test/reducer.test.ts, runtime-pool.test.ts, and client/screens.test.tsx — dashboard state, hydration, ordering, and visible route coverage
• README.md
• packages/coding-agent/README.md
• packages/coding-agent/docs/settings.md
• packages/coding-agent/docs/skills.md
• packages/coding-agent/docs/agent-models.md
• packages/coding-agent/docs/session.md
• packages/coding-agent/docs/json.md
• packages/coding-agent/docs/rpc.md
• packages/coding-agent/docs/dashboard.md

If implementation shows an existing adjacent test or UI file is a clearer home, cases may be consolidated there, but no behavior or affected documentation layer above may be dropped.

Acceptance criteria

• Arbitration is disabled by default and only global settings can configure it.
• A valid explicit arbiter model, non-empty live session candidate scope, and guide with exact scope coverage are required when enabled.
• The arbiter is a fully headless direct model call with a rolling parent-context buffer and zero tools or child-process capability.
• Exactly one successful or failed arbitration event occurs before every actual child spawn in single, parallel, and post-substitution chain paths.
• The arbiter can change only agent, scoped canonical model, and supported thinking; all other child inputs remain unchanged.
• Selecting another agent uses an already-available definition verbatim and cannot grant capabilities outside the existing subagent tool.
• Every listed failure prevents spawn() and surfaces a safe actionable error; no original-choice or parent-model fallback bypasses arbitration.
• Structured unchanged/changed/failure decisions persist in parent JSONL and reach TUI, JSON, RPC, and dashboard without raw model content.
• Secret scrubbing and deterministic bounds apply before inference, while child task/cwd remain untouched.
• Automated tests use injected calls and no live APIs.

Testing approach

• Guide parser: valid cold-start/history guides, malformed YAML, wrong schema, duplicate/missing/extra IDs, missing headings/subsections, over-size files, exact provider identity, and validate/read consistency.
• Headless arbiter: disabled path, complete bounded package, title-style rolling context, secret/custom-pattern redaction, no tools, configured model/thinking/API key, timeout/abort, provider/auth failure, strict exact-object parsing, one malformed retry, and no raw response in errors/events.
• Routing integration: unchanged and each changed field alone/together; inappropriate Explore corrected to feature-dev; routine lookup moved from a frontier model to a cheaper scoped model; unknown agent, out-of-scope provider/model, unsupported thinking, missing scope/guide/model, and all failures asserting zero child spawns.
• Mode coverage: single once, each parallel item once under the existing semaphore, each chain step once after literal {previous} substitution, chain stop on failed arbitration, cwd rejection before arbitration, and cancellation during arbitration.
• Immutability/security: byte-for-byte child task/cwd/context preservation, agent definition applied verbatim, normal tool exclusions retained, malicious instructions in task/guide/agent descriptions treated as data, and no prompt/response in transcript or JSONL.
• Observability: changed, unchanged, and failed event types; per-step chain identity; parent custom-entry persistence without LLM-context reconstruction; registry/TUI/RPC/dashboard final identity; snapshot/hydration behavior; and child-effective completion metadata remaining distinct.
• Regression and QA: targeted Vitest files, full npm test, npm run build, and npm run verify-workspace-links. After the required build, use the real dreb -p binary with a controlled guide/scope to verify disabled passthrough, unchanged routing, both priority corrections, one chain, and representative fail-closed configuration/guide/output failures.

Risks and boundaries

• Guide, task, conversation, and agent metadata are all prompt-injection inputs. The arbiter system prompt must label them as data, the host must validate the only accepted fields, and no model-authored rationale may cross the control boundary.
• The stage 1 guide is human-readable Markdown. Deterministic parsing and exact live-scope coverage are necessary, but this remains a small validator rather than a new guide service or policy engine.
• The immediate background-start event necessarily begins with the requested identity. The subsequent typed arbitration transition must update the registry and every consumer before child events arrive, preventing stale labels without delaying the tool's immediate background acknowledgement.
• Parallel arbitration can add provider pressure, but the existing background concurrency semaphore already bounds active calls; no second routing scheduler is needed.
• Native structured-output features differ by provider. Strict host parsing and validation preserve provider portability; one malformed-output retry is the only model-output recovery.

No blocking design question remains. The implementation should prefer the title setter's small dependency-injected pattern and the existing subagent convergence point over new orchestration abstractions.

---

Plan created by mach6

[m-aebrer]

Acceptance Gap — Arbiter User Controls

The current implementation exposes the global Dispatch Arbiter policy through settings.json and raw RPC settings, and displays arbitration decisions, but it does not yet provide normal user controls in either settings UI.

This is a hard acceptance criterion, not optional follow-up work. Before this implementation is pushed as complete, both control surfaces must expose:

• enable/disable;
• exact arbiter model selection;
• arbiter thinking level;
• routing-guide path;
• loud validation/readiness feedback rather than silently accepting an unusable configuration.

Required surfaces:

1. Interactive TUI /settings.
2. Web dashboard Settings.

The controls must write the global-only policy. Project settings must remain unable to enable, disable, or reconfigure arbitration.

---

Acceptance gap tracked by mach6

[m-aebrer]

Progress Update

Implemented the complete stage 2 Dispatch Arbiter, including the hard user-control acceptance requirement identified during implementation.

Headless arbitration

• Added the disabled-by-default, global-only subagentArbiter policy.
• Added strict stage 1 routing-guide parsing and exact live-scope coverage validation.
• Added a title-setter-style bounded rolling parent context and a direct, tool-less completeSimple decision call.
• Secret-scrubs the inference package, bounds optional metadata, preserves the child task/cwd, validates exact structured output, retries malformed output once, and fails closed on every configuration/guide/scope/provider/output/agent/model/thinking error.
• Runs once for single children, each parallel child, and each chain step after substitution.

Required user controls

• Interactive TUI /settings now exposes enable/disable, exact authenticated arbiter model, thinking, and guide path.
• TUI enablement validates model capability plus the current live scope and guide; rejected changes remain visibly unpersisted and surface a loud error.
• Dashboard Settings now exposes the same four controls, blocks model-less enablement, uses the existing model picker and RPC validation, rolls back rejected toggles, and shows readiness guidance.
• Both surfaces write only the global policy; project settings cannot shadow it.

Observability

• Persists and emits typed changed/unchanged/failure records without raw arbiter prompt, output, or reasoning.
• Updates final agent identity in the background registry, TUI, JSON/RPC, and dashboard.
• Dashboard parent/subagent views render safe final route and chain-step metadata.

Verification

• Commit hook passed 5,180 tests with 709 live tests skipped.
• Full npm test passed.
• Full npm run build passed.
• npm run verify-workspace-links passed.
• Biome and git diff --check passed.
• Focused settings/control suites passed across TUI, RPC, dashboard API/server, and dashboard screens.
• Isolated Playwright QA verified dashboard rendering, model-less enable rejection/rollback, model selection, enable persistence, and no browser console errors without touching the existing dashboard service.
• Real built-binary dreb -p smoke QA passed.

Commit: 88e575a

---

Progress tracked by mach6

[m-aebrer]

Code Review

Critical

1. Dashboard settings can report and persist arbitration as disabled after the user enables and configures it — packages/dashboard/src/client/screens/settings.tsx:348-350,470-545,1155-1160; packages/coding-agent/src/modes/rpc/rpc-mode.ts:726-778,871-872 (high confidence). Manual QA proves the scoped failure: Settings displays status: disabled even though arbitration is set on, a model is selected, and ~/.dreb/agent/model-routing-guide.md exists. Each dashboard sub-control fire-and-forgets a whole replacement subagentArbiter object copied from the current resource snapshot, while RPC persists that object as the complete policy. Overlapping saves can therefore restore stale enabled: false while retaining the other configured fields. Existing client tests inject an already-correct mocked settings DTO and never exercise a durable global setting through the real dashboard settings transport and reload path, so they do not protect this failure. Writes need serialization/authoritative merge or an optimistic local draft plus refresh, with a server-backed reopen/refresh regression test.

Important

2. Already-running sessions do not observe global arbiter changes made by another runtime — packages/coding-agent/src/core/settings-manager.ts:1228-1230; packages/coding-agent/src/core/agent-session.ts arbiter settings dependency (high confidence). getGlobalSubagentArbiterSettings() returns only the instance's cached globalSettings, while dashboard writes occur through a utility runtime. Existing sessions can therefore continue using stale disabled or old routing policy after the dashboard persists a change. The context trust policy already provides the appropriate fresh-global-file precedent.

3. The new external inference path lacks custom secret-pattern regression coverage — packages/coding-agent/test/dispatch-arbiter.test.ts; arbiter wiring in packages/coding-agent/src/core/agent-session.ts (high confidence). Tests cover built-in redaction but never supply getExtraSecretPatterns or prove that configured secretOutputPatterns reach the serialized completeSimple() package. A wiring regression could disclose user-defined secrets while the existing suite remains green.

4. TUI readiness and runtime arbitration resolve relative guide paths against different directories — packages/coding-agent/src/modes/interactive/interactive-mode.ts:3900-3903; packages/coding-agent/src/core/dispatch-arbiter.ts:285 (medium confidence). TUI enablement validates against process.cwd(), while runtime loads relative to the child request cwd. The control may accept a configuration that dispatch later rejects, or reject one that would work.

5. Project-scoped arbiter configuration is ignored without explaining the global-only rule — packages/coding-agent/src/core/settings-manager.ts:1228-1235; packages/coding-agent/src/modes/rpc/rpc-mode.ts:871-887 (medium confidence). Ignoring project configuration is correct, but a populated project policy disappears from merged/RPC state and the dashboard simply says disabled. Unlike the existing agentModels shadow warning, no actionable warning identifies the ignored project field.

6. Malformed enabled values are displayed as benignly disabled although runtime blocks spawning — packages/dashboard/src/client/screens/settings.tsx:348-350; packages/coding-agent/src/core/dispatch-arbiter.ts:249-252 (medium confidence). A hand-edited truthy non-boolean value such as "true" renders as disabled, while arbitration returns invalid_config and fail-closes every subagent launch. The settings status should distinguish intentional disablement from invalid configuration.

Suggestions

7. Chain fail-closed termination is not directly tested — packages/coding-agent/test/subagent-arbiter.test.ts (medium confidence). Success is covered for chain mode and failure for a direct single execution, but no test proves a failed chain arbitration prevents that spawn and every later step.

8. The total serialized arbiter-package bound is untested — packages/coding-agent/test/dispatch-arbiter.test.ts (medium confidence). No fixture exercises the separate 180,000-character context_too_large branch and proves inference is not called.

9. Arbitration failure handling repeats the same record-and-return block — packages/coding-agent/src/core/tools/subagent.ts:1183-1260 (medium confidence). A local failArbitration(code, message) helper would centralize record construction, swallowed record-emission failure, and the failed SubagentResult.

10. Dashboard server and client duplicate arbitration-event projection — packages/dashboard/src/server/runtime-pool.ts:517-530; packages/dashboard/src/client/state/reducer.ts:766-780 (medium confidence). Both construct the same DTO with repeated assertions and final-agent update. A shared typed projection helper would prevent drift.

11. Settings renders arbiter readiness from two independent calls — packages/dashboard/src/client/screens/settings.tsx:550-556 (medium confidence). Deriving the readiness object once is clearer and guarantees class/message use one snapshot.

12. Canonical provider/model parsing is repeated inline — packages/coding-agent/src/core/tools/subagent.ts:1211; packages/coding-agent/src/modes/rpc/rpc-mode.ts:764; packages/coding-agent/src/modes/interactive/interactive-mode.ts:3881 (low confidence). Reuse one shared parser rather than repeating slash-index validation.

Strengths

• Arbitration is headless, tool-less, dependency-injected, and consistently fail-closed before spawn.
• Guide parsing is deterministic, size-bounded, and exact about schema and live-scope coverage.
• Strict three-field decision parsing prevents model-authored explanations or arbitrary output from crossing the control boundary.
• Typed persistence and TUI/RPC/dashboard propagation keep arbitration records separate from reconstructed model context.
• Tests already cover disabled passthrough, successful single/parallel/chain routing, malformed-output retry, timeout/abort, direct no-spawn failures, immutability, and dashboard event projection.

Agents run: code-reviewer, error-auditor, test-reviewer, completeness-checker, simplifier

---

Reviewed by mach6

[m-aebrer]

Review Assessment

Review comment

Classifications

Finding	Classification	Reasoning
1. Dashboard can persist arbitration as disabled after enablement	genuine	Factual: Dashboard handlers fire-and-forget complete policy replacements derived from the current resource snapshot; overlapping saves can carry stale enabled: false, and RPC serializes rather than field-merges those stale objects. Scope: This is the user's confirmed manual-QA failure in a required first-class control surface.
2. Running sessions do not observe global arbiter changes	genuine	Factual: Dispatch reads the SettingsManager instance's cached global policy, while dashboard writes occur through another utility runtime; the context-trust policy already demonstrates fresh cross-process reads. Scope: Required dashboard changes must affect subsequent child spawns in active sessions.
3. No custom secret-pattern regression coverage	genuine	Factual: Production wiring passes compiled secretOutputPatterns, but arbiter and integration tests never prove they reach and scrub the outbound completion package. Scope: Custom-pattern scrubbing before inference is an explicit security/testing requirement.
4. Relative guide paths use different directories	deferred	Factual: TUI validates relative to process.cwd(), while dispatch resolves against request cwd. Scope: The required standard path is home-relative and explicit replacements can be absolute; runtime remains fail-closed. Consistent relative-path policy is optional follow-up.
5. No targeted warning for ignored project arbiter policy	deferred	Factual: A project arbiter object is ignored without a field-specific diagnostic, although the UI documents the global-only rule. Scope: Ignoring project policy is required; an extra warning for hand-edited project settings is optional diagnostics.
6. Malformed enabled appears disabled in dashboard	deferred	Factual: Non-boolean hand-edited values render as disabled while runtime returns invalid_config; RPC writes cannot create them. Scope: Runtime already fails closed and reports the dispatch error. Dashboard-specific malformed-file diagnostics are optional hardening.
7. Chain fail-closed termination is not directly tested	genuine	Factual: Success is tested for chain mode and failure for direct execution, but no chain arbitration-failure test proves later steps do not arbitrate or spawn. Scope: The authoritative testing plan explicitly requires chain stop on failed arbitration.
8. Total package-size rejection is untested	genuine	Factual: No test exercises the separate 180,000-character context_too_large branch or verifies zero completion calls. Scope: Deterministic aggregate bounds and pre-inference failure are explicit safety requirements.
9. Repeated arbitration failure blocks	nitpick	Factual: Similar record-and-return structures are repeated, but current branches preserve correct codes and fail before spawn. Scope: Helper extraction is readability-only.
10. Server and client duplicate event projection	nitpick	Factual: Both layers construct similar DTOs, but they serve separate state layers and currently agree. Scope: Shared projection would reduce possible future drift, not fix a current scoped defect.
11. Readiness is derived twice	nitpick	Factual: JSX calls the same pure helper twice against the same resource accessor. Scope: Computing once is marginally clearer and does not change behavior.
12. Canonical model parsing is repeated	nitpick	Factual: Slash parsing is repeated, with some contract differences around whitespace. Scope: Consolidation is a refactoring preference, not a demonstrated routing failure.

Action Plan

1. Make dashboard arbiter edits authoritative and race-free; serialize saves or maintain an optimistic draft, then verify durable enabled state through the real settings transport and reload path.
2. Refresh the global arbiter policy before each dispatch and test cross-runtime enable, disable, and policy changes.
3. Add an AgentSession-level custom secret-pattern test that captures the outbound arbiter completion package and proves redaction.
4. Exercise aggregate context_too_large rejection and prove inference is never called.
5. Fail arbitration in a middle chain step and prove that step and all later steps never spawn.

Counts: 5 genuine, 4 nitpicks, 0 false positives, 3 deferred.

---

Assessment by mach6

[m-aebrer]

Progress Update

Fixed all five genuine review findings selected for this batch:

• Serialized and optimistically projected dashboard Dispatch Arbiter edits so overlapping control changes cannot restore stale enabled: false state.
• Added dashboard client race coverage and settings API round-trip coverage for durable enabled status.
• Made active sessions refresh the global-only arbiter policy before each dispatch, including cross-runtime enable, disable, and policy changes; reload failures now fail closed with an actionable configuration error.
• Proved configured custom secret patterns scrub the outbound arbiter inference package.
• Added aggregate package-size rejection coverage proving inference is never called.
• Added chain failure coverage proving a failed arbitration prevents that child and every later chain step from spawning.

Verification passed:

• Commit hook: 5,187 passed, 709 live tests skipped
• Full npm test
• Full npm run build
• Targeted 350-test arbiter/dashboard suite
• Biome and git diff --check
• Workspace-link verification

Commit: 870c729

---

Progress tracked by mach6

[m-aebrer]

Code Review

Critical

None.

Important

1. Parent tool output can be sent to the arbiter provider — packages/coding-agent/src/core/dispatch-arbiter.ts:235-236,442, context-buffer.ts:120-137, agent-session.ts:1050-1058 (high confidence). recentActivity includes tool_execution_end output, so reads, diffs, shell output, and arbitrary file contents can enter the outbound package. Secret scrubbing only removes recognized/custom patterns and cannot enforce the explicit prohibition on file contents, diffs, or environment values. Record only tool name/status for arbitration, not tool output.

2. Slashful model IDs can lose the arbiter-selected provider at spawn — packages/coding-agent/src/core/tools/subagent.ts:365-373,1265-1269 (high confidence). The validated scoped model is reduced to selectedModel.id, while spawnSubagent() omits --provider whenever that ID contains /. For a canonical route such as openrouter/openai/gpt-oss-120b, the child receives only --model openai/gpt-oss-120b and may resolve under the wrong provider. Preserve the canonical route or pass the selected provider independently of slashes inside the model ID.

3. Strict exact-object output parsing lacks a control-boundary regression test — packages/coding-agent/src/core/dispatch-arbiter.ts and packages/coding-agent/test/dispatch-arbiter.test.ts (high confidence). The suite does not reject an otherwise valid decision with an extra model-authored key such as rationale. Add a two-attempt fixture proving malformed_output, one retry, and absence of arbitrary output from safe records/errors.

Suggestions

4. Dashboard disable can be rejected by a stale invalid model or thinking setting — packages/dashboard/src/client/screens/settings.tsx:324-336,531, packages/coding-agent/src/modes/rpc/rpc-mode.ts:763-775 (medium confidence). Turning arbitration off sends the entire old policy, and RPC still resolves/validates its model and thinking. If that model disappeared or became unsupported, users may be unable to disable a fail-closed policy through the required control. Allow a minimal disabled policy or skip availability/capability validation when disabling.

5. Guide-heading coverage and schema-version validation are not directly tested — packages/coding-agent/src/core/model-routing-guide.ts, packages/coding-agent/test/model-routing-guide.test.ts (medium confidence). The current missing/extra fixture fails at frontmatter scope comparison before heading validation, and no wrong-schema fixture exists. Keep frontmatter valid while independently varying headings, and add a schema_version: 2 case.

6. Arbiter API-key lookup failure is untested — packages/coding-agent/src/core/dispatch-arbiter.ts, packages/coding-agent/test/dispatch-arbiter.test.ts (medium confidence). Provider completion failure is covered, but registry.getApiKey() rejection is not. Test a secret-bearing auth error, safe arbiter_model failure, zero completion calls, and integration-level zero spawn.

7. In-flight cancellation is not tested through the spawn lifecycle — packages/coding-agent/src/core/tools/subagent.ts, packages/coding-agent/test/dispatch-arbiter.test.ts, subagent-arbiter.test.ts (medium confidence). Existing abort coverage aborts before arbitrate(). Hold inference pending, cancel the background agent after inference starts, and prove no child can spawn afterward.

8. Changed-agent system-prompt immutability is not asserted — packages/coding-agent/src/core/tools/subagent.ts, packages/coding-agent/test/subagent-arbiter.test.ts (medium confidence). Existing assertions cover final agent/model/thinking/tools but not --append-system-prompt. Assert the selected definition's prompt is passed verbatim and the requested definition's prompt is absent.

9. Parallel arbitration is not tested under the concurrency gate — packages/coding-agent/src/core/tools/subagent.ts, packages/coding-agent/test/subagent-arbiter.test.ts (medium confidence). Immediate promises prove exactly-once calls but not semaphore enforcement. Hold a batch of arbitration promises open and assert active inference never exceeds configured background concurrency.

10. Failed arbitration is not tested end-to-end through persistence and dashboard rendering — packages/coding-agent/src/core/agent-session.ts, packages/dashboard/src/client/screens/subagent.tsx, related tests (medium confidence). Current integration/UI cases use successful records. Drive a real failure with final: null, safe error fields, no spawn, non-context JSONL persistence, RPC/dashboard relay, and visible failure rendering without raw output.

11. Final routed identity is not tested after dashboard snapshot hydration/resync — dashboard state/runtime tests (medium confidence). Live event projection is covered, but browser reload/recovery could restore the requested identity. Hydrate a snapshot containing a changed arbitration and assert final agent identity, ordered history, and rendered route survive without a new event.

12. Synchronous git status can block the event loop on every arbitration — packages/coding-agent/src/core/agent-session.ts:409-424, git-repo-state.ts:22-37 (low confidence). Each child can invoke spawnSync("git", "status") with a five-second timeout. Reuse session metadata or use async/short-lived cached refresh to avoid TUI/RPC/dashboard stalls.

13. Dashboard arbitration DTO projection is duplicated — packages/dashboard/src/client/state/reducer.ts:769-779, packages/dashboard/src/server/runtime-pool.ts:517-527 (medium confidence). Extract the identical seven-field mapper into shared/protocol.ts so server and client cannot drift.

14. Failure record construction is repeated in executeSingle() — packages/coding-agent/src/core/tools/subagent.ts:1162-1270 (medium confidence). A local failArbitration(errorCode, errorMessage) helper can centralize the host record, best-effort emission, and failed result without changing behavior.

15. modelDefaults normalization uses a deeply nested ternary — packages/coding-agent/src/core/tools/subagent.ts:233-239 (low confidence). Normalize the scalar/array value with a small helper or equivalent flat expression.

16. The arbiter system prompt is a no-argument function returning a constant — packages/coding-agent/src/core/dispatch-arbiter.ts:203-212 (low confidence). A module constant states the invariant more directly.

17. Dashboard readiness is derived twice in one render — packages/dashboard/src/client/screens/settings.tsx:595-598 (low confidence). Compute the readiness object once, preferably with a memo if reused.

Strengths

• The implementation is genuinely headless and tool-less, with strict host validation, bounded context, one malformed-output retry, and fail-closed pre-spawn integration.
• Global-only settings refresh, dashboard save serialization, custom secret-pattern wiring, aggregate package bounds, and chain failure termination are now directly covered.
• Guide parsing and live-scope comparison are deterministic, and observability keeps validated host records separate from reconstructed model context.
• TUI, RPC, dashboard, documentation, and first-class settings controls comprehensively cover the approved stage 2 surface.

Agents run: code-reviewer, error-auditor, test-reviewer, completeness-checker, simplifier

---

Reviewed by mach6

[m-aebrer]

Review Assessment

Review comment

Classifications

Finding	Classification	Reasoning
1. Parent tool output can reach the arbiter	genuine	Factual: onToolEnd() feeds labelToolEnd(), which embeds result.output/result.content in parent.recentActivity sent to inference. Scope: The issue categorically prohibits file contents, diffs, and environment values in arbiter context; pattern scrubbing cannot enforce that rule.
2. Slashful model IDs lose the selected provider	genuine	Factual: Post-arbitration routing stores only selectedModel.id; spawn omits --provider whenever that ID contains /, allowing CLI resolution under another provider. Scope: Exact canonical scoped routing with no provider collapse is required.
3. No extra-key strict-output regression test	genuine	Factual: Exact three-key parsing exists, but tests never submit an otherwise-valid object with an extra model-authored key. Scope: Strict exact-object parsing, bounded retry, and exclusion of arbitrary output are explicit control-boundary test requirements.
4. Invalid stale policy can prevent dashboard disable	genuine	Factual: Disable merges and sends the old model/thinking, while RPC validates their current availability/capability even when enabled is false. Scope: Dashboard enable/disable is a hard requirement; users must be able to disable a fail-closed policy whose model became invalid.
5. Guide heading coverage and schema version lack direct tests	genuine	Factual: Current missing/extra fixtures fail at frontmatter comparison before heading validation, and no wrong-schema fixture exists. Scope: Wrong schema and independent missing/extra heading coverage are explicitly required in the plan.
6. API-key lookup failure is untested	genuine	Factual: getApiKey() has a distinct safe failure path, but tests only cover completion/provider rejection. Scope: Auth failure must fail closed, scrub errors, avoid inference, and prevent spawn; the plan explicitly requires this test.
7. In-flight cancellation lacks lifecycle coverage	genuine	Factual: Unit coverage aborts before arbitration starts, and no background lifecycle test aborts pending inference and proves spawn remains uncalled. Scope: Cancellation during arbitration is explicitly required to fail closed.
8. Changed-agent system prompt is not asserted	genuine	Factual: Changed-route tests assert agent/model/thinking/tools but not the selected definition's --append-system-prompt or exclusion of the requested prompt. Scope: Selecting another agent must apply that existing definition verbatim.
9. Parallel arbitration is not tested under semaphore pressure	genuine	Factual: Production appears gated, but tests use immediately resolved promises and never measure active arbitration concurrency. Scope: The plan explicitly relies on and requires testing the existing concurrency gate for parallel arbitration.
10. Failed arbitration lacks end-to-end persistence/dashboard coverage	genuine	Factual: Integration, persistence, reducer/runtime, and screen tests use successful records; no real failure proves final: null, safe errors, no spawn, non-context persistence, relay, and visible rendering. Scope: Typed failure observability across JSONL, RPC, and dashboard is required.
11. Routed identity lacks snapshot/hydration coverage	genuine	Factual: Live-event projection is tested, but snapshot/resync tests do not include changed arbitration or verify final identity/history/rendering after reload. Scope: Snapshot/hydration preservation is explicit in the plan.
12. Synchronous git status can block arbitration	deferred	Factual: Each enabled attempt can synchronously run git status with a five-second timeout. Scope: This is valid performance hardening, but async/cached metadata and latency guarantees are not required for stage 2.
13. Dashboard arbitration projection is duplicated	deferred	Factual: Server and client currently construct the same seven-field DTO and agree. Scope: Shared extraction is optional maintainability work, not a current correctness requirement.
14. Failure record construction is repeated	nitpick	Factual: Similar record/emission/result blocks are repeated but retain correct codes and fail before spawn. Scope: Helper extraction is readability-only.
15. modelDefaults uses a nested ternary	nitpick	Factual: The normalization expression is nested but produces correct values. Scope: Readability-only.
16. System prompt function returns a constant	nitpick	Factual: The function takes no inputs and returns fixed text, but is deterministic and correct. Scope: Constant extraction is stylistic.
17. Readiness is derived twice per render	nitpick	Factual: The same pure helper is called twice against the same synchronous accessor. Scope: Computing once is marginal cleanup with no behavior change.

Action Plan

1. Remove tool result/output from arbiter activity context; retain only host-derived tool name/status, and prove file/diff/shell content is absent.
2. Preserve the selected provider independently when spawning slashful model IDs; add a multi-provider exact-route regression.
3. Ensure users can always disable arbitration despite stale invalid model/thinking fields.
4. Add strict extra-key output tests covering one retry, final malformed_output, and no arbitrary output in safe records/errors.
5. Add API-key lookup rejection coverage for scrubbing, zero completion calls, and zero spawn.
6. Add lifecycle cancellation coverage that aborts after inference begins and proves no child spawn.
7. Add end-to-end failed-arbitration observability coverage through safe non-context persistence, RPC/dashboard projection, and visible rendering.
8. Add independent guide-heading coverage and wrong-schema-version tests.
9. Assert the changed route passes the selected agent's system prompt verbatim and excludes the requested prompt.
10. Hold parallel arbitration calls pending and prove active inference never exceeds configured background concurrency.
11. Hydrate/resync a changed arbitration snapshot and verify final identity, ordered history, and rendered route survive without a live event.

Counts: 11 genuine, 4 nitpicks, 0 false positives, 2 deferred.

Deferred follow-up: synchronous git metadata collection performance (finding 12) and shared dashboard projection extraction (finding 13). No tracking issues were identified.

---

Assessment by mach6

[m-aebrer]

Human Scope Correction — Arbiter Context

The linked issue body has been corrected to match the intended title-setter architecture:

• bounded recent assistant activity and tool outputs are intentional arbiter inputs;
• useful file contents, diffs, command output, and other tool results must not be categorically removed;
• there is no separate security boundary between parent-session activity and arbiter context;
• existing secret scrubbing and deterministic bounds still apply as normal model-call content hygiene.

This human-approved correction supersedes finding 1 in the latest review and assessment. That finding is now a false positive; implementation must not remove tool outputs from arbiter context.

Revised counts: 10 genuine, 4 nitpicks, 1 false positive, 2 deferred. The remaining genuine findings are findings 2 through 11 from the latest review.

The review session also proves that arbitration was active for all six spawned reviewers. Five routes were unchanged; the completeness checker changed from uptimize-deepseek/deepseek-v4-pro-gs at high to anthropic/claude-sonnet-4-6 at medium.

---

Scope corrected by human mandate

[m-aebrer]

Progress Update

Fixed all remaining genuine findings from the latest review after the human scope correction:

• Preserved the arbiter-selected provider when raw model IDs contain slashes, with an ambiguous multi-provider regression proving the exact child CLI route.
• Made explicit disablement succeed even when a retained arbiter model is no longer available or its thinking capability is stale; re-enabling still performs full validation.
• Added strict extra-key structured-output rejection and no-raw-output coverage.
• Added independent guide-heading coverage and unsupported schema-version tests.
• Added scrubbed API-key lookup failure coverage through both the headless arbiter and AgentSession, proving zero inference/spawn and safe non-context persistence.
• Added in-flight abort coverage through the direct model call and background-agent lifecycle.
• Proved changed routes pass the selected agent definition's system prompt verbatim.
• Proved pending parallel arbitration stays inside the existing four-agent concurrency gate.
• Added failed-decision projection/rendering coverage across parent JSONL, reducer, runtime snapshots, and dashboard drill-in without raw model content.
• Added hydrate and resync coverage preserving final routed identity, ordered chain records, and rendered route after reload.
• Corrected root/package/settings/agent-model documentation to explicitly retain bounded title-setter-style tool outputs with normal secret scrubbing. Tool outputs were not removed.

Verification passed:

• Commit hook: 5,204 passed, 709 live tests skipped
• 431 targeted arbiter/dashboard tests
• Full npm test
• Full npm run build
• Biome and git diff --check
• Workspace-link verification

Commit: cc7f85a

---

Progress tracked by mach6

[m-aebrer]

Code Review

Critical

None.

Important

None.

Suggestions

1. Repository metadata describes the parent cwd rather than the dispatched child cwd — packages/coding-agent/src/core/agent-session.ts:416-420, packages/coding-agent/src/core/dispatch-arbiter.ts:417,444-448 (medium, 82% confidence). A child dispatched to another repository receives the correct immutable child.cwd but parent-repository basename, branch, and dirty count, giving the arbiter contradictory context. Collect metadata from the request cwd, retaining session metadata only as a same-cwd fallback.

2. Tool-result retention is not asserted through the complete arbiter wiring path — packages/coding-agent/test/dispatch-arbiter.test.ts (medium, 99% confidence). The direct arbiter test checks the tool label but not the output sentinel, so dropping output between onToolEnd() and the serialized provider package would remain green despite the corrected requirement. Capture the completion package and assert a unique long output is retained and deterministically truncated.

3. Required routing-guide document headings lack fail-closed negative coverage — packages/coding-agent/test/model-routing-guide.test.ts (medium, 99% confidence). Tests do not independently remove the root Model Routing Guide heading or Routing safeguards section while leaving the rest valid. Add focused validator cases and prove invalid guides prevent inference.

4. Interactive settings readiness is tested only behind a mocked callback — packages/coding-agent/test/settings-selector-thinking-display.test.ts, packages/coding-agent/src/modes/interactive/interactive-mode.ts (medium, 98% confidence). No test drives the real callback through invalid guide, empty scope, unsupported thinking, and valid persistence paths. Add interactive-mode coverage proving errors are loud and rejected settings are not persisted.

5. Dashboard save rejection and durable rollback are untested — packages/dashboard/test/client/screens.test.tsx (medium, 98% confidence). Existing tests cover successful saves and overlap serialization, not an optimistic edit rejected by api.saveSettings() followed by refetch of the prior durable policy. Add a rejection case asserting visible rollback and surfaced server error.

6. Accepted cwd preservation is not asserted through arbitration and spawn — packages/coding-agent/test/subagent-arbiter.test.ts (medium, 96% confidence). Tests cover task immutability and invalid cwd rejection but not a valid distinct cwd reaching both the arbitration request and spawn unchanged. Add direct and chain assertions.

7. Synchronous git status can stall the parent event loop for each enabled arbitration — packages/coding-agent/src/core/agent-session.ts:415-423, packages/coding-agent/src/core/git-repo-state.ts:12-33 (low, 90% confidence). spawnSync() may block up to five seconds per dispatch and serializes parallel calls, freezing TUI/RPC/dashboard liveness. This was previously recognized as deferred performance hardening; an async or short-lived cached refresh would avoid the stall.

8. Arbitration failure record construction is repeated across executeSingle() — packages/coding-agent/src/core/tools/subagent.ts:1162-1270 (medium, 90% confidence). Extracting a local failArbitration(code, message) helper would remove repeated host-record, best-effort emission, and failed-result blocks without changing behavior.

9. Dashboard client and server duplicate arbitration-event projection — packages/dashboard/src/server/runtime-pool.ts:514-527, packages/dashboard/src/client/state/reducer.ts:766-779 (medium, 88% confidence). Move the identical seven-field projection to a typed helper in shared/protocol.ts to prevent drift.

10. Dashboard readiness is derived twice in one render — packages/dashboard/src/client/screens/settings.tsx:595-598 (low, 85% confidence). Compute one readiness value for both class and message to make the snapshot relationship explicit.

11. The arbiter system prompt is rebuilt by a no-argument function — packages/coding-agent/src/core/dispatch-arbiter.ts:203-210 (low, 82% confidence). A module constant more directly communicates the prompt invariant and removes repeated array construction.

Strengths

• Arbitration is genuinely headless, tool-less, strictly parsed, and consistently fail-closed before spawn.
• Single, parallel, and post-substitution chain routing share one enforcement path with robust abort and concurrency behavior.
• Exact canonical provider/model routing, refreshed global-only settings, secret scrubbing, and safe host-generated observability are well implemented.
• TUI, JSON/RPC, dashboard, persistence, hydration, and failure rendering coverage is broad.
• The implementation and documentation correctly retain bounded tool outputs after the human scope correction.

Agents run: code-reviewer, error-auditor, test-reviewer, completeness-checker, simplifier

---

Reviewed by mach6

[m-aebrer]

Review Assessment

Review comment

Classifications

Finding	Classification	Reasoning
1. Repository metadata uses the parent cwd	genuine	Factual: The package combines request child cwd with basename, branch, dirty count, and repository cwd derived only from the parent session cwd. Scope: The plan requires repository metadata for the dispatched child; contradictory routing context can affect the decision.
2. Tool-output retention lacks complete-path coverage	genuine	Factual: The arbiter test provides tool output but asserts only the host label, while lower-level buffer tests do not prove output reaches the serialized completion package. Scope: The human correction explicitly requires bounded tool outputs, so direct regression coverage must ship.
3. Required guide headings lack negative coverage	genuine	Factual: Tests never remove the required root heading or routing-safeguards section while preserving an otherwise valid guide. Scope: Required headings, guide validation, and pre-inference fail-closed behavior are explicit plan requirements.
4. Real TUI readiness callback is untested	genuine	Factual: Selector tests mock the callback and never exercise real model, thinking, scope, guide, error, or persistence behavior in InteractiveMode. Scope: Loudly validated first-class TUI controls are a human-mandated requirement.
5. Dashboard save rejection and rollback are untested	genuine	Factual: Tests cover successful and overlapping saves but not a rejected optimistic update followed by restoration of durable state. Scope: Required dashboard controls must surface server rejection and not display unsaved policy.
6. Accepted cwd preservation is untested	genuine	Factual: Production appears to pass cwd correctly, but tests cover default cwd and invalid escape only, not a valid distinct cwd through arbitration and spawn. Scope: Byte-for-byte cwd preservation is an explicit criterion and testing-plan item.
7. Synchronous git status can stall the event loop	deferred	Factual: Each enabled attempt can call synchronous git status with a five-second timeout, serializing calls on the event loop. Scope: Async/cached collection and latency guarantees are performance hardening outside Stage 2 and were not human-authorized later.
8. Arbitration failure construction is repeated	nitpick	Factual: Similar failure record/emission/result blocks repeat but currently retain correct codes and prevent spawn. Scope: Helper extraction is readability-only.
9. Dashboard arbitration projection is duplicated	deferred	Factual: Server and client independently construct the same DTO and currently agree. Scope: Sharing a mapper reduces future drift but fixes no current scoped defect.
10. Dashboard readiness is derived twice	nitpick	Factual: The same pure helper is called twice against the synchronous current state. Scope: Computing once is minor cleanup with no behavior change.
11. System prompt is rebuilt by a function	nitpick	Factual: The no-argument function constructs the same fixed prompt each time. Scope: Replacing it with a constant is stylistic.

Action Plan

1. Make repository metadata request-cwd-aware and add a distinct-repository regression test.
2. Add complete-path tool-output coverage, including deterministic truncation.
3. Add missing root-heading and routing-safeguards invalid-guide cases proving inference is not called.
4. Test the real interactive settings callback for invalid and valid readiness/persistence paths.
5. Test dashboard save rejection, loud error display, and durable rollback.
6. Test valid distinct cwd preservation through direct and chain arbitration/spawn paths.

Counts: 6 genuine, 3 nitpicks, 0 false positives, 2 deferred.

Deferred follow-up: synchronous git metadata performance and shared dashboard projection extraction. No tracking issues were identified; there are no deferred test gaps.

---

Assessment by mach6

[m-aebrer]

Progress Update

Fixed all six genuine findings from the latest review assessment:

• Repository metadata now follows each dispatched child cwd, so cross-repository routing receives the matching basename, branch, dirty count, and cwd rather than contradictory parent-session metadata.
• Added complete-path coverage proving bounded tool output reaches the serialized arbiter package with an explicit truncation marker.
• Added focused missing-root-heading and missing-routing-safeguards guide tests, including pre-inference fail-closed assertions.
• Extracted the real interactive settings callback into a testable method and covered invalid guide, empty scope, unsupported thinking, and successful global persistence.
• Fixed dashboard rejected-save rollback so the authoritative RPC error remains visible after refetch restores durable state; added regression coverage.
• Added direct and chain tests proving valid distinct cwd values reach arbitration and child spawn unchanged.

Verification passed:

• Commit hook: 5,213 passed, 709 live tests skipped
• 201 targeted arbiter/TUI/dashboard tests
• Full npm run build
• Full offline npm test
• npm run check
• Workspace-link verification and git diff --check

A separate live-enabled test run reached the external OpenAI Codex enterprise usage limit; the complete deterministic/offline suite passed.

Commit: d77ad9d

---

Progress tracked by mach6

[m-aebrer]

Code Review

Critical

None.

Important

None.

Suggestions

1. Dashboard/RPC disable can still be blocked by malformed retained arbiter fields — packages/coding-agent/src/modes/rpc/rpc-mode.ts:736-766, packages/dashboard/src/client/screens/settings.tsx:524-533 (medium, 86% confidence). The dashboard merges enabled: false into the complete retained policy, and RPC validates malformed model, thinking, and guidePath fields before treating the request as disablement. A hand-edited fail-closed policy such as model: "bad-model" can therefore block every spawn while the required dashboard disable control cannot recover it. Explicit disablement should persist minimally or short-circuit nested validation; re-enablement should remain strict.

2. Failed pre-spawn arbitrations cannot hydrate in drill-in when no child log exists — packages/dashboard/src/server/server.ts:824-828, packages/dashboard/src/server/subagent-log.ts:149-153 (medium, 84% confidence). The drill-in endpoint finds the background-agent metadata and then unconditionally reads a child log. A correctly fail-closed pre-spawn arbitration has no child JSONL, so direct load/reload throws instead of returning the safe arbitration record with messages: []. Keep unknown-agent errors loud, but allow known pre-spawn failures to hydrate without a child transcript.

3. Individual route changes lack spawn-level integration coverage — packages/coding-agent/src/core/tools/subagent.ts:1209-1322, packages/coding-agent/test/subagent-arbiter.test.ts:193-301 (medium, 99% confidence). Tests cover unchanged and all-fields-changed routes but not agent-only, model-only, and thinking-only decisions. Parameterized cases should prove each changed field is applied while the other two, the selected prompt/tools, result metadata, and final recorded route remain correct.

4. The valid local-history guide branch lacks a positive fixture — packages/coding-agent/src/core/model-routing-guide.ts:139-155, packages/coding-agent/test/model-routing-guide.test.ts:45-78 (medium, 99% confidence). Existing fixtures appear to use local_evidence: "cold-start"; add a valid "available" guide with non-null date bounds plus invalid available/cold-start date combinations so established users' guides cannot be rejected by an untested validator regression.

5. Arbitration failure construction is repeated across executeSingle() — packages/coding-agent/src/core/tools/subagent.ts:1160-1270 (medium, 95% confidence). Four branches repeat the same failure record, best-effort observer call, and failed SubagentResult. A local helper parameterized by error code/message would remove substantial duplication while preserving the intentional message-prefix distinction.

6. Dashboard arbitration-event projection is duplicated — packages/dashboard/src/server/runtime-pool.ts:514-530, packages/dashboard/src/client/state/reducer.ts:766-782 (medium, 95% confidence). Server and client construct the same seven-field DTO and apply the same final-agent update. A typed shared projection helper would prevent drift.

7. Dashboard readiness is derived twice in one render — packages/dashboard/src/client/screens/settings.tsx:597-601 (low, 92% confidence). Compute arbiterReadiness(current()) once so class and message come from one explicit snapshot.

8. modelDefaults uses a deeply nested ternary — packages/coding-agent/src/core/tools/subagent.ts:233-239 (low, 85% confidence). Extract or flatten the normalization branch so string, array, and absent defaults remain obvious without nested conditionals.

9. The fixed system prompt is rebuilt by a no-argument function — packages/coding-agent/src/core/dispatch-arbiter.ts:203-212 (low, 80% confidence). A module constant would communicate the invariant directly and avoid reconstructing the same string.

Strengths

• Core arbitration is tool-less, strictly parsed, bounded, and consistently fail-closed before spawn.
• Error auditing found no silent fallback, timeout/cancellation, settings freshness, or error-leakage defects at the reporting threshold.
• Exact provider/model propagation, selected agent definitions, rolling tool-output context, concurrency gating, and persistence/dashboard projection have broad regression coverage.
• Completeness review found every Stage 2 acceptance criterion and explicit TUI/dashboard control mandate implemented at the current HEAD.
• The internal arbiter uses clear discriminated result types and well-scoped helpers for decision validation, error scrubbing, signal handling, and changed-field calculation.

Agents run: code-reviewer, error-auditor, test-reviewer, completeness-checker, simplifier

---

Reviewed by mach6

[m-aebrer]

Review Assessment

Review comment

Classifications

Finding	Classification	Reasoning
1. Dashboard disable blocked by malformed retained fields	genuine	Factual: The dashboard merges enabled: false into the retained complete policy, while RPC validates malformed model, thinking, and guide-path values before its availability exception. Scope: Dashboard enable/disable is human-mandated, and users must be able to recover from a policy that fails closed on every spawn.
2. Failed pre-spawn arbitration cannot hydrate without a child log	genuine	Factual: The endpoint finds the agent and then unconditionally reads a child session file; pre-spawn failure records metadata but creates no child JSONL. Scope: Required dashboard failure observability and hydration/reload behavior must work for fail-closed attempts while unknown IDs remain loud errors.
3. Individual route changes lack spawn-level coverage	genuine	Factual: Integration tests change all three fields together or agent/model together, but do not cover agent-only, model-only, and thinking-only decisions with unchanged-field assertions. Scope: The explicit plan requires unchanged and each changed field alone/together.
4. Valid local-history guide branch lacks a positive fixture	genuine	Factual: Test guides use cold-start with null date bounds; the available-history branch requiring two non-null bounds and inverse invalid combinations is untested. Scope: The plan explicitly requires valid cold-start/history guides, and local-history evidence is a core guide contract.
5. Repeated arbitration failure construction	nitpick	Factual: Several branches repeat the same failure record, best-effort observer call, and failed result. Scope: Current branches preserve their distinct codes/messages and block spawn correctly; helper extraction is readability-only.
6. Duplicated dashboard arbitration projection	deferred	Factual: Server and client independently build the same seven-field DTO and update final identity. Scope: Both currently agree; sharing a mapper is optional maintainability work rather than a Stage 2 correctness requirement. No tracking issue exists or is necessary unless broader projection cleanup is planned.
7. Readiness derived twice in one render	nitpick	Factual: JSX calls the same pure readiness helper twice against current state. Scope: Output is correct; computing once is minor clarity cleanup.
8. Deeply nested modelDefaults ternary	nitpick	Factual: Scalar, array, and absent defaults are normalized through nested conditionals. Scope: The expression is correct and flattening it is stylistic.
9. Fixed system prompt rebuilt by a function	nitpick	Factual: The no-argument function joins the same fixed strings each call. Scope: Its deterministic output is correct; a constant is stylistic.

Action Plan

1. Make dashboard disablement recover from malformed retained policies by persisting a minimal disabled policy or bypassing nested validation only for explicit disablement; keep re-enablement strict and add malformed-field regressions.
2. Allow a known failed pre-spawn agent with no child log to hydrate as { agent, messages: [] }, while preserving loud errors for unknown agents.
3. Add parameterized agent-only, model-only, and thinking-only spawn integration tests covering unchanged fields, selected prompt/tools, spawn arguments, result metadata, and recorded route.
4. Add a valid local_evidence: "available" guide fixture with non-null dates and focused invalid available/cold-start date-bound cases.

Counts: 4 genuine, 4 nitpicks, 0 false positives, 1 deferred.

---

Assessment by mach6

[m-aebrer]

Progress Update

Fixed all four genuine findings from the latest review assessment:

• Explicit RPC disablement now bypasses malformed retained arbiter-field validation without clearing those fields, so hand-edited or otherwise malformed fail-closed policies can always be disabled through the dashboard toggle; re-enabling continues to validate fully. The test was updated to prove that malformed model, thinking, and guidePath values do not block the disable path, and the dashboard client now sends the complete retained policy on disable rather than a minimal object.
• Known pre-spawn arbitration failures (all arbitration records have status: "failure") now hydrate with messages: [] instead of throwing. A named SubagentSessionLogNotFoundError class makes the catch selective; unknown agent IDs and other missing-log cases still produce a loud 502.
• Added parameterized spawn-level integration cases for agent-only, model-only, and thinking-only routing changes, each asserting unchanged route fields, selected prompt and tool set, CLI arguments, result metadata, and the persisted record.
• Added a valid local_evidence: "available" guide fixture with non-null date bounds, plus four focused invalid date-combination cases covering available-without-start, available-without-end, cold-start-with-start, and cold-start-with-end.

Verification passed:

• Commit hook: 5,222 passed, 709 live tests skipped
• Full offline npm test
• npm run check
• npm run build
• Workspace-link verification and git diff --check

Commit: 220925f

---

Progress tracked by mach6

[m-aebrer]

Code Review

Round 4. Prior rounds fixed in 88e575a–220925f. Note: the arbiter itself blocked this review's subagent spawns (fail-closed on guide/scope mismatch — working as designed); it was temporarily disabled to run the review and will be restored.

Critical

None.

Important

1. Guide generation needs an UPDATE mode for scope rotation — packages/coding-agent/skills/model-routing-guide/SKILL.md (human manual QA, mandated genuine). When the session scope rotates (add/drop models), the guide's covered_model_ids no longer matches the live scope and every enabled dispatch fails closed — this happened live during this very review session (scope onehpc/Kimi-K3 vs guide covering onehpc/Kimi-K2.6, onehpc-agentworld/Qwen-AgentWorld-35B-A3B). Today recovery requires a full regeneration. Add a simple update workflow: check current enabledModels/scope, diff against the guide's covered IDs, remove unscoped model sections, and research+add newly scoped models — preserving existing entries rather than rebuilding from scratch.

Suggestions

2. Dashboard readiness helper crashes on malformed retained guidePath type, blocking the disable control — packages/dashboard/src/client/screens/settings.tsx:402-407; interacts with the disable bypass in packages/coding-agent/src/modes/rpc/rpc-mode.ts (medium, 86% confidence). The readiness helper calls arbiter.guidePath?.trim() assuming string-or-nullish, but hand-edited settings can hold guidePath: 123, and the round-3 RPC fix intentionally lets malformed retained fields persist while disabled. 123?.trim() throws during render, so the dashboard toggle — the required recovery path for a malformed fail-closed policy — is unreachable exactly when it is needed. Narrow with typeof === "string" before reading, and keep the disable control renderable regardless of retained field types.

3. Missing-log fallback hydration test does not exercise the safety boundary — packages/dashboard/test/server.test.ts:512-564; gate at packages/dashboard/src/server/server.ts:828-836 (medium, 99%). Tests prove the failed-arbitration-no-log case returns messages: [], but not that an ordinary spawned agent with a missing log still errors loudly. A regression returning messages: [] for every missing child log would silently hide real transcript loss. Add a negative case: non-arbiter agent, no sessionFile, expect non-200.

4. Re-enabling after fail-open disablement is not tested — packages/coding-agent/test/rpc-settings-commands.test.ts:507-545 (medium, 96%). The new test verifies disablement accepts retained malformed fields, but never attempts to re-enable that persisted malformed policy to prove it is fully validated, rejected, and leaves the durable policy disabled. A regression here could silently re-enable an unusable policy that blocks every spawn.

5. Duplicate covered_model_ids frontmatter entries lack a regression test — packages/coding-agent/test/model-routing-guide.test.ts:128-169; guard at packages/coding-agent/src/core/model-routing-guide.ts:159-170 (medium, 98%). Duplicate headings are tested, duplicate frontmatter IDs are not; set-based scope comparison alone would accept duplicates against a one-model scope. Add a duplicate-IDs fixture rejected specifically as duplicate coverage before inference.

6. No lifecycle test verifies the registry identity flip to the final agent — packages/coding-agent/src/core/tools/subagent.ts:2115-2122; lifecycle test at packages/coding-agent/test/subagent-arbiter.test.ts:487-564 deliberately returns an unchanged route (medium, 97%). No test drives a changed route (arbiter-a → arbiter-b) through the real background lifecycle and asserts getBackgroundAgents() exposes the final type before child completion. This is the acceptance-critical handoff for TUI/RPC/dashboard identity.

7. Interactive arbitration action rendering is untested — packages/coding-agent/src/modes/interactive/interactive-mode.ts:2791-2795 (medium, 95%). No test drives a subagent_arbitration event through Interactive Mode's handler to assert changed/unchanged records surface via showStatus, failures via showWarning, and updateBackgroundAgentStatus() runs.

8. Arbiter repo metadata is the cwd basename, not the repository — packages/coding-agent/src/core/agent-session.ts:416-423 (low, 84%). For a child cwd that is a subdirectory of a repo, repo: "foo" pairs with branch/dirty-count from the repo root; for non-git directories repo is populated with no git data. Slightly contradictory routing context. Derive from the git root when one exists; omit otherwise; keep cwd exact.

9. Custom clone helper reimplements structuredClone — packages/coding-agent/src/core/tools/subagent.ts:1716-1720 (low, 90%). Spread + selective deep-clone of arbitrations is equivalent to structuredClone(info) for this all-plain-data type; the single call is clearer.

Strengths

• The fail-closed guarantee held under live fire: the arbiter blocked this review's own subagent spawns on a genuine guide/scope mismatch rather than letting an un-arbitrated child through — the designed behavior, observed end-to-end.
• Error-auditor traced every failure path (inference, retry, settings refresh, RPC write, hydration, projection, identity ordering, scrubbing) and found no silent-failure mode at reporting threshold; the SubagentSessionLogNotFoundError swallow is correctly gated on all-failure arbitration records; the minimal-disable bypass is inert at runtime and full re-validation on enable.
• Completeness-checker verified all 13 stage-2 acceptance criteria, all 6 plan deliverables, the human-mandated TUI + dashboard controls, and every plan-listed file — zero gaps.
• Prior-round fixes verified in place with matching regression coverage; documentation stayed consistent with behavior after each round.

Agents run: code-reviewer, error-auditor, test-reviewer, completeness-checker, simplifier (+ human manual QA input)

---

Reviewed by mach6

[m-aebrer]

Review Assessment

Review comment

Classifications

Finding	Classification	Reasoning
1. Guide generation needs an UPDATE mode for scope rotation	genuine	Factual: skills/model-routing-guide/SKILL.md only instructs full generate-and-replace; no path diffs existing covered_model_ids against the new scope, removes stale sections, and researches only new models. validateModelRoutingGuideContent() requires exact live-scope coverage, so rotation fails every dispatch closed. Scope: Human-mandated genuine; the mismatch blocked this review session's own subagent spawns live.
2. Dashboard readiness crashes on malformed retained guidePath	genuine	Factual: arbiterReadiness() calls arbiter.guidePath?.trim(); optional chaining does not guard a non-nullish non-string like 123, so rendering throws. RPC intentionally preserves malformed retained fields while disabled. Scope: The dashboard control is the required recovery path for malformed fail-closed policies; a render crash makes it unreachable exactly when needed.
3. Missing-log fallback lacks negative safety test	genuine	Factual: Endpoint tests cover the all-failure-arbitration messages: [] case but never assert an ordinary registered agent with a missing child log still errors loudly through the same endpoint gate. Scope: The fallback exists for fail-closed hydration; its narrow boundary needs regression protection against silently emptying real transcripts.
4. Re-enabling a malformed disabled policy is untested	genuine	Factual: RPC skips retained-field validation only when enabled === false; the test stops after disablement and never attempts re-enable, validation rejection, and durable-disabled persistence. Scope: Loud validation and recoverable enable/disable are explicit hard requirements for the new transition.
5. Duplicate covered_model_ids guard lacks regression test	genuine	Factual: model-routing-guide.ts rejects duplicate frontmatter IDs via set-size comparison, but tests only cover duplicate model headings. Scope: Duplicate-ID coverage is explicitly required by the approved testing approach.
6. Final registry identity flip lacks lifecycle test	genuine	Factual: entry.agentType = record.final.agent runs before onArbitration emission, but the real AgentSession lifecycle test uses an unchanged route and never observes getBackgroundAgents() mid-flight on a changed route. Scope: Atomic registry identity update before child events is an explicit observability deliverable and testing criterion.
7. Interactive arbitration rendering untested	genuine	Factual: InteractiveMode.handleEvent() routes failures to showWarning, others to showStatus, and refreshes background status, but no test drives changed/unchanged/failed records through the handler. Scope: TUI rendering of all three outcomes is an explicit deliverable; formatter unit tests do not cover the dispatch.
8. Arbiter repo metadata is cwd basename, not repository	genuine	Factual: getRepoMetadata() sets repo: basename(cwd) while git status walks up to the real repo root; a subdirectory cwd pairs the leaf name with repo-root branch/dirty, and non-git cwds still get a repo label. Scope: Repository/cwd/branch metadata is a required arbiter input; internally contradictory context can mislead routing decisions.
9. Clone helper reimplements structuredClone	nitpick	Factual: The helper correctly deep-clones the only nested field; snapshots are properly isolated today. Scope: structuredClone(info) is clearer but behavior-identical; readability-only.

Action Plan

1. Add UPDATE mode to skills/model-routing-guide/SKILL.md: resolve authoritative current scope, diff against guide coverage, remove unscoped sections, research newly added models, refresh required evidence, run full final validation.
2. Harden dashboard arbiter rendering against malformed retained field types (narrow guidePath/model/thinking before use); add a non-string guidePath regression test proving the disable toggle stays reachable.
3. Derive arbiter repository identity from the git root via findGitRoot(cwd); omit repo for non-git directories; add nested-cwd and non-git tests.
4. Add endpoint-level negative test: ordinary registered agent with missing child log fails loudly, never messages: [].
5. Add RPC test: disable with malformed retained fields → attempt re-enable → loud rejection → durable policy remains disabled and byte-identical.
6. Add changed-route background lifecycle test (arbiter-a → arbiter-b) asserting getBackgroundAgents() exposes the final identity before child completion.
7. Add Interactive Mode handler tests for changed/unchanged/failed arbitration records: status vs warning selection and background-status refresh.
8. Add guide-parser regression test: duplicate covered_model_ids with unique model headings rejected with the dedicated duplicate-frontmatter error.

Counts: 8 genuine, 1 nitpick, 0 false positives, 0 deferred.

---

Assessment by mach6