refactor(engine): premise-witness drift + single-resolution prompt cascade + per-unit worktrees + studio trim

[jwaldrip]

What this is

This branch opened as the drift-model rewrite from DRIFT-CLEANUP.md. It grew — twice. It's now a broad engine-hardening + simplification pass plus a full rebuild of the review surface. 216 non-merge commits, ~1,560 files, +67.8k / −34.7k. The drift rewrite is still the flagship, but it ships alongside a single prompt-resolution path, per-unit worktree isolation, a studio-definition trim, a stack of engine reliability fixes, and — newest — the review SPA redesign + a native micro-app window.

The net deletion is still part of the headline: plugin/studios/ drops a wall of hat anti-pattern prose, redundant batons, and inlined prompt bodies as they move into a shared, overridable resolution path.

Workstreams

Area	What changed	Representative commits
Drift: premise-witness model	Witnesses snapshot premises (unit body + declared inputs), not deliverables (outputs). Output mutation no longer fires drift. v8→v9 migration backfills + strips + deletes dead sidecars. ~2.4k LOC of marker/baseline machinery removed.	f52cc445d 5477c7cc0 b47a76b2e ad1f4ffd3 ba5aba011 c80988457
Single prompt-resolution path	Full haiku_read_* tool family (haiku_read_hat, haiku_read_stage, discovery/output/phase) + build-time snapshot. Every subagent dispatch resolves studio assets through one shared reader instead of carrying inlined mandates. autotune→reflection rename; prompt bodies move to plugin/prompts/ with a project-override cascade.	9ce122421 720692d9f 5196a7734 9921200ca e4f8dbb23 b35153f50
Instruction-file dispatch form	Unit-hat, fix-hat, reviewer/approval/intent-review, and discovery dispatches all converted from inline prose to instruction-file form. Subagent reports a one-line work summary, then relays the engine's breadcrumb verbatim.	c8ad8a65e be2025b33 880725a74 a2de2b9e5 627e5eecc
Studio simplification	Verifier/planner hats added to 21 two-hat stages (plan→do→verify completeness). Per-unit batons compressed to mechanical handoff form. Heavy hat anti-pattern walls trimmed. Fix-scoped hat mandates for the stage fix loop.	f112cbfeb 6e3f6bf5e b764ec72f bd0a5e019 65b878a69
Per-unit worktree isolation	Unit hat dispatch forks a per-unit worktree (Phase 1a). Code-only isolation — iterations stay on the stage branch.	48d8c47ae 5954939a4 7d8da52d7 62c37f7cb
delivery-verifier + clean-tree gate	Global-tier intent-completion delivery-verifier (waits on delivery-PR CI, files findings, reads/resolves review threads). Pre-tick clean-tree gate blocks the tick on uncommitted agent work outside .haiku/.	bf7a439bf
Concurrency caps	Unit-execution waves cap + barrier at MAX_CONCURRENT_SUBAGENTS. Fix-loop hats pool at the same cap; target_unit dedup dropped.	42b7f5b33 d6f13a190
Review SPA redesign	Feedback moved from a blocking modal to a left-docked FEEDBACK rail + slide-out overlay drawer; the comment composer lives in the drawer; Approve is a standalone control, not part of the feedback flow. Every artifact (brief, observations, mockups, images) is annotatable. The gate decision bar stays actionable at every width. Measured CSS chrome-height vars bound the rail/drawer strictly between the header and the gate bar.	5ad0f9dab 0d0c551d7 eb1280348 6a56b37d1 bb692f430 2cb1d7bea
Micro-app window	The review SPA pops as a clean Chromium app-mode window against an isolated per-session profile — not a tab in the user's default browser. H·AI·K·U favicon for window identity; DPR-proof maximize so it never opens into the mobile layout; Apple-Silicon Playwright-build detection + full Chromium process-tree reap on close (no ghost browser).	b8f69b0fa 57f120ffb e00333f99 b7d6e5bc8
Presence heartbeat + gate launch	30s ACK'd heartbeat over the long-lived WS keyed to the MCP, not the tool call. The per-stage gate detects a never-attached SPA so a stale session stops suppressing the browser launch.	a2001a935 8026375db 0ecf3213f
Browse surface	Stage brief + observations + intent reflection surfaced on the browse/review page; feedback severity badges (blocker/high/medium/low); unit inputs linked; sign-offs show every expected role including pending.	ab25005c4 8ae6aaa31
Tool surface	haiku_dashboard deleted (broken active-stage derivation, redundant with haiku_intent_list). haiku-zap prose replaced with a real haiku_zap tool.	bc52589a0 26aa7aba1 25821303e
Statusline	Per-item pool bars + agent chips; sealed/intent-completion pipeline fixed. Persists dispatched position so the line stops jumping ahead; stops pinning an arbitrary off-branch intent; chips reflect artifact existence; atomic snapshot write.	5dcc74025 95483c9bb f6c050313 78e747b58
Engine reliability	Deadlock tick history persists across MCP restarts + batch-aware no-op backstop; wait-for-human signatures no longer poison the churn window (self-heal). Terminal stage close routes through the observations gate. Stage observations recorded via a forward-only merge gate. Stage-close merge recovers from untracked-clobber. Every output-owning hat is in fix_hats; non-owning fix hats advance, not reject. BRIEF.md writes to the repo stage dir, not global ~/.haiku.	24e618369 d01bd695f ed4bdf910 ea8412d8d d74e27e99 1e5a86745 9c8e40481 723c9ec3d
Rendering	Wireframe asset rendering hardening (SPA/view), bundled with fix-hat relays + drift/merge-state hardening.	c293b6d70
Providers / settings	settings.yml collapsed to studio + providers only (plugin/schemas/ drops ~1k lines).	7bff30b19
Optional stages (all studios)	intent.stages is the canonical materialized plan; the redundant skip_stages deny-list removed. A stage can be optional: true — the cursor leads the first elaborate tick with a keep-or-drop offer + a "what you're severing" dependents summary; haiku_drop_stage removes it from the plan (never auto-drops); cross-stage refs to a dropped stage auto-ignore (coverage gate skips a dir-less stage, prompts inject only .exists inputs, decompose plan-filters). Audited every studio for mandatory-core vs optional and marked 47 stages across 22 studios optional. software reshaped to its MVP core — inception/product/development mandatory, design/operations/security optional (the briefly-added release stage removed). deprecated: true hides a studio from new-intent pickers but keeps it resolvable (libdev). v9→v10 migration strips skip_stages + materializes stages. Fixed: user-gate next-stage routing follows the intent plan, not the studio superset.	bc0dcdf52 8370b974d 7920ccef4 7f0bd85f3 9521aa280 e3cb639ae 2ff39a80b 85f26bfbc 4ed2bbd5f 51ca584cb

The drift rewrite (flagship)

The bug this fixes (observed in production): a unit's output gets edited legitimately — a re-run or a fix-loop — the witness on the producer's approval no longer matches, and drift fires forever on the same file (terraform/environments/ops/fnox.toml was the prompting case). The agent eventually reaches for haiku_debug({op: "reset_drift"}). Debug becomes the recovery path for routine workflow. That's the smell.

Under the premise-witness model:

• Producers freely evolve their outputs — that's the job, not a drift signal.
• Consumers (downstream units that declared the file as an input) detect input drift on their own premise set.
• Cosmetic close re-stamps the consumer's input_witness; material close invalidates the slot and re-dispatch re-signs with current SHAs.
• reset_drift goes back to being a debug escape hatch, not a normal recovery path.

Witnesses live in unit/intent frontmatter, per signed slot — no separate files:

reviews:
  spec:
    at: 2026-05-16T...
    body_sha256: <unit body sha>
    input_witnesses:
      files:
        "intent.md": <sha>
        "knowledge/DISCOVERY.md": <sha>
      dirs:
        "stages/research/discovery/":
          "personas.md": <sha>
          "competitors.md": <sha>
approvals:
  spec:
    at: 2026-05-16T...     # bookkeeping-only, no witnesses field

Drift phases (all landed)

Phase	Description
1–2	input_witnesses schema + resolveInputWitnesses / extended buildReviewRecord
3	Sweep reads input_witnesses (new kinds: mutation / addition / deletion)
4	Cosmetic-close re-stamp via existing close-hook rebuild (no-op)
5–6	buildApprovalRecord returns {at} only; sweep stops reading approvals.witnesses
7	v8→v9 migration: backfills, strips approvals.<role>.witnesses, deletes dead sidecars
8	Surgical dead-code removal — drift-markers.ts + baseline-clear-marker.ts + haiku_baseline_init.ts deleted (~2,450 LOC)
9	drift_detected prompt rewritten + new input-drift test file

Deleted by v8→v9 migration (on-disk)

drift-markers.json, stages/<stage>/baseline.json, baseline-content/, .baseline-ack, baseline-thrash.json, and the approvals.<role>.witnesses field on every unit (stripped, slot kept).

Source files deleted (Phase 8)

src/orchestrator/workflow/drift-markers.ts, src/orchestrator/workflow/baseline-clear-marker.ts, src/tools/orchestrator/haiku_baseline_init.ts, test/haiku-baseline-init.test.mjs.

drift-baseline.ts stays — still used by haiku_human_write, baseline-content/upload routes, stamp-agent-write, and the drift-sweep kill switch (isDriftDetectionDisabled). It's no longer load-bearing for detection; further decoupling is its own refactor.

Review surface + micro-app (new since this PR opened)

The review SPA used to drop the human into a feedback modal that blocked the page and an Approve button tangled into the feedback flow, opened in whatever Chrome profile they last touched. That's now rebuilt:

• Feedback is a docked rail, not a modal. A thin left-edge FEEDBACK rail toggles a slide-out overlay drawer. Closed, it occupies a dedicated gutter and overlays nothing; open, it floats over content with no layout shift. The comment composer lives inside the drawer; Approve / Request Changes is a standalone gate bar. The rail and drawer are bounded strictly between the header and the gate bar via measured CSS custom properties (--review-header-h / --review-gatebar-h), so they sit over neither.
• Every artifact is annotatable. Brief, observations, mockups, and images all render from agent-owned files and all take inline annotations.
• The SPA pops as a micro-app. Instead of open-ing the URL into the user's default browser, the MCP launches a Chromium-family browser in --app mode against an isolated per-session profile — a standalone window with its own dock entry, no tabs, no extensions, no cross-talk. It carries a H·AI·K·U favicon, maximizes DPR-proof so it never trips the mobile breakpoint, and the whole Chromium process tree is reaped on close (the Apple-Silicon build-detection + process-group fix in b7d6e5bc8 killed a ghost-browser leak).
• Presence is heartbeat-driven. A 30s ACK'd heartbeat over the long-lived WS (keyed to the MCP, not the tool call) is refresh-tolerant; the gate detects a never-attached SPA so a stale session stops suppressing the launch.

Test coverage

96+ test files touched. New + reworked drift suites: drift-input-witnesses (input mutation/addition/deletion + cosmetic re-stamp + implicit intent.md), drift-no-false-positives (now includes a negative test — output mutation does not fire drift), drift-scenarios (output-drift case converted to a negative regression guard), plus the worktree-isolation and advance-hat-dispatch contract tests. The review SPA carries a 556-test vitest suite; the micro-app browser-discovery + process-tree reap has its own regression coverage (x64 + arm64 layouts). Migration chain intact through v9.

Review threads

All review threads on this PR are resolved — see the resolution summary mapping each finding to its fix commit.

[claude]

Claude finished @jwaldrip's task in 5m 2s —— View job

---

PR Review: refactor(drift): premise-witness model

Todo

• Read PR description and understand the scope
• Review full diff (gh pr diff 372)
• Read key changed files in depth
• Analyze correctness, bugs, and design
• Post inline comments for specific issues
• Post summary review

---

Overall Assessment

The design inversion is correct and well-argued. Witnessing premises (unit body + declared inputs) instead of deliverables (outputs) eliminates the class of permanent drift loops caused by normal output evolution, and the implementation is consistent about sign-time/check-time alignment. The fnox.toml loop bug this targets would be fixed. The migration is idempotent and leaves a clean story for Phase 8 deletion.

Five inline issues filed, ranging from a real correctness concern to documentation cleanup.

---

Issues (inline comments posted)

Medium — dirInventory uses full-file hash for .md files inside watched directories

dirInventory in sign-slot.ts:196 uses fileSha256 (FM-inclusive) for every file inside a witnessed directory. resolveInputWitnesses uses outputSha256 (body-only for .md/.txt) for direct file inputs. Sign-time and check-time are consistent within each path — no false mismatch today — but any .md file inside a watched input directory (e.g. discovery outputs in stages/research/discovery/) will trip input_mutation drift whenever the orchestrator re-stamps its FM, even if the human-authored prose is unchanged. The fix is to use outputSha256 in dirInventory and update the matching fileSha256 call in drift-sweep.ts:389.

Medium — DIR_INVENTORY_SKIP duplicated across two files

DIR_INVENTORY_SKIP in sign-slot.ts and DIR_INVENTORY_SKIP_SWEEP in drift-sweep.ts are exact copies with a comment saying "must stay in sync." Duplication is the problem — if someone extends one without the other, sign-time and check-time disagree on what files belong in a directory's inventory, producing spurious input_addition or input_deletion events. Export the constant from sign-slot.ts and import it in drift-sweep.ts.

Low — drift-sweep.ts:354 partial extension list

The sweep's explicit extension check for direct file inputs only covers .md | .markdown | .mdx, then falls back to outputSha256. The fallback is correct for .txt/.rst/.adoc (those also hit bodySha256 via TEXT_BODY_EXTENSIONS), but the partial list is misleading. Simplest fix: drop the explicit check and always call outputSha256(abs) directly — it already encodes the right strategy per-extension.

Low — Stale comment in haiku_await_gate.ts

The newly added comment says approvals witness output paths "(legacy — to be dropped in Phase 6)" but Phase 6 is done — buildApprovalRecord already returns { at } only and ignores its arguments. The comment implies witnessing is still happening here when it isn't.

Nit — v8-to-v9.ts migration outcome not surfaced

outcome.review_slots_backfilled and outcome.approval_slots_stripped are tracked but never written back to details. They disappear silently. Adding them to MigrationStepDetails would make the migration banner more useful for auditing a production upgrade.

---

Design Notes (non-blocking)

DRIFT-CLEANUP.md in repo root — Planning docs committed to the root tend to stay forever and confuse future readers once the work is done. Consider deleting it when Phase 8/9 close, or moving it to .haiku/ or a docs/internal/ dir.

buildApprovalRecord dead params at multiple call sites — dispatch-stamps.ts:270, debug-ops.ts:167/365, haiku_run_next.ts:1150 all still pass intentDirAbs, outputs args that buildApprovalRecord silently ignores. The PR description notes this as Phase 8 cleanup, which is the right call — just a reminder the dead call sites are spread across four files.

pickInputWitnesses null on empty block is correct — the guard that returns null when both files and dirs are empty avoids spurious input_addition drift for units that signed before input_witnesses existed. This is the right edge case handling.

---

Test Coverage Assessment

The existing test rewrites are solid — converting the old "output mutation fires drift" positive tests to negative regression guards is exactly right for the new model. The drift-self-clear-on-open-fb dedup tests now use spec drift instead of output drift, which keeps the dedup surface narrow and self-contained.

Noted gaps from the PR itself: Phase 9 input-drift tests (addition/deletion/dir-inventory mismatch, cosmetic-close re-stamp, material-close re-sign, migration backfill) are still TODO. The skip annotation on haiku_baseline_init is appropriate.

[claude]

Claude finished @jwaldrip's task in 7m 39s —— View job

---

PR Review: refactor(drift): premise-witness model

Todo

• Read PR description and understand the scope
• Review full diff (gh pr diff 372)
• Read key changed files in depth
• Analyze correctness, bugs, and design
• Post inline comments for specific issues
• Post summary review

---

Summary

The conceptual reframe here is correct and well-executed. The old model was witnessing outputs (things downstream of the decision), which caused the permanent drift loops you were seeing on fnox.toml and similar. Witnessing premises instead — unit body + declared inputs — is the right invariant: a reviewer signed saying "given these inputs I approve"; drift means those inputs changed. The three-layer dedup (exact ref → path → basename) in collectOpenDriftFbDedup is also a solid fix for the production re-emission problem.

The code is generally clean. A few specific issues found:

---

Bug 1 (fragile): Input file hashing diverges at check-time — drift-sweep.ts:354–371

The comment says "same strategy" as sign-time outputSha256, but the code manually checks for .md/.markdown/.mdx and calls bodySha256 directly, while sign-time (resolveInputWitnesses:254) just calls outputSha256 for all input files.

Functionally equivalent today, because the uncovered extensions (.txt, .rst, .adoc) fall to the else branch which calls outputSha256 anyway. But if TEXT_BODY_EXTENSIONS in sign-slot.ts ever changes (e.g., .md removed, or .yaml added), the hardcoded check in the sweep would diverge — sign-time and check-time hashes would disagree, producing permanent false drift on every markdown input file.

The fix is one line: replace the entire manual check with const currentSha = outputSha256(abs). Inline comment posted.

---

Bug 2 (correctness): Drift-FB close in haiku_run_next overwrites approval timestamps — haiku_run_next.ts:~1148

The close_feedback loop (unchanged context, not in this PR's diff) re-stamps every approvals.<role> with buildApprovalRecord() on drift-FB close, calling new Date() for each slot. feedback-close-hook.ts (the canonical drift-close implementation added/refined in this PR) explicitly does the opposite:

"Leave existing approval timestamps untouched on drift-FB close. The audit trail of 'who approved when' stays intact; nothing to refresh."

buildApprovalRecord() now ignores its arguments and returns { at: new Date() } — so the close_feedback path silently overwrites "when did role X approve this unit" for every approval on the targeted unit. The two drift-refresh paths need to behave identically; the approval loop in haiku_run_next.ts should be dropped. Inline comment posted on the nearest changed line.

---

Stale comment: haiku_await_gate.ts:153

The comment says approval witnesses are "legacy — to be dropped in Phase 6 of the drift cleanup." Phase 6 is already done in this PR — buildApprovalRecord returns { at } only. The outputs variable below is fetched and passed but silently ignored. Inline comment with suggested replacement posted.

---

Missing: dedicated v8→v9 migration test

The new migration (v8-to-v9.ts) has three distinct behaviors (backfill input_witnesses, strip approvals.witnesses, purge sidecar files) plus an idempotency contract, none of which appear to have a targeted test. The new drift behavior tests exercise the post-migration state but don't verify that legacy on-disk layouts (units with approvals.witnesses, sidecar files present) migrate correctly. Inline comment on v8-to-v9.ts posted.

---

Everything else looks good

• The pickInputWitnesses empty-block short-circuit (null when both files and dirs are empty) is correct — avoids false drift on units that never had input coverage.
• Directory inventory hashing is consistent: both dirInventory (sign) and the sweep's listDirFiles path use fileSha256, and the skip lists (DIR_INVENTORY_SKIP / DIR_INVENTORY_SKIP_SWEEP) are in sync.
• resolveInputWitnesses intent-relative-first resolution mirrors sweep-side resolution exactly.
• The implicit intent.md witness on every review (without requiring units to declare it) is clean and the test at drift-input-witnesses.test.mjs:285 pins it.
• The three-layer dedup in collectOpenDriftFbDedup is well-reasoned; the body-regex fallback is intentionally loose and the closed-FB exclusion limits its blast radius.
• The v8→v9 migration's idempotency skip (if (slotRec.input_witnesses !== undefined) continue) is correct.
• DRIFT-CLEANUP.md as a committed design doc is fine — it's a useful rationale reference that explains the "why" in a way code comments can't fully capture.

The Phase 8 dead code (buildOutputWitnesses, the _intentDir/_outputs params on buildApprovalRecord, the legacy skip entries in DIR_INVENTORY_SKIP) is appropriately deferred; it's non-breaking and cleaning it up after Phase 9 tests are green is the right sequencing.

[claude]

Claude finished @jwaldrip's task in 4m 45s —— View job

---

PR Re-Review: refactor(drift): premise-witness model

Todo

• Read PR description and understand the scope
• Get full PR diff (gh pr diff 372)
• Read key changed files in depth
• Check if prior inline issues were addressed in new commits
• Analyze correctness, bugs, and design
• Post inline comments for specific issues
• Post summary review

---

Status: Same 5 commits as prior review — no new code landed

All issues from the previous inline review remain open. Repeating them here for completeness, with updated notes from deeper analysis.

---

Confirmed Open Issues

Medium — dirInventory uses fileSha256 for .md files inside watched dirs (sign-slot.ts:196, drift-sweep.ts:398)

Sign-time and check-time are consistent with each other today (both use fileSha256 for dir contents), so there are no false mismatches right now. The problem is that any .md file inside a watched input directory — e.g., discovery output files in stages/research/discovery/ — carries engine-managed FM (output_sha256, mandate_sha256, hat, etc.). When the engine re-stamps that FM without touching the prose body, fileSha256 returns a new hash and input_mutation drift fires. The same FM-noise problem that motivated outputSha256 for direct file inputs applies here too. Fix: use outputSha256 in dirInventory and fileSha256(fileAbs) → outputSha256(fileAbs) at drift-sweep.ts:398.

Medium — DIR_INVENTORY_SKIP duplicated (sign-slot.ts:160, drift-sweep.ts:64)

The skip-list is copy-pasted with a "must stay in sync" comment. The comment names the failure mode correctly (sign-time and check-time would disagree on inventory, causing spurious input_addition / input_deletion). Export the constant from sign-slot.ts, import it in drift-sweep.ts. Inline comment already filed on the prior review.

Low — Stale comment in haiku_await_gate.ts:152

// Reviews witness the unit body + input premises; approvals
// witness the declared output paths (legacy — to be dropped
// in Phase 6 of the drift cleanup).

Phase 6 is done. buildApprovalRecord already returns {at} only. The comment implies output witnessing is still happening here when it isn't.

Low — review_slots_backfilled / approval_slots_stripped not surfaced in migration banner (v8-to-v9.ts:197-199)

MigrationStepDetails in migrate-registry.ts has no fields for these two counters. They're tracked in V9Outcome but written to details as:

details.units_migrated = outcome.units_migrated
details.drift_artifacts_deleted = outcome.drift_artifacts_deleted
// review_slots_backfilled and approval_slots_stripped disappear here

The banner shown to users after migration gives no signal about how many witness blocks were backfilled or stripped — useful for auditing a production upgrade. Add the two fields to MigrationStepDetails + emptyMigrationDetails.

Low — Partial extension check at drift-sweep.ts:358-362

const currentSha =
  ext === ".md" || ext === ".markdown" || ext === ".mdx"
    ? bodySha256(abs)
    : outputSha256(abs)

The explicit list covers only .md | .markdown | .mdx. The fallback (outputSha256(abs)) already body-hashes .txt | .rst | .adoc via TEXT_BODY_EXTENSIONS. The three-way conditional is misleading (appears to say "everything not in this list is full-file") and incomplete vs the sign-time set. Simplest fix: delete the branch and always call outputSha256(abs) directly.

Nit — v8-to-v9.ts:96-97 redundant self-assignment

slotRec is a reference to the same object at reviewsRec[role]. Mutating it and then writing it back (reviewsRec[role] = slotRec) is a no-op that reads as a structural replacement to future editors.

---

New Observation (not in prior review)

dispatch-stamps.ts re-signing now populates input_witnesses (the diff at line 248-260)

The change in drainPendingDispatches correctly passes intentDir and unitInputs to buildReviewRecord so re-dispatch cycles get a full premise snapshot. This was a gap — a slot re-signed via drain would have gotten body_sha256 only and no input_witnesses. The fix is correct.

---

Design Notes

dirInventory / resolveInputWitnesses path resolution is correct — intent-relative-first with repo-relative fallback mirrors the sweep exactly. ✓

pickInputWitnesses null guard on empty block is the right call — avoids input_addition drift for pre-v9 units that get migrated but have no declared inputs. ✓

feedback-close-hook.ts cosmetic re-stamp calls buildReviewRecord with unitInputs, so it rebuilds input_witnesses from current on-disk state. Next sweep compares fresh SHAs against fresh SHAs → no re-fire. ✓

intent-scope approval asymmetry is intentional: unit approvals use buildApprovalRecord (bookkeeping-only, no hash), while intent.md approvals use buildReviewRecord(intentMd) without args (body-hash only, no input_witnesses). The sweep detects intent body drift under kind: "spec" for unit: "(intent)". This matches the design doc. ✓

---

Overall Verdict

The design inversion is correct and the implementation is internally consistent. The fnox.toml permanent-loop bug would be fixed. Migration is idempotent. The five issues above are the outstanding work before merge — the two medium ones (hash strategy for dir contents, skip-list deduplication) are straightforward fixes and address a real FM-noise risk for projects whose discovery outputs carry engine-managed frontmatter.

[claude]

Claude finished @jwaldrip's task in 6m 13s —— View job

---

PR Re-Review: refactor(drift): premise-witness model

Todo

• Read PR description and understand scope
• Get full PR diff and new commits since last review
• Check which prior issues were addressed
• Read key changed files in depth
• Analyze correctness, bugs, and design
• Post inline comments for specific issues
• Post summary review

---

Phase 8 Commits (new since last review) — clean

Two new commits since the prior review pass:

• ba5aba0 — surgical deletion of drift-markers.ts, baseline-clear-marker.ts, haiku_baseline_init.ts, and removal of their callers from state-tools.ts, side-effects.ts, tool-defs.ts, index.ts. All marker-clear call sites correctly updated; the v9 model replaces them with open FB as the dedup signal. No new issues introduced.
• 20ef4aa — test file deletions matching the deleted source. Clean.

Prior Issues: Not Yet Addressed

None of the issues raised in the two previous review passes were resolved in the new commits. Re-filing the unresolved ones:

Bug (medium) — haiku_run_next.ts approval re-stamp overwrites audit timestamps

The review re-stamp in haiku_run_next.ts was correctly updated to pass unitInputs (the change in this PR). But the approval re-stamp loop that immediately follows (unchanged, around lines 1149-1153) still runs buildApprovalRecord(intentDirAbs, outputs) for every approval role and writes a fresh at: new Date(). Since buildApprovalRecord now returns { at } only, this silently overwrites "who approved when" on every drift-FB close via haiku_run_next. feedback-close-hook.ts is explicit about not doing this. Inline comment posted on the surrounding diff hunk.

Medium — DIR_INVENTORY_SKIP_SWEEP still duplicated

sign-slot.ts defines DIR_INVENTORY_SKIP (unexported); drift-sweep.ts redefines it verbatim as DIR_INVENTORY_SKIP_SWEEP. The comment says "must stay in sync" — but duplication is the problem. Export from sign-slot.ts, import in drift-sweep.ts. Inline comment posted.

Low — stale comment in haiku_await_gate.ts:152

Still references "legacy — to be dropped in Phase 6 of the drift cleanup." Phase 6 is done: buildApprovalRecord ignores all arguments and returns { at } only. Inline comment posted with suggested replacement.

Low — partial extension list in drift-sweep.ts:354–371

The manual .md/.markdown/.mdx check before the outputSha256 fallback is functionally equivalent today but diverges from sign-time's direct outputSha256(abs) call. If TEXT_BODY_EXTENSIONS in sign-slot.ts changes, this check becomes a silent source of hash strategy divergence. The fix is one line: call outputSha256(abs) directly. Inline comment with suggestion posted.

Low — v8-to-v9.ts outcome fields not surfaced

outcome.review_slots_backfilled and outcome.approval_slots_stripped are computed but never written into details. The migration banner only shows units_migrated and drift_artifacts_deleted. Inline comment posted.

Low — no dedicated v8→v9 migration unit test

drift-input-witnesses.test.mjs covers post-migration drift behavior but doesn't verify that legacy on-disk state (units with approvals.witnesses, sidecar files present) migrates correctly, or that the idempotency skip (input_witnesses !== undefined) actually prevents re-backfill. Inline comment posted on the migration's registerMigrator call.

Everything Else Remains Good

• The feedback-close-hook.ts design is correct and well-commented — approvals left untouched, only reviews re-witnessed.
• resolveInputWitnesses and dirInventory in sign-slot.ts are consistent: sign-time and check-time use the same strategies per surface (direct file → outputSha256, directory file → fileSha256). The theoretical risk for FM-stamped .md files inside witnessed directories is still present but low-impact in practice (discovery output files don't carry engine-managed FM).
• Three-layer dedup in collectOpenDriftFbDedup (exact ref → path → basename) is solid.
• pickInputWitnesses returning null on empty block is correct — avoids spurious input_addition for pre-v9 units.
• drift-input-witnesses.test.mjs (5 tests) covers the core premise-witness behaviors well: mutation, addition, deletion, cosmetic-close re-stamp, implicit intent.md.
• Phase 8 dead code deletion is complete and correct.

[claude]

Claude finished @jwaldrip's task in 2m 26s —— View job

---

PR Re-Review: refactor(drift): premise-witness model

Todo

• Read PR description and understand the scope
• Check new commits since last review
• Get full PR diff
• Verify which prior issues were addressed
• Read key changed files in depth
• Post updated review

---

New Commit: c809884 — gate-session.json cleanup

Since the last review, one commit landed: fix(drift): kill gate-session.json + auto-purge dead engine sidecars.

This commit is clean. The investigation is solid (gate-session.json had a writer in server/tool-call.ts but no reader — haiku_await_gate reads gate session pointers from intent.md FM at gate_review_session_<stage> keys, not from a sidecar). The fix is correct:

• server/tool-call.ts — writer and orphaned imports removed cleanly.
• purge-dead-sidecars.ts — the new per-tick sweep is well-structured. The dead-lists are annotated with why each entry is safe to remove. The tryDelete wrapper is non-fatal, idempotent by construction, and correctly handles both file and directory targets. The deleted count tracking (snapshot-before, count-missing-after) is correct.
• run-tick.ts — purgeDeadSidecars(iDir) fires after migration but before the cursor walk, which is the right position: migration already cleaned up its one-shot artifacts; the sweep catches anything that reappears afterward.
• v8-to-v9.ts — gate-session.json added to the migration's purgeDriftArtifacts for the one-shot cleanup path.

One observation: the deleted return value from purgeDeadSidecars(iDir) in run-tick.ts:284 is silently discarded. Not a bug — the cleanup is non-critical and the function's comment says "Returns the count... for callers that want to log/telemetry." Just noting that a future telemetry signal here would help diagnose post-downgrade sidecar reappearance.

---

Prior Issues — Status (none resolved in new commits)

Bug (medium) — approval re-stamp loop overwrites audit timestamps · haiku_run_next.ts:1149–1153

Still unaddressed. The loop at lines 1149–1153 runs buildApprovalRecord(intentDirAbs, outputs) for every approval role on drift-FB close, writing at: new Date() and overwriting the original "who approved when" timestamp on every role. feedback-close-hook.ts is explicit that approvals are bookkeeping-only under the premise-witness model and must not be re-stamped on drift-FB close.

This is a correctness issue: every drift-FB closed via haiku_run_next silently corrupts the approval audit trail. Fix this →

Medium maintenance — DIR_INVENTORY_SKIP duplicated · sign-slot.ts:160, drift-sweep.ts:64

The constant is still copy-pasted between the two files with a "must stay in sync" comment. sign-slot.ts defines it as const (unexported); drift-sweep.ts redeclares it as DIR_INVENTORY_SKIP_SWEEP. The new purge-dead-sidecars.ts adds gate-session.json to STAGE_SCOPE_DEAD — which is correct for that purpose — but the skip lists in sign-slot.ts and drift-sweep.ts were not updated to match (gate-session.json wouldn't be a top-level input dir, so not a bug, but it illustrates how the duplication creates maintenance surface). Fix this →

Low — stale comment in haiku_await_gate.ts:152

The comment still reads "witness the declared output paths (legacy — to be dropped in Phase 6 of the drift cleanup)". Phase 6 landed in this PR. buildApprovalRecord now returns { at } only. Fix this →

Low — partial extension list in drift-sweep.ts:358–362

The manual .md/.markdown/.mdx check still present. Sign-time calls outputSha256(abs) directly for all input files; the sweep's check is an incomplete subset of TEXT_BODY_EXTENSIONS. The fallback path (the else branch calling outputSha256) makes it work today, but if TEXT_BODY_EXTENSIONS changes, the explicit shortcut list becomes a silent hash strategy divergence. Replacing the entire conditional with outputSha256(abs) is one line and removes the drift risk. Fix this →

Low — v8-to-v9.ts migration outcome not surfaced

outcome.review_slots_backfilled and outcome.approval_slots_stripped are computed but still not written into details at line 203–204. They disappear silently; the migration banner only shows units_migrated and drift_artifacts_deleted. Fix this →

---

Overall Assessment

The core design inversion is correct and well-executed across all 7 commits. The fnox.toml permanent-loop bug is fixed. The new commit's gate-session.json cleanup is the right call (write-only artifact, correct sidecar removal strategy). Five issues remain from prior review passes — one medium correctness bug (approval timestamp corruption), one medium maintenance issue (skip-list duplication), and three low items. None of the five are blockers on the conceptual change, but the approval timestamp bug is worth fixing before merge.

[claude]

Claude finished @jwaldrip's task in 6m 28s —— View job

---

PR Re-Review: refactor(drift): premise-witness model

Todo

• Read PR description and understand scope
• Check new commits since last review
• Verify which prior issues were addressed
• Read key changed files in depth
• Post updated review

---

New Since Last Review

Six commits landed since c809884 (the prior review's head). The drift-relevant one is c293b6d (fix(rendering+engine): wireframe asset rendering, fix-hat relays, drift + merge-state hardening). The others are studio/prompt/provider refactors and haiku_dashboard deletion — clean and correct.

---

New Feature: Input==Output Baton Exemption (c293b6d) — Correct

stageProducedRelPaths in drift-sweep.ts now builds a set of intent-relative paths the current stage produces (discovery/output template location: fields + every current-stage unit's outputs:). The guard at line 465 skips input_mutation firing for direct file inputs that match:

if (mismatched && !stageProducedRel.has(path)) {

This fixes the designer-prep convergence loop from the 2026-05-20 report — six findings on two baton files, one misclassified as a material deletion against a 393-insertion append. The test file drift-input-output-baton.test.mjs covers the discovery-location case, unit-output case, and the control (non-produced upstream inputs still fire). Implementation is correct.

New low-severity gap: the baton exemption only covers direct file input_mutation events (line 465). The directory inventory scan at lines 530–557 has two unguarded paths:

• Line 542: input_mutation for files inside a watched directory
• Line 554: input_addition for new files inside a watched directory

If a stage watches a directory and produces files inside it (e.g., a stage that both reads and writes discovery files in stages/research/discovery/), the exemption won't fire from the dir-watch path. Low-severity in practice since batons are typically declared as specific file paths, not directory watches — but worth noting. Fix this →

---

Prior Issues — Status (none resolved)

Bug (medium) — approval re-stamp overwrites audit timestamps · haiku_run_next.ts:1149–1153

Still unresolved. The drift-FB close block now correctly passes unitInputs when rebuilding review records (good). But the approval loop immediately following still runs:

for (const role of Object.keys(approvals)) {
    approvals[role] = buildApprovalRecord(intentDirAbs, outputs)
}
setFrontmatterField(unitPath, "approvals", approvals)

buildApprovalRecord returns { at: new Date().toISOString() } — every drift-FB close via haiku_run_next silently overwrites the original "who approved when" timestamps on the unit. feedback-close-hook.ts (the canonical drift-close path) does the opposite — its restampSurvivingWitnesses only updates body_sha256 in-place and leaves at untouched. The two paths need to behave identically. Drop the approval loop and its setFrontmatterField call; leave only the reviews re-stamp. Fix this →

Medium maintenance — DIR_INVENTORY_SKIP duplicated · sign-slot.ts:271, drift-sweep.ts:70

The skip-list is still copy-pasted. sign-slot.ts defines it unexported; drift-sweep.ts redeclares it as DIR_INVENTORY_SKIP_SWEEP with a "must stay in sync" comment. This is now the fourth review pass flagging the same issue. The fix is one change: export from sign-slot.ts, import in drift-sweep.ts. Fix this →

Low — stale comment in haiku_await_gate.ts:151–152

Still reads: "witness the declared output paths (legacy — to be dropped in Phase 6 of the drift cleanup)". Phase 6 is done. buildApprovalRecord returns { at } only. The comment is now misleading. Fix this →

Low — partial extension list in drift-sweep.ts:457–462

The manual .md/.markdown/.mdx check before the outputSha256 fallback is still present. Sign-time calls outputSha256(abs) directly for all input files; this check is an incomplete subset of TEXT_BODY_EXTENSIONS that diverges silently if the set changes. Replace the isMarkdown ternary with a direct outputSha256(abs) call. Fix this →

Low — v8-to-v9.ts outcome fields not surfaced

outcome.review_slots_backfilled and outcome.approval_slots_stripped are still computed in V9Outcome but not written into details (lines 203–204 only write units_migrated and drift_artifacts_deleted). MigrationStepDetails in migrate-registry.ts still doesn't have these two fields. The migration banner gives no signal about how many witness blocks were backfilled or approval maps stripped. Fix this →

---

Everything Else Looks Good

The HTML wireframe rendering (html-inline.ts) is security-conscious: only local relative hrefs are inlined, path traversal is clamped to intentDir, @import depth is limited to 3, and the octet-stream/attachment content-type is preserved at the serve layer so HTML artifacts never execute under the tunnel origin.

The merge-in-progress guardrail suspension (guard-workflow-fields.ts) is correct — it detects MERGE_HEAD, cherry-pick, revert, and rebase state files in $GIT_DIR, and falls back to "not in a merge" on any error so the normal guard fires in non-git mode.

haiku_unit_get correctly restricts to AGENT_AUTHORABLE_UNIT_FIELDS and refuses FSM-driven fields with a stable named error code.

haiku_dashboard deletion (bc52589) is clean — the tool file is gone, no lingering imports or references in the rest of packages/haiku/src/.

---

Overall Assessment

The design inversion is correct and the implementation is internally consistent across all 19 commits. The fnox.toml permanent-loop bug is fixed; the new baton exemption addresses the designer-prep convergence loop from the 2026-05-20 report. Five issues carry forward from prior review passes — the approval timestamp corruption bug in haiku_run_next.ts is the only one worth holding merge on; the rest are cleanup. Twelve prior items (all the ones raised in the first four passes except these five) have been addressed or are correctly deferred.

[claude]

Claude finished @jwaldrip's task in 7m 5s —— View job

---

Pass 67 — No Issues · Ready to Merge

Trigger: pull_request synchronize — new commits since Pass 66 (26da904)

CI: 2 runs in progress (Claude Code Review + CI).

---

New commits reviewed

Commit	Description	Verdict
a6c612ee	fix(browse): clearer note when declared output isn't on disk	✅ Clean
e99d2f76	docs(drift): correct stale drift-detection-gate.ts references	✅ Clean
8a1a2d04	merge: absorb concurrent review/SPA commits into optstages-work	✅ Clean
4a373c2b	chore(website): refresh paper-revisions tracking data	✅ Clean

---

Commit notes

a6c612ee — IntentDetailView.tsx: two "Couldn't load this file." strings replaced with a precise message explaining the file is declared in outputs but not on disk yet. Both text and binary not-found paths updated identically. Correct.

e99d2f76 — Comment-only. Updates three files (action-log.ts, drift-baseline.ts, state-tools.ts) to correctly describe the orphaned action-log attribution read path after the v9 witness/SHA drift rewrite. Accurately flags readActionLogSync / readIntentScopeActionLogSync / readActionLogForTick as having no production consumer now that drift-detection-gate.ts and manual_change_assessment are gone. tsc + biome clean per commit message.

8a1a2d04 — Merge integrating the intent-root "Other" tab (from the drift-cleanup branch) into optstages-work. 13 files, 213 insertions. Reviewed the full merge diff:

• stage-internal-entries.ts: INTENT_ROOT_INTERNAL_ENTRIES set is comprehensive — correctly excludes intent.md, stages/, knowledge/, feedback/, reflection.md, intent-tick.json, action-log.jsonl, write-audit.jsonl, all baseline/drift sidecars, and VCS plumbing. System journals never leak to the SPA.
• parser.ts: walkIntentRootRecursive skips INTENT_ROOT_INTERNAL_ENTRIES at depth 0 only (correctly allows all files once inside a non-internal subdir). parseIntentRootFiles sets stage: "" as the intent-scope marker and tags directory for top-level-dir grouping. Correct.
• server/tool-call.ts: intentOtherFiles wired into both handleToolCall and prepareGateReviewSession; included in the URL-rewriting loop alongside outputArtifacts and otherFiles. Consistent.
• sessions.ts / haiku-api/src/schemas/session.ts: intentOtherFiles / intent_other_files optional on both sides — old sessions without the field degrade gracefully.
• IntentReview.tsx: "Other" tab added with disabled: !hasOther guard; shows count. OutputArtifactsTab.tsx: stage === "" path renders "Intent root" header instead of a blank string.
• Test: parse-output-artifacts.test.mjs new case explicitly verifies action-log.jsonl and write-audit.jsonl are hidden and that stray NOTES.md / subdirectory files surface with correct stage: "" and directory tagging.

4a373c2b — Pure data: paper-revisions.json commit hash + line stats refreshed. No logic changes.

---

Outstanding prior item

The dead allow variable in v9-to-v10.ts:63–66 (always null inside the !hasStages branch, making !allow || allow.has(s) always true) was filed as an inline comment in Pass 66 and reportedly resolved. The code is unchanged; this appears to have been acknowledged / won't-fixed. Not a correctness issue — the migration output is identical either way.

---

Overall: 67 passes, no open issues. The PR is clean and ready to merge.

[claude]

Claude finished @jwaldrip's task in 3m 3s —— View job

---

Pass 68 — Review SPA output rendering + dir-tab 404 fix

New since Pass 67 (4a373c2b): one commit — 6e231683 (fix(review): software code outputs render highlighted; dir-tab detail no longer 404s)

What changed

Two targeted bug fixes in the Review SPA's detail view, neither touching the core drift/witness/migration machinery.

parser.ts — repo-root output path resolution

The parseUnitOutputs function previously only resolved declared outputs against the intent dir. Software-studio units declare their outputs as repo-root-relative paths (e.g. web/apps/admin/src/WorkerDates.tsx) because those are the codebase files the unit edited — they live at the repo root, not under .haiku/intents/<slug>/. Result: buildArtifactEntry couldn't find them, returned a contentless stub, and the reviewer saw a blank box.

The fix derives repoRootAbs from the /.haiku/ segment in the intent path (with a resolve("..", "..", "..") fallback for non-standard layouts), then falls back to the repo root when the intent-dir path doesn't exist. Path-containment security is preserved: anything resolving outside both the intent dir and the repo root is still continued. serveRel = undefined correctly prevents the intent-scoped tunnel from attempting to serve a file it can't reach; content is inlined instead.

One detail worth noting: the mutation entry.relativePath = undefined after buildArtifactEntry returns is a bit asymmetric — buildArtifactEntry sets relativePath internally, and the caller immediately clears it. The comment makes the intent clear and it's functionally correct, but it's the kind of thing that could be tightened in a follow-up (pass the serveRel into buildArtifactEntry so the function sets it right the first time). Not a blocker.

$kind/$name.tsx — dir-tab detail route

The route's isKind guard rejected kind values that were dynamic directory names (e.g. proofs) rather than members of ReviewDetailKind. This matched the prior $tab.tsx fix (0e019fdfb). The widened guard accepts any safe slug (DIR_KIND_RE = /^[a-z0-9][a-z0-9._-]*$/i), which is appropriately narrow — no traversal characters. The kind as ReviewDetailKind cast is sound at runtime because StageReview treats detail.tab as a string id throughout the render path.

StageReview.tsx — empty body fallback

ArtifactBody now shows an informative note + open-link instead of a blank <pre> when artifact.body is empty. Clean, unremarkable.

Test coverage

• parse-output-artifacts: 30/30 (+2 new cases: repo-root resolve + inlined content, and missing-both-places stub surface)
• haiku-ui: 572/572 (+3 new cases: code output highlighted, empty-body note, dir-tab renders without 404)

Both suites pass. tsc and biome clean. Bundle 857 KB gz (147 KB under the 1 MB cap).

---

Assessment: no blockers. PR is ready to merge.

Prior passes found and fixed all substantive issues (drift-witness model, migration chain, deadlock detector, within-wave refill, optional-stage routing, non_actionable lifecycle guards, restamp audit timestamp preservation, quality_gates_env_blocked FSM protection, SPA gate presence detection, Biome 2.x compliance). This commit closes two reported UX bugs in the Outputs detail view with correct, tested fixes and no regressions introduced.

[claude]

Claude finished @jwaldrip's task in 4m 3s —— View job

---

Claude Code Review — Pass 69

Head: 2d9dec84 · No new commits since last pass · confirming verdict

---

What this PR does

A large-scope refactor across the workflow engine, SPA review UI, and migration layer:

1. Premise-witness drift model — witnesses now snapshot what a unit consumes (body + declared inputs), not what it produces. Output mutation no longer fires drift. Fixes the fnox.toml permanent drift loop that was blocking intent completion.

2. Per-unit worktree isolation — each dispatched unit runs in its own git worktree (elaborate Phase 1a, execution Phase 2, fix-chain Phase 6). Eliminates the class of dirty-working-tree collisions between concurrent unit dispatches.

3. Single prompt-resolution path — haiku_read_* tool family (haiku_read_hat, haiku_read_stage, etc.) and studioReadRef() shared reader. Dispatch prompts emit breadcrumbs instead of inlined bodies. Removes the path-dependency on import.meta.url that crashed the production bundle.

4. FEEDBACK rail — left-docked full-height rail with slide-out non-blocking drawer, replacing the old FAB + full-screen modal.

5. Micro-app window — Chromium --app mode with isolated per-session profile, DPR-proof maximize, Apple-Silicon Playwright build detection, and whole process-tree reap on close.

6. Gate presence heartbeat — 30s ACK'd heartbeat over long-lived WS, keyed to the MCP session. Never-attached sweep (NEVER_ATTACHED_GRACE_MS = 60_000), gate_awaiting_client response when no SPA is attached.

7. non_actionable terminal disposition — new terminal state for findings with no code fix (user questions, out-of-scope notes). Prevents reject→classifier→reject loops.

8. Optional stages — intent.stages as canonical materialized plan, haiku_drop_stage MCP tool, StageConfig.optional, StudioConfig.deprecated.

9. v8→v9 / v9→v10 migrations — backfills input_witnesses, strips approvals.*.witnesses, purges dead sidecars; strips skip_stages, backfills stages from studio minus deny-list.

10. Engine reliability — deadlock tick history persistence, batch-aware signatures, forward-only observations gate, untracked-clobber stash-retry, engineProtectedMergeInCwd for all merge paths.

---

Correctness

Drift model (drift-sweep.ts, sign-slot.ts, haiku_run_next.ts)

• Witness-at-sign captures body_sha256 + input_witnesses (declared input files hashed). stageProducedRel exemption applied at all three event-push sites prevents false alarms on the baton itself.
• Drift-FB close correctly refreshes body_sha256/input_witnesses in-place while preserving the original at timestamp. Approvals are NOT re-stamped — the fix from Pass 61 holds.

non_actionable lifecycle — haiku_feedback_move, haiku_feedback_write, and detectSettledDuplicate all guard this terminal state (commit ee2ee9e). The reject→classifier→reject loop is closed.

quality_gates_env_blocked in FSM_DRIVEN_UNIT_FIELDS_LIST (unit.ts) — engine-stamped field now protected against agent overwrite.

DecisionKind union — "changes_requested" is now in the union, not just cast. DECISION_LABELS and DECISION_ANNOUNCE both have entries; no more undefined in screen-reader announce.

useReviewChromeHeightVars — useEffect with [] dep array + MutationObserver for the conditionally-mounted gate bar. No more ResizeObserver recreation on every render.

Hat sequence migration (hat-sequence-migration.ts) — trim filter uses !isOrphan(it) to preserve hat-absent entries rather than the earlier typeof hat === "string" && validHats.has(hat) which dropped them.

mergeUnitWorktree (unit-branch-recovery.ts) — checks for unmerged commits before short-circuiting on a missing worktree. Prevents false "already merged" no-ops on re-entry.

Session never-attached suppression (sessions.ts) — hasAttachedReviewSessionForIntent checks isBrowserAttached (not just session existence). Grace window prevents indefinite suppression.

Schema parity (tool-defs.ts) — HAIKU_INTENT_CREATE_INPUT_SCHEMA is the single source of truth; studio_candidates is declared in both the schema and the tool def. Parity guard test in server-tools.test.mjs validates this at CI.

v9→v10 dead allow variable — the allow set in the !hasStages branch was always null (testing the exact condition already known false). Simplified to studioStages minus skip_stages. Logic is correct and clearer.

Parser repo-root fallback (parser.ts) — parseUnitOutputs falls back to the repo root when a declared output isn't under the intent dir. Unsafe paths (outside intent AND outside repo) still skip. The empty-box regression is fixed; 30/30 parse tests pass including two new cases.

Dir-tab isKind guard — widened to accept safe slug DIR_KIND_RE = /^[a-z0-9][a-z0-9._-]*$/i in the $kind/$name route. Dynamic directory tabs no longer 404.

Deadlock detector EXTERNAL_INPUT_ACTIONS — dead entries removed ("await_gate", "await_design_direction", "await_visual_answer"). recordTickResult early-returns on actionWaitsOnExternalInput; progressionSignatures() filters them from churn check. The false-halt on pickup-after-gate-park is closed.

Churn window — wait-for-human actions (user gate, etc.) no longer accumulate in the churn window, preventing the false "progress stalled" halt on the next real progress tick.

---

Security

No new attack surfaces introduced. The repo-root fallback in parser.ts bounds reads to within the repo (.haiku/ segment anchor + path normalization). No shell injection, no credential exposure, no new external network surfaces.

---

Performance

Per-unit worktree isolation adds process overhead for worktree create/destroy but eliminates the serialization bottleneck from shared-worktree dirty-state collisions. Net win for multi-unit waves.

The @tscircuit/runframe@0.0.2004/+esm pin replaces @tscircuit/runner@latest. Predictable bundle size; the latest resolution was a dependency-drift risk.

---

Design/Architecture

The premise-witness model is the right call. Witnessing outputs conflated "what did this unit declare it would produce" with "has anything changed in the codebase." The new model cleanly separates intent-plan stability (premises) from artifact mutation (not drift-relevant). The stageProducedRel baton exemption is the necessary complement — without it, the baton file itself would always trigger drift on the producing stage.

The @canon: sentinel replaces import.meta.url-relative template loading in the production bundle — correct fix for the MCP boot crash. The canonical root is resolved once at startup; all downstream loaders use the sentinel.

The non_actionable terminal state closes a structural gap in the feedback FSM. Previously, a finding that the classifier couldn't actionize would cycle indefinitely. The new state is terminal, correctly blocks re-entry, and is guarded in all three write paths.

The schema parity guard (server-tools.test.mjs) is a good systemic fix — it catches the class of bugs where the advertised schema drifts from the handler's actual validation.

---

CI (as of this review)

Check	Status
Biome Lint	✅ SUCCESS
MCP Tests	⏳ In progress
claude-review	⏳ In progress (this run)

No regressions introduced in the last two commits (6e231683, 2d9dec84). Both were straightforward fixes: repo-root output resolution + dir-tab routing, and biome format + dead-variable simplification.

---

Verdict

Approve — merge when CI green.

The premise-witness drift model is a foundational correctness fix. The supporting changes (worktree isolation, non_actionable disposition, schema parity guard, engine-reliability fixes, SPA rail/micro-app/heartbeat) are all well-motivated, correctly implemented, and properly tested. No issues to block merge.

[claude]

Claude finished @jwaldrip's task in 8m 26s —— View job

---

PR #372 Review — Pass 70

PR: refactor(engine): premise-witness drift + single-resolution prompt cascade + per-unit worktrees + studio trim
Branch: refactor/drift-cleanup-witness-model → main
CI: in progress at time of review

---

New commit since pass 69: c963b3ef

fix(engine): catch + repair phantom declared outputs at terminal hat and intent closeout

This fixes a real production bug: a unit could declare an output that never existed on disk (e.g. WorkerDates.test.ts when the actual file shipped as WorkerDates.test.tsx), and the intent would reach closeout with nothing blocking. Two root causes: (1) the per-unit terminal-hat gate only validated outputs once at completion, not after subsequent fix-loop merges that could rename/delete files; (2) a whole family of intent-level output validators had been dead since the v4 refactor — defined and unit-tested but never wired into the cursor.

The fix is well-architected:

• repairDeclaredOutput (state-tools): finds a same-stem sibling differing only in the final extension; returns null when ambiguous or absent — never guesses.
• Terminal-hat gate: attempt repair before blocking. Extension typos self-correct; genuinely absent outputs still block.
• validate-output-existence-gate.ts: intent-closeout sweep guarded on findCurrentStage === null so it never flags in-flight units. Auto-repairs near-misses in place; files deduplicated high FBs for the rest. Best-effort try/catch wrapping in run-tick.ts means a sweep failure can't wedge a tick.
• 175-line test file with 6 cases covering all code paths.

One issue found: the dedup check in validate-output-existence-gate.ts (line 73–80) skips FBs with closed_at from openRefs, intending to allow re-filing when a fixed output later disappears. But non_actionable also sets closed_at. If the fix loop closes a missing-output FB as non_actionable (gave up, accept the drift), the next closeout tick would skip that FB from openRefs, see the output still absent, and file a new FB — creating an infinite loop at closeout. Inline comment filed with a one-line fix.

---

Overall assessment

This is a 249-commit, 100-file refactor that landed in excellent shape after 70 review passes. The headline changes are all sound:

• Premise-witness drift model: witnesses capture what a unit consumes (inputs + body), not what it produces. Output mutation no longer fires drift. Fixes the fnox.toml permanent-drift production loop. Schema correct, baton exemption guards all three event push sites, consistent hashing via isTextBodyExtension() at sign-time and check-time.
• Single prompt-resolution path: haiku_read_* + studioReadRef() + @canon: sentinel form. MCP boot crash fixed.
• Per-unit worktree isolation: Phase 1a in place; isolation architecture documented and wired.
• Optional stages: intent.stages as canonical plan, haiku_drop_stage, StageConfig.optional, StudioConfig.deprecated, v9→v10 migration, libdev folded into software.
• non_actionable terminal disposition: Correctly guarded in all three state-tools.ts lifecycle paths.
• Gate heartbeat + micro-app window: Presence detection correct, DPR-proof maximize, Apple-Silicon Playwright support.
• Review SPA redesign: Left-docked FeedbackRail, GateDecisionBar, useReviewChromeHeightVars with ResizeObserver + MutationObserver, DecisionKind union complete including "changes_requested".
• Engine protections: FSM_DRIVEN_UNIT_FIELDS_LIST includes quality_gates_env_blocked, engineProtectedMergeInCwd on all 4 merge paths, deadlock detector cleaned of dead entries.
• Biome 2.x compliance: All format/lint issues resolved.

One non-blocking issue remains (inline comment above): the non_actionable dedup gap in validate-output-existence-gate.ts could cause an infinite-refile loop at closeout in the edge case where the fix loop marks a missing-output FB non_actionable. The one-line fix is in the comment.

Everything else reviewed across 70 passes is correct and merge-ready. The premise-witness inversion is the right architectural fix and the implementation is faithful to the design.

---

Review by Claude · CI run

[claude]

Claude finished @jwaldrip's task in 5m 55s —— View job

---

PR #372 Review — Pass 70

Two new commits since the last review pass (2d9dec84):

---

c963b3ef — fix(engine): catch + repair phantom declared outputs at terminal hat and intent closeout

Scope: state-tools.ts, validate-output-existence-gate.ts (new), run-tick.ts, output-existence-repair.test.mjs (new)

This fixes a real correctness gap: a unit could declare WorkerDates.test.ts, ship WorkerDates.test.tsx, and the intent would close out with the SPA surfacing a phantom "declared output isn't on disk" and nothing blocking. The fix is a two-layer backstop with sound design throughout.

Layer 1 — terminal-hat repair (in haiku_unit_advance_hat): Before blocking on a missing output, repairDeclaredOutput looks for a same-stem sibling differing only in the final extension. If exactly one candidate exists, the declaration is rewritten in-place and the hat completes against reality. Ambiguous (two candidates) or absent → null → block as before. finalExtStem correctly strips only the last extension (WorkerDates.test.ts → WorkerDates.test), so .test.ts ↔ .test.tsx and .ts ↔ .tsx both match.

Layer 2 — closeout sweep (autoRepairOrFileMissingOutputs): When findCurrentStage === null (all stages done), walk every unit's declared outputs, attempt repair, and file a deduplicated high FB for the rest. Dedup key is source_ref: missing-output:{unit}:{path} — re-running every tick is a no-op once the FB is open. Wrapped in try/catch in run-tick.ts so it's genuinely best-effort and never wedges a tick.

outputResolutionRoots() mirrors the exact four roots unitOutputExists() checks in the same order (intent-relative → worktree intent dir → repo root → worktree root), so repair and existence checks see the same filesystem. This is the important invariant — repair guessing at roots that the existence check doesn't would produce false negatives.

Test coverage (output-existence-repair.test.mjs): pins repairDeclaredOutput for the extension-typo case, no-sibling null, ambiguous null, and extension-less null; pins the closeout sweep for repair-in-place and FB-filed-then-deduped. All four edge cases of the repair contract are covered.

No issues found. Clean, defense-in-depth fix.

---

a5596fd0 — fix(review): disabled-mode gate hint is composer-aware + accurate

Scope: GateDecisionBar.tsx, gate-decision-hint.test.tsx (new)

A one-line conditional replacing a single incorrect string. When the reviewer is viewing a non-active stage with nothing pending (decideMode → "disabled"), the old copy — "Type a comment above or click into another stage" — was wrong in the docked-rail layout where the composer isn't rendered inline. The new copy branches on the composer prop:

• composer=true → "Type a comment above to leave feedback on this stage. Only the active stage can be approved — select it to approve."
• composer=false → "Open the feedback panel to leave a comment on this stage. Only the active stage can be approved — select it to approve."

Both strings also fix the secondary inaccuracy: the old "click into another stage" implied the non-active stage was approvable, which it isn't.

The test mocks the three context hooks the bar depends on and asserts exact substring matches for both layouts, plus verifies the stale "click into another stage" copy is absent.

No issues found. Small, focused, tested.

---

Summary

Both commits are clean. No blocking issues across either. CI is running. This PR remains merge-ready pending CI green. 69+ passes of review, all critical/medium findings resolved, the last two rounds of commits are correctness/UX fixes with solid test coverage.

[claude]

Claude finished @jwaldrip's task in 5m 42s —— View job

---

Claude Code Review — Pass 71

Triggered by: pull_request synchronize (commits 51ca584c, 2d01f23a)
CI: In progress — no failures reported yet.

---

New commits reviewed this pass

Commit	Description
a5596fd0	fix(review): GateDecisionBar disabled-mode hint is now composer-aware
4ed2bbd5	refactor(software): drop the release stage; product mandatory, security optional
51ca584c	feat(studios): 47 stages marked optional: true across 22 studios (audit)
2d01f23a	Merge origin/refactor/drift-cleanup-witness-model → optstages-work2

---

One open issue (carried from Pass 70, still unaddressed)

packages/haiku/src/orchestrator/workflow/validate-output-existence-gate.ts lines 73–79 — non_actionable FBs bypass the dedup gate → infinite re-file loop at closeout.

When the fix loop closes a missing-output FB as non_actionable, state-tools.ts sets status: "non_actionable" and closed_at: <timestamp>. The current guard fires on closed.closed_at → continue, never adding the ref to openRefs. On the next closeout tick: output still absent, no matching ref in openRefs, new FB filed → fix loop closes it non_actionable → next tick files again → unbounded loop.

Inline comment filed with a one-line fix and a suggested test case. The test suite in output-existence-repair.test.mjs does not cover this scenario.

---

Pass 71 findings per commit

a5596fd0 — GateDecisionBar hint fix
Clean. The composer boolean prop is already threaded to the bar; the ternary is the correct branch point. gate-decision-hint.test.tsx renders the bar in disabled mode for both composer={false} and composer={true} and asserts the exact copy and the absence of the stale "click into another stage" string. No issues.

4ed2bbd5 — software studio release-stage removal
STUDIO.md now lists six stages: [inception, design, product, development, operations, security]. The mandatory core description in the preamble matches the implementation. optional-stages.test.mjs and studio-config.test.mjs updated. Workflow diagram regenerated. No issues.

51ca584c — 47 stages marked optional
Pure STAGE.md data change (optional: true added). The commit message correctly notes this is additive and conservative — marking a stage optional only enables the keep-or-drop prompt, never auto-drops. Studios with safety-critical depth (QA, migration, security-assessment) preserve deep mandatory cores. No issues.

2d01f23a — merge commit
Brings validate-output-existence-gate.ts (177 lines), state-tools.ts additions (repairDeclaredOutput, unitOutputExists), run-tick.ts closeout call, and output-existence-repair.test.mjs (175 lines) onto the branch. The gate integration in run-tick.ts is correct — best-effort try/catch, telemetry on action, placed at the right point in the tick sequence (before fix-chain merge completion). All reviewed in prior passes except the non_actionable gap above.

---

Cumulative status (71 passes)

All prior issues resolved except the non_actionable dedup gap above (filed as an inline comment). This is the only remaining correctness issue: it creates an unbounded FB-file loop at intent closeout for any missing-output finding that the fix loop accepts as non-actionable. One-line fix; suggested test case included in the inline comment.

Everything else across the premise-witness drift rewrite, per-unit worktree isolation, single prompt-resolution path, optional stages (haiku_drop_stage, StageConfig.optional, v9→v10 migration), gate heartbeat, Review SPA redesign, migration reliability, deadlock detector, schema parity guard, and Biome 2.x compliance is solid. The PR is merge-ready after that one fix lands.

[claude]

Claude finished @jwaldrip's task in 5m 38s —— View job

---

Pass 71 — reviewing commits since a5596fd0

---

New commits

SHA	Subject	Verdict
5cec721f	fix(browse): pipeline stage status is monotonic	✅ Clean
261e0cf4	fix(engine): closeout output gate non_actionable + gray-matter mutation	✅ Fixes both open issues
00543a64	build(review): regenerate inlined SPA bundle (+38 bytes gzipped)	✅ Clean
4d218eee	Merge remote-tracking branch into branch	✅ Clean

---

Open issue from Pass 70: CLOSED

validate-output-existence-gate.ts:87 — non_actionable dedup leak fixed in 261e0cf4. The commit message nails the diagnosis: a non_actionable closure sets closed_at, so the old closed.closed_at → continue guard skipped it without adding the ref to openRefs, letting the next closeout tick re-file → fix loop re-closes non_actionable → infinite loop. The fix correctly distinguishes suppressive closures (non_actionable — intentionally absent, don't re-file) from permissive ones (closed / rejected — fixed or invalid, a re-file is appropriate if the output is still missing). Logic confirmed correct after tracing through deriveFeedbackStatus: resolution: "non_actionable" + closed_at → derived status "non_actionable" → falls through to openRefs.add(ref) in the new code path.

The same commit also fixes a second bug I hadn't caught: gray-matter shared-cache mutation in both validate-output-existence-gate.ts and state-tools.ts::repairDeclaredOutput. Both sites did parsed.data.outputs = corrected then re-stringified. gray-matter caches parse results by content and returns a shared object, so mutating it corrupts every byte-identical unit's next parse (two skeleton units swap outputs). Both sites now spread into a fresh data object before stringifying. The test suite's cross-test contamination was the surface symptom; the real hazard is a production race between two units with identical initial content.

---

5cec721f — browse pipeline monotonicity

normalizeStageProgression in intent-parsing.ts is clean. The algorithm finds the highest-indexed complete stage, then back-fills everything below it. Edge cases all handled: all-pending (loop doesn't run), all-complete (no-op), legitimate active stage between two complete stages (correctly normalized). All three providers (GitHub, GitLab, local) wire it in before calling deriveV4ActiveStage, so the derived active stage lands on the correct position. Two tests cover the contradiction case (Operations active while Security complete) and the monotonic no-op case.

---

CI

Two runs still in-progress at review time (Claude Code Review + CI, both started at 2026-05-28T05:24:26Z). No failures in any completed run.

---

Verdict

No open issues. The two previously-reported bugs are fixed with tests. PR is merge-ready pending CI green.

[claude]

Claude finished @jwaldrip's task in 4m 26s —— View job

---

Claude Code Review — Pass 72

CI is running; reviewing the two commits pushed since Pass 71.

---

eefef339 — fix(engine): feedback-assessor is fix-loop-only, never a unit hat

Correct and clean. The old resolveUnitHats appended feedback-assessor to any unit declaring closes:, but in v4 the closes: field is an informational breadcrumb only — closure is stamped on the FB by the fix loop's haiku_feedback_advance_hat, not by the unit. The injected hat was vestigial and confused status derivation.

Changes:

• resolveUnitHats now returns resolveStageHats(intent, stage) unconditionally. The unit parameter is renamed _unit (unused, no lint violation). The docstring is updated to clearly state the invariant: units run the stage hats: only, fix hats stay in fix loops.
• 71-line test addition to hat-sequence-migration.test.mjs covers both halves: resolveUnitHats no longer appends the assessor to closes: units, and reconcileOrphanedHatSequences strips the stale iteration from existing workflows.

The existing reconcileOrphanedHatSequences pre-cursor gate already handles migration of stale iterations, so live workflows degrade gracefully — no migration script needed. Correct.

---

f8efa3b0 — test(engine): pin that the engine NEVER merges intent work into mainline

Solid trust-checkpoint test. 214 lines, no mocks — real git repo + bare origin. Covers both halves of the delivery invariant:

1. Engine sees intent work ahead of mainline and touches nothing. reconcileIntentBranches runs; origin/main and local main SHAs are asserted byte-identical afterward.
2. External PR merge is the only close signal. A human merge commit is pushed to origin/main; isBranchMerged returns true; another reconcileIntentBranches still leaves origin/main at the human's SHA with the human's author.

Good test hygiene: _resetIsGitRepoForTests() clears memoized isGitRepo state between git state changes, HAS_GIT guard for CI environments, try/finally cleanup. The commit message explicitly calls out that injecting an engine merge+push makes assertion (1) fail — the test has teeth.

---

Overall

No open issues. Both commits are well-reasoned, well-tested, and tighten semantics that were previously ambiguous or outright wrong. The PR is ready to merge pending CI green.

[claude]

Claude finished @jwaldrip's task in 4m 1s —— View job

---

Claude Review — Pass 73

One new commit since Pass 72: a246d196 — fix(engine): validate unit & feedback iterations only carry valid hats for their kind

CI: in progress at time of review.

---

a246d196 — Generalize hat-sequence reconciler to cover feedback iterations

This commit extends the pre-cursor reconciler from units-only to all three entity scopes: unit iterations, stage-scope feedback iterations, and intent-scope feedback iterations. The rule is now uniformly enforced: a hat is valid for an iteration only when (1) it's the right kind of hat for that entity, and (2) a mandate file actually exists for it.

Architecture assessment: correct. The semantics are clean — hats: is the unit rotation, fix_hats: is for feedback/fix loops, and the two must not bleed into each other. The reconciler runs pre-cursor, is idempotent, and produces no git/worktree side effects. The enforcement was already proven sound for units; this commit extends it consistently.

Implementation review:

The refactor into trimIterations() + reconcileDir() is a meaningful improvement. The shared helper handles the gray-matter cache-poisoning risk (structuredClone before mutation) in one place rather than three. The isInvalid predicate is identical in shape to what was reviewed and approved in the prior unit-only path — it correctly distinguishes hat-absent entries (kept) from wrong-kind-hat entries (trimmed).

The valid-hat set computation differs slightly by layer:

• validUnitHats() uses resolveStageHats + resolveHatPath — stage-scoped, correct, no per-unit hat overrides exist in the architecture.
• validStageFixHats() uses resolveStageFixHats + resolveFixHatPath — symmetric.
• validStudioFixHats() uses readStudioFixHatPaths for existence, consistent with the studio-reader API.

The removal of resolveUnitHats(slug, stage, unit) in favor of resolveStageHats(studio, stage) is a tightening: the old call accepted a per-unit scope that the architecture doesn't actually use (all units in a stage share the same hat rotation), and it required the intent slug rather than the studio name. The new approach is more direct.

One minor observation: the new test covers stage-scope feedback (unit hat stripped, fix hats kept). Intent-scope feedback (studio fix_hats: layer) is exercised by the same reconcileDir → trimIterations code path but has no dedicated test case. Not a blocker — the logic is identical — but it's worth a follow-up test if the studio fix_hats: layer gets exercised more heavily.

No issues found. The commit is correct, well-scoped, and leaves the reconciler in a more defensible state than it was before.

---

Overall

Pass 73 finds no open issues. The PR continues to be ready to merge pending CI green. All findings from earlier passes have been resolved; this final commit closes a correctness gap that was the motivating bug for eefef339 (feedback-assessor must never appear on a unit) but now also covers the symmetric case (unit-rotation hat must never appear on a feedback item) and both feedback scopes.

✅ Approved — merge when CI passes.

[claude]

Claude finished @jwaldrip's task in 4m 28s —— View job

---

Claude Code Review — Pass 74

Reviewed 3 new commits (4018772a, c914b442, bdfaf8ec) — all three are targeted, correct bug fixes traced to the worker-new-badge incident on 2026-05-28. No issues found.

---

4018772a — healDuplicateFeedbackIds pre-tick gate

Correctness: clean.

The race is well-understood: nextFeedbackNumber is local-max + 1, so two parallel reviewers operating against a behind-origin intent-main can both allocate the same number, and the non-fast-forward rebase recovery keeps both files on disk. The fix is correctly scoped — scanning all stages (not just the current one) is right since intent-completion reviewers write at intent scope.

Implementation details that hold up:

• maxNum is initialised from the max over all groups before any renames, then incremented in-place as each loser is processed. Successive renames within a single tick cannot re-collide with each other or with existing unique files. ✓
• Multiple losers per group (sorted.slice(1)) each get a fresh maxNum += 1 — handles 3+ collisions without gaps. ✓
• Sidecar renamed first, then the body URL rewritten, then the MD written, then the old MD removed. Safe ordering: the new name always resolves on disk before body text points at it. ✓
• writeFileSync + rmSync (write-then-delete) rather than renameSync for the MD: if rmSync fails after a successful write the gate is still idempotent on re-run — created_at is preserved in the new file, so the tiebreak correctly identifies the original as keeper. ✓
• Best-effort try/catch at both the per-scope and per-file level; emits telemetry on heal events. Never blocks the tick. ✓

Test suite covers: the shadow bug reproduction, correct keeper selection (earliest created_at), sidecar rename + URL rewrite, and idempotency on an already-unique tree.

---

c914b442 — intent quality gates before terminal user gate

Correctness: clean.

The reorder is a one-block move: userMissing → intent_review block relocates from before the intent_quality_gates check to after it. The new sequence (spec → adversarial reviewers → intent quality gates → user → reflection → seal) is the only correct ordering — the human gate is "the final check before reflection," not a pre-automated-bar signature.

The e2e assertion is precise: lastIndexOf("dispatch_quality_gates") < lastIndexOf("intent_review"). Using lastIndexOf correctly identifies the intent-scope QG dispatch (the last one, since stage QGs fire earlier in the run) vs. the terminal intent_review action (distinct from per-stage user_gate). All five modes still seal per the existing coverage.

---

bdfaf8ec — mergeStageBranchForward via engineProtectedMergeInCwd

Correctness: clean.

mergeStageBranchForward was the one remaining plain git merge --no-ff in the intent-completion path — unlike unit/fix-chain/downstream-sync merges, it never re-asserted engine-owned frontmatter. Routing it through engineProtectedMergeInCwd brings it into the same contract as every other merge in the engine.

The target-authoritative choice is correct: HEAD = intent main at merge time, which holds the cross-stage user approval and the authoritative spec stamp. Stage-only files (unit work) still merge forward; only genuine conflicts on agent content surface.

Error-path handling:

• r.ok → clean or already up-to-date → return empty conflict list. ✓
• r.conflictFiles.length > 0 → genuine agent content conflict → surface it. ✓
• !r.ok + empty conflicts → hard pre-merge refusal (dirty tree, etc.) → tryRun merge --abort (harmless no-op if nothing started) + throw, caught by outer try/catch which maps to success: false. ✓

toBranch is in scope at the throw site (defined at line 1894). ✓

The regression test is the exact reproduction of Bug 2: three-branch setup with divergent intent.md spec timestamps + a user approval on main. Post-fix: no MERGE_HEAD, main's timestamps win, stage unit file still merges forward.

---

Summary

Pass 74 finds no open issues. These three commits close out the worker-new-badge incident's Bug 2 (FB-id collision → shadowed finding, divergent intent.md FM → stage-forward-merge conflict) and Bug 4 (premature human gate before automated quality bar). Each fix is narrowly targeted, has a regression test that would have caught the original bug, and leaves no loose edges.

The PR remains ready to merge pending CI green.