feat(open-claude-design)!: discovery, init, reference discovery, live QA

[flora131]

Summary

Restructures the builtin open-claude-design workflow around the accessible impeccable skill (/skill:impeccable …). The workflow now opens with a discovery interview, always bootstraps project design context via init, runs design-system onboarding + gallery reference discovery + reference import in a single concurrent context phase, and uses impeccable live for in-browser element-level QA. Also fixes annotation feedback being silently dropped between preview and refinement stages, and bundles a pending impeccable skill v3.8.0 refresh.

Breaking Changes

Three top-level inputs are removed; the discovery stage now gathers them interactively.

Removed input	Replacement
reference	Provide in prompt or answer the discovery interview
output_type	Confirmed during discovery (prototype/wireframe/page/component/theme/tokens)
design_system	Auto-detected and bootstrapped by the always-run init stage

Remaining inputs: prompt, discover_references (default true), max_refinements.

Migration: drop reference=…, output_type=…, design_system=… from existing invocations, or fold them into prompt. The discovery interview will ask for anything it can't infer.

- /workflow open-claude-design prompt="Design a billing page" reference=https://stripe.com/billing output_type=page
+ /workflow open-claude-design prompt="Design a billing page like Stripe's"

- /workflow open-claude-design prompt="Generate spacing and color tokens" output_type=tokens design_system=./DESIGN.md
+ /workflow open-claude-design prompt="Generate spacing and color tokens"

Key Changes

open-claude-design workflow — new phase order

1. Discovery (/skill:impeccable shape): new first stage interviews the user for a confirmed brief, output type, and references via the structured question tool, returning { brief, output_type, references }. User-supplied references take precedence over DESIGN.md/PRODUCT.md (DESIGN.md fills gaps; PRODUCT.md governs register/voice). A REFERENCE_PRECEDENCE note threads into the import, generator, and apply-changes prompts. Headless runs infer a defensible brief without blocking.

2. Init (always runs) (/skill:impeccable init): detects PRODUCT.md/DESIGN.md (root, .agents/context/, or docs/, case-insensitive), creates whichever is missing, and reconciles existing files against the discovery brief without clobbering them. Reuses discovery answers so nothing is asked twice. Best-effort; never blocks the run.

3. Combined context phase (single concurrent fan-out): design-system onboarding (ds-locator → ds-analyzer → ds-patterns), gated gallery reference-discovery (Awwwards, recent.design, Dribbble, Monet, Motionsites — playwright-cli clicks into standout work, records a scroll-through video for animation fidelity, falls back to full-page screenshot + real URL, persists to <artifact_dir>/references.md; web-search fallback if browser is unavailable), and per-reference import (web-capture-<n> / file-parser-<n>) all run in one ctx.parallel fan-out before design-system-builder synthesizes the design system. Set discover_references=false to skip the gallery pass.

4. Generation → 5. Live QA → 6. Export gate

Live QA (/skill:impeccable live)

preview-display-* stages drive /skill:impeccable live against the static preview.html — pick elements in the browser, annotate them, compare three on-brand variants, accept one that is written back in place. Degrades to playwright-cli show --annotate, then a manual file path.

New modules

• open-claude-design-setup.ts: runDiscovery, detectDesignContextFiles, ensureProjectDesignContext, buildReferenceDiscoveryPrompt, persistReferencesBrief, buildLivePreviewDisplayPrompt
• open-claude-design-feedback.ts: feedback parser, merged-brief builder, guardrail
• discoveryDecisionSchema / discoveryDecisionFromResult / REFERENCE_PRECEDENCE added to utils

New unit tests

Added coverage for: discovery schema/parse, always-run init + reconcile + discovery-context threading, combined context fan-out, reference-discovery prompt/persist, live-QA prompt, trimmed input contract, full-run init path, live_changes capture/threading/persistence, feedback parser, merged-brief ordering, guardrail, and artifact persistence.

Bug Fixes

Annotation feedback dropped between preview and refinement (#1464)

preview-display-* results were awaited-and-discarded, so a user could annotate a preview and watch the next iteration apply only internal critique while the design stayed visually unchanged.

Fix: Captured annotations are now parsed (tolerant of heading/bullet/bold/backtick label styles and placeholder values), carried through the loop as previewFeedbackHistory, written to <artifact_dir>/feedback/iteration-<n>.md/.json, and ordered above reviewer decisions and impeccable critique in the merged refinement brief. user-feedback-* receives them in a dedicated <user_annotations> block and will not approve export while honest feedback is unaddressed. A guardrail asserts annotation threading before each apply stage and fails loudly if it's missing.

Artifact dir pollution in test runs

prepareArtifactDir now writes to the OS tmpdir when NODE_ENV=test, preventing per-run folders from appearing in specs/design/.

Export gate + terminal annotation hardening

• Export gate refuses ready_for_export while unaddressed annotations exist (forces an apply pass via assertUserAnnotationsThreaded)
• Final refinement iteration and post-export final-display are read-only (no longer solicit user_notes/live_changes they cannot apply)
• assertUserAnnotationsThreaded now also enforces accepted live_changes threading
• Deterministic browser-centric early exit: when playwright-cli is unavailable the run calls ctx.exit() with install instructions instead of generating a design no one can review
• copyAnnotationArtifacts refuses to copy a model-supplied snapshot path that resolves outside the project/artifact dir

Bundled impeccable skill v3.8.0 refresh

• Reorganizes live-mode helpers under scripts/live/ and shared utilities under scripts/lib/
• Adds detector/design-system.mjs (design-system detection) and inline-ignore support (scripts/detector/shared/inline-ignores.mjs)
• Removes deprecated standalone scripts (cleanup-deprecated.mjs, impeccable-paths.mjs)
• Significant expansion of live-browser.js, live-server.mjs, scripts/context.mjs, and detector rules
• New scripts/live/ modules: manual-apply.mjs, manual-edit-routes.mjs, svelte-component.mjs, sveltekit-adapter.mjs, ui-core.mjs, vocabulary.mjs

Notes

• All checks pass: typecheck, lint, check:file-length, test:unit (2546 pass / 0 fail), pre-commit/pre-push hooks.
• Design spec: specs/2026-06-21-open-claude-design-init-references-live-qa.md.
• The impeccable skill changes were pre-existing in the working tree and are intentionally bundled in this PR.

[mintlify]

Preview deployment for your docs. Learn more about Mintlify Previews.

Project	Status	Preview	Updated (UTC)
bastani	🟢 Ready	View Preview	Jun 22, 2026, 3:36 AM

💡 Tip: Enable Workflows to automatically generate PRs for you.

[claude]

Review: feat(open-claude-design)!: discovery-first restructure + live QA

Reviewed the authored workflow logic (open-claude-design-*.ts), the new tests, and the input/output contract. I scoped the bundled impeccable-skill refresh out of the detailed pass (see PR-hygiene note). Overall this is a clean, well-structured change — nicely modularized to stay under the 500-line gate, strong graceful-degradation discipline, and good test coverage for the issue-1464 feedback-threading guardrail. A few things worth addressing before merge.

🐞 Potential bugs

1. file:// URL construction is malformed on Windows — open-claude-design-runner.ts:65-66 builds previewFileUrl/specFileUrl as file:// + path. On Windows previewPath is e.g. C:\Users...\preview.html, so this yields file://C:\Users... (invalid — should be file:///C:/Users/...). The code explicitly supports Windows elsewhere (the where / shell: isWindows branches in ensurePlaywrightCli), and these URLs are passed to playwright-cli open ... and printed for the user to open, so this breaks the browser-open path on Windows. Recommend pathToFileURL(previewPath).href from node:url.

2. isFileLike treats any non-URL string as a path — open-claude-design-utils.ts:58-61. A freeform reference from the discovery stage (e.g. a brand name rather than a URL/path) returns true, so the runner spawns a file-parser-n stage that tries to read a nonexistent file. It degrades gracefully, but it is a wasted LLM stage per junk reference. Consider normalizing references to URLs/real paths in the discovery stage, or a light existence/heuristic check before creating file-parser-* steps.

🧹 Code quality / maintainability

3. Loose re-declared context types. OpenClaudeDesignContext (runner), DesignContext (phases), and SetupDesignContext (setup) each hand-roll a narrow shape with task(name, options: object). That drops type-checking on the task options object (model config, schema, tools, previous) — the surface most likely to drift from the real SDK contract, and at odds with the repo strict / noUnusedLocals posture. Prefer importing the SDK WorkflowRunContext / WorkflowTaskOptions so a contract change surfaces at compile time.

4. Stale jsdoc in prepareArtifactDir — open-claude-design-utils.ts:177-181 says it prefers /.atomic/workflows/open-claude-design/, but the code writes join(cwd, "specs", "design", runId). The downstream comment confirms specs/design is intended — so just the jsdoc is wrong. Worth fixing since artifact location is user-facing.

5. Typo: "You are a staff design enginer." — open-claude-design-runner.ts:320 (design-system-builder role).

⚙️ Security & runtime notes (non-blocking)

6. ensurePlaywrightCli runs npm install -g @playwright/cli@latest at runtime (utils.ts:308). Best-effort, skipped under NODE_ENV=test, and mirrors the documented playwright-cli bootstrap, so reasonable. Two notes: (a) the which/where + install args are all literals → no injection risk, good; (b) the test-skip relies on Bun implicitly setting NODE_ENV=test. True for bun test, but it is an implicit dependency guarding a 180s networked global install plus writes into the repo specs/design tree — brittle to hinge on. Consider an explicit workflow-controlled guard env.

✅ Things that are good

• result.name filtering in the combined context fan-out is safe — the task primitive returns { name, stageName: name, ... }, so the ds-* / reference-discovery / web-capture-* partitioning works in production, not just mocks.
• The issue-1464 guardrail (assertUserAnnotationsThreaded) plus ordering (user annotations above internal critique in buildRefinementBrief) is well-tested and the verbatim-inclusion invariant holds across iterations.
• Robust headless fallbacks in discoveryDecisionFromResult and every .catch(() => undefined) display stage.
• Changelog (Breaking Changes + Changed) and docs/workflows.md properly updated, including the dropped reference/output_type/design_system inputs.

📦 PR hygiene
The bundled impeccable-skill refresh (commit b0f490f, the bulk of the +13k/-3.4k) is an independent vendored sync mixed into a behavioral-workflow PR. You already flagged willingness to split — I would encourage it: it makes this PR reviewable on its own merits and keeps the skill-sync history separate.

Nice work overall — items 1 and 2 are the only ones I would consider blocking.

Automated review via Claude Code.

[claude]

Code Review — PR #1470: open-claude-design discovery-first restructure

Reviewed the non-vendored workflow code (the open-claude-design-*.ts modules, tests, docs, changelog). The impeccable skill assets are vendored, so I focused there only lightly — the CodeQL hardening in commits 3–4 looks reasonable.

Overall this is a high-quality, well-tested change. The decomposition into setup / feedback / phases / runner / utils keeps every file under the 500-line gate, the parallel fan-out is keyed by stage name (robust to ordering), and the #1464 feedback-threading guardrail (assertUserAnnotationsThreaded) is a nice "fail loud" safety net. Changelog and docs are comprehensive. Test coverage is strong: discovery schema/parse + headless fallback, always-run init, combined fan-out, live-QA prompt, annotation threading/ordering, and artifact persistence are all exercised.

A few mostly-minor things worth considering:

1. max_refinements: 0 can't disable refinement (behavior gotcha)

positiveInteger(value, fallback) (open-claude-design-utils.ts:41) returns the fallback for any value <= 0, so a user explicitly passing max_refinements: 0 to skip the refinement loop silently gets 3 instead. If "zero refinements" is a legitimate request, consider distinguishing "unset" from "0". If a minimum of 1 is intended, no change needed — just flagging the surprise.

2. Init summary can contradict itself when init is best-effort

ensureProjectDesignContext (open-claude-design-setup.ts:218) runs init via .catch(() => undefined) (best-effort), but the returned summary asserts created missing PRODUCT.md and DESIGN.md based purely on the before detection. If init actually failed, the same summary then appends Detected after init: PRODUCT.md=pending, … — a self-contradictory blob that gets threaded into the design-system-builder prompt as project_design_context. Consider deriving the status line from the after detection (only claim "created" for files that now exist) so the downstream model isn't told a file exists when it doesn't.

3. design-system-builder is now gated behind reference discovery

The combined Phase-3 parallel barrier (open-claude-design-runner.ts:289) bundles ds-* onboarding, reference-discovery, and per-reference imports together, then runs design-system-builder afterward. But the builder only consumes onboardingAnalysis (previous: onboardingAnalysis), not the references. Since reference-discovery involves slow browser work (scroll-through video capture), the builder now waits on it despite not using its output. The generator needs both, so references gate generation regardless — but the builder itself could overlap reference discovery. Reasonable simplicity-vs-latency tradeoff; just noting the builder no longer starts as early as it could.

4. The rich design system is dropped from outputs

design_system is hardcoded to the literal "project-derived design system" (runner.ts:465) while the actual synthesized DESIGN.md spec (designSystem) is only used internally. Defensible since it's written to disk, but a workflow-composition consumer reading outputs.design_system gets a label, not the system. If by design, the output description is accurate; otherwise consider surfacing the content (or a path to it).

5. Minor: stray indentation in the runner

runOpenClaudeDesignWorkflow (runner.ts:48–479) is indented an extra level — looks like leftover from a removed wrapper block. Cosmetic (tsc/lint don't catch it), but worth tidying.

Nits

• extractField truncation: if a user's user_notes body contains a line that canonicalizes to a known field label (e.g. a note starting Preview path: make it wider), multi-line collection stops early. Edge case, low risk given the label set.
• Tests use // @ts-nocheck — consistent with ~97/325 existing unit files, in line with repo convention.

Could not run bun test / bun run typecheck locally (sandbox blocked execution), so I'm trusting the PR's stated green checks for those. The logic review above is from reading the diff.

Nice work overall — the feedback-threading guardrail and the discovery-first flow are solid improvements. 🤖

[claude]

Code Review — feat(open-claude-design)!: discovery, init, reference discovery, live QA

Reviewed the workflow TypeScript (open-claude-design-{runner,phases,setup,feedback,utils}.ts, entrypoint, .d.ts) and the three new test files. The bulk of the +13K diff is the vendored impeccable .mjs skill (pre-existing, intentionally bundled), so I focused on the ~2,150 lines of first-party logic and tests. Overall this is a well-structured, carefully-defensive change: best-effort persistence that never throws, headless/mock fallbacks on every LLM stage, tmpdir isolation under NODE_ENV=test, all files under the 500-line gate, and genuinely thorough unit tests for the parsing/threading/persistence surface. The #1464 fix is the right shape. A few findings:

🟠 Medium — the early-break export gate bypasses the annotation guardrail (#1464 can recur)

In refineOpenClaudeDesign (open-claude-design-phases.ts:104-107), when user-feedback-N returns ready_for_export=true the loop breaks before apply-changes and therefore before assertUserAnnotationsThreaded. So if the reviewer model approves export while previewFeedbackHistory still holds meaningful, unaddressed user notes, those annotations are silently dropped — the exact #1464 failure mode, just relocated from the apply stage to the gate.

The apply path is protected deterministically (assertUserAnnotationsThreaded), but the gate relies only on a prompt instruction (phases.ts:84 "never approve export while honest user feedback remains unaddressed"). That asymmetry is worth closing — e.g. a deterministic check that refuses an immediate ready_for_export=true when the latest iteration captured meaningful notes that haven't yet been through an apply pass. There's also no test covering this branch; the threading test forces refinementDecision(false) so apply always runs. A ready_for_export=true-with-pending-annotations test would lock in the intended behavior.

🟡 Low — guardrail covers userNotes but not liveChanges

buildUserAnnotationsSection (feedback.ts:196-220) and persistPreviewFeedback both treat liveChanges as meaningful feedback, and the apply prompt is told to honor accepted live variants, but assertUserAnnotationsThreaded (feedback.ts:282-296) only asserts userNotes threading. Live-changes-only feedback can be dropped without tripping the guard. Lower severity since accepted variants are already written into the preview HTML in place, but it's inconsistent with the stated contract.

🟡 Low — extractField value termination is permissive

extractField (feedback.ts:120-143) only ends a multi-line value at a known FIELD_LABELS entry or a horizontal rule. If a stage emits an unrecognized labeled line between user_notes and the next known field (e.g. priority: high), it gets swallowed into the captured notes. The output format is model-controlled and the labels are fixed, so risk is low, but the parser leans toward over-capture.

🔵 Nits

• isPlaceholderValue (feedback.ts:73-80) lists "a" and "n" as placeholder tokens, so a one-character real note (user_notes: a) would be discarded. Realistically never a problem, just noting the edge.
• Prompt typos: "staff design enginer" (runner.ts:320, and the role line at runner.ts:317). Cosmetic, but these strings go straight into model prompts.
• ensurePlaywrightCli (utils.ts:307-308) runs npm install -g @playwright/cli@latest at runtime — a global, env-mutating side effect on an unpinned @latest. It's correctly gated (skipped under tests, best-effort, never throws) and mirrors the documented skill bootstrap, so acceptable; flagging only so the global-install-on-run behavior is a conscious choice.
• copyAnnotationArtifacts (feedback.ts:298-331) copies whatever path the model emits (resolved against workflowCwd) into the feedback dir. Guarded by isFile and best-effort, fine in the trusted local context — noted for completeness.
• Test files use // @ts-nocheck (open-claude-design-feedback.test.ts:1, setup.test.ts:1), which opts the whole file out of the strict typing CLAUDE.md emphasizes. Common for mock-heavy tests, but the recorder/mock shapes could be typed instead.

✅ Confirmed good

• File-length gate respected (largest is runner.ts at 480 lines).
• Docs (docs/workflows.md) and both CHANGELOG.mds updated per CLAUDE.md.
• Combined ctx.parallel fan-out for ds-* + reference-discovery + imports is a sound performance improvement over sequential phases.
• Structured schemas (discoveryDecisionSchema, etc.) with tolerant *FromResult fallbacks for headless runs.
• Persistence helpers are uniformly best-effort and never block the workflow.

Nice work overall — the only thing I'd treat as a blocker is the Medium finding, since it re-opens the bug class this PR set out to fix.

Note: I reviewed the diff statically; I was unable to execute bun test in this environment, but the test code itself looks correct and the PR reports 2546/0 passing.

[flora131]

Thanks for the thorough review — addressed in cbde5080b.

🟠 Medium (the blocker) — early-break export gate. Fixed deterministically. refineOpenClaudeDesign now tracks unappliedMeaningfulFeedback (seeded from the initial preview, set when a preview-display-* round captures meaningful notes/live-changes, cleared after each apply). The loop only honors an immediate ready_for_export=true when that flag is false; otherwise it falls through to a forced apply pass — which runs assertUserAnnotationsThreaded — so captured annotations can never be dropped at the gate. Added builtin-workflows-open_claude_design-02.test.ts with two cases: (1) ready_for_export=true while the initial preview holds unaddressed annotations now forces apply-changes-1 (and threads "I don't like this background" / "Apple website") with no spurious second apply, and (2) the no-annotations fast-path still exports immediately without an apply.

🟡 Low — guardrail vs liveChanges. assertUserAnnotationsThreaded now enforces liveChanges threading in addition to userNotes, matching buildUserAnnotationsSection/persistPreviewFeedback. New unit test covers a live-changes-only feedback round (throws when dropped, passes when threaded).

🔵 Nits

• Dropped "a"/"n" from PLACEHOLDER_TOKENS so a one-character real note survives ("na" stays for N/A); added an assertion.
• Fixed the staff design enginer prompt typo.

Deliberately left as-is (flagging the rationale):

• extractField over-capture (Low). The preview-display-* output labels are a fixed, model-controlled set and all live in FIELD_LABELS, so a stray unknown label between user_notes and the next field shouldn't occur in practice. Terminating on any word:-style line would truncate legitimate multi-line notes (which are frequently bullet lists containing colons), so I left the over-capture-leaning behavior rather than risk under-capture. Happy to tighten it if you'd prefer a stricter contract.
• ensurePlaywrightCli global install / copyAnnotationArtifacts / // @ts-nocheck tests — kept as conscious choices per your notes (gated best-effort bootstrap, trusted-local artifact copy, mock-heavy test files).

Full suite green locally: typecheck, lint, check:file-length, and test:unit (2549 pass / 0 fail).

[claude]

Code Review — feat(open-claude-design)!: discovery, init, reference discovery, live QA

Reviewed the core TypeScript (open-claude-design{,-runner,-phases,-setup,-feedback,-utils}.ts) and the new tests. The bundled impeccable .mjs skill files were treated as vendored and reviewed only at a glance. Overall this is a high-quality, well-documented change: the module split keeps every file under the 500-line gate, the #1464 feedback-threading machinery is clean and genuinely tested end-to-end, and the graceful-degradation/headless handling is careful throughout. A few things worth addressing before merge.

🟠 1. Annotations captured on the final preview-display iteration are still silently dropped

This is the same class of bug #1464 fixes, but at the tail of the loop. In refineOpenClaudeDesign, each iteration ends by running preview-display-${iteration}, capturing its feedback, persisting it, and pushing it onto previewFeedbackHistory (open-claude-design-phases.ts:271-295). On the last iteration (iteration === maxRefinements) the loop then exits, so that final capture is never threaded into any apply-changes pass.

It also never reaches the export phase: exportOpenClaudeDesign doesn't receive previewFeedbackHistory or the unappliedMeaningfulFeedback flag (ExportOptions, open-claude-design-phases.ts:302-315; called at open-claude-design-runner.ts:446-459), and export runs unconditionally regardless of approvedForExport. So a user who annotates the final refined preview gets their notes written to feedback/iteration-N.json but the design is never updated — exactly the "annotate and watch nothing change" experience #1464 set out to eliminate. The in-loop comment at phases.ts:62-66 even anticipates this ("the export gate must not honor an immediate ready_for_export while such feedback is still pending"), but that guard lives only inside the refinement loop.

Suggestions (either):

• Thread previewFeedbackHistory/pending-feedback into the export gate so a terminal annotation forces a forced-fix (or another bounded apply) before handoff, or
• Don't solicit user_notes/live_changes on the terminal preview-display if they can't be acted on, to avoid setting a false expectation.

🟠 2. final-display asks for user_notes but discards them

exportOpenClaudeDesign's final-display stage prompts for user_notes and annotated_snapshot (phases.ts:473) but the result is .catch(() => undefined) and dropped (phases.ts:445-478). This is the awaited-and-discarded pattern the PR is fixing elsewhere. Since it's post-export there's no loop to feed, but it still invites the user to annotate a spec where the notes go nowhere. Consider either capturing/persisting them as a final artifact, or dropping the user_notes ask from this stage.

🟡 3. ds-locator / ds-analyzer / ds-patterns prompts imply a pipeline but run flat-parallel

All three onboarding steps are now in a single ctx.parallel fan-out (runner.ts:121-207,289), yet ds-analyzer is told to "evaluate the located design-system evidence" and ds-patterns to extract patterns — wording that presumes ds-locator's output is available. In a concurrent fan-out it isn't, so each agent independently re-scans the repo. The merge happens fine in design-system-builder (previous: onboardingAnalysis), so output is correct, but the prompts are misleading and the concurrent re-scan is somewhat wasteful. Either keep a short pipeline (locator → analyzer/patterns) or reword the analyzer/patterns objectives so each clearly does its own independent scan. (The PR description's ds-locator → ds-analyzer → ds-patterns arrow notation reinforces the sequential impression.)

🟡 4. copyAnnotationArtifacts copies an LLM-supplied path verbatim

feedback.annotatedSnapshot comes from model output and is resolved (absolute paths pass through) then copied into the artifact dir, including a sibling .yaml/.yml lookup (open-claude-design-feedback.ts:310-343). Low risk in a local single-user context, but it will copy an arbitrary file (e.g. an absolute path outside the project) if the model emits one. Consider constraining the source to within workflowCwd/artifactDir before copying.

🔵 Nits

• runner.ts: the entire runOpenClaudeDesignWorkflow body is over-indented (looks like a removed try/wrapper), and const designContext = ctx; (runner.ts:46) is a no-op alias. Cosmetic, but it makes the diff noisier.
• Test files use // @ts-nocheck. Understandable for terse mocks, but it forfeits type checking on the suite given the repo's strict settings — typing the mock ctx/results would catch contract drift.
• extractField (feedback.ts:118-141): a user note whose line begins with a known field label (e.g. Spec path: …) will truncate collection at that line, since matching is by canonical label. Unlikely in practice; just noting the edge.

✅ Strengths

• The feedback-threading design is solid: tolerant label parsing, placeholder detection (incl. the 1-char "a" survives test), latest-first ordering, durable artifact persistence, and the assertUserAnnotationsThreaded guardrail that fails loudly rather than refining without user input.
• Excellent test coverage of the open-claude-design drops Playwright annotation feedback between preview and refinement stages #1464 path: parse styles, placeholder handling, live_changes, brief ordering, guardrail (both notes and live-changes), persistence, plus full-run threading and the no-annotations fallback.
• NODE_ENV=test short-circuits (tmpdir artifacts, skipped global npm install -g) are the right call for keeping the suite hermetic.
• Inputs/outputs, .d.ts, changelog, and docs are all kept in sync; breaking-change migration is clearly documented.

Finding #1 is the one I'd most want resolved (or explicitly acknowledged as out of scope), since it's a residual instance of the very bug this PR targets. The rest are polish.

[flora131]

Thanks — addressed in 5b7a06492.

🟠 1 — terminal-iteration annotations dropped (residual #1464). Fixed. buildLivePreviewDisplayPrompt gained a final mode: on the last refinement iteration the preview is shown read-only — it no longer solicits user_notes/live_changes it can't apply and instead tells the user how to re-run — and the loop skips capture/threading on that iteration, so a terminal annotation is never captured-then-orphaned. (Combined with the export-gate guard from the prior round, captured feedback now always goes through an apply pass or is never solicited.)

🟠 2 — final-display solicited then discarded notes. Fixed. It no longer runs show --annotate or asks for user_notes/annotated_snapshot; it just opens the spec for review and emits a next_action_hint on how to re-run for changes.

Browser-centric early exit (per the maintainer's note that ctx.exit() is available). Added a deterministic guard right after the startup ensurePlaywrightCli() check: when the playwright-cli browser can't be made available, the run ctx.exit()s up front — surfacing the would-be artifact paths and install instructions — instead of generating a design no one can review interactively. Gated via shouldEarlyExitForBrowser(available, NODE_ENV) so the unit suite (NODE_ENV=test) and runtimes without ctx.exit still run to completion.

🟡 3 — parallel onboarding prompts implied a pipeline. Reworded the ds-analyzer and ds-patterns objectives to state each does its own independent repo scan and explicitly that they run in parallel with the locator, so the "evaluate the located evidence" phrasing no longer implies a dependency.

🟡 4 — copyAnnotationArtifacts copied a model-supplied path verbatim. Now constrained: the resolved source must be within workflowCwd or the run's artifact dir, otherwise it's skipped (the notes .md/.json are still persisted).

🔵 Nits. The extractField known-label-termination edge and the // @ts-nocheck test files are left as-is (acknowledged tradeoffs). The runOpenClaudeDesignWorkflow over-indentation / const designContext = ctx; alias is cosmetic and from the earlier rewrite — re-indenting the whole body now would balloon this PR's diff far more than it'd clean up, so I left it; happy to do a dedicated cosmetic pass if you'd prefer.

New tests cover the read-only final prompt, the early-exit predicate, and the snapshot-containment check; docs and both changelogs updated. Full suite green locally: typecheck, lint, check:file-length, test:unit (2552 pass / 0 fail).

[claude]

Code Review — PR #1470: feat(open-claude-design)!: discovery, init, reference discovery, live QA

Reviewed the authored TypeScript (open-claude-design-{runner,phases,setup,feedback,utils}.ts, the workflow definition, and the four test files), the docs, and the CHANGELOG. The bundled impeccable .mjs refresh was spot-checked for stale import paths only. Overall this is a strong, well-tested PR — the #1464 feedback-threading fix is well-architected, the modularization keeps every file under the 500-line gate, and the docs/CHANGELOG are comprehensive and rule-compliant.

Strengths

• Excellent test coverage. The feedback parser, merged-brief ordering, both guardrail branches (user_notes + live_changes), placeholder handling, artifact persistence, and the full-run export-gate behavior are all covered — including a real path-traversal regression test (does not copy a snapshot outside the project/artifact dir).
• open-claude-design drops Playwright annotation feedback between preview and refinement stages #1464 fix is durable by construction. Annotations are parsed tolerantly, carried through previewFeedbackHistory, ordered above internal critique in buildRefinementBrief, persisted as artifacts, and the assertUserAnnotationsThreaded guardrail makes silent drops impossible. The final-iteration / final-display read-only handling correctly prevents capturing terminal annotations that can't be applied.
• Good security hygiene in copyAnnotationArtifacts — model-supplied snapshot paths are constrained to the project/artifact dir before any copy, and all FS persistence is best-effort (never blocks the run).
• Docs (docs/workflows.md) and both CHANGELOGs are updated consistently with the breaking input change; no stale reference=/output_type=/design_system= references remain.

Considerations (none blocking)

1. runner.ts is at 499/500 lines — one line from the file-length gate. Compounding this, the entire function body (open-claude-design-runner.ts:50-498) is over-indented by 2 extra spaces, as if it still lived inside a since-removed try block. The next edit to this file will trip check:file-length. Worth de-indenting and/or extracting the dsSteps/importSteps builders into -setup.ts (where the other prompt builders already live) to buy headroom.

2. assertUserAnnotationsThreaded is a hard throw after all expensive work. Today it can't fire spuriously — annotations.text always embeds the verbatim notes and the only transformation downstream is .trim(), so prompt.includes(notes) holds. But it's a brittle invariant: any future change that escapes, truncates, or summarizes the annotations block would convert a best-effort UX feature into a fatal crash that discards a completed generation+refinement run. Consider degrading (log + re-inject the block) instead of throwing, or scoping the assertion more tightly. At minimum, a comment noting "must stay verbatim" near buildRefinementBrief would help.

3. Accepted live_changes redundantly force an apply pass. Live variants are described as "written into the preview HTML in place" during the session, yet hasMeaningfulFeedback counts them, so unappliedMeaningfulFeedback forces an extra apply-changes pass and the guardrail requires them in the prompt. This is defensible (protects the edits from being reverted), but it's an extra full-file-rewrite stage re-applying changes already on disk. Worth a one-line comment clarifying the intent is preservation, not re-application.

4. copyAnnotationArtifacts containment uses resolve, not realpath (open-claude-design-feedback.ts:311-314). An in-tree symlink pointing outside the project could slip past isWithin. Very low severity (needs a model-controlled path and a pre-existing malicious symlink), but realpathSync on the source before the check would close it for free.

5. Trivial: copyAnnotationArtifacts' YAML-sibling derivation source.replace(/\.[A-Za-z0-9]+$/, yamlExt) is a no-op when the snapshot path has no extension, making sibling === source. Practically unreachable (snapshots always carry an extension); noting for completeness.

6. approved_for_export semantics: when the loop exhausts maxRefinements with a final apply (rather than an early break), approvedForExport stays false even though the design went through the full refinement budget. Pre-existing behavior, but consumers reading that output field may find it counterintuitive.

Note

I was unable to execute the test suite in this review environment (sandbox denied the bun test invocation), so I relied on reading the tests and the PR's stated 2546 pass / 0 fail. The tests are well-structured and the assertions match the implementation.

Nice work overall — items above are polish/future-proofing, not correctness blockers.

🤖 Generated with Claude Code