fix: engine FM-merge protection + browse perf & detail-view overhaul by jwaldrip · Pull Request #374 · gigsmart/haiku-method

jwaldrip · 2026-05-28T17:02:36Z

Summary

Three themes, all on this branch, building cleanly on current main (24 commits, 56 files).

Browse — list view performance & correctness

Lean GitLab + GitHub list loaders: batch the per-*/main-branch reads in one aliased GraphQL request, dropping the old O(3N) per-branch fan-out.
Paginate the intent-branch query on both providers — intents whose branch sorted past feat(builder): add stack-specific implementation guidelines #100 were silently dropped (e.g. worker-new-badge).
Surface raw-GraphQL errors + fall back so an intent never vanishes from the list.
Show the created date on each list card (the list sorts newest-first by it).

Browse — detail view

Resolve an unmerged intent's artifacts/code from its haiku/<slug>/main branch (was reading the default branch → "no file on disk").
Code inputs/outputs are clickable + syntax-highlighted; stage-view outputs open in a modal to match the unit view.
Shared sign-off component across unit + intent scope: every role opens a modal — studio agents show their mandate, engine roles their prompt, user the gate description, quality_gates the command list. Bundles intent-review-agents + engine-review bodies.
Hat/fix history rendered as a horizontal stepper (markdown handoff on click); FB badge shows the filing agent's name, not its classification; raw frontmatter renders as highlighted YAML.
Unit stats wired to real data (iterations-derived bolt/hat, active time = summed hat-dispatch durations) and irrelevant tiles dropped.
Stop the Relay duplicate-id console warning (content-addressed TreeEntry.id collides for identical files at two paths) by reading the recursive tree via raw GraphQL.
Path-shaped loading skeletons (list/intent/stage/unit) instead of the portfolio skeleton everywhere; unit segment is now the keyword …/stage/<stage>/unit/<unit>/.

Engine

pending_seal: an intent is not sealed until its work lands on the default branch — a sealed stamp is reinterpreted against merge state at the display layer, and only short-circuits once merged.
Engine-protect the intent-main→stage enforcement merge so a 3-way auto-resolve never hands the agent intent.md conflict markers.
System journals (action-log.jsonl, write-audit) never surface as unit outputs.

Test plan

CI green (biome packages/haiku/src, engine test suite, website build/typecheck)
Browse list + detail verified live against the GitLab monorepo (Playwright)
website typecheck + biome clean across all touched files
Smoke an end-to-end intent to confirm pending_seal holds until merge

🤖 Generated with Claude Code

…no intent.md markers) The version auto-bump to 10.0.0 activated the v9→v10 migration, which materializes `stages` onto intent.md. When that ran on intent main mid-flight, it diverged intent main's intent.md from a stage branch's — and `ensureOnStageBranch` Stage 2 (the per-tick main→stage enforcement merge) was a PLAIN `git merge` with no engine-state guard. It conflicted on intent.md and left conflict markers in a workflow-managed file, so the seal choked parsing it (`YAMLException`) and the cursor reported "stage-branch enforcement failed". CI on the bump commit went red on seal-intent-commits. Route Stage 2 through `engineProtectedMergeInCwd` like every other engine merge — target/HEAD-authoritative. Per the stage-branch invariant the stage is ahead of main and authoritative for its own state (downstream-sync-clobber), so this keeps the stage's rich FM AND deterministically resolves intent.md to one valid copy — no markers ever reach the agent. Also hardens `mergeStageBranchIntoMain` the same way (settleEngineConflicts on each conflict-return path). Tests (at v10): seal-intent-commits green again; new ensure-stage-branch-engine-fm pins that ensureOnStageBranch resolves divergent intent.md without markers; downstream-sync-clobber + unit-worktree-sibling-clobber still green (target- authoritative invariant preserved). Full suite 2070/0 at v10. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

…og, write-audit) The engine appends to intent-root journals (action-log.jsonl, write-audit.jsonl, drift baselines) during a unit's work, so they show up in the unit's git diff — and `autoPopulateOutputs` was missing them from its bookkeeping exclusion, so they leaked into `outputs:` and rendered on the SPA output surface. - Source: `autoPopulateOutputs` now excludes `INTENT_ROOT_INTERNAL_ENTRIES` (the same set the intent-root file parser already hides) — no new leaks. - SPA display: `parseUnitOutputs` drops any declared output whose basename is an intent-root journal — covers intents that leaked before the source fix. - Website already filters `.jsonl` from the outputs list (isBookkeepingArtifact). Test: parse-output-artifacts pins that action-log.jsonl / write-audit.jsonl declared in a unit's outputs never surface, while the real deliverable does. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

…p the O(3N) fan-out The list view was O(3N): per haiku/<slug>/main branch it did a readFile + an MR fetch + a per-stage unit probe, plus a separate MR fetch for every stage branch. That's the slowness. Lean path: - Drop the per-stage-branch MR fan-out entirely (MR/PR status loads lazily in the detail view, where it's already fetched). - Replace the per-main readFile + MR + per-stage-probe loop with ONE raw aliased GraphQL request (`batchMainBranchData`): for every main branch, read `intent.md` + the immediate `stages/` subdir names as per-branch aliases, chunked. Relay queries are static and can't fan out across a variable set of refs, so this uses a raw POST to /api/graphql. - Active stage + progress derive from the stage-dir set (the lean equivalent of the old per-stage unit probe) — the card's `Stage:` + `N/total` bar. - Removed the now-dead `probeStagesWithUnits`. Default-branch catalog still loads via listIntentsFromRef (tree + batched blobs). Per-main is now one batched request instead of 3N. (Folding the default-branch blobs into the same raw batch — to hit a strict ≤3 queries — is a follow-up.) Network path verified by typecheck + the pure-helper browse tests; full load needs in-browser verification against a real GitLab project. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

…ity with GitLab) Mirror the GitLab lean loader on the GitHub provider. The per-main fan-out was readFile + per-stage unit probe per haiku/<slug>/main branch; replace it with ONE raw aliased GraphQL request (`batchMainBranchData`) that reads each main branch's `intent.md` text + its `.haiku/intents/<slug>/stages/` subdir names (stage-dir count) via per-branch `object(expression:"ref:path")` aliases, chunked. Active stage + progress derive from the stage-dir set. PR metadata is already inline in the branch-list query, so it's kept at no extra cost (no separate PR fetch). Removed the now-dead `probeStagesWithUnits`. Relay queries are static (can't fan out across refs), hence the raw POST to api.github.com/graphql. Network path verified by typecheck + the pure-helper browse tests; full load needs in-browser verification against a real GitHub repo. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

…ult branch An intent is no longer sealed the moment its approvals sign. Sealing is the terminal write-lock (`sealed_at`), and the work isn't actually delivered until `haiku/<slug>/main` lands on the repo's default branch. So the cursor now gates the seal on that merge. Engine: - New `orchestrator/workflow/intent-delivery.ts` — `intentDeliveryState` / `isAwaitingMerge`, squash-merge-aware via the existing `isBranchMerged` (local `--is-ancestor` first, gh/glab merged-PR fallback), plus a cheap local-only mode for per-request display paths. - Cursor terminal gate emits a new `pending_seal` action and HOLDS (no `sealed_at`) when the hub branch isn't an ancestor of the default branch. Falls through to `seal_intent` when merged, in filesystem mode, or when no default branch resolves. The engine never merges itself (honors "never merge unless asked"); the next tick re-checks and seals once the merge lands. - `pending_seal` prompt builder + template, registered. - On `/haiku:haiku-pickup` of a held intent, `haiku_run_next` re-opens the `PR_INTERACTION_ROLES` approvals (`delivery-verifier`) so it re-audits the open change request and files feedback for new review comments before sealing. Keyed on the role-class set, so cascade overrides are honored. Phase indicators: - Statusline: new `pending_seal` phase (render kind + violet hue, "pending seal" label, gated). - Derived status `pending_seal` on `haiku_intent_list`. Left `haiku_capacity` sealed_at-only (throughput report, not a phase indicator — held intents count as in-flight there). - SPA wire: `current_state.seal_status` + `awaiting_merge_into` (Zod + engine type + `current-state.ts`); `IntentCompleteView` renders a "Pending seal" badge and "awaiting merge into <default>" copy. - Website architecture map post-intent card + actors/payload sync; CLAUDE.md concept row; PROMPTS.md folder listing. Tests: - `seal-intent-commits` now proves both halves: it holds at `pending_seal` with no `sealed_at`, then seals after a simulated delivery. - New shared `deliverIntent` test helper (merges the hub branch onto the default branch — what the human/host does); the 8 git-backed lifecycle/e2e drivers call it on `pending_seal` so they still reach `sealed`. - Refreshed the stale `parity.spec.tsx` DOM snapshot (a pre-existing GateDecisionBar copy drift unrelated to this change). Full suites green: haiku 2069, haiku-api 171, haiku-ui 574; haiku-ui + website typecheck clean; MCP build green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…intents never vanish A main-only intent (haiku/<slug>/main not yet on the default branch) disappeared from the list view: the new raw aliased batch returned no intent.md for it and the loader skipped it silently. Two fixes: - rawGraphql now console.warns GraphQL `errors[]` (they come back HTTP 200 with null data) + HTTP/transport failures — a dropped intent was otherwise indistinguishable from "no such intent". - batchMainBranchData falls back PER BRANCH to the proven single-ref reads (readFileFromRef for intent.md + a stages-dir tree query) whenever the batch yields no intent.md — correctness first; the batch stays the fast path. The console warning will show why the batch missed so the fast path can be fixed definitively. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

…wse-perf Consolidate the pending-seal feature onto this branch so it ships as one PR alongside the engine-FM-merge / browse-perf work. Conflicts resolved: - state-tools.ts: import collision only (kept both `isAwaitingMerge` and `INTENT_ROOT_INTERNAL_ENTRIES`); the derived-status edits auto-merged. - plugin/bin/haiku.mjs: regenerated by rebuild (now bakes v10). Consolidated suites green: haiku 2071, haiku-ui 574; haiku-ui + website typecheck clean; MCP build clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…anch #100 were dropped) `branchNames(searchPattern:"haiku/*", limit:100)` returned a single 100-branch page. A busy monorepo has far more than 100 `haiku/*` branches (every unit branch counts — 168 here), so any intent whose `*/main` branch sorted past the cutoff silently vanished from the list (worker-new-badge: branch #100+ → missing). Two changes: - Narrow the glob to `haiku/*/main` — the lean list only needs the per-intent main branches, not unit/stage branches. 168 → 53 results here. - Paginate: loop `offset` by 100 until a short page signals the end, so it scales past any single-page cap. Verified live (Playwright, real GitLab monorepo): list went 88 → 98 intents and the worker-new-badge card renders correctly — "Add \"New\" badge to recent workers", completed, Security, 6/6 stages. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

…ab fix) `refs(refPrefix: "refs/heads/haiku/", first: 100)` capped at a single page, so on a repo with >100 `haiku/*` branches any intent whose `*/main` ref sorted past #100 dropped off the list — same bug just fixed for GitLab. GitHub's `refs` is cursor-paginated (not offset), so loop on `pageInfo.hasNextPage`/`endCursor` until exhausted. Done via raw GraphQL rather than the static Relay artifact so `after:` can be threaded without regenerating compiled queries; the artifact + its import stay for the single-slug detail path (scoped to one intent's branches, no cap risk). Trimmed the per-node selection to the fields actually consumed (name + PR number/url/state). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

…the display layer A sealed_at stamp written before the merge gate landed (or any premature seal) shouldn't read as "done" while the work is still ahead of the default branch. The write-gate stops NEW premature seals, but the display surfaces still trusted sealed_at blindly, so a legacy sealed-but-unmerged intent kept showing "sealed". Now the status surfaces re-check the merge locally and show pending-seal when the hub branch hasn't landed — honoring the original ask: "while the cursor signal may say sealed, if the branch isn't merged its status should be pending seal." - statusline: the sealed render block flips to the `pending_seal` phase ("pending seal", gated) when `isAwaitingMerge` (local-only); pickActiveIntent keeps a sealed-but-unmerged intent in the live set so it still shows. - current-state (SPA wire): `seal_status`/`awaiting_merge_into` derive pending_seal for a sealed-but-unmerged intent, not just an unsealed one. - haiku_intent_list: a sealed stamp resolves to "pending_seal" when the hub branch isn't merged, else "completed". All local probes (no gh/glab) so the per-prompt status line stays fast. Full suites green: haiku 2071, statusline 43, intent-list 127. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…puts viewable The detail view loaded an intent's metadata from its haiku/<slug>/main branch but then read each artifact's bytes from the DEFAULT branch (provider.readFile used this.branch, which is empty on a slug-only URL). An unmerged intent's deliverables don't exist on the default branch, so every output/input rendered "no file is on disk" (website) / "(empty)" (SPA) even though the files were right there on the intent branch. - Add an optional `ref` to BrowseProvider.readFile / resolveAssetUrl (GitLab + GitHub honor it; local ignores it). The detail views pass the intent's branch so artifacts resolve from where they actually live. - Thread intent.branch through IntentDetailView -> StageDetail/UnitOutputsSection (stage UNIT OUTPUTS) and -> UnitDetailView -> Artifacts/Refs sections (unit view). - Unit view: a declared output is either repo-root code (web/.../X.tsx) or intent-relative (stages/.../plan.md). It only ever tried the intent-relative shape; add the same as-is-then-intent-relative candidate resolution the stage view already used, so code outputs resolve too. - Code-type inputs/outputs are now clickable + syntax-highlighted: the unit view had its own narrow text allowlist that classified .tsx/.ts/.js as binary -> a dead download link. Switch to the shared FilePreview/isTextFile (CODE_LANG map -> hljs), so code opens in the doc modal highlighted like the stage view. Verified live (Playwright, GitLab monorepo, worker-new-badge intent): both the stage UNIT OUTPUTS rows and the unit-detail inputs/outputs resolve real content from the branch (no "no file on disk"), and WorkerDates.test.tsx opens with full TSX syntax highlighting (252 hljs tokens) on the unit view, 50 on the stage view. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

…FM as YAML Unit detail view, Reviews & Approvals: - Each non-engine role now links to its studio definition on the stage page (`/studios/<studio>/stages/<stage>/#agent-<role>` for the review walk, `#approve-agent-<role>` for the approval walk — anchors the stage page already emits). Engine-owned roles (spec, continuity, cross-stage-consistency) and the gate slots (quality_gates, user) have no def page, so they stay plain text. Threads `intent.studio` through to the sign-offs section. Raw-frontmatter accordion: - Renders syntax-highlighted YAML (the on-disk format) via the shared FilePreview instead of dumping raw JSON. Serialized with gray-matter's stringify (the website's existing YAML lib), fences stripped. Verified live (Playwright, worker-new-badge unit): six studio agents link out on each of the review + approval walks; the five engine/gate roles are not linked; the FM accordion shows highlighted YAML (977 hljs tokens). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

…y as markdown Feedback cards (unit + intent scope): - The mono pill showed the FB's `origin` (a classification — adversarial-review, engine-review, studio-review). The engine already stamps the filing agent's role in `author` (every review/approval/intent-review/continuity dispatch passes author=<role>), so surface that instead: a finding from the security agent reads "security", not "adversarial-review". Falls back to origin when no author is stamped. Hat history (unit) + Fix history (feedback): - Each iteration's handoff baton now renders as markdown inside a collapsed-by-default <details>, with the hat/result/commit (or bolt) line as the summary. Was a plain-text paragraph, always expanded. Extracted a shared IterationHeading for the summary row. Verified live (Playwright, worker-new-badge): FB pills show agent names (cross-stage-consistency, etc.) with no adversarial-review/engine-review; hat history renders 3 collapsed details that expand to rendered markdown. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

…s merged Closes the legacy/coherence gap: the cursor's sealed short-circuit fired on `sealed_at` alone, so an intent sealed before the merge gate (or any premature seal) whose `haiku/<slug>/main` is still ahead of the default branch was treated as terminal — a pickup couldn't re-audit it. Now the short-circuit fast-paths to `sealed` ONLY when the work has landed (squash-merge-aware `intentDeliveryState`). A sealed-but-unmerged intent falls through to the full walk, so a pickup re-opens the delivery-verifier (the PR_INTERACTION re-audit already in haiku_run_next) and the terminal re-emits `pending_seal` until the merge lands. The terminal merge gate now fires whether or not `sealed_at` is set. Inapplicable in filesystem mode / no default branch → seals exactly as before. Tests: - seal-intent-commits gains a legacy guard: a stamped-but-unmerged intent stays held at pending_seal (doesn't short-circuit to sealed), then seals once delivered. - Removed the two degenerate cursor-walk "sealed_at → sealed" sanity tests — that invariant is gone by design; realistic sealed paths are covered on the completed fixture in seal-intent-commits. Full suite green: 2069 passed, 0 failed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…inking out The unit-detail Reviews & Approvals roles linked to the studio stage page. Open the agent's mandate in a modal in-place instead — less context loss. A small client loader (studio-defs.ts) lazy-fetches the bundled studio content (/prototype-stage-content.json, the same asset the architecture map reads) and resolves the review-agent body for (studio, stage, role), falling back to any stage in the studio for a borrowed agent. Engine roles (spec/continuity/ cross-stage-consistency) and the user/quality-gate slots ship no def, so they stay plain text. Verified live (Playwright, worker-new-badge unit): clicking "Architecture" opens a modal titled Architecture showing the rendered mandate from plugin/studios/software/stages/development/review-agents/architecture.md. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

The engine already merges the default branch into the hub branch every tick/pickup and surfaces genuine conflicts as pre_cursor_sync_conflict, so no new conflict-resolve mechanics are needed (confirmed). The pending_seal prompt now says the branch was kept current and to resolve any surfaced conflict. Also rebuilds plugin/bin/haiku.mjs (v10) so the committed bundle matches the Part 1 engine changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…lockquotes Consistency: the stage view's UNIT OUTPUTS expanded inline while the unit view opened a modal. Standardize on the modal — a unit-output row now opens its file in a centered modal (new OutputFileModal shell, mirrors the unit DocModal), rendered via the same FilePreview, so outputs open the same way on both surfaces. Hat/Fix history: a `>` aside in a handoff baton rendered with the default `prose` blockquote style — oversized italic text wrapped in curly quotation marks, which looked off for a one-line note. Tone it down to a quiet left-ruled, non-italic note and drop the auto-inserted quote glyphs (shared BATON_MARKDOWN_CLASS across hat + fix history). Verified live (Playwright): clicking a stage UNIT OUTPUTS row opens the file in a modal (plan-unit-001-render-new-tag.md rendered inside). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

The collapsed-row list (per-row left-border stubs, a chevron per hat) read as cluttered and sparse. Replace it with a horizontal stepper: the hat sequence as connected dots (result-colored, filled when selected) with the hat name + result under each, and a single handoff panel below that shows the selected step's baton as rendered markdown. Nothing selected initially — the timeline reads as a flow first, detail on demand. Shared IterationStepper backs both the unit Hat history and the feedback Fix history. Verified live (Playwright, worker-new-badge unit): planner → builder → reviewer render as a connected stepper; clicking builder opens its handoff markdown. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

…tion Intent-level approvals now use the SAME sign-off component as the unit view (extracted SignOffGroup), and every role — unit or intent — opens a modal explaining what it does, not just studio review agents: - studio review / intent-review agents -> their `*.md` mandate - engine roles (spec / continuity / cross-stage-consistency) -> the engine prompt body for the phase walk (review = pre-execute, approve = post-execute, intent = completion) - user -> a description of the human gate - quality_gates / intent_quality_gates -> the executable command list (unit's own gates; intent scope shows the deduped union across all units) Bundling (build): `_build-prototype-content.mjs` now captures studio `intent-review-agents/`, the global `plugin/intent-review-agents/` tier (delivery-verifier), and the engine-review bodies (review/approve/intent × spec/continuity/cross-stage-consistency) into prototype-stage-content.json. `loadRoleDef` in studio-defs.ts resolves all of the above; the unit's IntentApprovalsCard collapses to a thin wrapper over the shared component. Verified live (Playwright, worker-new-badge intent): Delivery Verifier opens its global mandate, Spec opens the intent-scope engine prompt, User opens the gate description, Intent Quality Gates opens the command union. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

The unit "Duration" stat measured earliest-iteration-start to latest-end — a wall clock that counts every idle gap (waiting on the user, queueing, time between bolts). Each iteration carries `started_at`/`completed_at`, so sum each hat-dispatch's own duration instead: that's the actual working time. The stat tile now leads with "Active" (the summed dispatch time) and keeps the wall-clock span as secondary context ("21h elapsed · <dates>"). Falls back to the old Duration/Elapsed when no iteration carries both stamps (v3 units). Adds a `formatDurationMs(ms)` helper to @haiku/shared (minutes-aware, seconds for sub-minute totals) alongside the existing span-based formatDuration. Verified live (Playwright, worker-new-badge unit-001): ACTIVE 10m vs 21h wall-clock — the unit had ~10m of hat work but sat for ~21h. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

The unit stat tiles read dead v4 fields: `bolt` and `hat` were removed from the unit FM (the schema says so outright — both derive from the iterations log now), so the browse read `data.bolt`/`data.hat` and always showed "Bolt 0" / "Hat —". Criteria parsed body checkboxes most v4 units don't use, so it showed "0/0". - Derive `bolt` = iteration count (hat dispatches) and `hat` = the most recent iteration's hat in the unit parse, so both carry real values (also fixes the KanbanView card, which already guarded on them). - Unit detail tiles now render only when they carry signal: Criteria only when the unit declared checkbox criteria; Iterations only when the unit has run; Current Hat only while in-flight (a completed unit's hats are in the Hat history stepper). Relabel "Bolt" -> "Iterations" (it's a dispatch count, not the removed bolt field); Kanban "Bolt N" -> "N iters". Verified live (Playwright, completed worker-new-badge unit-001): tiles are now Iterations 3 · Status Completed · Active 10m — no more Bolt 0 / Hat — / 0/0. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

The card only rendered a date when `started_at` was present (a start→end span + duration), so list intents that carry `created_at` but no `started_at` showed no date at all — and the list sorts newest-first by created date, so it was the one temporal field worth surfacing. Add a "Created: <date>" to the meta row (`createdAt ?? startedAt`). Verified live (Playwright, GitLab monorepo): every card shows its created date (May 26 / May 21 / May 19 …), matching the newest-first order. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

GitLab's `TreeEntry.id` is content-addressed (the blob SHA), so two identical files at different paths — e.g. a DESIGN-BRIEF.md copied into both `knowledge/` and `stages/design/` — come back with the SAME id and different `path`. The compiled Relay artifact auto-injects `id` on the Node-typed tree entries, so RelayResponseNormalizer floods the console with "Invalid record … conflicting field … same id" and lets one path clobber the other. The recursive intent-tree fetch only reads `name`/`path` into plain objects and never touches the Relay store, so route it through `rawGraphql` (the same raw POST the lean list loaders use) requesting exactly `name`/`path` — no `id`, no normalization, no warning. Drop the now-unused artifact import; the generated `$data` type still types the response. Verified live (Playwright, location-timesheet-summary — the intent from the report): detail view loads with 0 console errors and no RelayResponseNormalizer warning. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Every browse page flashed the portfolio (list) skeleton while loading, even a deep link straight to a unit. Now the skeleton matches the page being loaded. - New BrowseSkeletons.tsx: list / intent / stage / unit skeletons mirroring each view's real layout, with a `BrowseSkeleton({ location })` picker (unit > stage > intent > list). - RemoteBrowseView's init (provider-auth) loading and PortfolioView's deep-link-detail loading both render the path-aware skeleton instead of the hardcoded list skeleton. (Wired up the previously-unused loadingDetail flag for the deep-link guard.) Pathing: the unit segment is now keyword-delimited like its siblings — `…/intent/<slug>/stage/<stage>/unit/<unit>/` instead of the positional `…/stage/<stage>/<unit>/`. buildBrowseUrl emits the `unit/` form; parseBrowsePath accepts it and still parses the legacy positional + pre-stage-keyword forms for back-compat. Verified live (Playwright, GitLab monorepo): the new `/unit/` URL resolves to the unit detail (breadcrumb Development › Unit 001); during load the page is the unit-width container, not the portfolio list. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

claude · 2026-05-28T17:02:55Z

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

PR #374 Review

Gather full PR diff
Review engine changes (pending_seal, FM-merge protection)
Review browse/list performance changes
Review browse/detail view changes
Review test coverage
Post inline comments for specific issues
Post overall assessment

Overall Assessment

Solid branch. Three well-scoped themes that each have clear motivation, adequate test coverage, and no correctness holes I could find. The pending-seal merge gate is the most architecturally significant change; the browse perf work is the most impactful for users. I found one true perf issue (double delivery probe per tick), one caching regression worth noting (GitLab detail tree), and three minor nits. Nothing blocking.

Engine — `pending_seal` + FM-merge protection

Correctness: good. The split between localOnly (display paths, statusline) and authoritative (cursor gate, actual seal decision) is the right call — the fast path stays fast and the gate that writes sealed_at is squash-merge-aware.

The ensureOnStageBranch fix (engineProtectedMergeInCwd for Stage 2) is exactly the right shape: same driver as every other engine merge, same engine-state authority rule. The new test (ensure-stage-branch-engine-fm.test.mjs) is a proper regression — it forces the divergent-intent.md condition directly, independent of migration version.

The settleEngineConflicts closure in mergeStageBranchIntoMain mirrors the same pattern cleanly. When only engine files conflicted, the merge commits automatically and returns []; genuine agent-code conflicts surface unchanged. The target-authoritative rule (HEAD = intent main, the cross-stage authority) is correct for this merge direction.

One perf flag → see inline on cursor.ts:2130: for a sealed-but-unmerged intent, both the early-exit check and the merge gate call intentDeliveryState(slug) in authoritative mode. In a squash-merge workflow that's two gh pr list calls per tick. Easy fix by passing the first result through.

Pickup logic (re-opening PR-interaction approvals on /haiku:haiku-pickup): correct. The PR_INTERACTION_ROLES key is used rather than a hardcoded role name, so a studio override is covered automatically. The edge case where the delivery-verifier hasn't yet been stamped (not in approvals) is handled by the normal-tick path — see inline on haiku_run_next.ts:1346 for a comment clarity note.

System journals filter: the INTENT_ROOT_INTERNAL_ENTRIES fix stops both the source leak (autoPopulateOutputs) and the display-layer leak (parser.ts). The basename check in parser.ts is deliberately broader — see inline note on whether that's a concern for output files named baseline.json.

Browse — List-view performance

Both providers: the O(3N) fan-out (per-branch readFile + MR fetch + per-stage probe) is replaced with one chunked aliased GraphQL batch. The change is clean and the fallback path in GitLab (readFileFromRef when the batch alias returns null) is the right defensive layer. GitLab now narrows the branch-list query to haiku/*/main (far fewer results), while GitHub keeps haiku/* to capture the inline associatedPullRequests — both approaches fit their provider's API shape.

Pagination fix: both providers now cursor-paginate past the 100-branch cap. The GitLab approach uses offset pagination (short-page termination); GitHub uses cursor pagination with hasNextPage. Both break out of infinite-loop territory: GitLab on names.length < PAGE, GitHub on !hasNextPage || !endCursor. Correct.

Minor nit → inline on github-provider.ts:158: CHUNK is local; CHUNK_SIZE is module-level in GitLab. Cosmetic but worth aligning.

Silent fallback drop logging → inline on gitlab-provider.ts:247: when both the batch and the readFileFromRef fallback fail, the intent silently uses the default-branch baseline — fine behavior, but a console.warn would make this traceable in production.

Browse — Detail view

Artifact resolution from the correct branch: passing intentBranch through to readFile / resolveAssetUrl is the right fix — before, an unmerged intent's files were read from the default branch (where they didn't exist yet). The prop threads cleanly through UnitDetailView, InputsSection, OutputsSection, and RefsSection.

SignOffGroup + studio-defs.ts: clean abstraction. The prototype-stage-content.json sidecar already existed for the architecture map; loading it lazily here (one fetch, cached module-level) is the right static-export-friendly pattern. The cascade (stage review agent → studio intent-review agent → global intent-review agent → engine body → user gate) mirrors the engine's own resolution order.

parseUnit fix for v4: reading bolt/hat from the append-only iterations array instead of the dropped FM fields is the correct v4 fix. lastIterationHat is a clean helper.

GitLab tree listing uncached → inline on gitlab-provider.ts:610: the intent tree listing in fetchIntentTreeFromRef is now rawGraphql (to avoid the Relay id collision), which removes the old cachedQuery cache. Blob contents remain cached. Worth noting for high-frequency detail view navigation.

Test coverage

ensure-stage-branch-engine-fm.test.mjs: genuine regression test, creates the divergent git state directly. ✓
seal-intent-commits.test.mjs: covers pending_seal hold, legacy-seal guard, and deliverIntent+seal path. The tick parser fix (splitting on \n\n---\n\n) is also correct — the prompt body was leaking into parsedResp.action before. ✓
parse-output-artifacts.test.mjs: pins the journals filter on both the source and display paths. ✓
Existing cursor-walk sealed-state sanity tests removed with a clear explanation. The replacement coverage in seal-intent-commits.test.mjs is more realistic (actual git state, full tick sequence). ✓

Review comments: - cursor.ts: a sealed-but-unmerged intent probed intentDeliveryState twice per tick (each falls back to `gh pr list` in a squash-merge repo). Hoist the result from the seal short-circuit and reuse it at the terminal merge gate. - gitlab-provider: cache the recursive intent-tree listing in glCache (TTL + already cleared by clearBranchCache's `intentTree` purge) — the rawGraphql fetch that fixed the Relay dup-id warning bypasses Relay's store, so a detail re-mount otherwise re-fetched the tree every time. - gitlab-provider: warn when the batch AND the readFileFromRef fallback both return null for a branch's intent.md (it silently falls back to the default-branch baseline — now debuggable). - github-provider: extract the inline `CHUNK = 10` to a module-level `CHUNK_SIZE` matching GitLab. - parser.ts: note the journal filter is a basename check (intentionally drops a reserved internal name anywhere, not just the intent root). - haiku_run_next.ts: tighten the pickup re-open comment — it only re-opens already-stamped delivery approvals; if the verifier hasn't run, the next tick dispatches it. Biome: fix import ordering in the three pending-seal files CI flagged. Engine tests pass (cursor-walk 31/31, seal-intent, squash-merge-fallback, engine-never-merges-mainline). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

claude · 2026-05-28T17:25:20Z

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

PR #374 Re-review (after new commits)

Get full PR diff
Review engine changes (pending_seal, FM-merge, system journals)
Review browse list performance (GitHub + GitLab providers)
Review browse detail view changes
Review test coverage
Review-fix commit verification
Post inline comments
Post overall assessment

Re-review Summary

The fix(review) commit addressed every item from the first pass. All six prior flags are resolved; no new blockers found. The branch is ready to merge.

Previous review comments — all addressed

Item	Status
Double delivery probe (two `gh pr list` calls per tick for sealed-but-unmerged)	Fixed — `sealedDelivery` hoisted from the seal short-circuit, reused at the terminal merge gate via `const delivery = sealedDelivery ?? intentDeliveryState(slug)`
GitLab tree listing uncached after Relay fix	Fixed — `rawGraphql` tree fetch now writes to `glCache` with the same TTL and is cleared by `clearBranchCache`'s `intentTree` purge
`CHUNK` vs `CHUNK_SIZE` naming inconsistency	Fixed — `CHUNK_SIZE = 10` at module scope in `github-provider.ts`, matches GitLab
Silent fallback drop with no log	Fixed — `console.warn` fires when both the batch alias AND the `readFileFromRef` fallback return null
`basename` filter reach undocumented	Fixed — code comment explains the intentional trade-off (reserved names filtered anywhere in the tree)
Pickup comment: misleading "always re-audits"	Fixed — comment now clarifies it only re-opens already-stamped slots; if the verifier hasn't run yet, the next regular tick dispatches it

New observations (informational, non-blocking)

settleEngineConflicts is correctly wired. The closure in mergeStageBranchIntoMain calls restoreEngineStateFromBase, which does git checkout <ref> -- <path> && git add -- <path> per engine-owned file — so the restored files are staged before git commit --no-edit fires. No missing git add. The mergeMessage captured from outer scope is the correct stage→main merge message. ✓

GitHub batch has no per-item retry. Unlike GitLab's batchMainBranchData (which falls back to readFileFromRef per item when the aliased batch yields null), the GitHub batch falls through directly to the default-branch catalog on a null. Intentional and acceptable — the default catalog is still shown; the behavior difference is noted in the GitLab comment but not in the GitHub one. Minor documentation gap only.

Chunk fetches are serial, not parallel. Both providers process chunks with a sequential for loop. For a repo with 150 */main branches that's 15 sequential batch requests. Correct behavior; a future Promise.all(chunks.map(...)) would be a nice follow-up optimization without any correctness implication here.

Legacy seal guard test is solid. The test in seal-intent-commits.test.mjs manually writes sealed_at to an unmerged intent and verifies the cursor still returns pending_seal — the short-circuit correctly ignores a pre-existing stamp until the work lands. Then deliverIntent fast-forwards the default branch and the intent seals normally. Good regression shape.

pending_seal prompt template is clear. Accurately describes the state (built, signed, reflected, but not merged), correctly instructs the agent to tell the user to merge rather than doing it, and explains the pickup re-audit path. ✓

Test coverage

The new test suite is good:

ensure-stage-branch-engine-fm.test.mjs — version-independent regression; forces the divergent intent.md condition directly, confirms no conflict markers survive.
seal-intent-commits.test.mjs — covers pending_seal hold, legacy seal guard, deliverIntent+seal path, and the tick response-parser fix (split on \n\n---\n\n).
parse-output-artifacts.test.mjs — pins the journals filter on both source and display paths.
Old degenerate "sealed_at unconditionally → sealed" tests removed with a clear explanation; the replacement coverage is more realistic. ✓

jwaldrip · 2026-05-28T17:26:51Z

Addressed all six review comments in 3696672d9:

cursor.ts double delivery probe — hoisted sealedDelivery from the seal short-circuit and reuse it at the terminal merge gate (sealedDelivery ?? intentDeliveryState(slug)), so a squash-merge repo probes gh pr list once per tick, not twice.
gitlab tree listing uncached — cache the raw tree listing in glCache under an intentTree: key (TTL + already purged by clearBranchCache), so a detail re-mount doesn't re-fetch.
github inline CHUNK — extracted to module-level CHUNK_SIZE = 10, matching GitLab.
gitlab silent fallback — added a console.warn when the batch AND readFileFromRef fallback both miss a branch's intent.md.
parser basename filter — added a note that the basename check intentionally drops a reserved internal name anywhere in the tree (not just the intent root), and the trade-off.
haiku_run_next pickup re-open — tightened the comment: only already-stamped delivery approvals are re-opened; if the verifier hasn't run, the next tick dispatches it.

Engine tests pass locally (cursor-walk 31/31, seal-intent, squash-merge-fallback, engine-never-merges-mainline); Biome Lint green.

jwaldrip and others added 24 commits May 28, 2026 08:10