feat(runtime-verifier): script-driven proof with video, gitignored + uploaded to per-stage PRs

[jwaldrip]

What & why

The runtime-verifier drove the app through the haiku-playwright MCP and wrote screenshots into .haiku/.../proof/ — which was not gitignored, so every verification run committed regenerated binary churn into history, and the MCP gave screenshots only (no video). This reshapes proof end to end and adds the per-stage delivery PR for discrete mode.

Changes

• Driver → self-installed Playwright script with video. The web/GUI verifier now writes a self-contained Playwright script (own scratch dir, never the project's deps) that records video + step screenshots. haiku_view boot still returns the URL; the haiku-playwright MCP is the documented fallback. Doctrine + all nine runtime-verifier.md mandates updated; browser_* tool prose softened to capability language (doctrine owns how to drive, mandates own what to verify).
• Proof is gitignored. ensureWorktreesGitignored → ensureHaikuGitignored seeds the worktree pool + both proof globs idempotently; root .gitignore matches. Engine git add paths honor .gitignore, so proof stays out of commits and out of dirty_tree_blocking_tick. The SPA still serves it live off disk.
• Proof is uploaded to the PR. runtime-verifier joins PR_INTERACTION_ROLES (now in BOTH role classes). Subagent templates grant additive scopes (proof-write AND PR-upload). Dispatch builders inject a mode-aware proofTargetPrUrl. GitLab uploads-API / GitHub release-asset doctrine in git.md.
• Per-stage delivery PR. In discrete / discrete-hybrid the engine opens a DRAFT stage PR at stage start (openStageDraftPullRequest, base = haiku/<slug>/main), records it in a stage_prs map on intent FM, and flips it draft→ready at the gate — not a second PR. That stage PR is where the stage's proof lands. continuous / autopilot / quick keep the single intent-main draft PR. stage_prs is not a tamper-checksummed INTENT_FIELD (like draft_pr_*).

Sync surface

CLAUDE.md concept rows, ARCHITECTURE.md §2, the website architecture map (actors.ts + payload-for.ts).

Verification

• Engine suite: 2080 passed / 0 failed (242 files) — new stage-pr-and-proof-gitignore.test.mjs (10, incl. mode-aware URL selection) + updated verification-doctrine.test.mjs to lock the script/video/upload + additive-scope contract.
• Website tsc + next build: clean. MCP bundle rebuilt.

Scoped out (follow-up)

• discrete-hybrid external-only stages — v1 opens a stage PR for every stage in both discrete modes; narrowing hybrid to external-review stages needs a stage-level declaration that doesn't exist yet (external_review is a runtime human decision today).
• Paper — left untouched; it describes the runtime/delivery lens conceptually, below the abstraction level of capture mechanics / PR-per-stage.

🤖 Generated with Claude Code

[claude]

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

---

PR Review Complete

• Gather context: read PR diff and key changed files
• Review git-worktree.ts changes (gitignore + stage PR logic)
• Review orchestrator/side-effects.ts (stage PR opening)
• Review state-tools.ts changes (stage_prs field)
• Review haiku_await_gate.ts (draft→ready flip)
• Review role-class changes (runtime-verifier joins PR_INTERACTION_ROLES)
• Review prompt builder changes (proofTargetPrUrl injection)
• Review mandate file updates (runtime-verifier.md across studios)
• Review test coverage
• Post final review summary

---

Overall Assessment

This is a well-structured PR with a clear problem statement and a coherent solution. The four changes (Playwright-script driver, proof gitignored, proof uploaded to PR, per-stage delivery PR) are all tightly coupled and ship together correctly. The 2080-test suite passing is a strong signal. Here's my detailed read:

What works well

ensureHaikuGitignored refactor. The conversion from a single-entry function to a pattern array is clean. The idempotency logic (build a Set of existing lines, filter to missing patterns, early-return if nothing is missing) is exactly right and the test coverage for the partial-upgrade case (worktrees present, proof absent) is solid.

openStageDraftPullRequest design. The shape mirrors openIntentDraftPullRequest exactly — same OpenIntentMrResult return type, same best-effort error handling, same compare-URL fallback. The branch/base pair haiku/<slug>/<stage> → haiku/<slug>/main is correct (stage into intent-main, not into repo mainline), and the idempotency guard in openStageDraftPrIfDelivery (readStagePr(slug, stage)?.url) correctly skips re-opening on revisit.

Role-class additive scopes. Adding runtime-verifier to PR_INTERACTION_ROLES while keeping it in RUNTIME_OBSERVATION_ROLES, and then composing the subagent template's write-scope section additively (doctrineRef || prInteraction → combined block), is a clean extension of the existing pattern. The old mutually-exclusive if/else if is gone in favor of two independent if blocks.

stage_prs not in INTENT_FIELDS. Keeping it out of the tamper-checksum list (same as draft_pr_*) is the right call and the test explicitly locks this invariant. The comment explaining why is helpful.

Mandate updates. All nine runtime-verifier.md files consistently replace direct browser_* MCP calls with "your Playwright script" phrasing, softening the MCP to a documented fallback. The doctrine doc itself now has a complete Playwright provisioning recipe (scratch dir, npm init -y, PLAYWRIGHT_BROWSERS_PATH reuse, video enable). This is a meaningful UX improvement.

Issues found

1. PER_STAGE_PR_MODES defined in two places (inline comment at side-effects.ts:109). If a new mode is added, both the PR-opening and the proof-upload-target selection need updating independently. This is the most actionable issue — a single extracted constant in a shared file removes the drift risk entirely. Fix this →

2. Double-banner on partial upgrade (inline comment at state-tools.ts:4695). Repos that already ran ensureWorktreesGitignored (old banner text) will get a second comment block appended. Cosmetic but messy for users upgrading. Easy to fix with a one-pass strip of the old comment before appending.

3. status: "failed" fallthrough in haiku_await_gate (inline comment at haiku_await_gate.ts:583). The code is correct — falling through to openStagePullRequest when there's no URL is the right recovery. But the comment on the branch says "Flip it to ready instead of opening a SECOND PR" which reads as if url being absent means we're in a non-per-stage-PR mode, not that the draft creation failed. Misleading for future readers.

4. GitHub release-asset privacy risk (inline comment at providers/git.md:40). The doctrine correctly flags that release asset URLs are public even for private repos, but it's easy to miss. Worth a visual callout for the agent reading at runtime.

5. Minor: if (stage && ...) unnecessary guard in dispatch_approval/index.ts:52. stage is always truthy at this call site. Fix this →

6. Test helper serializes nested objects as JSON-in-YAML (inline comment at stage-pr-and-proof-gitignore.test.mjs:101). seedIntent writes stage_prs: {"development":{"url":"..."}} which is valid YAML but differs from what setStagePrField actually writes. Not a correctness bug today, but a future test probing raw FM text would be surprised.

What's scoped out correctly

The PR notes that discrete-hybrid with external-only stages is deferred — the engine opens a stage PR for every stage in both discrete modes. That's a reasonable v1 that doesn't break anything; the narrowing can happen once external_review is a stage-level declaration rather than a runtime human decision.

The paper is intentionally left untouched — proof mechanics are below the abstraction level of the paper's runtime/delivery section. Agreed.

The architecture map entries (actors.ts + payload-for.ts) received one-line updates describing the per-stage PR addition. These are minimal but sufficient for now.

Verdict

The core feature is correct and well-tested. Issues 1 and 2 are the only ones I'd call pre-merge candidates — everything else is follow-up or editorial. The test suite coverage (role classification, FM round-trip, gitignore idempotency, mode-aware URL selection) is solid and directly locks the contracts introduced here.

[claude]

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

---

Re-review: feat(discrete-hybrid) follow-up commit + full PR

• Read full PR diff
• Review new commit (discrete-hybrid external-only stage PR narrowing)
• Check if previous review issues were addressed
• Post inline comments on new issues
• Update summary

---

What the new commit adds

210737c feat(discrete-hybrid): per-stage PR only for external-review stages addresses the "scoped out" item from the first review. In discrete-hybrid mode, the engine now opens a per-stage draft PR only for stages whose review: gate is or includes external — continuous stages keep their work on the intent-main PR. discrete still opens one per stage.

New function stageRequiresExternalReview (orchestrator/studio.ts). Reads the STAGE.md review: frontmatter field. Handles string "external", compound arrays ["external", "ask"], and missing values (returns false as the safe default for unknown stages). Tests against real software STAGE.md files confirm the live behavior: development/design → external; inception/operations → not. Clean and correct.

Mode gate in openStageDraftPrIfDelivery. The discrete-hybrid short-circuit fires before the idempotency check, so non-external stages never touch stage_prs. The haiku_await_gate fallback (openStagePullRequest) handles the edge case where a stage winds up at external review without a pre-staged PR (e.g., a project overlay adds external to a gate mid-flight). Documentation sync (CLAUDE.md, ARCHITECTURE.md, actors.ts, git.md) is complete and accurate.

Proof upload target fallback. resolveProofTargetPrUrl in dispatch_approval/index.ts correctly needs no change: a non-external hybrid stage has no stage_prs[stage] entry, so it already falls back to draft_pr_url. The design is correct.

Suite: 2081 passed / 0 failed.

---

Issues from first review — status

#	Issue	Status
1	PER_STAGE_PR_MODES defined in two places (side-effects.ts:110 + dispatch_approval/index.ts:47)	Still open — inline comment re-filed
2	Double-banner on partial upgrade (ensureHaikuGitignored appends new banner after old one)	Still open
3	status: "failed" comment in haiku_await_gate.ts misleading ("not a second PR" only true when status=draft)	Still open
4	GitHub release-asset privacy risk callout (flagged in git.md but easy to skim)	Addressed in docs — editorial only
5	if (stage && PER_STAGE_PR_MODES.has(mode)) redundant stage && guard	Still open — inline comment re-filed
6	seedIntent test helper serializes stage_prs as JSON-in-YAML rather than YAML object syntax	Still open

Issues 1 and 5 are the only pre-merge candidates from this round; 2, 3, and 6 remain low-risk follow-up candidates.

---

Overall verdict

The new commit ships the correct narrowing for discrete-hybrid with minimal surface area — one new function, one added guard, tests against real STAGE.md files, and complete documentation sync. No regressions introduced. The fallback behavior in haiku_await_gate and resolveProofTargetPrUrl handles all edge cases correctly without changes.

The one structural concern that carries across both commits is the duplicated PER_STAGE_PR_MODES constant. It controls two independent behaviors (whether to open a stage PR, and which PR to target for proof upload) and now also implicitly shapes the stageRequiresExternalReview call site — a new mode added to one side won't propagate to the other. Worth extracting before additional delivery modes appear.

[claude]

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

---

Re-review: 8610db2 (address claude-review findings + fix Biome)

• Read latest commit changes
• Verify previous issues were addressed
• Review full PR diff for remaining issues
• Post final summary

---

Status of all prior review findings

#	Issue	Status
1	PER_STAGE_PR_MODES defined in two places	✅ Fixed — extracted to delivery-modes.ts, imported from both sites
2	Double-banner on partial upgrade	✅ Fixed — hasManagedLine check: emits a short one-line note instead of a second full banner
3	haiku_await_gate comment misleading	✅ Fixed — comment now says "covers both no-per-stage-PR modes AND the case where the stage-start open failed"
4	GitHub release-asset privacy callout	Kept — already documented in git.md, author's rationale accepted
5	stage && guard in resolveProofTargetPrUrl	Kept — defensive on a param (string type still admits empty strings); harmless
6	seedIntent JSON-in-YAML test seed	Kept — valid YAML, test-only, explicitly documented in commit message

---

What the fix commit does well

delivery-modes.ts is a clean, neutral leaf module — 19 lines, no heavy deps, well-commented. The documentation correctly distinguishes discrete (every stage gets a PR) from discrete-hybrid (only external-review stages), and the note "membership here means 'this mode CAN open per-stage PRs'" is exactly the right framing given the narrowing that stageRequiresExternalReview applies on top.

hasManagedLine heuristic uses lines.some((l) => l.startsWith(".haiku/")) to detect partial upgrades. Edge cases are all handled correctly: old worktree-only entry (.haiku/worktrees/ starts with .haiku/) → short note; fresh repo (no .haiku/ lines) → full banner; proof globs already present (nothing in missing) → early return before the check even fires. Solid.

openStageDraftPrIfDelivery adds a detectPrTool() === null guard before calling into the PR-open path. This is an improvement over the openStageDraftPullRequest function itself's best-effort handling — it short-circuits cleanly when no provider CLI is present, avoiding a push attempt that would fail anyway.

Biome fix is included — the import sort was red on npx biome check packages/haiku/src before this commit. Clean CI path now.

Test coverage is thorough: 11 tests across role classification, stageRequiresExternalReview against real STAGE.md files, setStagePrField/readStagePr FM round-trips, ensureHaikuGitignored idempotency (including the partial-upgrade case), and mode-aware proof-upload target selection. These lock exactly the contracts introduced in this PR.

---

Overall verdict

All three pre-merge blockers from prior reviews are resolved. No new issues found in the fix commit. The PR is clean and ready to merge.

[claude]

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

---

PR #376 Final Review

• Gather context from previous review rounds
• Review full PR diff (gh pr diff 376)
• Assess all changes holistically
• Post final review summary

---

This is a comprehensive review of the full PR diff across all three commits. The PR has already gone through two prior rounds of review; this pass covers the complete merged state.

What the PR does

Four tightly-coupled changes shipped together:

1. Playwright-script driver with video — the web/GUI verifier now writes a self-contained Playwright script (scratch dir, never the project's deps) that records video + step screenshots. The haiku-playwright MCP is the documented fallback.
2. Proof is gitignored — ensureWorktreesGitignored renamed to ensureHaikuGitignored, now seeds worktree pool + both proof globs idempotently.
3. Proof uploaded to the PR — runtime-verifier added to PR_INTERACTION_ROLES (stays in RUNTIME_OBSERVATION_ROLES too). Dispatch builders inject mode-aware proofTargetPrUrl. Additive scopes: proof-write AND PR-upload.
4. Per-stage delivery PR — discrete opens a draft PR for every stage; discrete-hybrid narrows to stages with external in their review: gate (stageRequiresExternalReview). Gate flips draft→ready instead of opening a second PR.

Architecture assessment

delivery-modes.ts is the right call architecturally. The 19-line neutral leaf exports a single PER_STAGE_PR_MODES set with clear comments distinguishing discrete (every stage) from discrete-hybrid (external-review stages only via stageRequiresExternalReview). The comment "membership here means 'this mode CAN open per-stage PRs'" is exactly the right framing given the narrowing applied on top.

openStageDraftPrIfDelivery ordering is correct: the discrete-hybrid short-circuit fires before the idempotency check (readStagePr(slug, stage)?.url), so non-external stages in hybrid mode never touch stage_prs. The detectPrTool() === null guard short-circuits before attempting a push to nowhere. The idempotency on revisit (stage already has a URL) is clean.

resolveProofTargetPrUrl fallback chain is exactly right: stage PR (if available) → intent-main draft PR → empty string (no upload, proof still on disk). No mode where the proof is lost.

review-role-classes.ts comment block explains the additive scopes clearly: "A role may be in BOTH sets — runtime-verifier is." The old mutually-exclusive if/else if is gone; dispatch builders use two independent if blocks that compose correctly.

stageRequiresExternalReview handles all three gate shapes cleanly — string "external", array ["external", "ask"], missing/unknown (returns false as safe default). Verified against real software STAGE.md files in the test suite.

hasManagedLine heuristic in ensureHaikuGitignored handles the partial-upgrade case correctly: any .haiku/-prefixed line → use a short inline note instead of the full banner. Edge cases confirmed: old worktree-only entry (.haiku/worktrees/ starts with .haiku/) → short note; fresh repo → full banner; proof globs already present → early return. The accept: list in HAIKU_GITIGNORE_PATTERNS handles the trailing-slash-or-not variant.

haiku_await_gate fallthrough is now correctly documented: "covers both no-per-stage-PR modes AND the case where the stage-start open failed (no URL recorded)" — so status: "failed" entries are correctly recovered by opening on demand at gate time.

Test coverage

The new stage-pr-and-proof-gitignore.test.mjs (11 tests) locks exactly the right contracts:

• runtime-verifier in both role-class sets
• stageRequiresExternalReview against real software STAGE.md files (development/design → external, inception/operations → not)
• setStagePrField/readStagePr FM round-trip + multi-stage non-clobber + null for absent
• stage_prs not in INTENT_FIELDS (tamper-checksum invariant)
• ensureHaikuGitignored idempotency including partial-upgrade path (worktrees present, proof absent)
• resolveProofTargetPrUrl mode-aware selection (discrete → stage PR; continuous → intent-main)

verification-doctrine.test.mjs additions lock the script/video/upload + additive-scope contract, including a negative assertion that the strict no-write block does NOT render for a both-sets role.

Prior review issues — final status

#	Issue	Resolution
1	PER_STAGE_PR_MODES defined in two places	✅ Extracted to delivery-modes.ts, imported from both sites
2	Double-banner on partial upgrade	✅ hasManagedLine check: short inline note on upgrade, full banner only on first write
3	haiku_await_gate comment misleading	✅ Comment now covers both no-per-stage-PR modes AND failed draft-open case
4	GitHub release-asset privacy callout	✅ Documented in git.md; kept without a visual callout (author's call, low risk)
5	stage && guard redundant	✅ Kept — defensive on string type that admits "", harmless
6	seedIntent JSON-in-YAML in test helper	✅ Kept — valid YAML, test-only, not a correctness issue

Verdict

All three pre-merge blockers from prior reviews are resolved. The implementation is correct, well-tested (2081 tests, 0 failed), and the architecture is clean. The discrete-hybrid narrowing (commit 2) and the address-review-findings commit (commit 3) both land without introducing new issues.