feat(harness): add stage-only multi-model orchestration

[haxiorz]

Goal

Add an opt-in, stage-only multi-model harness that separates implementation from independent judgment, runs bounded validation and repair phases, and leaves every successful run at Cezar's existing human review gate.

Problem and root cause

The original single-agent path could not provide an independently judged, quorum-based implementation flow. This PR adds that flow behind the default-off multiModel capability.

The requested-changes review also found three issues in the first implementation:

• the host-side advisor probe ran before the vendored runtime's endpoint guard, so a raw repository config could pair an ambient OpenCode auth-store credential with an arbitrary OpenAI-compatible endpoint;
• new-task-harness was statically imported by the eager New Task route even though multi-model mode is opt-in;
• three historical specs contained agent-directed execution directives that should not be trusted as repository data.

What changed

• Added a durable TypeScript phase graph for preflight, capture, specification, spec council, implementation, validation, final council, bounded repairs, exact staging, recovery, and the final review gate.
• Added per-run orchestrator, implementer, and two-to-five reviewer bindings with cross-family quorum, while keeping OpenCode advisors read-only and Claude/Codex in stage-only writing roles.
• Added trusted skill/runtime/config pinning, provider readiness probes, exact staged-path derivation, and typed API/cockpit surfaces for readiness, phases, findings, decisions, and certification.
• Kept the feature behind repository multiModel configuration, off by default, with a synthesized safe harness config when .ai/agentic.config.json is absent.
• Centralized the only auth-store-enabled advisor presets and their exact official provider endpoints. Profile resolution, custom-role canonicalization, binding synthesis, and the transport now reject redirected or mismatched auth-store bindings before reading credentials or making a request.
• Split the harness picker/dialog into a lazy Vite chunk loaded only when multi-model mode is opened, with an accessible loading state and a source-level bundle-boundary regression test.
• Removed the three agent-directed execution directives from specs while preserving their factual implementation context.
• Added regression coverage at the safety helper, profile planner, transport, driver, HTTP API, and New Task bundle boundary.

The matched zero-config evaluation delivered 5/5 contained, tested, green-gate multi-model changes versus 2/5 contained and 4/5 green-gate single-agent changes. All 90 reviewer invocations completed and produced 401 findings; the multi-model arm cost 2.9× more and took 6.5× longer wall-clock.

Validation

• npm run typecheck — passed in all workspaces; the contract inliner completed for 13 files.
• npm test — passed: 366 files and 6,896 tests.
• npm run test:unit — passed: 35 tests, with one expected platform skip because setsid is unavailable.
• npm run build — passed for service and cockpit; check:pack passed with 612 package files, including 102 under web/dist.
• npm run test:package — passed: 16 installed-tarball and release tests.
• Focused harness/server regression suites — passed: 156 tests.
• Focused New Task route, project-route, and bundle-boundary suites — passed: 114 tests.
• git diff --check — passed.
• Regression proof — each new security and bundle-boundary test was confirmed failing with its production fix stashed, then passing after restoration.

The configured validation gate was run twice in repository order; the second complete run followed the final test edit and passed every command.

A prior full browser comparison on this PR produced 196 passed / 13 failed / 6 skipped, versus 31 failures on an exact detached origin/main baseline. The residual failures reproduced on the base branch. That broad browser comparison was not rerun for this remediation commit; the affected New Task path was covered by its focused route suites and production build, and remains marked for manual QA.

Automated review

• om-code-review — approve after one remediation round resolving one blocker, one major finding, and one minor finding.
• Fresh independent review — the GitHub requested-changes review supplied the independent findings; all were reproduced and remediated, but the maintainer's post-fix verdict is still pending.
• Unresolved findings — None in the post-fix local review; external re-review remains required.

Breaking changes

None. No existing route, response schema, environment variable, dependency, migration, or default workflow behavior changed.

Labels and delivery

• review — the requested changes are implemented and the branch is ready for maintainer re-review.
• feature — the PR introduces the optional multi-model orchestration capability.
• needs-qa — provider-backed execution and the lazy-loaded cockpit interaction should be exercised manually before merge.
• priority-medium — this is an ordinary feature delivery rather than a release outage or emergency fix.
• risk-high — the PR spans orchestration, credentials, process execution, API behavior, and the cockpit, so it warrants deep review despite the default-off gate.

Manual QA

1. Open New Task in default task mode and confirm the new-task-harness asset is not requested.
2. Switch to Multi-model and confirm the controls load once, preserve keyboard/focus behavior, and can open and close the setup dialog.
3. Configure an auth-store advisor with a non-official endpoint and confirm the probe reports it unavailable without sending an outbound request.
4. Configure the official DeepSeek or OpenCode preset and confirm readiness probing still succeeds when the corresponding local credential exists.
5. Run a disposable multi-model task and confirm it reaches review, stages only certified paths, and creates no commit, push, PR, or merge.

Evidence

• The reviewed head's eager main chunk measured 355,262 bytes raw / 94,546 bytes gzip. After the split it measures 331,982 / 88,981 bytes, a reduction of 23,280 raw and 5,565 gzip bytes.
• The harness UI now emits as a separate 25,390-byte raw / 7,310-byte gzip chunk and is absent from the initial HTML module-preload list.
• Earlier live zero-config QA used a disposable repository, real provider transports, Codex Luna for orchestration/implementation, and Codex Terra plus OpenCode Nemotron Lightning for cross-family review. The run completed one bounded repair, revalidated green, received fresh two-family approval, staged only the expected paths, and parked at review without publishing.

Documentation

• docs/multi-model-harness.md
• .ai/specs/2026-07-23-harness-orchestration.md and follow-ups
• .ai/specs/2026-08-23-multi-model-eval.md
• CHANGELOG.md

[pat-lewczuk]

🤖 om-auto-review-pr started by @pat-lewczuk at 2026-08-28T21:45:52Z. Other auto-skills will skip this PR until the lock is released.

[pat-lewczuk]

🔍 Code Review: feat(harness): add stage-only multi-model orchestration

🎯 Summary

This PR adds an opt-in, stage-only multi-model harness: a durable phase graph (preflight → capture → specification → spec council → implementation → validation → final council → bounded repairs → exact staging) conducted from packages/cezar/src/harness/driver.ts, executed against a sealed copy of the vendored cez-harness runtime, and surfaced through a new project-scoped API family plus a Multi-model composer tab, Settings section and task-harness cockpit route. It is 219 files, +38,320/−604, and I reviewed it as a code PR (not spec-only) against the full changed-file set, with attention concentrated on the safety boundary (stage-only enforcement, the Claude guard, the seal, the ledger-backed publish gate), the public contract surface, and the changes this PR makes to existing code paths.

The engineering standard here is high and unusually well-evidenced. The safety story is layered rather than trusted once: the publish gate lives server-side in harnessPublishBlockReason and refuses POST /runs/:id/git/push and POST /runs/:id/pr until the durable ledger records a staged stage plus a ready outcome (or a contested outcome explicitly accepted, cross-checked against a matching decisions entry rather than a bare flag); the runtime is copied out of the model-writable worktree and re-verified by digest before every op instead of being trusted once; commandStage in harness.mjs validates every allowlist path for absoluteness, traversal and .git targeting, verifies refs and reflogs for drift, and refuses any staged path outside the git-derived allowlist and any residual unstaged file; advisor family is canonicalized from trusted configuration in advisor-identity.ts so a client cannot relabel a model to manufacture council diversity; and mechanicalVerdict derives the review verdict from findings rather than believing a reviewer that says "approve" while listing a blocker. BACKWARD_COMPATIBILITY.md is updated additively and correctly, the new contract schemas in packages/contract/src/harness.ts are purely additive, the feature is gated off by default behind multiModel with a documented no-strand story for in-flight runs, and the comment density explains why rather than what throughout — several comments cite the specific run id that motivated the rule. Test coverage on the new code is genuinely thorough (driver, ledger, quorum, probe, safety, staged-only guard, certification, profile plan, reviewer binding, plus web-side route tests).

The problems I found are not in the harness itself. They are in the three-line-per-call-site refactor of packages/cezar/src/git-worktree.ts that this PR does along the way — replacing the index-mutating git add -N . with a scratch GIT_INDEX_FILE. That refactor is correct in intent and is the right fix for the run 5f6fe8ae race, but it drops the cleanup half of the pattern it copied from server/git-changes.ts, and it omits the scratch environment on one of the two git runners inside worktreeShortstat — which is exactly the write the change exists to eliminate, on a hot path that runs at the end of every agent turn. Plus the head currently conflicts with origin/main.

Verdict

❌ request changes — the head does not merge cleanly into origin/main (blocker), and the git-worktree.ts scratch-index refactor leaks a temporary git index file on every diff read and still writes the real index on the repointed-worktree path (two majors). None of these are in the harness feature itself; all three are small, local fixes, after which I expect this to approve.

🧪 Validation Gate

Run in an isolated worktree at the PR head (f3b78f31) after npm ci.

Command	Status	Notes
npm run typecheck	✅ PASS	All four projects (contract, api-client, server, web) clean.
npm test	✅ PASS	6,648 tests. Two full-suite runs each reported a handful of failures, but they are not attributable to this PR: one set (git-worktree.test.ts, git.test.ts, health-forge.test.ts, projects-api.test.ts, git-changes.test.ts, automations-api.test.ts) is an artifact of my sandbox's TMPDIR living inside a git repository, which makes "outside a git repository" assertions fail; with a clean TMPDIR the run reduced to 3 failures in files this PR does not touch (automations-gate.test.ts, repo-git.test.tsx, task-changes.test.tsx), and all three pass in isolation, so they are load-sensitive flakes rather than regressions.
npm run test:unit	✅ PASS	36 passed, 0 failed.
npm run build	✅ PASS	Server, web, and check:pack (592 files, 91 under web/dist).
npm run test:package	✅ PASS	15 installed-tarball/release tests.

CI on the PR is green: the only reported check, license/cla, is SUCCESS, and no check is pending. This review therefore does not depend on any outstanding CI signal.

Findings

⛔ Blocker

The PR head does not merge cleanly into origin/main. GitHub reports mergeable: CONFLICTING / mergeStateStatus: DIRTY, and a local git merge-tree origin/main HEAD confirms exactly one conflicting file: CHANGELOG.md. The branch is 49 commits ahead and 1 commit behind origin/main, and main has since added its own ## ✨ Features entry at the top of the Unreleased section, which collides with the entry this PR adds in the same place. This is a blocker on its own terms — the diff being reviewed is not the diff that will merge — but it is mechanical: merge origin/main and keep both feature bullets under the single ## ✨ Features heading, newest first. Nothing else in the 219-file diff conflicts.

⚠️ Major

packages/cezar/src/git-worktree.ts:68 — scratchIndexEnv never deletes the temporary index file it creates, leaking one file per diff read for the life of the process. Every call allocates join(tmpdir(), 'cez-wt-scratch-<pid>-<seq>'), seeds it with git read-tree HEAD, populates it with git add -N ., and returns it — and nothing ever removes it. The pattern this was copied from does clean up: packages/cezar/src/server/git-changes.ts:314 runs rmSync(scratchIndex, { force: true }) in a finally. The call sites make this unbounded rather than theoretical. worktreeShortstat is invoked from RunManager.recordTurnEnd (packages/cezar/src/workflows/run.ts:3861), which runs at the end of every agent turn of every run; worktreeDiffStat is invoked inside a .map over runs at packages/cezar/src/server/server.ts:5126, so one request over N runs leaks N files; and worktreeDiff is on GET /api/v1/runs/:id/diff (server.ts:4875), which the cockpit polls. A seeded index is proportional to the repository's tree, so on a large repo a long-lived cezar server accumulates tens of thousands of multi-megabyte files in $TMPDIR — on a machine where /tmp is a tmpfs that is RAM. Fix: wrap each caller (or scratchIndexEnv itself, by returning a disposable handle) so the file is removed in a finally, mirroring git-changes.ts; note the file must survive until the last git call that uses the env, so the cleanup belongs at the end of worktreeDiff / worktreeDiffStat / worktreeShortstat, not inside the helper's own body.

packages/cezar/src/git-worktree.ts:613 — worktreeShortstat hands resolveTaskDiffBase a git runner without the scratch-index environment, so the read path still writes the real index. Line 611 obtains the scratch env and line 617 correctly uses it for the reported git diff --shortstat, but line 613 passes (args) => git(worktreePath, args) — the plain runner. resolveTaskDiffBase is not a pure ref lookup: when taskBranch is set and HEAD has been repointed onto another branch, it calls changedLines twice, and changedLines runs git diff --shortstat <ref>, which refreshes and rewrites the index it reads. packages/cezar/src/git-diff-base.ts:143-148 states this requirement explicitly — "Callers that keep their diffs off an index they do not own … must therefore hand this module a runner carrying that same environment" — and git-changes.ts:290 complies (resolveTaskDiffBase((args) => git(dir, args, env), …)). Two consequences follow. First, on any repointed worktree (review and QA runs, which are precisely the ones that repoint) the "read paths must not write ANY index" invariant this PR sets out to establish is still violated, at end-of-turn frequency — the same class of race as run 5f6fe8ae, and it can still trip commandStage's residual check in harness.mjs, which reads git status --porcelain and rejects any entry whose second status column is not a space (an intent-to-added file shows exactly that). Second, the anchor decision is measured against a different index than the number that is finally reported, so the two probes can pick an anchor the reported stat then contradicts. Fix: pass (args) => git(worktreePath, args, env) on line 613, matching git-changes.ts.

🔹 Minor

packages/cezar/src/harness/claude-guard.ts:15 — the stage-only PreToolUse guard fails open on its own internal errors. const worktree = realpathSync(process.env.CEZ_HARNESS_WORKTREE) and the outputDir line below it run at module top level, outside any try, and realpathSync is typed and behaves as throwing on undefined input or a vanished path. Claude Code's hook protocol treats exit code 2 as "block the tool call" and any other non-zero exit as a non-blocking error that is surfaced but lets the call proceed — so if either environment variable is missing, or the worktree is removed while the session is live, the guard crashes with exit 1 and the very next Edit/Write/Bash runs unguarded. The sandbox layer still stands behind it on both platforms, so this is defence-in-depth rather than the only boundary, but a guard should fail closed. Fix: wrap the two realpathSync calls (and ideally the whole body) so any failure routes through the existing refuse() helper, which already exits 2.

packages/cezar/vendor/skills/cez-harness/scripts/harness.mjs:2206 — commandStage reads staged paths without -z, so a path git chooses to quote makes staging fail with a misleading error. git diff --cached --name-only C-quotes any path containing non-ASCII bytes, a double quote, or a backslash ("caf\303\251.txt") unless -z is passed or core.quotePath is off. The same file already knows this — line 1375 uses git diff --cached --name-only -z and splits on NUL. Here the quoted spelling will not match any entry in paths (which came from validateStagePath on the raw allowlist), so the unexpected check throws Staged paths outside allowlist: "caf\303\251.txt", and if it somehow got past that, samePathSet(stagedPaths, [...allowlist]) in packages/cezar/src/harness/driver.ts:5151 would reject it as "staged paths do not exactly match the complete git-derived handoff". The behaviour is fail-closed, which is the right direction, but a harness run that touches a single accented or space-and-quote filename can never produce a handoff, and the error blames the allowlist rather than the encoding. Fix: use -z and NUL-split, as line 1375 already does.

**packages/cezar/src/harness/runtime.ts:267 (copyTreeByValue) — the seal walks the model-writable tree following symlinks, with no cycle or depth guard.** sealHarnessRuntimecopies/.claude/skillsout of the worktree, andcopyTreeByValuedeliberately usesstatSyncrather thanlstatSync so links are dereferenced ("copy by value") — which is the right call for the threat it is defending against. But a symlink that points at an ancestor (.claude/skills/cez-harness/x -> ../../..) then reports isDirectory()forever, and the function recurses without bound: the run's host process hits either aRangeError: Maximum call stack size exceededorENOSPC, and since this runs inside the cezar server rather than a child, an uncaught throw here is a server-level failure rather than a run-level one. The premise of the seal is that the worktree contents are untrusted, so the guard should be explicit. Fix: track visited real paths in a Set` and refuse (or skip) on revisit, and/or cap the recursion depth.

.ai/HANDOFF-feat-multi-modal.md — a branch-scoped working note is committed to the permanent tree, and is already stale. The file opens with "Branch is pushed to fork/feat/multi-modal, currently d768f071, 46 commits ahead of origin/main … 215 files, +37.7k/−583"; the head under review is f3b78f31, 49 commits ahead, 219 files, +38,320/−604. .ai/specs/ is the durable home for design content and this PR already adds seven spec documents plus docs/multi-model-harness.md there, so the handoff's lasting value is covered. It will only drift further after merge. Suggest dropping it from the PR, or folding anything still worth keeping into docs/multi-model-harness.md.

💅 Nit

**packages/cezar/package.json:46 — @anthropic-ai/sandbox-runtimeis a hard dependency of the published package but is only reachable on macOS.**sandboxRuntimeCliPath()is called exclusively from theuseStandaloneSandbox (platform === 'darwin') branch of createClaudeStageOnlySettings, yet every Linux and Windows install of @open-mercato/cezarnow downloads it. Pinning it exactly at0.0.61is the right instinct for a pre-1.0 package; consider moving it tooptionalDependencies(with the existingrequire.resolve` already positioned to fail loudly if it is genuinely needed and absent) so non-macOS installs stay lean.

.claude/launch.json — an editor launch configuration lands in the repo root, unrelated to the PR's stated scope. It is harmless and plausibly useful to other contributors, but it is not multi-model orchestration and it arrives without a note in the PR body. Either mention it in the description or split it out; author's call.

💥 Breaking Changes

• No exported/public symbol removed or renamed without a deprecation path — the diff removes no export from packages/contract/src or packages/api-client/src; packages/contract/src/harness.ts is entirely new.
• No function signature changed in a breaking way — git() in git-worktree.ts gains an optional trailing env parameter; worktreeShortstat's opts was already optional. RunManager.startRun is unchanged in arity.
• No required type field removed or narrowed — RunRecord.harness (packages/cezar/src/runs/store.ts:326) is .optional(), so pre-existing runs.json files still parse, and multiModel in config.ts is .default(false).catch(false), so a malformed value degrades rather than throwing.
• No HTTP route URL removed or renamed; no method changed for an existing operation — the six harness routes are new and project-scoped; harnessRoutes is mounted alongside the existing families.
• No field removed or retyped in an existing response shape — GET/PUT /api/v1/config gains multiModel additively, GET /api/v1/health is untouched, and POST /api/v1/runs gains an optional harness body object.
• No event or message name renamed or removed; no payload field removed — the five harness.* UiEvent variants in packages/api-client/src/protocol/ui-events.ts are added to the union; no existing variant is touched.
• No CLI command or flag renamed or removed; no machine-parsed output format changed.
• No database table or column renamed or removed — the project uses plain files, and runs.json's schema change is the optional field noted above.
• No config key renamed and no default changed silently — multiModel defaults to false, and the PR documents that flipping it off cannot strand an in-flight run because revival reads the run's own persisted workflowDef and the run-scoped harness reads stay live.
• Where a contract had to change: old surface kept working through a deprecation window, with migration notes — BACKWARD_COMPATIBILITY.md gains an accurate additive entry for the harness route family, the multiModel gate and its 409 semantics, and the certification field on GET /api/v1/harness/status, which is exactly the discipline this repository asks for.

One item worth calling out explicitly rather than as a checkbox: POST /runs/:id/messages now behaves differently for harness runs — manager.deferMessage is skipped and the text is queued as a phase-boundary message answering {queuedForPhase: true} instead of {deferred: true}, and images are refused with a 409. That is a new shape on an existing route, but it is reachable only for runs carrying the harness stub, which can only exist once the feature flag has been on, so no pre-existing client can observe the change. It is correct as designed; it just deserves a line in BACKWARD_COMPATIBILITY.md next to the harness entry so the next reader does not have to derive it from the diff.

🧪 Test Coverage

Coverage on the new surface is strong and specific rather than nominal. packages/cezar/src/harness/ ships tests for the driver (3,474 lines exercising phase recovery, council folding, fix rounds and the stage gate), the ledger's valid/missing/corrupt/unsupported branches, council quorum including the two-family rule and the degraded-but-ok path, the probe and its live transports, certification resolution including the stale window, profile planning, reviewer binding and the OpenCode seating refusal, model-family resolution — with a test that reads packages/web/src/routes/new-task-form.ts and asserts the browser mirror of FAMILY_BY_NAME and GATEWAY_PREFIXES matches the server table byte-for-byte, which is exactly the right way to hold a duplicated table honest — plus dedicated safety.test.ts, staged-only-guard.test.ts and seal-runtime.test.ts for the boundary. packages/cezar/src/server/harness-api.test.ts (1,007 lines) covers the route family, and the web side adds tests for the composer tab, readiness, the harness rail, timeline, state and the settings section. packages/web/e2e/agents-dock.e2e.ts was rewritten to exercise scoped routing, progressive history, real overflow and viewport restoration rather than being weakened; the other e2e edits are small waits for hit-testing to settle, not relaxed assertions, and I did not find an assertion that was deleted to make a red test green.

The gaps map onto the findings above, and each is a small, well-located test:

• No test asserts that the scratch index file is removed. Add a case to packages/cezar/src/git-worktree.test.ts that snapshots readdirSync(tmpdir()).filter(f => f.startsWith('cez-wt-scratch-')) before and after worktreeDiff, worktreeDiffStat and worktreeShortstat and asserts the set is unchanged — that pins the leak fix and keeps a future refactor from re-dropping the finally.
• No test asserts that worktreeShortstat leaves the real index alone on a repointed worktree. git-changes.test.ts already has the equivalent shape for collectChanges; add the sibling to git-worktree.test.ts — create a worktree, repoint HEAD onto another branch, record git status --porcelain (and the index mtime) before and after worktreeShortstat, and assert no untracked file was promoted to intent-to-added.
• packages/cezar/src/harness/staged-only-guard.test.ts covers what the guard refuses but not what it does when it cannot run. Add a case that invokes the guard script with CEZ_HARNESS_WORKTREE unset and asserts the exit code is 2, not 1 — that is the assertion that makes "fails closed" a property rather than an intention.
• commandStage has no case for a path git would quote. Add one to the harness runtime's stage tests using a filename with a non-ASCII character (and one with a space and a double quote) and assert the handoff stages successfully rather than reporting "Staged paths outside allowlist".
• seal-runtime.test.ts has no adversarial input. Add a case that plants a symlink cycle under .claude/skills/ and asserts sealHarnessRuntime returns null (or skips the link) instead of recursing.

---

Reviewed by om-auto-review-pr. autofix: skipped (not my PR — re-run with --autofix to fix it here).

[pat-lewczuk]

🏷️ label rationale

• changes-requested — the review verdict. The head does not merge cleanly into origin/main (CHANGELOG.md conflict) and two majors remain in the git-worktree.ts scratch-index refactor.
• feature — this adds a new opt-in capability (the stage-only multi-model harness), not a fix or a refactor.
• priority-medium — a substantial new feature, but gated off by default behind multiModel, so nothing ships to existing users until the flag is flipped. Not release-blocking.
• risk-high — 219 files and +38,320 lines, and it touches shared, hot paths well outside the new feature: git-worktree.ts diff reads, workflows/run.ts revival and turn bookkeeping, runs/store.ts step ordering, skill discovery precedence, and the POST /runs + POST /runs/:id/messages routes. Wide blast radius; review deeply.
• needs-qa — the change ships real UI: a Multi-model composer tab, a Settings → Harness section, and a whole task-harness cockpit route. qaGate is on for this repository, so qa-approved is required before merge and is earned by manual QA, never inferred from a diff.

in-progress is a meta lock held by this review run and is released with the completion comment below.

[pat-lewczuk]

👤 Handing this back to @haxiorz — changes requested.

Three things stand between this and an approval, and none of them are in the harness itself:

1. Merge origin/main. The head conflicts on CHANGELOG.md only — main added its own ## ✨ Features entry since this branch forked. Keep both bullets under the one heading.
2. packages/cezar/src/git-worktree.ts:68 — scratchIndexEnv never deletes the GIT_INDEX_FILE it creates. server/git-changes.ts:314 does (rmSync(..., { force: true }) in a finally); this copy dropped that half. It leaks once per turn end and once per run per list request.
3. packages/cezar/src/git-worktree.ts:613 — resolveTaskDiffBase is handed a runner without the scratch env, so on a repointed worktree changedLines' git diff --shortstat still rewrites the real index. git-diff-base.ts:146 states the requirement explicitly and git-changes.ts:290 complies.

The minors (guard fail-open, --name-only without -z, the seal's unguarded symlink recursion, the stale .ai/HANDOFF-*.md) and the two nits are worth picking up in the same pass but do not gate the verdict on their own.

No fixes were pushed to this branch — it belongs to another author and this run was not invoked with --autofix. Re-request review (or re-run /om-auto-review-pr 925) once the blocker and the two majors are addressed.

[pat-lewczuk]

🤖 om-auto-review-pr completed: CHANGES REQUESTED. Lock released.

1 blocker (merge conflict with origin/main in CHANGELOG.md), 2 majors (temp-index leak and a missing scratch-index env in git-worktree.ts), 4 minors, 2 nits. Validation gate run locally at the PR head: npm run typecheck, npm test, npm run test:unit, npm run build, npm run test:package — all pass. CI (license/cla) is green, so no CI follow-up is pending. autofix: skipped (not my PR — re-run with --autofix to fix it here).

[haxiorz]

🤖 om-auto-continue-pr started by @haxiorz at 2026-08-30T21:42:51Z. Other auto-skills will skip this PR until the lock is released.

[haxiorz]

🤖 om-auto-continue-pr — 📋 adoption plan

This PR carried no execution plan, so I reconstructed one from its own context rather than stopping: .ai/runs/2026-08-30-multi-modal.md (committed on this branch as 1fdd271c). This interactive invocation uses adoption ask mode: the plan is being handed to the user for confirmation before implementation begins.

🎯 Goal as I understand it

Finish the opt-in, stage-only multi-model orchestration feature described by PR #925 while resolving every actionable review finding, preserving the existing zero-config path, and returning the branch to a green review gate.

The branch already contains the feature, safety model, typed API, cockpit UX, specifications, tests, and evaluation evidence. The remaining work is therefore deliberately narrow: synchronize with main, correct the review findings and their regression gaps, run the complete repository gate, and obtain a fresh authoritative review.

📋 Remaining plan

• Phase 2: Synchronize the reviewed branch
  • Merge the current origin/main and resolve CHANGELOG.md by retaining both feature entries under one heading.
• Phase 3: Correct scratch-index isolation
  • Make every worktree diff helper dispose its scratch index in finally and add leak regression coverage.
  • Pass the scratch environment through diff-base resolution and prove repointed-worktree reads leave the real index unchanged.
• Phase 4: Harden harness safety boundaries
  • Make the Claude stage-only guard fail closed on missing or invalid environment paths and add the exit-code regression test.
  • Parse staged paths as NUL-delimited data and add quoted and non-ASCII filename coverage.
  • Refuse symlink cycles while sealing the model-writable runtime and add adversarial cycle coverage.
• Phase 5: Resolve review cleanup and compatibility notes
  • Remove the stale branch handoff and unrelated editor launch artifact.
  • Resolve the macOS-only sandbox runtime packaging nit without weakening supported-platform behavior.
  • Document harness message-queue behavior on the existing route in BACKWARD_COMPATIBILITY.md.
• Phase 6: Validate and return to review
  • Run targeted regression tests and the full configured validation gate, fixing branch-attributable failures.
  • Run om-auto-review-pr 925 --autofix, apply actionable findings, and leave the PR ready for human review and QA.

🔍 What I based this on

• High confidence: the PR title and description, the 219-file branch diff, and the committed harness specifications consistently state the opt-in, stage-only multi-model goal.
• High confidence: @pat-lewczuk's CHANGES_REQUESTED review and hand-back comment identify one merge blocker, two majors, four minors, two nits, and a compatibility-documentation note.
• High confidence: branch history through f3b78f31 shows the feature and its prior fixes are already landed; the previous review also reports a green full local gate at that head.
• High confidence: current CI reports license/cla as successful, and there are no inline review threads or linked issues adding independent scope.

⚠️ Assumptions and non-goals

• Both CHANGELOG.md feature bullets will be retained under one feature heading; no other conflict resolution is expected.
• The four minors are treated as required hardening because the reviewer explicitly asked that they be picked up in the same pass.
• The unrelated .claude/launch.json and stale branch handoff will be removed. The macOS-only sandbox dependency will move only if package behavior and installed-artifact tests stay correct.
• This plan does not enable the feature by default, weaken safety or validation gates, refactor unrelated baseline browser failures, rewrite history, merge the PR, or grant QA approval.

Correct me by editing the plan file, or by commenting here and re-running /om-auto-continue-pr 925 — the plan is a document, not a decision.

[haxiorz]

🤖 om-auto-continue-pr completed. Status: in-progress. Lock released.

[haxiorz]

@pat-lewczuk, please re-review the latest head, 2af45082.

The findings from your review are addressed:

• Scratch indexes are cleaned up after diff reads, and repointed-worktree diff-base probes use the same scratch environment.
• The Claude guard blocks calls when its writable roots cannot be initialized; staging handles quoted/non-ASCII filenames; runtime sealing rejects symlink cycles. The follow-up also makes seal cleanup tolerate a read-only destination without throwing.
• The stale handoff and unrelated launch config were removed, sandbox-runtime is optional, and the harness message-queue behavior is documented in BACKWARD_COMPATIBILITY.md.
• Merged current origin/main (e8c95f3a) and resolved all 11 conflicts, preserving the newer attachment, model-discovery, and mobile behavior. Fixed the resulting model-picker integration gaps: Claude discovery now feeds harness roles, and catalogs stay scoped to their runner.

The original regression tests were checked against the pre-fix code and fail on the reported defects.

Validation on the updated branch:

• npm run typecheck: passed.
• npm test: 365 files, 6,890 tests passed. The first attempt hit the already documented auto-resume.test.ts ENOTEMPTY teardown flake; the unchanged rerun passed.
• npm run test:unit: 35 passed, 1 expected platform skip.
• npm run build: passed, including check:pack.
• npm run test:package: 16 passed.
• Focused browser checks: 20 passed across task creation, plan editing, agent panels, and mobile task layout/header.
• git diff --check: passed.

This is ready for another review pass; the focused browser checks do not replace the repository's human QA gate.