feat(workflows): add open-claude-design builtin workflow

[flora131]

Summary

Adds open-claude-design as a built-in Atomic CLI workflow — an open-source replica of Anthropic's Claude Design product (launched April 17, 2026). Orchestrates the existing design skill ecosystem (impeccable, critique, shape, polish, audit, etc.) into a deterministic 5-phase pipeline via the Claude Agent SDK using a hybrid fan-out + bounded iterative loop topology.

Key Changes

Workflow (src/sdk/workflows/builtin/open-claude-design/)

• claude/index.ts — Full 5-phase workflow driven by the Claude Agent SDK:
  • Phase 1: Design System Onboarding — parallel headless fan-out (ds-locator, ds-analyzer, ds-patterns) + HIL approval via design-system-builder; persists tokens to Design.md
  • Phase 2: Import — parallel headless capture of URL/file references (web-capture, file-parser)
  • Phase 3: Generation — single visible agent producing the first design version
  • Phase 4: Refinement Loop — bounded HIL loop (≤5 iterations) with parallel critique + screenshot sub-agents and apply-changes; exits early on completion signal phrases ("approved", "ship it", "done", etc.)
  • Phase 5: Export/Handoff — HTML export + deterministic Claude Code handoff bundle (handoff-prompt.md, README.md, design-intent.md, component-specs.md); export assets tailored to output-type
• copilot/index.ts — Full Copilot provider implementation of the same 5-phase pipeline; handles Copilot-specific concerns: fresh session per stage (F5), empty assistant turn on tool-call exit (F1), and SessionEvent[] message retrieval (F9)
• opencode/index.ts — OpenCode adapter stub for future support
• helpers/constants.ts — Shared constants: MAX_REFINEMENTS=5, DESIGNS_DIR, headless (claude-sonnet) vs. visible (inherits Opus) agent options, impeccable design bans, IMPECCABLE_SCAN_CMD
• helpers/design-system.ts — Design system persistence: load/save Design.md, token extraction, slugified scratch directories
• helpers/export.ts — Deterministic handoff bundle generation; LLM exporter stage handles design-content-aware files; adapts output to output-type (prototype/wireframe/page/component)
• helpers/import.ts — URL/file reference aggregation for the Import phase
• helpers/prompts.ts — System prompts for all phase agents across all providers
• helpers/scan.ts — Deterministic impeccable scan gate: runs impeccable detect --json against the design directory and returns structured ScanFinding[]; findings are surfaced to the apply-changes stage to fix banned anti-patterns alongside user feedback; gracefully degrades when CLI is unavailable
• helpers/validation.ts — Refinement-complete detection via completion signal phrases (regex + substring matching)

Research & Spec

• specs/2026-04-17-open-claude-design.md — RFC: architecture decisions, goals, phase-by-phase spec
• research/docs/2026-04-17-open-claude-design.md — SDK mapping: existing Atomic primitives cover ~70% of required capabilities
• research/docs/2026-04-17-claude-design-product-analysis.md — Product analysis of Anthropic's Claude Design (6-phase pipeline, capabilities, target users)
• research/web/2026-04-17-claude-design-anthropic-labs.md — Raw primary sources from the Anthropic Labs announcement

Architecture

Phase	Pattern	Key Agents	Model
1 – Design System Onboarding	Parallel headless fan-out + HIL approval	ds-locator, ds-analyzer, ds-patterns, design-system-builder	Sonnet (headless) / Opus (HIL)
2 – Import	Parallel headless capture	web-capture, file-parser	Sonnet
3 – Generation	Visible single-agent	generator	Opus (inherited)
4 – Refinement Loop	Bounded loop (≤5 iter) with HIL + impeccable scan gate	user-feedback, critique, apply-changes	Sonnet (critique) / Opus (apply)
5 – Export/Handoff	Visible + deterministic bundle	exporter	Opus (inherited)

Headless stages use claude-sonnet with bypassPermissions for cost efficiency and unattended operation; visible/creative stages inherit the orchestrator model (Opus). The impeccable scan gate runs deterministically (no LLM call) each refinement iteration, grouping findings by anti-pattern and injecting them into the apply-changes prompt.

Usage

atomic workflow -n open-claude-design -a claude --prompt "Create a dashboard for tracking API usage"
atomic workflow -n open-claude-design -a claude --prompt "Landing page for a dev tool" --reference https://example.com

Inputs

Name	Required	Description
prompt	yes	What to design
reference	no	URL, file path, or codebase path for design reference
output-type	no	prototype (default), wireframe, page, component
design-system	no	Path to existing Design.md — skips onboarding if provided

[Copilot]

Pull request overview

Adds a new built-in workflow (open-claude-design) to the workflow SDK, implementing a multi-stage Claude-orchestrated design pipeline that generates and refines HTML/CSS/JS artifacts and exports a handoff bundle.

Changes:

• Introduces core workflow orchestration for open-claude-design (Claude provider) including onboarding, import, generation, refinement loop, and export/handoff.
• Adds helper modules for reference classification, validation parsing/formatting, design system persistence, export directory management, and deterministic handoff bundle packaging.
• Adds extensive Bun unit tests plus supporting spec + research documentation and new Claude agent definitions.

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/sdk/workflows/builtin/open-claude-design/helpers/web-capture.ts	Reference classification helpers (URL/file/codebase) and standard viewports.
src/sdk/workflows/builtin/open-claude-design/helpers/web-capture.test.ts	Unit tests for reference classification + viewport sizes.
src/sdk/workflows/builtin/open-claude-design/helpers/validation.ts	Refinement decision parsing + critique/screenshot parsing + summary formatting.
src/sdk/workflows/builtin/open-claude-design/helpers/validation.test.ts	Unit tests for refinement decision parsing and validation merging/formatting.
src/sdk/workflows/builtin/open-claude-design/helpers/prompts.test.ts	Prompt-builder test coverage (strings contain expected sections/constraints).
src/sdk/workflows/builtin/open-claude-design/helpers/handoff.ts	Deterministic packaging of a Claude Code handoff bundle from design outputs.
src/sdk/workflows/builtin/open-claude-design/helpers/handoff.test.ts	Tests for handoff bundle structure, file writing, and sensitive file exclusion.
src/sdk/workflows/builtin/open-claude-design/helpers/export.ts	Timestamped output/export dir helpers + recursive copy with sensitive filtering.
src/sdk/workflows/builtin/open-claude-design/helpers/export.test.ts	Tests for timestamps, dir creation, filtering, and recursive copy behavior.
src/sdk/workflows/builtin/open-claude-design/helpers/design-system.ts	Design system type/validation + load/persist logic to JSON on disk.
src/sdk/workflows/builtin/open-claude-design/helpers/design-system.test.ts	Tests for default DS, validation guard, persistence, and loading behavior.
src/sdk/workflows/builtin/open-claude-design/claude/index.ts	Main workflow orchestration implementing the 5-stage pipeline + refinement loop.
src/sdk/workflows/builtin/open-claude-design/claude/index.test.ts	Lightweight definition-shape tests for the compiled workflow export.
specs/2026-04-17-open-claude-design.md	RFC/spec documenting architecture, phases, directory layout, and goals.
research/web/2026-04-17-claude-design-anthropic-labs.md	Source collection on Claude Design announcement/coverage.
research/docs/2026-04-17-open-claude-design.md	Research mapping Claude Design phases to Atomic SDK primitives.
research/docs/2026-04-17-claude-design-product-analysis.md	Product analysis of Claude Design phases/capabilities for reference.
.claude/agents/design-system-builder.md	New Claude agent definition for DS onboarding + user approval flow.
.claude/agents/design-refiner.md	New Claude agent definition for iterative refinement + AskUserQuestion enforcement.
.claude/agents/design-generator.md	New Claude agent definition for initial artifact generation (HTML/CSS/JS).
.claude/agents/design-exporter.md	New Claude agent definition for producing export/handoff documentation sections.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Unused imports in this test file (e.g., readdir, and PackageHandoffOptions) will trigger lint warnings. Remove unused imports or use them in assertions to keep the test suite clean.

[Copilot]

This test creates const stat = await Bun.file(newDir).exists(); but never uses it. That unused variable will raise a lint warning; either assert on it (e.g., expect(stat).toBe(true)) or remove it.

Suggested change

const stat = await Bun.file(newDir).exists();

[Copilot]

FindingSeverity is imported but not used in this test file, which will cause a lint warning. Remove the unused type import (or add coverage that uses it).

[Copilot]

parseRefinementDecision logs via console.warn(...), but this module-level header and multiple docstrings claim all functions are pure (no side effects). Either remove the logging (or inject a logger callback) so the helpers remain pure, or update the docs to reflect the side effect (and consider gating it behind an explicit debug flag).

Suggested change

	// If we reach here, the agent did NOT call AskUserQuestion. Log a warning
	// so this failure mode is visible in workflow logs.
	console.warn(
	"[open-claude-design] WARNING: AskUserQuestion tool was not called during " +
	"refinement. Falling back to JSON/regex extraction from agent text. " +
	"The user was NOT prompted for input.",
	);
	// If AskUserQuestion was not called, fall back to parsing the agent text.

[Copilot]

validateDesignSystem() checks that typography.fontFamily and typography.scale are objects, but it never validates required nested keys/types (e.g. fontFamily.heading/body and scale.h1/h2/body/small as strings). As a result, malformed design systems can pass validation and later produce undefined values in prompts/handoff output. Tighten the guard to verify the required nested fields are present and strings (matching the function docstring).

Suggested change

	const fontFamily = typography.fontFamily as Record<string, unknown>;
	if (
	typeof fontFamily.heading !== "string" ||
	typeof fontFamily.body !== "string"
	) return false;
	const scale = typography.scale as Record<string, unknown>;
	if (
	typeof scale.h1 !== "string" ||
	typeof scale.h2 !== "string" ||
	typeof scale.body !== "string" ||
	typeof scale.small !== "string"
	) return false;

[Copilot]

classifyReference() treats bare www. strings as a URL, but the workflow passes reference through to playwright-cli unchanged. Playwright navigation typically requires a scheme; www.example.com will fail in many cases. Consider normalizing URL references (e.g., prefix https:// when the ref starts with www.) before calling buildWebCapturePrompt() / launching capture.

[claude]

Code Review — feat(workflow): add open-claude-design builtin workflow

Thanks for this contribution — it's a large, well-documented PR that closely mirrors the patterns in deep-research-codebase and ralph. The helper modularization (prompts / design-system / handoff / validation / export / web-capture) is good, and the parallel fan-out in Phase 1+2 plus bounded refinement loop from Ralph are nicely reused. Below is what I'd push back on before merging.

🚨 Critical: AskUserQuestion is not granted to the agents that must call it

The SDK docs are explicit: "If you specify a tools array, include AskUserQuestion for this to work." (docs/claude-code/agent-sdk/guides/user-input.md:42). The frontmatter in the new agent files is missing it for two agents whose prompts loudly demand it:

• .claude/agents/design-system-builder.md:4 — tools: Read, Write, Glob, Grep, Edit, Bash → missing AskUserQuestion
• .claude/agents/design-refiner.md:4 — tools: Read, Write, Edit, Bash → missing AskUserQuestion

Both agents' prompt bodies (and the matching helpers in prompts.ts, e.g. ASK_USER_QUESTION_ENFORCEMENT, buildRefinePrompt, buildDesignSystemBuilderPrompt, buildDesignReviewPrompt) instruct the agent to call the tool — but with the restricted tools: list it simply cannot. This silently breaks the HIL approval gate for Phase 1 and the entire refinement loop in Phase 4. Add AskUserQuestion to those frontmatter lists (or drop the tools: line to inherit defaults).

Bugs & correctness

1. validateDesignSystem accepts malformed typography/fontFamily (helpers/design-system.ts:146-164). The check only asserts typography.fontFamily and typography.scale are objects — it never verifies fontFamily.heading/body or scale.h1/h2/body/small are strings, though the DesignSystemContext type requires them. Later, handoff.ts:151 does designSystem.typography.fontFamily.heading unconditionally, which would crash on a validated-but-missing field. Either tighten validateDesignSystem or defensively format the handoff prompt.

2. extractDesignSystem greedy fallback can swallow unrelated JSON (helpers/design-system.ts:266). The regex /\{[\s\S]*\}/ matches from the first { to the last } in the whole builder output. If the agent's narrative contains any JSON-like snippet before the design-system block (e.g., an example), tryParseAndValidate will attempt to parse a huge junk blob; if that fails, it silently falls back to the default design system — the user's intended tokens vanish without warning. At minimum, log a warning on fallback (you did this for parseRefinementDecision, do it here too).

3. getTimestampedOutputDir and getTimestampedExportDir each call getTimestamp() separately (helpers/export.ts:52-62). If callers use these two helpers rather than the composite ensureOutputDirs, the two timestamps can diverge across a clock-second boundary, silently decoupling the pair. The composite already guards against this; consider either deleting the two public helpers or having them take a shared timestamp argument.

4. copyDesignFiles has no atomicity guarantees (helpers/export.ts:105-143). A mid-copy failure leaves a partial handoff/design/ directory and an unresolved packageHandoffBundle promise. For a "deterministic, no-LLM" final step this is worth hardening — wrap in try/catch and clean up (or at least surface a clear error message).

5. Sensitive-file filter is basename-only (helpers/export.ts:83-86, helpers/handoff.ts:60-62). A file under .../credentials/logo.png will be copied because only the basename is tested. For a design bundle this is mostly theoretical, but since you already added the filter, consider testing the full relative path.

6. Duplicated isSensitiveFile (handoff.ts:60 uses regex patterns; export.ts:83 uses substring patterns). packageHandoffBundle delegates copying to copyDesignFiles, which uses export.ts's version — so handoff.ts's copy is dead code at the path that matters. Consolidate into one source of truth.

7. Cross-platform open/xdg-open instructions (prompts.ts:863, 933, 1003). Per CLAUDE.md, the project supports Windows, but the prompts tell the generator and refiner to run open (macOS) or xdg-open (Linux). Add the Windows start equivalent, or drop the shell command and just tell the agent to write the path — the workflow's own console.log at claude/index.ts:552 already surfaces it.

8. Stale validationFeedback across iterations (claude/index.ts:406-502). validationFeedback is a loop-scoped string that only gets overwritten by a validation pass. If the user chooses option 2 once, then runs through several option-3 multi-turn rounds (inner while loop), then requests validation again, the outer iteration still carries forward feedback from the prior validation into buildRefinePrompt. Clear it when the user takes option 3, or timestamp each feedback block, to avoid the refiner acting on findings that no longer match the current state.

9. "Iteration X of 8" labelling is misleading (prompts.ts:904-905). MAX_REFINEMENTS is the outer validation-cycle bound, but the user can iterate many turns within a single stage via option 3. The prompt's "Iteration X of 8" implies per-turn counting.

Minor / polish

• SUBAGENT_OPTS sets permissionMode: "bypassPermissions" + allowDangerouslySkipPermissions: true for every headless sub-agent (claude/index.ts:79-82). This matches ralph/deep-research-codebase, but consider adding a one-line comment that this is acceptable because the sub-agents run over the project repo and not attacker-controlled input. A URL pulled from reference flows into buildWebCapturePrompt and could, in theory, include markdown that breaks the prompt structure — trust boundary assumption is worth documenting.
• prompts.ts is 1408 lines of template literals. It reads fine, but exporting the constants (ANTI_PATTERN_GUARDRAILS, ASK_USER_QUESTION_ENFORCEMENT) and splitting builders by phase into per-file modules would make future diffs more surgical.
• parseRefinementDecision swallowing the "user typed 3 alone, no feedback" case by returning feedback: null is intentional (the agent is supposed to re-ask), but there's no test covering that branch — worth adding.
• The console.log in claude/index.ts:545-555 bypasses whatever logger the SDK uses elsewhere. Check whether the workflow framework exposes a preferred logger (the other builtins may be using ctx.log or similar).

Tests

Coverage on helpers (validation, handoff, export, design-system, web-capture, prompts) is substantive — good. Gaps worth filling:

• No test asserting extractDesignSystem falls back to the default when the agent returns malformed output vs. when it returns a valid block — that's the hottest failure path.
• No test that validateDesignSystem rejects a design system with a missing typography.fontFamily.heading string (see bug add agent instructions #1).
• claude/index.test.ts tests only the workflow shape (name, inputs, run function). That's expected (tmux/CLI required to run), but consider extracting captureImport's dispatch logic (url vs file vs codebase routing) into a pure helper so it can be unit-tested without mocking ctx.stage.

Nits

• deletions: 0 on a +8942-line PR — be sure this isn't actually deleting the .claude/agents/design-*.md files that git-show from HEAD currently has (the PR branch's merge state shows them deleted locally; confirm they land in main after merge).
• research/ and specs/ files are useful context; consider whether they need to ship in main or belong in a docs branch.

Overall approach is solid; the AskUserQuestion tool-list issue is the one that I'd block on, since it silently defeats the workflow's core interaction pattern.

[claude]

Review: PR #667 — feat(workflow): add open-claude-design builtin workflow

Thanks for the thorough planning work here. Before diving into line-level feedback, there is one structural issue that probably needs to be addressed first.

1. PR title/description do not match the diff (primary concern)

The PR description announces an implementation with:

• src/sdk/workflows/builtin/open-claude-design/claude/index.ts (5-phase orchestrator)
• Four new .claude/agents/*.md sub-agent definitions
• Six helper modules under helpers/
• A full test suite (*.test.ts) for helpers + orchestrator integration tests

However, gh pr view 667 --json files shows only 4 files changed, all markdown, all additive:

File	+lines
research/docs/2026-04-17-claude-design-product-analysis.md	519
research/docs/2026-04-17-open-claude-design.md	589
research/web/2026-04-17-claude-design-anthropic-labs.md	269
specs/2026-04-17-open-claude-design.md	875

No TypeScript, no agent definitions, no tests. The feat(workflow): Conventional Commits prefix is misleading for what is effectively a spec + research PR. Suggested actions:

• Retitle to docs(spec): open-claude-design RFC and research (or similar), or
• Push the implementation commits that the description refers to before merging.

A reviewer pulling this branch expecting to exercise the workflow will hit nothing. This also affects CI signal — none of the implementation claims (bun lint, bun typecheck, bun test coverage for the helpers) can actually be verified from this diff.

2. Quality of the docs themselves

Taking the PR as a docs/spec PR, content quality is high:

• Source attribution is solid — frontmatter in the research docs correctly separates primary_source, raw_research, additional_sources. The raw web collection (research/web/...) is kept distinct from the synthesized analysis in research/docs/..., which matches the project's research-codebase skill convention.
• Cross-references resolve — spec links to ../research/docs/... from specs/ are correct relative paths.
• SDK mapping is grounded — Part 2 and Part 3 of research/docs/2026-04-17-open-claude-design.md cite concrete files and line ranges (src/sdk/define-workflow.ts:186-195, ralph/claude/index.ts:54-248, etc.), which is the right level of specificity for a planning doc.
• Non-goals are explicit (NG1–NG7 in the spec). That's the part of an RFC that's most often missing.

3. Internal inconsistencies in the spec

A few things a future implementer will stumble on:

• design-exporter agent is referenced but not declared. Section 5.8 spawns a visible stage with chatFlags: [\"--agent\", \"design-exporter\", ...], but Section 5.11 ("Agent Definitions") lists only three new agents: design-system-builder, design-generator, design-refiner. Either add design-exporter.md to 5.11, or rewrite 5.8 to use an existing agent (e.g. worker) with a skill-loaded prompt. The PR description lists four agents including design-exporter.md, so 5.11 is likely the one that's wrong.
• Refinement exit condition contradicts itself. Section 5.7 code snippet checks if (isRefinementComplete(refine.result)) break; — i.e. parses the assistant's text for a completion signal. A few paragraphs later, the "Exit condition (via AskUserQuestion)" subsection says the exit is an explicit AskUserQuestion with three options (Approve / Continue / Start over). Section 9 (Q2 Resolved) confirms the intent is the latter — so the code snippet in 5.7 should be updated to show the askUserQuestion(...) call instead of the text-parsing helper, otherwise an implementer will keep building isRefinementComplete() and drift away from the resolved decision.
• Design system lifecycle is ambiguous. Section 5.1 shows a single .open-claude-design/design-system.json (shared across runs) but output-<timestamp>/ + export-<timestamp>/ (per-run). Explicitly state: "design system JSON is a singleton that survives across runs; re-runs may update it via the onboarding stage" (or whatever the intent is). Also spell out whether the timestamped output dirs are ever garbage-collected — otherwise .open-claude-design/ will grow unbounded.
• output-type enum drift. The research doc lists [\"prototype\", \"wireframe\", \"deck\", \"collateral\", \"frontier\"]. The spec (5.2) lists [\"prototype\", \"wireframe\", \"mockup\", \"landing-page\"]. Pick one; the research doc's "deck / collateral / frontier" values are more faithful to Claude Design's product, but several are in NG (Non-Goals), so pruning makes sense — just make the two docs agree.
• Reused agent codebase-online-researcher tool surface. 5.11 says it handles "Web capture via playwright; screenshot validation." Before implementation, confirm this agent actually has the playwright-cli skill or Bash(playwright-cli:*) permission in its current definition — otherwise Phase 2 URL capture and Phase 4 screenshot validation will silently degrade to text-only.

4. Risks and open questions worth flagging before implementation

• MAX_REFINEMENTS = 8 with a visible HIL stage per iteration + parallel headless critique + screenshot per iteration. That's up to 8 × 3 = 24 stages and ~8 tmux windows in one run. Worth confirming GraphFrontierTracker handles this and that the TUI graph panel stays readable. Ralph caps at 10 but isn't generally run to its limit.
• --allow-dangerously-skip-permissions + --dangerously-skip-permissions both appear in SKIP_PERMS. One is presumably a typo/alias — double-check against docs/claude-code/cli/permissions.md before shipping.
• CLAUDE.md rule: "Avoid ambiguous types like any and unknown." The snippets use DesignSystemContext and ImportContext but never define them in the spec. Section 5.4 hints at a JSON shape; please inline the full TS interface (or point to where it will live) so the type contract is reviewable before code lands.
• Privacy claim vs. reality. 7.1 says "Source code is never uploaded to external servers." That's true for the headless in-process Agent SDK path, but visible stages invoke the Claude Code CLI which does send content to Anthropic. Consider rewording to "no data leaves the normal Claude provider path" to avoid an overclaim that Anthropic Trust & Safety would flag.
• Handoff bundle & sensitive files. 7.1 says the export helper "explicitly filters" .env/credentials. Nail down the filter list (glob of denied patterns) in the spec — if this lives only in code it's easy to accidentally ship a permissive version.

5. Minor polish

• The research_at date, the git_commit in research/docs/2026-04-17-open-claude-design.md, and the last_updated field are all fine for a research artifact, but consider whether git_commit: 200d34dc... (pre-branch) is worth keeping — it will be stale the moment this PR rebases.
• specs/2026-04-17-open-claude-design.md Section 5.3 has a duplicate rendering of the "┌─→ web-capture..." ASCII diagram (same diagram appears in 4.1 as Mermaid and again in 5.3 as ASCII). Keep one canonical source.

Recommendation

• Blocking: Split or relabel this PR. As-is, the feat(...) commit message will land on main and future git log --grep=workflow / release-note generation will claim a workflow was shipped when only the RFC was. Either push the implementation commits the description refers to, or re-title to docs(spec): and open the implementation as a follow-up PR that references this spec.
• Non-blocking: Fix the internal inconsistencies in Section 3 above (design-exporter agent, refinement exit snippet, output-type enum, directory lifecycle). These will save the implementer a round-trip.

The research and spec work themselves are in good shape — just the packaging of this PR needs adjusting.

[claude]

Code Review — PR #667

Thanks for putting this up! Capturing the research and RFC as standalone artifacts before the implementation lands is the right call. Comments below, scoped to a docs-only PR (no code = no perf/security concerns; test coverage N/A).

Blocking / should-fix

1. PR description references a file that isn't in the diff.
The body lists specs/2026-04-17-open-claude-design.md under "Specification" as the full TDD/RFC. That file is not in the changeset and does not exist in specs/. Either:

• The spec file was meant to be included — please add it, or
• The PR body should be updated to clarify that the RFC content lives inside research/docs/2026-04-17-open-claude-design.md and no separate spec is being shipped yet.

This matters because the specs/ directory is the canonical home for RFCs in this repo (see the long history of specs/2026-*-*.md); leaving a dangling reference will confuse future readers.

2. RFC references skills that don't exist in .agents/skills/.
research/docs/2026-04-17-open-claude-design.md cites the following as "Exists" / available, but a quick ls .agents/skills/ shows no such directories:

• extract — referenced in Phase 1 mappings (line ~189) and Part 5 ("Existing Capabilities")
• normalize — Phase 1 stage skill list (line ~218)
• handoff — Phase 6 stage skill list (line ~368)
• Also Part 6 maps /design-system to "extract + normalize skills"

Anyone reading this as an implementation guide will hit the wall immediately. Please either correct these to skills that exist (e.g., impeccable, audit, polish, liteparse) or explicitly mark them as "New — needs to be built" alongside the other to-be-built items in the same table.

Non-blocking suggestions

3. Frontmatter inconsistency across the three files.
The repo's research docs use a fairly stable frontmatter format (date, researcher, git_commit, branch, repository, topic, tags, status, last_updated, last_updated_by). Your three files use three different shapes:

• 2026-04-17-open-claude-design.md follows the convention ✓ (but repository: atomic-open-claude-design is wrong — it's just atomic).
• 2026-04-17-claude-design-product-analysis.md uses a custom shape (topic, researched_at, primary_source, raw_research, purpose).
• 2026-04-17-claude-design-anthropic-labs.md uses a third shape (source_url, fetched_at, fetch_method, additional_sources).

For research/web/ the source-collection shape is reasonable, but the product-analysis doc should probably match the standard research/docs/ frontmatter for consistency with the ~100 other files there.

4. Cross-reference prior overlapping research.
research/docs/2026-02-14-frontend-design-builtin-skill-integration.md already explored adding frontend-design as a built-in skill. Your RFC proposes a built-in workflow, which is a different (and IMO more interesting) integration point — but a one-line "Why a workflow, not a skill — see 2026-02-14 for the skill-based alternative" in Part 4 would help future readers reconcile the two and prevent re-litigation.

5. Capture the why of the implementation revert.
The PR description notes "implementation was added in this branch and subsequently reverted." Future readers will want to know whether it was reverted for scope reasons, blocking technical issues (e.g., HIL didn't work as expected), or something else. A brief addendum in either the PR body or the RFC's "Open Questions" section would preserve that context.

6. skip-onboarding as enum: ["true", "false"] is brittle.
The SDK's WorkflowInputType is "string" | "text" | "enum" (src/sdk/types.ts:152), so encoding a boolean as a 2-value enum is the only option today, but it's worth flagging in Open Questions whether the SDK should grow a boolean input type before this workflow ships — handling \"true\"/\"false\" as strings is a paper cut that will repeat.

7. Minor accuracy nits in code references.

• ralph/helpers/prompts.ts:1-1078 — file is 1077 lines.
• The exact defineWorkflow line range (define-workflow.ts:186-195) is plausible but worth re-verifying when you commit since these line numbers will rot quickly. Consider linking by symbol name only (defineWorkflow() in src/sdk/define-workflow.ts) for the long-lived doc.

What's good

• The phase-by-phase mapping table in Part 3 is genuinely useful and the right level of abstraction for an RFC.
• Distinguishing "Already Exists / Needs to Be Built / Not Applicable" in Part 5 is exactly the structure that makes scoping decisions easy in review.
• "Not Applicable (CLI/TUI limitations)" is honest and avoids over-promising — adjustment sliders, drawing, and multi-user collaboration are correctly identified as web-only.
• Borrowing Ralph's bounded-loop pattern for refinement (Part 3, Phase 4) is the right reuse choice — that pattern is already battle-tested.
• The Open Questions section is well-formed and surfaces the right decisions for follow-up discussion (persistence format, exit condition, validation approach, multi-provider scoping).

Summary

Solid research + RFC content. Two things to fix before merge: (1) reconcile the missing specs/ file with the PR description, and (2) correct the references to non-existent skills (extract, normalize, handoff). The rest are polish items that don't block.

[claude]

Code Review — PR #667

Thanks for the thorough research on Claude Design. I reviewed both the research docs and the workflow implementation. The research artifacts are high-quality and well-sourced. The implementation has some concrete issues worth addressing before this lands.

PR Description vs. Diff Mismatch

The PR body states "The implementation and spec were added in this branch and subsequently reverted. Only the research artifacts remain…", but the diff clearly includes:

• specs/2026-04-17-open-claude-design.md (806 lines)
• src/sdk/workflows/builtin/open-claude-design/claude/index.ts (434 lines)
• 5 helper files (~1,233 lines)
• opencode/ and copilot/ stubs (58 lines each)

Totals: 3,961 additions across 13 files, roughly 40% implementation/spec and 60% research. Please either update the description to reflect what's actually in the diff, or split off the implementation into a separate PR as the description suggests.

---

Blockers (likely behavioral bugs)

1. isRefinementComplete() will false-positive on iteration 1. (helpers/validation.ts:11-21)

The completion signals include bare words like "approved", "done", "export", "looks good", and "approve and export". Checked via .toLowerCase().includes(signal). The user-feedback stage's prompt (prompts.ts:550-555) instructs the agent to present the options verbatim — including the string "Approve and export". That assistant text is then fed to isRefinementComplete, which will match on both "approve and export" and "export" and exit immediately.

In practice the loop never iterates. Suggested fix: require a stricter sentinel (e.g., the agent writes a JSON marker like {"decision":"approve"}) and parse it, similar to Ralph's parseReviewResult / ReviewResultSchema pattern.

2. Visible stages pass HEADLESS_OPTS to session.query(). (claude/index.ts:316-325, 411-421)

HEADLESS_OPTS is the Agent SDK's query options (permissionMode, model: "sonnet", etc.). For visible tmux-based stages the Claude provider drives the TUI via keystrokes — SDK query options are not applied. So the model: "sonnet" override on visible user-feedback-* and exporter stages is silently ignored; they inherit the parent's Opus model, contradicting the "tiered model strategy" described in specs/2026-04-17-open-claude-design.md:639-676.

3. Visible stages don't set --agent. All visible stages pass {} as clientOpts, so no chatFlags and no --agent <type> flag. Compare to Ralph (ralph/claude/index.ts:75-94, 97-112, 192, 202, 221-232), which explicitly pins --agent planner/orchestrator/reviewer/debugger. The research doc proposes custom agents (design-system-builder, design-generator, design-refiner, design-exporter) but none of the five visible stages select any agent at all. As a result, prompt-level instructions like "load the /impeccable skill" (prompts.ts:481, 771) are a hope, not a guarantee.

4. VISIBLE_OPTS is defined but never imported or used. (helpers/constants.ts:22-25). Either wire it into the visible stages (via sessionOpts) or delete it.

---

Reliability / Error Handling

5. persistDesignSystem() throws a raw ENOENT if the agent didn't write Design.md. (helpers/design-system.ts:45-51). If the design-system-builder stage's HIL flow is interrupted or the agent forgets to write the file, the workflow dies with an opaque node error. Catch and rethrow with a message pointing to the stage and expected path.

6. Handoff bundle silently incomplete on exporter failure. writeHandoffBundle (helpers/export.ts:36-64) only writes handoff-prompt.md + README.md + copies Design.md. If the exporter stage failed to copy the design files or write design-intent.md / component-specs.md, the README advertises files that don't exist. Consider verifying the expected files exist and failing fast, or at least noting "(missing)" in the generated README.

7. No existence check on --design-system input. loadDesignSystem(designSystemPath) reads arbitrary paths without validation; ENOENT propagates as a raw node error. Validate and surface a friendly error.

---

Code Quality

8. aggregateImportResults() is an identity helper. (helpers/import.ts:40-52). It just constructs the same object shape from the same fields. Delete it and build the object inline.

9. isFilePath() heuristic misclassifies bare domains. example.com/foo.html is not a URL (no scheme) but matches \.\w{1,6}$ and is classified as a file. Low impact since users typically include a scheme, but a fs.stat existence check would make this deterministic.

10. Regex-via-string-flag in COMPLETION_SIGNALS (validation.ts:19-20, 33-35): "user selected.*approve" as a string is turned into a regex via .includes(".*") detection. This overloading is fragile; either use a discriminated union ({ kind: "literal" | "regex", value: string }) or just use RegExp instances directly.

11. IMPECCABLE_BANS duplicates the impeccable skill. constants.ts:40-45 hardcodes the BAN list and reflex-font list. If the skill's list changes, this drifts silently. Reference the skill source or read it at runtime.

12. Scratch directory inside research/. research/designs/.scratch/ (design-system.ts:85) and research/designs/<slug>/ for final output (DESIGNS_DIR) mix ephemeral intermediate artifacts and long-lived research docs. Prefer something under .atomic/ or project-rooted designs/ to keep research/ documentation-only (consistent with how it's used in this very PR).

13. Dynamic import of mkdir inside ensureScratchDir (design-system.ts:84) is unnecessary — static-import it at the top like the other fs functions.

14. specs/ vs. research/ mismatch for MAX_REFINEMENTS. The spec and research docs reference "max 8 iter" (and 5-phase vs 6-phase framing); constants.ts:6 sets MAX_REFINEMENTS = 5. Align the docs with the code or vice versa.

15. Stub files duplicate the full inputs block. opencode/index.ts and copilot/index.ts each re-declare the same inputs (58 lines each, ~116 total) just to throw. Either skip registering them until implementation lands, or share a single buildStubWorkflow(agent) helper.

---

Security

16. Web-capture + file-parser pipe untrusted content into downstream prompts. Expected for this use case, but worth a comment in buildWebCapturePrompt / buildFileParserPrompt reminding that the fetched content may contain prompt injection and the downstream generator should treat it as data, not instructions.

17. path.resolve(designSystemPath) with no normalization check means --design-system=../../../etc/passwd reads arbitrary files. The agent currently has full FS access anyway, but input validation at the workflow boundary is cheap.

---

Testing

18. Zero test coverage on pure helpers. isUrl, isFilePath, slugifyPrompt, aggregateImportResults, and especially isRefinementComplete are ideal targets — all pure functions, all currently broken or ambiguous in subtle ways. Per CLAUDE.md (bun test), these should each have a short test file. isRefinementComplete in particular would catch blocker #1 on the first run.

---

Summary

The research is solid work and worth landing. The implementation has a probable refinement-loop-never-runs bug (#1), several cases where config is declared but not wired (#2, #3, #4), and no tests. Suggested path forward:

1. Either revert the implementation/spec (as the PR description claims) and land only research, or
2. Fix blockers add agent instructions #1–Flora131/feat/add skills #4, add tests for the pure helpers, and update the PR description to reflect the actual scope.

Happy to dive deeper on any of these if useful.

[claude]

Code Review — open-claude-design workflow

Nice scope and the phased structure reads cleanly alongside the existing deep-research-codebase workflow. The research artifacts and RFC make the intent easy to follow. A handful of concrete issues and rough edges below — grouped by impact.

Bugs / correctness

1. isRefinementComplete exits the loop on common feedback words — src/sdk/workflows/builtin/open-claude-design/helpers/validation.ts:11-21

COMPLETION_SIGNALS includes \"done\", \"export\", \"approved\", \"looks good\". These are everyday words in design critiques. A user saying "I'm done with the header color but the padding looks off" will silently exit the refinement loop on iteration 1. Same for "the hero section looks good, but fix the nav". Even worse, the feedback prompt itself (buildRefineFeedbackPrompt) lists \"Approve and export\" as one of four options — whichever option the user picks, the agent's echo is very likely to contain \"export\" or \"approved\" verbatim. Suggest replacing the substring heuristic with a structured signal: have the feedback stage return a typed value (a sentinel like COMPLETED on its own line, or better, capture the AskUserQuestion selection index directly from the transcript) and compare with exact equality.

2. MAX_REFINEMENTS = 5 contradicts the docs — helpers/constants.ts:6 vs. the PR description and claude/index.ts:12 which both say "Bounded loop (max 8 iter)". Pick one and align.

3. HEADLESS_OPTS silently dropped on visible stages — claude/index.ts:324,420

user-feedback-${iteration} and exporter are visible tmux stages (no headless: true), but both pass { ...HEADLESS_OPTS } to s.session.query(). Looking at ClaudeSessionWrapper.query → claudeQuery (src/sdk/providers/claude.ts:523), claudeQuery destructures only tmux-relevant options; permissionMode, allowDangerouslySkipPermissions, and model: \"sonnet\" are silently ignored. The misleading part: a future reader will assume these stages run on Sonnet with bypassed permissions — they don't. Either drop the spread on visible stages or rename the constant to something like SDK_QUERY_OPTS and add a VISIBLE_OPTS sibling that's actually empty.

4. Non-URL / non-file references are silently dropped — helpers/import.ts:22-33

The README/workflow description says reference accepts "URL, file path, or codebase path". But in claude/index.ts:228-270 the Promise.all only fires branches when isUrl(reference) or isFilePath(reference) is true. A codebase path like src/components (no leading ./, no extension) matches neither predicate — the reference just evaporates with no warning. Either drop "codebase path" from the user-facing copy, or add an explicit branch/warning.

isFilePath's \\.\\w{1,6}$ regex also over-matches: login.page, v1.0, or user.email register as file paths. Consider fs.stat for the ambiguous case.

5. Handoff bundle references files the exporter may never have written — helpers/export.ts:122-126 and claude/index.ts:428-432

writeHandoffBundle always writes README.md and handoff-prompt.md that reference design-intent.md, component-specs.md, design/index.html, etc. These are produced by the exporter LLM stage — which the export prompt explicitly lets the user decline ("HTML only"). The deterministic write runs unconditionally after the exporter, so any user who picks "HTML only" or any exporter failure leaves a README promising files that don't exist. Either gate writeHandoffBundle on the exporter's actual output, or collapse the branch and always write all bundle files.

6. Agents chosen by name don't match the task — claude/index.ts:242,263,368

• web-capture uses agent: \"codebase-online-researcher\" but the prompt tells it to run Playwright to screenshot a live URL — the researcher agent is for docs lookups, not visual capture.
• file-parser and screenshot-validation both use agent: \"codebase-analyzer\" to parse DOCX/PPTX/PDF or render HTML via Playwright — outside that agent's described scope.

These may work by accident because the agents inherit Bash, but an agent whose description says "Analyzes codebase implementation details" running Playwright is a surprise. Either pick agents whose descriptions align, or use the default (no agent: key).

Code quality

7. outputTypeInstructions is typed loosely — helpers/prompts.ts:445

Typed as Record<string, string> with a ?? outputTypeInstructions.prototype fallback. Since the output-type input is an enum with four fixed values, prefer Record<\"prototype\" | \"wireframe\" | \"page\" | \"component\", string> — that way adding a new enum value surfaces the missing branch as a compile error. CLAUDE.md asks to avoid loose types.

8. Dynamic import inside ensureScratchDir — helpers/design-system.ts:84

const { mkdir } = await import(\"node:fs/promises\") — the module already imports readFile at the top. Just import mkdir statically for consistency.

9. Slug collisions — helpers/design-system.ts:68-78

The scratch dir is {scratchDir}/{slug} without any timestamp component (the isoDate is only used for the final export dir). Two runs with the same prompt write to the same scratch folder and silently overwrite each other.

10. The "please echo this phrase" prompt pattern is brittle — helpers/prompts.ts:560-561

If the user chose "Approve and export", include the phrase "user approved"...

Relying on the LLM to reliably repeat a specific string every time is a weak control channel. Combined with issue #1, this is the main source of loop-control fragility. A structured signal would be far safer.

Testing

11. Pure helpers aren't unit-tested

isUrl, isFilePath, slugifyPrompt, isRefinementComplete, aggregateImportResults, renderBans, and buildHandoffPrompt are all pure and trivially testable under bun test. Given that #1, #4, and the slug edge cases are all in pure functions, even a small test file would have caught the false-positive completion signals and the regex over-match. Other builtin workflows don't ship tests either, but the fragile logic here argues for adding some now.

Minor / stylistic

• buildHandoffPrompt emits :root { /* Import all tokens from Design.md... */ } as a literal placeholder comment. Either serialize the tokens or drop the block.
• PR description mentions "~70% of required capabilities" from the research, but the OpenCode and Copilot entry points are stubs that throw. Consider marking the workflow Claude-only in the description until the other providers land, or skip shipping the stub files until they have a real implementation.
• The HIL instruction tells the agent to use AskUserQuestion in several visible stages — worth a sanity-check that the claude-code CLI running in the tmux pane actually exposes that tool in the session configuration this workflow spawns.

Positives worth keeping

• Forward-only data flow (each stage embeds upstream output verbatim) matches the pattern in deep-research-codebase and avoids re-discovery costs.
• Deterministic boundaries around LLM stages (persistDesignSystem, aggregateImportResults, writeHandoffBundle) are the right instinct — pure TS where pure TS suffices.
• Embedding IMPECCABLE_BANS directly in the generator and refinement prompts (rather than relying on a separately-loaded skill) is a good prompt-engineering call.
• The RFC and research docs are genuinely useful and tie the implementation back to an explicit design.

Happy to dig further on any of the above.

[claude]

PR Review: feat(workflow): add open-claude-design builtin workflow

Reviewed the full workflow (claude/index.ts, all helpers, stubs, constants, and prompts) against CLAUDE.md conventions and the existing deep-research-codebase pattern. Overall the topology and SDK-primitive usage are solid and consistent with existing builtins. A few items to address before merge.

---

HIGH PRIORITY

1. isRefinementComplete completion detection is too permissive (helpers/validation.ts)

The COMPLETION_SIGNALS list contains bare substrings like done, export, approved. These produce false positives on natural phrases the user is likely to say in the feedback stage: (not done yet), (do not export yet), (have not approved it) all match. Since the feedback stage is exactly where users will naturally mention these words, the refinement loop is likely to exit after iteration 1 in common cases. Consider either:

• Anchoring to a sentinel the prompt asks the agent to emit (e.g. [[USER_APPROVED]]), which buildRefineFeedbackPrompt already half-does — just enforce an exact phrase and test with an anchored regex, or
• Parsing the structured AskUserQuestion result (the user selected option) rather than the agent free-text paraphrase.

2. MAX_REFINEMENTS inconsistency

• helpers/constants.ts sets it to 5
• PR description says max 8 iterations
• JSDoc in claude/index.ts says bounded HIL loop (max 8 iterations)

Pick one. If 5 is intentional, update the description and the topology comment.

3. npx in buildCritiquePrompt violates CLAUDE.md Bun-only rule

The critique prompt emits: npx impeccable --json . CLAUDE.md has an EXTREMELY_IMPORTANT block forbidding npx — use bunx impeccable --json ... instead. Separately, there is no impeccable CLI binary in the repo (only .agents/skills/impeccable/ docs plus a cleanup script), so this command will always fall through to the scanner not available path. Either remove the scanner branch or wire up a real binary.

4. No tests in the final diff

The first commit of this branch added ~2,000 lines of unit tests for the pure helpers (design-system.test.ts, export.test.ts, handoff.test.ts, prompts.test.ts, validation.test.ts, web-capture.test.ts, claude/index.test.ts). A later revert dropped them and they were not restored. CLAUDE.md explicitly instructs contributors to write tests with bun test. At minimum, the pure functions should have coverage — they all have deterministic inputs/outputs and are the easiest kind to test:

• isUrl, isFilePath, aggregateImportResults (helpers/import.ts)
• isRefinementComplete (helpers/validation.ts) — currently the weakest link (see add agent instructions #1)
• slugifyPrompt, ensureScratchDir (helpers/design-system.ts)
• buildHandoffPrompt, buildReadme (helpers/export.ts)

---

MEDIUM

5. isFilePath heuristic silently drops valid references (helpers/import.ts:28-34)

The extension-detecting regex mis-classifies prompts like (notes about v1.2) as a file path. Conversely, codebase paths like src/components (no leading ./~ and no extension) return false for both isUrl and isFilePath, so the Import phase does nothing — despite the input description advertising URL, file path, or codebase path to import as design reference. Either narrow the input description or add a codebase-reference branch.

6. persistDesignSystem has no fallback when the builder agent does not write Design.md

If HIL is cancelled, the tool budget is exhausted, or the agent drifts, readFile(designPath) throws ENOENT and aborts the whole workflow with an unhelpful error. Consider catching ENOENT and surfacing a workflow-specific message (design system builder did not produce Design.md — rerun with --design-system= or re-approve via HIL), and/or persisting the transcript as a fallback source so the error points at what the agent actually produced.

7. .claude/settings.json flips repo-wide permission posture

skipDangerousModePermissionPrompt: true plus permissions.defaultMode: bypassPermissions means every Claude Code user opening this repo now inherits bypass-permissions by default. That may be intentional for a workflow-heavy repo, but it is a non-local security change bundled inside a feature PR. Worth calling out explicitly in the description and getting a second review from whoever owns the project security posture.

8. ensureScratchDir uses a dynamic import of an already-statically-imported module (helpers/design-system.ts:83-88)

readFile from node:fs/promises is statically imported at the top of the file. Just add mkdir to the existing import line instead of await import(...).

9. Output-directory collisions

finalDesignDir = isoDate-slug can collide across runs on the same day with similar prompts (the slug is capped at 6 words / 60 chars). deep-research-codebase disambiguates with startedAt.getTime(). Consider the same here so repeated runs do not silently overwrite.

---

LOW / NITS

10. HEADLESS_OPTS sets both permissionMode: bypassPermissions and allowDangerouslySkipPermissions: true. deep-research-codebase SUBAGENT_OPTS sets the same pair — consistent, but worth hoisting to a single shared constant in the SDK rather than duplicating per-workflow.

11. DESIGNS_DIR is exported from constants.ts but ensureScratchDir hardcodes research/designs and .scratch rather than composing them from constants.

12. buildReadme in helpers/export.ts emits a claude CLI invocation as the suggested handoff command. claude is not guaranteed on PATH for bundle consumers; consider a tool-agnostic phrasing or at least a Prerequisites: Claude Code CLI installed note.

13. copilot/index.ts and opencode/index.ts are throwing stubs. If the workflow registry can expose not yet supported for agent X at listing time, this is a nicer UX than a runtime throw after the user has already supplied inputs.

14. The PR bundles ~2,200 lines of research/spec docs (research/docs/, specs/). These are genuinely useful but make the diff harder to review. Splitting them into a docs-only PR is a common pattern that would let reviewers focus on the workflow itself.

---

WORKS WELL

• Topology mirrors deep-research-codebase (parallel headless fan-out + synthesis + deterministic TS aggregation) — good consistency.
• Prompts are position-aware with top/bottom request framing and an explicit TRAILING_PROSE_REMINDER, matching the convention already established in this repo.
• The IMPECCABLE_BANS constant being interpolated into both generation and refinement prompts is a clean way to keep anti-patterns enforced across stages.
• Deterministic handoff-bundle writes (writeHandoffBundle) correctly avoid an unnecessary LLM call.

Happy to look at a revision once the high-priority items are addressed — especially the isRefinementComplete tightening and restoring the pure-function tests.

[claude]

Code Review — open-claude-design

Solid, well-documented workflow. The topology comment, RFC, and prompt-builder structure are excellent. Most of my notes are around the refinement-loop exit detection, a handful of minor bugs/drift, and missing tests for the easy-to-test pure helpers.

Bugs / correctness

1. isRefinementComplete substring matching is dangerously loose — helpers/validation.ts:11-21. These signals are matched via raw .includes() against lowercased feedback:

   • "done" → fires on "I'm not done yet, tighten spacing"
   • "export" → fires on "Don't export the gradient to the icon"
   • "approved" → fires on "the disapproved color is still there"
   • "ship it" is fine, but "looks good" will fire on "the header looks good, everything else needs work"

   Given the loop silently jumps to Phase 5 on a false positive, this is the highest-impact bug in the PR. Recommend a single explicit sentinel (e.g. [REFINEMENT_COMPLETE]) emitted by the agent only on the "Approve and export" branch, matched by a plain includes(SENTINEL). The buildRefineFeedbackPrompt already instructs the agent to include "user approved" on approval — keep just that one exact-phrase match and drop the rest.

2. Dead code: VISIBLE_OPTS is defined in helpers/constants.ts:22-25 but never imported anywhere. Either wire it into the visible ctx.stage calls in claude/index.ts or delete it.

3. PR description / code drift — the description says "bounded HIL loop (max 8 iterations)" but MAX_REFINEMENTS = 5 in both code and spec (specs/2026-04-17-open-claude-design.md:531). Update the description.

4. Hardcoded scratch path drifts from DESIGNS_DIR — helpers/design-system.ts:85 hardcodes "research", "designs", ".scratch" instead of path.join(DESIGNS_DIR, ".scratch"). If DESIGNS_DIR ever changes, the scratch dir silently diverges.

5. Unnecessary dynamic import — helpers/design-system.ts:84 does const { mkdir } = await import("node:fs/promises") even though the file already does import { readFile } from "node:fs/promises" at the top. Lift mkdir into the static import.

6. persistDesignSystem produces an opaque ENOENT on failure — helpers/design-system.ts:45-51. If the Phase-1 builder stage ends without writing Design.md (model didn't follow step 6, user aborted HIL, etc.), Phase 2 fails with a bare fs error. Wrap with a clear message pointing at the builder stage contract.

7. finalDesignDir can collide — claude/index.ts:405 uses ${isoDate}-${slug}. Two runs of the same prompt in the same day overwrite each other's bundle silently. Append startedAt.getTime() (same pattern as deep-research-codebase).

8. README.md asserts files that may not exist — helpers/export.ts:118-125 unconditionally lists design-intent.md and component-specs.md in its Contents table, but those files are written by the LLM exporter stage (not writeHandoffBundle). If the LLM skips them, the README lies. Options: (a) check the files exist before listing, (b) fall back to deterministic stubs when missing, or (c) run writeHandoffBundle before the exporter so the README can rely on mkdir -p only.

9. reference has an invisible "neither URL nor path" bucket — claude/index.ts:230/251. If the user passes a reference that's e.g. a bare domain (example.com) or a hypothetical repo ref (org/repo), both Promise.all branches return null and the generator silently sees empty import context. Consider either warning up front or broadening detection.

10. isFilePath doesn't stat the path — helpers/import.ts:28-34. The file-parser stage then bounces off an LLM error. A cheap access() up front gives a clean diagnostic and shaves a headless query.

Consistency

11. Copilot/OpenCode stubs throw at runtime — copilot/index.ts:52-57 and opencode/index.ts:52-57 throw new Error(...) after inputs are parsed. Existing builtin workflows (ralph, deep-research-codebase) ship full implementations for all three providers; shipping throwing stubs is a regression of that pattern. Consider either implementing them (ideal) or gating registration so atomic workflow -n open-claude-design -a copilot ... fails cleanly at discovery with a user-friendly message, rather than post-parse.

12. Triplicate inputs block across the three provider files. Extract to a shared module so they stay in sync as the workflow evolves.

13. Magic filenames — index.html, styles.css, script.js appear in prompts.ts, export.ts, and writeHandoffBundle. One constant module keeps prompts and deterministic bundle layout aligned.

Test coverage

14. No unit tests for the pure helpers. Easy wins:

    • isRefinementComplete — document false-positive expectations after item 1 is fixed
    • slugifyPrompt — edge cases (empty, emoji, non-Latin, very long)
    • isUrl / isFilePath — boundary cases (file://, ~/x, ./a.b.c)
    • aggregateImportResults — contract documentation

    The RFC references test coverage but none landed. Given the workflow orchestrates 10+ LLM calls, the helpers are the cheapest layer to gain confidence at.

Security (informational)

15. HEADLESS_OPTS applies allowDangerouslySkipPermissions: true + bypassPermissions to web-capture (playwright against an untrusted reference URL) and file-parser (arbitrary user-supplied path). This matches existing workflow conventions, but combined with free-form user input, it's worth calling out in the docs — a malicious URL gets navigated with no sandbox opt-in, and a malicious path can be parsed. isUrl correctly restricts to http/https, which blocks file:// / javascript: — good.

Nits

• renderBans() emits the entire reflex-font list as a single bullet — splitting by category would make prompt compliance easier to audit.
• aggregateImportResults just shallow-copies its input; at the call site the literal object would be clearer than a pass-through function.
• HEADLESS_OPTS pins model: "sonnet" — worth keeping, but note it breaks symmetry with deep-research-codebase's SUBAGENT_OPTS (no model override). Maybe a comment pointing to the cost-optimization rationale in the RFC.

What's great

• Topology ASCII diagram in claude/index.ts:17-40 is exceptionally clear.
• Forward-only data flow with embedded upstream outputs — correct application of context-engineering principles.
• TRAILING_PROSE_REMINDER / HIL_INSTRUCTION constants prevent prompt drift across 11 stages.
• Deterministic writeHandoffBundle instead of a 12th LLM call to concatenate markdown is the right call.
• The RFC is detailed and the research docs make the design lineage auditable.

Happy to re-review once item 1 and the triplication in items 11/12 are addressed — the rest are polish/minor.

[claude]

Code Review: open-claude-design workflow

Overall this is a well-structured, thoughtfully documented workflow that mirrors the existing deep-research-codebase and ralph patterns nicely. Great ASCII topology in the header, clean phase separation, and good use of Promise.all for parallel fan-out. A handful of concerns worth addressing before merge.

🔴 Bugs / Correctness

1. Fragile loop-exit detection — high false-positive risk (helpers/validation.ts:11-21)
isRefinementComplete() substring-matches bare words like \"done\", \"export\", \"approved\", \"looks good\" against the lower-cased feedback transcript. The feedback stage's own prompt explicitly asks the agent to echo user choices, including phrases like "export the design", "the current state is done", or an option literally labeled "Run full critique" that includes the word "critique" but may include "export" in the follow-up summary. Any of these will prematurely exit the loop.

ralph solves the analogous problem with JSON parsing (parseReviewResult). Recommend a unique structured marker instead — e.g. instruct the agent to emit <REFINEMENT_APPROVED/> or a JSON block {\"action\":\"approve\"} and grep for that literal. It's a one-line prompt change plus a one-line validator change and eliminates an entire class of bugs.

2. Visible HIL stage gets headless model/permission override (claude/index.ts:309-329)
user-feedback-${iteration} has no headless: true flag (it's a visible HIL stage — the whole point is to present to the user), yet it passes { ...HEADLESS_OPTS } to s.session.query(), which forces model: \"sonnet\" and permissionMode: \"bypassPermissions\". The constant's doc-comment for VISIBLE_OPTS even explicitly says "Visible/creative stages: inherit orchestrator model (Opus). No model override." Either drop the spread or switch to VISIBLE_OPTS.

The exporter stage (line 420) has the same pattern — visible stage, headless opts spread in.

3. VISIBLE_OPTS is dead code (helpers/constants.ts:22-25)
Exported but never imported anywhere in the workflow. Either wire it into the visible stages (see #2) or remove it.

4. Design.md at repo root is silently overwritten (helpers/design-system.ts:45-51, prompts.ts:268-269)
The builder prompt instructs the agent to write to ${root}/Design.md without checking whether a user-curated file already exists, and persistDesignSystem() reads it back unconditionally. If a user runs this workflow in a repo where they've hand-tuned Design.md, it will be clobbered with no prompt. Recommend either: (a) check-and-ask when a file exists, (b) write to research/designs/.scratch/Design.md and only promote on explicit confirmation, or (c) document the overwrite behavior and add an --overwrite flag.

5. isFilePath regex over-matches (helpers/import.ts:33)
/\.\w{1,6}\$/ treats any string ending in .ext as a file path, so domain names without scheme (example.com, mycorp.co) silently route to the file parser. Either tighten to /^[./~]/.test(trimmed) only, or do a real access() check. Also ~ is not expanded — if the user passes ~/foo.png the downstream Read tool won't resolve it. Normalize via os.homedir() in the helper.

6. Handoff bundle written regardless of exporter outcome (claude/index.ts:407-432)
writeHandoffBundle() runs after the exporter stage unconditionally and generates a README referencing design/index.html, design/styles.css, etc. If the exporter stage failed or produced no files, the bundle is misleading (README points at missing files). Consider guarding on the expected files existing, or moving the bundle-write into the exporter's deterministic post-step.

7. MAX_REFINEMENTS exit is silent (claude/index.ts:307-399)
When the 5-iteration cap is hit without user approval, the loop falls through to export with no notification. At minimum surface a message ("refinement cap reached — exporting current state") and consider requiring explicit approval even at cap.

🟡 Quality / Style

8. aggregateImportResults is an identity wrapper (helpers/import.ts:40-52)
It receives an object with fields prompt/reference/webCapture/fileParse and returns an object with the identical shape. Per CLAUDE.md's "avoid introducing abstractions beyond what the task requires" — inline it or delete.

9. Prompt size (helpers/prompts.ts — 993 lines)
Each builder already generates a sizable prompt; when these are concatenated with upstream stage outputs (locatorOutput, analyzerOutput, patternsOutput all verbatim) the design-system-builder prompt will be very large. Worth considering a size budget or truncation for the fan-in stage. Not a blocker.

10. outputTypeInstructions[opts.outputType] is string | undefined (helpers/prompts.ts:470)
Since output-type is an enum validated by the input schema, it will always be one of the four known keys. Type narrowing would remove the ?? outputTypeInstructions.prototype fallback — or a Record<OutputType, string> with OutputType = \"prototype\" | \"wireframe\" | \"page\" | \"component\" imported from a shared types module would make this safer per the CLAUDE.md rule "Avoid ambiguous types."

11. designDir (scratch) collides across runs of the same prompt (claude/index.ts:126-130)
${scratchDir}/${slug} is reused if the same prompt is re-run; stale files from a prior run can leak into a new one. Same for ${isoDate}-${slug} in the final dir on a same-day re-run. Add a timestamp component (slug-${Date.now()}) or clear the dir first.

12. Subagent names referenced in prompts aren't validated
The workflow dispatches agent: \"reviewer\", codebase-locator, codebase-analyzer, codebase-pattern-finder, codebase-online-researcher. If any of these aren't installed in the running project's .agents/, the stage will fail opaquely. deep-research-codebase has the same exposure, so not a blocker for this PR — but worth a doc note in the README/spec.

🟢 Security

13. Web-capture launches user-supplied URLs under bypassPermissions (claude/index.ts:231-247)
This is intentional for the unattended flow and consistent with the other builtin workflows, but isUrl only validates http(s) — no host allowlist, no confirmation. Worth a line in the README describing the trust model.

🔵 Tests

14. No unit tests on pure helpers
isUrl, isFilePath, slugifyPrompt, isRefinementComplete, aggregateImportResults, and buildHandoffPrompt/buildReadme in export.ts are trivial to test with bun test and cover most of the correctness bugs above (esp. #1, #5). Consistent with existing builtin workflows not having tests, but these helpers genuinely warrant coverage given the loop-exit stakes in #1.

✨ Nits

• constants.ts:44 bans Fraunces alongside reflex display fonts — it's a genuine workhorse serif, not a "reflex" pick. Worth a second pass on that list.
• Phase 2 prompts mention liteparse as a tool the file-parser agent can use — confirm it's available in the sub-agent tool set.
• claude/index.ts:405 computes finalDesignDir via ${isoDate}-${slug} but if DESIGNS_DIR doesn't exist the first mkdir inside writeHandoffBundle creates it — fine, just an observation.

Summary

The architecture is sound, parallelism is correctly applied, and the documentation (spec + RFC + research docs) is exceptional. The blocking concerns are #1 (fragile completion detection), #2 (headless opts on a visible HIL stage), and #4 (silent Design.md overwrite) — each is a small change. Everything else is polish.

Nice work — once #1/#2/#4 are addressed this is in great shape.

[claude]

Review — open-claude-design workflow

Thanks for the thorough RFC and phased breakdown — the topology is clear and the deterministic handoff bundle is a nice touch. Below is constructive feedback grouped by area, with a few items I'd consider blockers for correctness.

Bugs / correctness

1. isRefinementComplete will false-positive constantly (helpers/validation.ts:11-21). The signal list includes "done", "export", "approved" — extremely common English words that appear in the feedback prompt itself and in the agent's trailing prose. Worse, the buildRefineFeedbackPrompt enumerates "Approve and export" and "Request specific changes" as AskUserQuestion options, and instructs the agent to echo the user's choice. If the user picks "Request specific changes" but the agent's echo contains the literal option label "Approve and export" — or even just "you can export later" — the loop terminates after iteration 1. Tighten this to a single, unambiguous marker (e.g., require OPEN_CLAUDE_DESIGN_COMPLETE::APPROVED on its own line) and have the prompt emit that sentinel only on the approve branch.

2. Visible HIL stages run on Sonnet with bypass-permissions (claude/index.ts:324, 420). user-feedback-${iteration} and exporter are visible stages (no headless: true), yet both pass { ...HEADLESS_OPTS } into session.query, which forces model: "sonnet" and permissionMode: "bypassPermissions". The README and constants.ts both state visible/creative stages should inherit Opus. Either drop the options object (so Opus inherits) or use VISIBLE_OPTS — currently VISIBLE_OPTS is exported but never imported, which is the smoking gun that these two call sites picked the wrong constant.

3. isFilePath is too permissive (helpers/import.ts:33). /\.\w{1,6}$/ matches any two tokens joined by a dot, so prompts like "design for app.v3" or "dashboard for section.a" are routed through file-parser, which will then fail to Read a non-existent file. Tighten to require a leading path char (^[./~] or contains /), or actually stat the candidate before classifying.

4. persistDesignSystem has no fallback (helpers/design-system.ts:45-51). If the HIL builder stage was cancelled or the agent wrote Design.md somewhere else, readFile throws a raw ENOENT at the user and all downstream phases are lost. Consider catching and emitting a workflow-level error with actionable guidance, or verifying the file before returning.

5. Final export directory silently overwrites (claude/index.ts:405). ${isoDate}-${slug} collides on same-day repeat runs with similar prompts. writeHandoffBundle uses mkdir recursive and writeFile which blindly overwrite. At minimum add an incrementing suffix when the directory exists, or include a short hash/timestamp.

6. Scratch directory is shared and never cleaned (design-system.ts:83-88 + claude/index.ts:129). Re-running the same prompt reuses research/designs/.scratch/<slug>/ from the previous run — stale index.html/styles.css can leak into the new generation. Consider per-run timestamp under the slug, or clear before write.

Code quality

7. Duplicate input schema across provider stubs (claude/index.ts:87, copilot/index.ts:20, opencode/index.ts:20). The same ~30-line inputs array is repeated verbatim three times. Hoist to helpers/inputs.ts and import — otherwise adding a new input means editing three files and will silently drift.

8. aggregateImportResults is a pass-through (helpers/import.ts:40-52). It accepts {prompt, reference, webCapture, fileParse} and returns the same shape with no transformation. Either inline the object literal at the call site or give the helper a real job (e.g., normalize empty strings, compute a hasReference flag).

9. Dynamic import for mkdir (design-system.ts:84). ensureScratchDir does const { mkdir } = await import("node:fs/promises") even though readFile is already statically imported from the same module. Add mkdir to the top-level import.

10. VISIBLE_OPTS is dead code. See bug updates to readme and instructions #2 — either remove or wire it up.

11. Redundant flags on RegExp (helpers/validation.ts:34). Input is already lower-cased one line above, so the "i" flag is pointless.

12. outputTypeInstructions[opts.outputType] ?? outputTypeInstructions.prototype (prompts.ts:470). outputType is already an enum-constrained string — the fallback can never fire. Use a Record<typeof outputType, string> typed literal and drop the fallback; TypeScript will enforce exhaustiveness.

13. IMPECCABLE_BANS typing. as const is good but the items are prose strings; the BAN 1/BAN 2 prefixes on only some items reads inconsistently (items 3–4 don't have a number). Either number all four or none.

Performance / robustness

14. Parallel import fan-out always does Promise.all with nulls. Not a bug — but since at most one branch runs, a simple if/else is cheaper to read and equivalent.

15. No timeout / cancel on refinement loop. Five iterations × (feedback + critique + screenshot + apply) = up to 20 nested agent invocations. If any one hangs, the whole workflow hangs. Worth confirming the Atomic stage infra enforces timeouts here (ralph has a similar shape so maybe it's fine, but document it).

16. Screenshot file path is shared across iterations (prompts.ts:677). Every screenshot-${iteration} stage writes to ${scratchDir}/screenshot-validation.png, so each iteration clobbers the prior one — fine for the current flow, but if future work wants to diff iterations, make the filename iteration-specific.

Security

17. loadDesignSystem path escape (design-system.ts:29). Uses path.resolve(designSystemPath) with no confinement to project root. For a user-supplied path the current behaviour is expected, but flag it in the input description so the user knows the workflow will read arbitrary files from disk.

18. Prototype server path check is good. server.ts in buildPrototypeAssetsBlock correctly uses normalize + startsWith(ROOT) to prevent traversal. 👍

Tests

19. Zero tests added for 4,142 lines. The pure helpers are trivially testable and have obvious edge cases already surfaced above:

    • isUrl — "https://…", "http://" without host, "ftp://", whitespace.
    • isFilePath — see bug update readme and mcp servers #3; add cases for "app.v3", "./foo.css", "~/bar", "plain prompt".
    • slugifyPrompt — empty string → "design", unicode, 60-char cap.
    • isRefinementComplete — bug add agent instructions #1 false-positive cases.
    • aggregateImportResults — once it does something beyond pass-through.
    • writeHandoffBundle — snapshot the generated README/handoff-prompt against fixtures.

    Per CLAUDE.md (bun test + test-driven-development skill) these are expected.

Nits

• The ASCII topology diagram at the top of claude/index.ts shows the loop arrow returning to the top of the workflow — should return to user-feedback-i, not all the way up.
• helpers/constants.ts:44 — the long font-ban string would be cleaner as an array of fonts joined at render time.
• buildPrototypeAssetsBlock emits a package.json with a literal <slug> placeholder that the agent is asked to replace — safer to interpolate the slug at build time since we have it in TS scope already.

---

Overall: the architecture is well-reasoned and the spec docs are strong. The two items I'd block on are #1 (loop will exit early almost always) and #2 (visible stages silently downgraded to Sonnet). The rest are polish.

[claude]

Code Review — open-claude-design workflow

Nice piece of work — the 5-phase pipeline, headless/visible split, and deterministic handoff bundle all make sense. The prompt engineering (positional framing, trailing-prose reminder, explicit HIL-via-AskUserQuestion) is thoughtful, and reusing existing specialist agents (codebase-locator, codebase-analyzer, reviewer, etc.) keeps the design consistent with deep-research-codebase. A few substantive issues to resolve before merging.

High priority

1. Visible/HIL stages are forced onto Sonnet, contradicting the architecture table

The PR description and constants.ts comments both say visible/creative stages inherit the orchestrator model (Opus), and the architecture table lists user-feedback and exporter under Opus. But the code applies HEADLESS_OPTS (which hard-codes model: "sonnet") at:

• claude/index.ts:324 — user-feedback-${iteration} (visible HIL)
• claude/index.ts:420 — exporter (visible)

Either pass VISIBLE_OPTS (which intentionally omits model) or nothing. As-is, the declared model routing and the effective model routing disagree — and the unused VISIBLE_OPTS export in constants.ts:22 is dead code, reinforcing that this was a wiring slip.

2. isRefinementComplete false-positives on normal assistant prose

helpers/validation.ts:11-21 treats the assistant's full extracted text as the completion signal. Signals like "done", "export", "approved", "looks good" are high-frequency words in ordinary recaps — e.g. the feedback agent's own summary "I'm done gathering feedback; I'll pass your request to the apply-changes stage to export..." contains three triggers. That exits the loop before iteration 1 even runs.

Recommend restricting to an explicit, unambiguous marker (REFINEMENT_COMPLETE sentinel the prompt instructs the agent to emit, or a leading-line match on /^user approved/i). The prompt at prompts.ts:563-564 already asks the agent to include "user approved" — make that the only accepted signal, and check it exists as a standalone token rather than a substring.

3. Reference input is silently dropped for valid-but-unrecognized shapes

The PR description advertises --reference as "URL, file path, or codebase path", but claude/index.ts:228-270 only runs a stage when isUrl() or isFilePath() matches. Inputs like src/components/Button (no extension, no leading ./) fall through both branches and the reference is lost — no warning, no error, the generator just proceeds without it. Either add a third branch that treats the value as a codebase path, or error out when the reference is non-empty but unclassified.

4. isFilePath false-positives on bare domains

helpers/import.ts:33 — /\.\w{1,6}$/ matches example.com, site.io, company.co.uk → all classified as file paths. If a user forgets the https:// prefix on a URL, the file-parser stage runs against a nonexistent path and fails. A quick existsSync check before picking the branch, or rejecting hosts that look like domains (contain a dot and no path separator), would make this robust.

Medium

5. persistDesignSystem error is opaque

helpers/design-system.ts:45-51 calls readFile(${root}/Design.md) immediately after the builder stage. If the agent didn't write the file (user aborted, permission issue, model drift), the user sees a raw ENOENT. Wrap with a message that points to what happened and how to recover (e.g. "design-system-builder did not produce Design.md — rerun with --design-system=<path> to skip onboarding").

6. let designSystem; in strict mode

claude/index.ts:136 — with "strict": true + "noImplicitAny", an uninitialized let is an implicit-any unless flow analysis assigns on every path (which it does here). Even so, annotate it: let designSystem: DesignSystemData; — CLAUDE.md's "avoid ambiguous types" rule applies.

7. Dynamic import of node:fs/promises inside a helper

helpers/design-system.ts:84 — const { mkdir } = await import("node:fs/promises"); is done dynamically, even though the rest of the file already imports from that module statically at the top. Move it up with readFile.

8. The three-file input-spec duplication across providers

copilot/index.ts and opencode/index.ts copy-paste the entire inputs array. Any future change to the input schema will silently desync if only one is updated. Extract the inputs into helpers/inputs.ts and import from all three providers. (This pattern is also present in deep-research-codebase and ralph, so fixing it repo-wide is a separate PR — but worth noting.)

Minor / style

• VISIBLE_OPTS exported but unused (helpers/constants.ts:22-25). Either wire it into the visible HIL stages (see add agent instructions #1) or remove.
• outputTypeInstructions[opts.outputType] ?? outputTypeInstructions.prototype (prompts.ts:470) — unreachable fallback because the input is validated as an enum. Harmless but dead.
• Handoff-prompt CSS placeholder (export.ts:93-97) — emits a :root {} block with only a comment; reads like broken CSS to a human. Either populate with the tokens parsed from Design.md or remove the code block and reference Design.md in prose.
• persistDesignSystem naming — the function only reads; "persist" implies it writes. loadPersistedDesignSystem or readDesignSystemFromRoot matches intent.
• Bun server path-traversal check (prompt at prompts.ts:911-923) is correctly constructed (normalize + startsWith(ROOT)), but note that the server is emitted by the LLM exporter stage, not written deterministically. If you care about the security property, write the server.ts from helpers/export.ts the same way you write handoff-prompt.md — don't trust the LLM to reproduce it exactly.

Testing

Zero tests added. Consistent with ralph/deep-research-codebase — but several helpers are pure and trivial to cover with bun test:

• slugifyPrompt (edge cases: empty, unicode, 60-char truncation)
• isUrl / isFilePath (covers issues update readme and mcp servers #3 and Flora131/feat/add skills #4 above)
• isRefinementComplete (would have caught updates to readme and instructions #2)
• aggregateImportResults (shape contract)

A small helpers/*.test.ts file alongside each helper would pay for itself the first time one of these functions breaks downstream.

Security

No critical concerns. bypassPermissions + allowDangerouslySkipPermissions is consistent with sibling workflows. The Bun server template handles path traversal correctly; my only note is that its correctness depends on the LLM reproducing it verbatim (see the minor bullet above).

Summary

Solid architecture and prompt design. Main blockers: model-routing wiring (#1), completion-signal false positives (#2), reference handling gaps (#3, #4). Medium-priority cleanups around error messages and type annotations. Once those are in, this is a valuable addition.

[claude]

Review — open-claude-design workflow

Overall: the architecture is well-factored (clear phase boundaries, helpers split by responsibility, parity between Claude and Copilot adapters). Below are issues found with a focus on correctness and code quality.

---

Potential bugs

1. isRefinementComplete is too permissive — likely short-circuits the refinement loop — helpers/validation.ts:11

const COMPLETION_SIGNALS = [
  \"approve and export\", \"approved\", \"looks good\",
  \"ship it\", \"done\", \"export\", ...
]

The signals \"done\", \"export\", \"approved\", and \"looks good\" are extremely common in design-feedback prose. Worse, the feedback prompt itself (helpers/prompts.ts:555) instructs the agent to present the option string "Approve and export — finalize the design and create handoff bundle". The moment the agent echoes that option set, \"approve and export\" matches and the loop exits — likely on iteration 1 regardless of user choice.

Suggested fix: keep only the narrow sentinel (\"user approved\") the prompt is already instructed to emit, and drop the liberal keywords. Add a unit test that exercises several realistic transcripts.

2. Path-traversal guard in generated server.ts template is off by one separator — helpers/prompts.ts:917

if (!resolved.startsWith(ROOT)) return new Response(\"Forbidden\", { status: 403 });

If ROOT is /tmp/design, a normalized path /tmp/designA/foo satisfies startsWith but escapes the directory. Use resolved === ROOT || resolved.startsWith(ROOT + sep). Low severity (prototype, local-only), but this is boilerplate that will be copy-pasted.

3. Command-injection surface in generated README — helpers/export.ts:141

claude \"$(cat handoff-prompt.md)\"

The user's raw prompt is embedded verbatim into handoff-prompt.md. If the prompt contains backticks or $(...), bash expands them in the subshell before claude sees them. Prefer claude < handoff-prompt.md (or the tool's file-input flag).

---

Code quality

4. VISIBLE_OPTS is defined but never imported — helpers/constants.ts:22. Dead code; remove or wire it in.

5. Passing HEADLESS_OPTS to a visible session is misleading — claude/index.ts:324 (the user-feedback-${iteration} stage).

The stage is interactive (no headless: true); ClaudeSessionWrapper#query silently ignores SDK options (src/sdk/providers/claude.ts:999-1008). So permissionMode/model: \"sonnet\" do nothing here. Dropping the argument makes the intent match the runtime behavior.

6. Implicit any on designSystem — claude/index.ts:136, copilot/index.ts:137

let designSystem;

CLAUDE.md explicitly calls out: "Avoid ambiguous types like any and unknown." Annotate as let designSystem: DesignSystemData;.

7. HandoffBundleOptions.outputType: string should be narrowed — helpers/export.ts:20. The input is enum-constrained to \"prototype\" | \"wireframe\" | \"page\" | \"component\"; reuse that union so the if (outputType === \"prototype\") switches are exhaustiveness-checked.

8. Inconsistent node:fs/promises import style — helpers/design-system.ts:84

ensureScratchDir does const { mkdir } = await import(\"node:fs/promises\") while readFile is imported statically at the top. Make it consistent (top-level static import).

9. aggregateImportResults is a pure pass-through — helpers/import.ts:40. It just returns its argument object. Either inline the literal at the call site or document what the helper is for (e.g., a place to hang future normalization).

10. .scratch dir is never cleaned up. Each run writes research/designs/.scratch/<slug>/ and leaves it. Consider cleaning (or rotating) on success, or document the intended lifetime.

11. Copilot: !m.data.parentToolCallId relies on falsy coercion — copilot/index.ts:77. If the SDK ever emits an empty string vs null/undefined, both currently work — but m.data.parentToolCallId == null is more defensive against future typing.

---

Test coverage

No tests accompany the PR. The deterministic helpers are straightforward to unit-test and would guard against the issues above:

• isRefinementComplete — cover transcripts that mention "export" / "done" casually
• slugifyPrompt — unicode, punctuation, empty strings
• isUrl / isFilePath
• writeHandoffBundle — files created at expected paths
• aggregateImportResults

CLAUDE.md encourages bun test and the testing-anti-patterns skill for this.

---

Nice-to-haves (non-blocking)

• process.cwd() as the project root (claude/index.ts:125) — deep-research-codebase uses a dedicated getCodebaseRoot(). Consider reusing that for consistency across built-in workflows.
• Signal-phrase hard-coding: lifting COMPLETION_SIGNALS to a single source of truth that the feedback prompt and the detector both read from would prevent drift.
• TRAILING_PROSE_REMINDER and HIL_INSTRUCTION are great — consider documenting them briefly where buildRefineFeedbackPrompt diverges (FIRST — before any tool call…) so future authors don't re-invent.

[claude]

PR Review — feat(workflow): add open-claude-design builtin workflow

Thanks for the thorough spec and well-structured workflow. The implementation cleanly follows the patterns established by deep-research-codebase and ralph, and the 5-phase decomposition is easy to follow. Below is feedback grouped by severity.

---

🔴 High — loop exit detection has serious false-positive risk

helpers/validation.ts → isRefinementComplete() matches any of "approved", "ship it", "done", "export", "approve and export", etc., as a case-insensitive substring of the agent's full assistant transcript.

The feedback-stage prompt (buildRefineFeedbackPrompt) explicitly tells the agent to echo all four AskUserQuestion options back to the user — options that contain the literal strings "Approve and export" and "Request specific changes". As soon as the agent echoes its own multi-choice options prompt, the transcript will contain \"approve and export\" and isRefinementComplete() returns true on iteration 1 regardless of what the user actually chose.

\"export\" as a bare substring is especially loose — any phrase like "once you approve, the exporter will..." would trigger exit.

Suggested fixes:

• Have the agent return a structured JSON decision ({\"decision\":\"approve\"|\"refine\"|\"abort\",\"feedback\":\"...\"}) via its last assistant message and parse it. This is exactly what ralph/helpers/review.ts::hasActionableFindings does with ReviewResult.
• Or, require an unambiguous sentinel like <COMPLETION_SIGNAL>approved</COMPLETION_SIGNAL> and match on the tag, not the keyword.
• At minimum, drop \"export\" / \"done\" / \"approve and export\" (which is a literal option label) from the signal list.

There are no tests for this helper; a single unit test would catch it immediately.

---

🟡 Medium

1. VISIBLE_OPTS is dead code. Declared in helpers/constants.ts but never imported anywhere. Either remove it or wire it up to the visible stages for consistency.

2. Missing test coverage. No tests exist for any of: slugifyPrompt, isUrl, isFilePath, aggregateImportResults, isRefinementComplete. These are all pure functions, trivial to cover with bun test, and the bug above is a good motivation.

3. isFilePath misclassifies bare hostnames.

   return /^[./~]/.test(trimmed) || /\.\w{1,6}$/.test(trimmed);

   Input example.com → isUrl returns false (no scheme) → isFilePath returns true (matches .com extension heuristic). Web capture is skipped, file parser tries to read example.com and fails. Consider requiring a path separator or a leading ./ / / / ~/, and either reject bare hostnames or coerce them through isUrl with a https:// prefix.

4. screenshot-{i} uses the wrong sub-agent. The prompt instructs "Use playwright-cli to open index.html" but binds agent: \"codebase-analyzer\", whose default tool set (Grep, Glob, Read, Bash, LSP) does not include playwright-cli. bypassPermissions may incidentally allow it, but semantically this should use codebase-online-researcher (which you already use for web capture and which does list Bash(playwright-cli:*) in its tools).

5. HIL prompts may not work on Copilot. The prompts hard-code AskUserQuestion instructions (HIL_INSTRUCTION in helpers/prompts.ts) and those are shared verbatim across providers. Copilot CLI does not expose a tool with that exact name/semantics. For the Copilot path these steps will degrade to plain natural-language questions, which defeats the intent. Either add a provider-neutral HIL hint ("use the user-question tool available in your environment") or branch the prompt per provider.

6. Unbounded context embedding. designSystem.raw and existingImpeccable are embedded verbatim into every downstream prompt. If Design.md or .impeccable.md grow (which they will across iterations), every stage pays the full token cost and may blow context. Consider a byte/token cap with a truncation notice, or extract a summary once and pass the summary forward.

---

🟢 Low / polish

1. ensureScratchDir uses a dynamic import(\"node:fs/promises\") even though mkdir is already statically imported in the same file's callers (claude/index.ts, copilot/index.ts, export.ts). Make it a static top-level import — both for readability and to match the rest of the codebase.

2. Prompt-injection surface. User-supplied prompt and reference are inlined inside XML-like tags (<DESIGN_REQUEST>…</DESIGN_REQUEST>) with no escaping. Low impact in a local CLI where the user is the operator, but worth an XML-attribute-style escape (e.g. replace </DESIGN_REQUEST> in user content) if this is ever invoked over a non-trusted channel.

3. Inconsistent ban labels. IMPECCABLE_BANS lists BAN 1: and BAN 2: but the remaining two entries are unnumbered. Either number all four or drop the prefix entirely.

4. No safety net if Design.md write fails. persistDesignSystem() does a bare readFile(path.join(root, \"Design.md\")) immediately after the builder stage. If the agent doesn't actually write the file (tool refused, wrong path, early exit), the whole workflow crashes with an opaque ENOENT. A friendlier error message ("Design.md was not written by the builder stage — re-run or provide --design-system") would save users time.

5. slugifyPrompt can cut mid-word when substring(0, 60) runs after join(\"-\"). Trim trailing - and avoid partial words if trivial.

6. Duplicated getAssistantText helper. Already identical to the ones in deep-research-codebase/copilot/index.ts and ralph/copilot/index.ts. Consider promoting it into a shared helper under src/sdk/workflows/ — the three copies will drift.

7. writeHandoffBundle doesn't verify the exporter actually copied the design files. If the exporter LLM hallucinates success but skips the copy, the bundle ships with an empty design/ directory. Since the source paths are known deterministically, this could be done in TypeScript rather than delegated to the exporter. That is also what the export.ts docstring says ("deterministic TS helper"), so moving the three copyFile calls into writeHandoffBundle would align behavior with the documented contract.

---

✅ Things I liked

• Clear phase decomposition with a topology diagram in the header.
• Good context-engineering hygiene: trailing-prose reminder, bans embedded at the prompt level, position-aware framing.
• Reusing established patterns (HEADLESS_OPTS, ctx.stage, Promise.all fan-out) keeps the workflow consistent with its siblings.
• Thoughtful output-type handling in the exporter (prototype vs wireframe vs page vs component) with a runnable Bun server that has proper path-traversal protection.
• Excellent RFC and research docs — easy to understand the rationale.

Overall this is a solid foundation — the main blocker for me is the isRefinementComplete false-positive bug, since it will silently exit the refinement loop on the very first iteration. Everything else is tractable.

[claude]

Code Review — open-claude-design workflow

Thorough review of the new workflow. Overall this is well-structured: clean separation between deterministic helpers and LLM stages, good documentation, the forced-fix gate is a nice safety net, and the three-provider implementations mirror each other carefully. A handful of concerns worth addressing before merge.

🔴 Correctness

1. PR description contradicts the code (opencode/index.ts).
The description says "OpenCode adapter stub for future support," but the file is a full 570-line implementation. Either the description is stale, or the file is provisional but documented in helpers/prompts.ts as production-ready. Please reconcile — if the opencode provider isn't actually ready/tested, add a runtime guard (e.g. throw new Error("opencode provider not yet supported")) rather than shipping a silent half-implementation.

2. isRefinementComplete() — false-positive risk on "export". helpers/validation.ts:18

const COMPLETION_SIGNALS = [... "export", ...];

"export" is far too loose: the refinement feedback prompt literally contains the option label "Approve and export", and the agent naturally echoes words like "ready to export later" or "before export." Any such phrasing exits the loop even when the user didn't approve. Recommend removing the bare "export" and keeping only the specific "approve and export" / "user approved" phrases, or better — have the feedback prompt emit a sentinel string the agent must include verbatim (e.g. <REFINEMENT_STATUS>approved</REFINEMENT_STATUS>).

3. Redundant Extract<> casts after a type predicate. claude/index.ts:429-431, 455-457 (and mirrored in copilot/, opencode/)

if (hasBlockingFindings(preExportScan)) {
  const findings = (preExportScan as Extract<typeof preExportScan, { available: true }>).findings;

hasBlockingFindings is already a user-defined type guard (scan is { available: true; findings: ... }), so TS already narrows preExportScan inside the block. The Extract<> cast is dead weight and actively harmful because it masks a type regression if the predicate ever loses its is-annotation. Drop the cast: const findings = preExportScan.findings;

4. isFilePath is permissive in ways that may surprise users. helpers/import.ts:33

return /^[./~]/.test(trimmed) || /\.\w{1,6}$/.test(trimmed);

A reference like v1.2 or my.app matches as a "file path" and gets fed to the file-parser stage, which will then fail on a missing file. Consider requiring at least a path separator or confirming the file exists synchronously (e.g. existsSync) before classifying.

🟡 Code quality

5. Unnecessary dynamic import() in ensureScratchDir. helpers/design-system.ts:84

const { mkdir } = await import("node:fs/promises");

Already imported statically at the top of every call site. Move the import to the top of the file.

6. let designSystem; lacks annotation. claude/index.ts:142, mirrored in the other two providers. TS infers, but an explicit let designSystem: DesignSystemData would match the rest of the codebase's style and help future readers.

7. isoDate captured at workflow start. claude/index.ts:133
The refinement loop is HIL-gated and can run for hours or straddle UTC midnight. The finalDesignDir path then embeds a "wrong" date. Consider computing isoDate just before building finalDesignDir.

8. HEADLESS_OPTS has both permissionMode: "bypassPermissions" and allowDangerouslySkipPermissions: true. helpers/constants.ts:20-22. These are redundant in the Agent SDK — bypassPermissions already skips the permission prompt. Keeping both isn't harmful but muddles the intent.

9. model: "sonnet" shorthand. helpers/constants.ts:22. CLAUDE.md explicitly calls out the current canonical IDs (claude-sonnet-4-6). The SDK may accept the alias today but pinning would be more robust and survives the next alias re-resolution.

🟢 Minor

10. No tests. The helpers are pure functions (isUrl, isFilePath, aggregateImportResults, slugifyPrompt, isRefinementComplete, renderScanFindings, hasBlockingFindings) that are trivial to unit-test — and isRefinementComplete's signal set in particular is exactly the kind of regex/substring logic that benefits from table-driven tests. CLAUDE.md calls out testing as expected practice; other builtin workflows in tests/sdk/ are untested too, so this matches precedent, but given the loose completion-signal concern above, a few targeted tests would pay off.

11. Scanner is a soft dependency. runImpeccableScan swallows "scanner missing" gracefully and proceeds, but the hasBlockingFindings gate only triggers when the scanner runs AND returns findings. Consider logging prominently when the scanner is unavailable during the pre-export check — users may be surprised to learn their design shipped without being scanned.

12. Prompt word "done" in COMPLETION_SIGNALS. Similar to "export" — it's a generic word an agent might say in many non-approval contexts ("I'm done with the critique"). Consider tightening.

✅ Things I liked

• Clean helper split: constants / design-system / export / import / prompts / scan / validation. Each has a single responsibility and is import-safe across all three providers.
• Forced-fix gate + rescan + throw is a solid defense-in-depth pattern — prevents the "user approved early, banned patterns leak out" failure mode.
• Prompts inline the design rules verbatim rather than relying on /impeccable skill loading — robust against missing skills.
• Bun.spawn + JSON parse for the scanner is the right call (no eval, bounded output, path-sanitized).
• writeHandoffBundle is deterministic TS, not an LLM call — matches the description and keeps the bundle reproducible.
• Per-provider failure-mode comments in copilot/index.ts (F1/F5/F9) and opencode/index.ts (F3/F5/F6/F9) are excellent — these are exactly the non-obvious gotchas a future editor needs to know.

Questions

• Was this tested end-to-end on all three providers, or only claude? The provider-specific workarounds (F1/F5/F9/F3/F6) are detailed enough to suggest real testing, but confirming would help.
• DESIGNS_DIR = "research/designs" — is it intentional that the final design output lives under research/? That directory usually holds research docs (the two big .md files in this PR, for example). A top-level designs/ or .atomic/designs/ might be clearer.

Nice work overall — the pipeline structure, the forced-fix gate, and the per-provider parity are the strongest parts.