feat(workflows): add coding-backwards-design workflow for copilot and claude agents

[pranavsankar2]

Summary

Adds coding-backwards-design workflows for the Copilot and Claude agents, codifying the "Coding Backwards" methodology into an automated, 7-stage generative pipeline for design-driven UI implementation. Also includes a reference implementation — a fully-built Big Sur–themed portfolio site — generated by running the workflow against jamesbuckhouse.com.

Changes

Workflows

Copilot variant (.atomic/workflows/design-driven/copilot/index.ts):

• Typed via defineWorkflow<"copilot"> and finalized with .compile()
• 7-stage pipeline: Design Discovery → Design Critique → Architecture Plan → Scaffold → Progressive Build → Visual Analysis → Documentation
• Uses defineTool with Zod schemas for structured outputs (component plans, review results) since Copilot lacks a native json_schema output mode
• Runs per-phase Playwright verification gates with impeccable anti-pattern detection after each priority phase
• Ralph-style bounded QA loop (max 5 iterations) with parallel visual analysis + dual reviewers + debugger

Claude variant (.atomic/workflows/design-driven/claude/index.ts):

• Typed via defineWorkflow<"claude">, uses @anthropic-ai/claude-agent-sdk directly for structured outputs via json_schema output format
• Same 7-stage pipeline as Copilot variant, adapted for Claude's session model (s.session.query vs s.session.send/sendAndWait)
• Stage 5 uses queryWithComponentPlan with JSON schema to extract a structured ComponentPlan, driving parallel component builds via captureBranchChangeset
• Stage 6 uses queryWithStructuredOutput for visual analysis and dual reviewers; merges all three review sources via mergeReviewResults
• Delegates infra discovery to named sub-agents via @"agent-name (agent)" syntax

Both variants share the same inputs (spec, design_reference, dev_command) and reuse helpers from src/sdk/workflows/builtin/ralph/.

Generated Site (Workflow Output)

Reference implementation produced by running the workflow:

• preview/index.html — single-page portfolio with 45 artworks, 12 film posters, 64 library resources, Delphi embed, and About section
• preview/server.js — local static file server for the preview
• src/css/ — complete CSS design system: tokens (colors, spacing, radii, shadows, transitions, typography), component styles, utilities, and interaction states
• src/js/ — vanilla JS component modules (Navbar, GalleryGrid, ArtworkCard, Lightbox, HotlineEmbed, LibrarySection, FilterBar, FilmSection, Footer, router, main) plus data files with unit tests

Config

• bunfig.toml — adds coverage exclusions for src/css/**, src/js/**, and preview/** (generated site assets, not part of the core app)

Notes

• Requires the playwright-cli skill to be available to the agent for Stage 1 (visual analysis) and the QA pass
• dev_command is optional — stages fall back to bun run preview/server.js if not provided
• The generated site preserves all original content (real image URLs, text, routing) and applies only visual changes
• opencode variant is not included in this PR

[claude]

Claude's Review

Thanks for adding this — codifying "Coding Backwards" into a reproducible pipeline is a neat use of the staged workflow API. Leaving findings below, ordered by severity.

Blocking

1. defineWorkflow<"copilot">({...}) is not a valid API. src/sdk/define-workflow.ts:186 only declares one generic, and it is for the inputs tuple (const I extends readonly WorkflowInput[]), not the agent type. Passing "copilot" as that generic should fail type-checking because "copilot" does not extend readonly WorkflowInput[].

Every other workflow in .atomic/workflows/**/copilot/index.ts uses the fluent form — see .atomic/workflows/hello-world/copilot/index.ts:43:

export default defineWorkflow({ name: "…", inputs: [...] })
  .for<"copilot">()
  .run(async (ctx) => { … })
  .compile();

Please apply the same pattern here; otherwise bun typecheck should fail.

2. Load-bearing inputs are required: false but used unconditionally. target_url and design_reference are interpolated straight into prompts with no null check or default:

prompt: `…crawl ${ctx.inputs.target_url}. …`
prompt: `…target aesthetic: "${ctx.inputs.design_reference}". …`

If a user omits them, the agent gets crawl undefined and aesthetic: "", which silently degrades the whole pipeline. Either set required: true or guard with a fail-fast check at the top of .run. dev_command is genuinely optional and already handled via devInstructions — good.

Important

3. Prompt-injection surface via design_reference. It is a type: "text" multi-line input interpolated verbatim into multiple downstream prompts (critique, DESIGN_README, progressive build, final QA). A malicious or sloppy value can redirect the entire pipeline. Minimum mitigation: wrap it in a delimited block and instruct the agent to treat it as data, not instructions.

4. target_url is similarly untrusted. Playwright will actually navigate to it, so at least validate the shape (new URL(ctx.inputs.target_url) in a try/catch at the top of .run) to fail fast on garbage input.

5. Template-literal indentation bleeds into prompts. Because the template literals live inside an indented callback, every line after the first carries ~10 leading spaces (e.g. Extract the DOM...). Not a correctness bug, but it inflates tokens and is inconsistent with the other workflows. Consider extracting prompt construction into small top-level helpers the way .atomic/workflows/hello-world/copilot/index.ts:8 and .atomic/workflows/parallel-hello-world/copilot/index.ts:4 do — which also makes the workflow easier to unit-test.

6. Stage 3 saving DESIGN_README.md to the repo root can clobber an existing file. Instruct the agent to read-first-and-merge, or place it in a dedicated subpath.

Nitpicks

• Missing trailing newline on .atomic/workflows/design-driven/copilot/index.ts (diff shows No newline at end of file).
• "Restylize" (in both the description and the PR body) isn't standard English — "Restyle" reads cleaner.
• Workflow description is much longer than peers ("A simple single-session hello world workflow"). Tighten to one clause.
• "ralph loop" appears as raw jargon in the stage 5 prompt. There is internal context for it (research/docs/2026-02-15-ralph-loop-manual-worker-dispatch.md), but the Copilot agent will not have that in context. Either expand inline ("iteratively: implement → observe → fix → repeat") or reference an actual skill.
• No tests. CLAUDE.md calls out testing as a first-class concern, and src/sdk/define-workflow.test.ts is a good template. A short compile-time check (assert the workflow compiles, inputs are declared, stage names are present) would catch issue add agent instructions #1 in CI automatically.
• Prior-stage handoff. Other workflows pass assistant text through SessionHandle.result via a getAssistantText() helper (see .atomic/workflows/headless-test/copilot/index.ts:5). Here every downstream stage re-reads the previous transcript from disk through s.transcript(prev).path. It works, but it shifts synthesis into the agent on every hop — a short summary on the handle would be cheaper.

Nice bits

• The staged decomposition maps cleanly to the Coding Backwards phases — each stage has one job and a clear transcript-based contract.
• Conditional devInstructions is the right shape for making dev_command genuinely optional.
• Scoping to Copilot only is sensible given the Playwright-CLI dependency — worth a // TODO that Claude/OpenCode variants would need equivalent browser tooling.

Once the API-shape fix (#1) lands and the required-input gap (#2) is closed, the rest is polish. Happy to look again after a push.

[claude]

PR Review — coding-backwards-design workflow

Thanks for adding this! The Coding Backwards methodology maps nicely onto a staged workflow. That said, I think the PR as-is shouldn't merge. A summary of findings below, roughly ordered by severity.

---

🔴 Blocking issues

1. defineWorkflow<\"copilot\">(...) is a type error

.atomic/workflows/design-driven/copilot/index.ts:3 uses:

export default defineWorkflow<\"copilot\">({ ... })

The single generic parameter on defineWorkflow is I extends readonly WorkflowInput[], not an AgentType — passing \"copilot\" violates the constraint. bun typecheck will fail. Worth noting the JSDoc on WorkflowBuilder.for() explicitly calls this out:

Use .for<\"copilot\">() before .run() instead of passing the agent as a type parameter to defineWorkflow.

Every existing workflow in this repo (hello-world, headless-test, parallel-hello-world, built-in ralph) follows the correct pattern:

defineWorkflow({ name: ..., inputs: [...] })
  .for<\"copilot\">()
  .run(async (ctx) => { ... })
  .compile();

Please switch to that form. This appears not to have been locally typechecked before opening the PR — running bun typecheck as part of the submission flow would have caught it.

2. PR scope: 46 files, ~4,479 additions of generated artifacts

The PR title/description says "add a copilot workflow," but only .atomic/workflows/design-driven/copilot/index.ts (129 lines) belongs to that feature. The other ~4,350 lines look like they are the output of running the workflow, not the workflow itself:

• DESIGN_README.md (1,082 lines) — a Big Sur design spec for jamesbuckhouse.com
• preview/index.html (1,115 lines) + preview/server.js (254 lines) — a generated static site + dev server
• src/css/** + src/js/** — portfolio site source (components, data, tokens)
• bunfig.toml coverage exclusions added specifically to silence coverage on the generated src/css/**, src/js/**, preview/**

These all appear to be example output from a single run of the new workflow and shouldn't be committed. Either (a) split into two PRs — one for the workflow, one for any demo output that needs to live in-repo — or (b) remove the generated artifacts entirely and point to them externally. The current shape makes the workflow itself hard to review and pollutes the repo with a portfolio site unrelated to Atomic CLI.

3. Path traversal in preview/server.js

resolveFilePath() (preview/server.js:111-123) joins the request pathname onto PREVIEW_DIR / ROOT with no normalization or containment check. join(ROOT, \"/../../etc/passwd\") escapes the intended root. Even for a "dev server," this reads arbitrary filesystem paths off localhost.

Minimal fix would be to resolve + assert the final path starts with the allowed root:

const resolved = resolve(filePath);
if (!resolved.startsWith(PREVIEW_DIR) && !resolved.startsWith(SRC_DIR)) {
  return new Response(\"403 Forbidden\", { status: 403 });
}

(This whole file falls away if #2 is addressed, though.)

---

🟡 Workflow correctness issues

4. Undefined inputs get interpolated literally into prompts

All three inputs are required: false, but the prompts interpolate them with no fallback:

// Stage 1
`Use 'playwright-cli' to crawl ${ctx.inputs.target_url}.`
// Stage 2
`...target aesthetic: \"${ctx.inputs.design_reference}\".`

If a user doesn't supply them, the agent receives literally crawl undefined or aesthetic: \"undefined\". Either:

• Mark target_url and design_reference as required: true (they seem load-bearing), or
• Provide defaults and null-coalesce: ctx.inputs.target_url ?? \"https://example.com\" (see .atomic/workflows/headless-test/copilot/index.ts:31 for the pattern used elsewhere in this repo).

5. dev_command is embedded in the prompt unescaped

`Run '${ctx.inputs.dev_command}' to start the local web server...`

If the value contains a single quote, the prompt's own quoting breaks. Low severity given the agent is the one parsing, but worth tightening.

---

🟢 Design suggestions (non-blocking)

6. Directory structure implies siblings that don't exist

The PR creates .atomic/workflows/design-driven/copilot/ but not claude/ or opencode/. The description acknowledges this, which is fine — but all other workflow triples in the repo have all three variants. Either add stubs for the missing agents (even if they're stubbed-out) or drop design-driven/ up a level and name the folder design-driven-copilot/ to be explicit that only copilot is supported for now.

7. Stage context passing is lossy

You're chaining stages via s.transcript(prev), which writes the path and instructs the next agent to re-read it. .atomic/workflows/headless-test/copilot/index.ts shows a cleaner pattern: extract the assistant text from session.getMessages() and pass the text itself into the next stage's prompt. That avoids the agent re-opening files and gives you typed access to the prior stage's result via the SessionHandle<T>.result field.

8. File-level header comment is missing

Other workflows open with a multi-line block explaining the methodology and when to use it (e.g. src/sdk/workflows/builtin/ralph/copilot/index.ts:1-16). Since this workflow codifies a specific methodology ("Coding Backwards"), a short header explaining what each stage corresponds to would help maintainers who aren't already familiar with the term.

9. Minor style nits

• Trailing comma missing on the last input at line 27 (inconsistent with other workflows).
• Multi-line template literals (e.g. lines 37–38, 65–70) preserve leading indentation in the sent prompt. Other workflows either use dedent or write prompts as single-line string concats.

---

✅ What's working

• Stage naming and descriptions are clear and map cleanly to the Coding Backwards steps.
• Input schema (name/type/placeholder) matches the patterns used by other workflows in .atomic/workflows/.
• Using s.transcript() to chain stages is the right shape — it just needs to be paired with stage return values for richer context passing (see updated readme #7).

---

Suggested path forward

1. Drop the generated site artifacts (DESIGN_README.md, preview/**, src/css/**, src/js/**) and the related bunfig.toml exclusions from this PR.
2. Fix the defineWorkflow<\"copilot\"> type error by switching to .for<\"copilot\">().
3. Either mark target_url and design_reference required, or add defaults.
4. Run bun typecheck and bun lint locally and include the output in the PR description.

Happy to re-review once the workflow is isolated — the underlying idea is solid.

[claude]

PR Review: feat(workflows): add coding-backwards design workflow for copilot

Thanks for this PR — the "Coding Backwards" methodology is an interesting approach and the workflow structure is well-thought-out. Below is my review covering code quality, bugs, security, performance, and test coverage.

---

Security (P0 — must fix before merge)

1. Path traversal vulnerability in preview/server.js

The resolveFilePath function is vulnerable to directory traversal attacks:

// preview/server.js:131
if (pathname.startsWith("/src/")) {
    return join(ROOT, pathname);
}
return join(PREVIEW_DIR, pathname);

path.join does not prevent escaping the root. A request to /src/../../etc/passwd would pass the startsWith("/src/") check and resolve to a file outside the project root (e.g., /etc/passwd). Similarly, /../../../etc/passwd escapes PREVIEW_DIR.

Fix: Resolve the path and then verify it stays within the allowed root:

function resolveFilePath(pathname) {
  const decoded = decodeURIComponent(pathname);
  let resolved;
  if (decoded === "/" || decoded === "") {
    resolved = join(PREVIEW_DIR, "index.html");
  } else if (decoded.startsWith("/src/")) {
    resolved = resolve(ROOT, decoded.slice(1)); // remove leading /
  } else {
    resolved = resolve(PREVIEW_DIR, decoded.slice(1));
  }
  // Guard against directory traversal
  if (!resolved.startsWith(PREVIEW_DIR) && !resolved.startsWith(SRC_DIR)) {
    return null; // Return null and handle as 404
  }
  return resolved;
}

2. iframe allows microphone and camera without user consent context

// src/js/HotlineEmbed.js:16
iframe.allow = 'microphone; camera';

Granting camera permissions to a third-party iframe (delphi.ai) is broad. Consider whether camera is truly needed — the design spec only mentions microphone.

---

Code Quality & Architecture (P1 — should fix)

3. Generated site placed inside src/ pollutes the core source tree

The generated portfolio outputs (src/css/, src/js/) live alongside the project's core application code (src/lib/, src/sdk/, src/commands/). This is confusing and creates a risk that future tooling changes (linters, bundlers, imports) accidentally pick up the generated site.

The bunfig.toml coverage exclusions (src/css/**, src/js/**) are a workaround that could hide future legitimate source code from coverage reporting.

Suggestion: Move generated site assets to a self-contained directory, e.g., .atomic/workflows/design-driven/output/ or examples/design-driven-portfolio/. This keeps src/ clean for actual application source code and eliminates the need for broad coverage exclusions.

4. DESIGN_README.md (1082 lines) committed to the project root

This is a site-specific design specification for jamesbuckhouse.com. It should live alongside its workflow output, not at the repository root where it looks like project-level documentation.

5. Dual content sources — HTML vs JS data files

Content exists in both preview/index.html (1115 lines of hardcoded HTML with all 45 artwork cards, 64 library items, etc.) AND src/js/data/*.js (structured data files). This creates a maintenance burden: if content changes, two files must be updated. The JS modules in src/js/main.js dynamically create DOM elements from data files, but the HTML already has everything hardcoded — the JS modules would overwrite or duplicate the static content.

6. File structure diverges from DESIGN_README.md

The README specifies TypeScript files (Navbar.ts, GalleryGrid.ts) in src/components/ and CSS in src/styles/, but the implementation uses JavaScript (.js) in src/js/ and CSS in src/css/. This is a bun/TypeScript project per CLAUDE.md, so .ts would be more appropriate.

7. Copilot workflow uses npm run dev as placeholder

// .atomic/workflows/design-driven/copilot/index.ts:42
placeholder: "npm run dev",

Per CLAUDE.md: "This is a bun project. Do NOT use node, npm, npx, yarn, or pnpm commands." The placeholder should be bun run dev or bun run preview/server.js.

8. Missing newline at end of copilot/index.ts

The file doesn't end with a newline character (line 129). Most linters and POSIX conventions expect a trailing newline.

---

Potential Bugs (P1)

9. main.innerHTML = '' on every route change leaks event listeners

// src/js/main.js:64-66
'#/': () => {
    main.innerHTML = '';
    main.appendChild(createGalleryGrid(artworks));
},

Setting innerHTML = '' removes DOM nodes but doesn't clean up event listeners added by addEventListener in components like FilterBar.js and Lightbox.js. The Lightbox module uses a module-level lightboxEl variable that will reference a detached node after route changes, breaking reopening.

10. Claude workflow as Record<string, unknown> type assertions

// .atomic/workflows/design-driven/claude/index.ts:118-122
if (msg.subtype === "success" && (msg as Record<string, unknown>).structured_output) {
    structured = (msg as Record<string, unknown>).structured_output as ComponentPlan;
}

These cascading type assertions are fragile and bypass type safety entirely. If the SDK types are incomplete, consider augmenting them with proper interface extensions rather than using Record<string, unknown> casts throughout.

11. Claude workflow asAgentCall may not match agent invocation syntax

// .atomic/workflows/design-driven/claude/index.ts:159-161
function asAgentCall(agentName: string, prompt: string): string {
    return `@"${agentName} (agent)" ${prompt}`;
}

This constructs an agent invocation via string interpolation. If the SDK's agent invocation syntax changes, this silently breaks. Verify this matches the current Claude Agent SDK's expected format.

---

Performance (P2)

12. 45 artwork cards + 12 film cards + 64 library cards loaded at once

preview/index.html loads all 1115 lines of DOM at once. For the src/js/ approach, createGalleryGrid(artworks) creates all 45 <img> elements immediately. While loading="lazy" helps with network, the DOM node count is high. Consider virtual scrolling or pagination for the library section (64 cards).

13. CSS loaded via @import url() chains

/* src/css/main.css */
@import url('./tokens/colors.css');
@import url('./tokens/radii.css');
/* ... 14 more imports */

Each @import url() triggers a separate HTTP request in development. For production, these should be bundled. For the dev server, consider inlining or using a CSS build step.

---

Test Coverage (P2)

14. Tests only cover data integrity and module signatures

The test files are well-structured and validate data shape, but there's no coverage for:

• preview/server.js — no tests for MIME type resolution, path handling, SSE, or the path traversal issue
• router.js — no tests for route matching, wildcard patterns, or hash change handling
• Filter behavior — FilterBar.js and LibrarySection.js filtering is unimplemented (TODO comments)
• Lightbox behavior — open/close state management is untested

At minimum, server.js and router.js should have unit tests since they have non-trivial logic (path resolution, pattern matching).

---

Minor Issues

• Unused _event and _filename in server.js watcher — using _ prefix is fine but the watcher callback fires on every file change in the watched directories (including node_modules if present). Consider adding a debounce.
• preview/server.js error handler swallows errors — catch (err) returns a generic 500 without logging. Consider console.error(err) for dev server debugging.
• Inconsistent IMDB URL trailing slashes — some film entries have trailing slashes on IMDB URLs (e.g., tt6875952/) and some don't (e.g., tt0479952). Minor, but inconsistent.

---

Summary

Category	Findings
Must fix (P0)	Path traversal in server.js
Should fix (P1)	Generated files in src/, DESIGN_README.md at root, dual content sources, event listener leaks, type assertion fragility
Nice to fix (P2)	Test coverage for server/router, CSS import performance, DOM performance

The workflow definitions (both copilot and claude variants) are well-designed with clear stage separation and the progressive build + QA loop pattern is solid. The main concerns are around where the generated output lives and the path traversal security issue in the dev server.

[claude]

Review

Thoughtful work on codifying the "Coding Backwards" methodology into a pipeline. The three-SDK parallel (Claude / Copilot / OpenCode) is a nice demonstration of the workflow authoring surface. A few findings grouped by severity.

Critical — bugs that silently disable parts of the workflow

1. s.session.query() return shape is misused in the Claude variant. ClaudeSessionWrapper.query() returns Promise<SessionMessage[]> (see src/sdk/providers/claude.ts:722-733), so result.output is always undefined.

   • .atomic/workflows/design-driven/claude/index.ts:674,686,701 — the three infra-* stages return String(result.output ?? ""), which is always "".
   • .atomic/workflows/design-driven/claude/index.ts:882 — return result.output; makes debuggerReport empty, so every iteration after the first has nothing to fix from.
   • Fix: use extractAssistantText(result, 0) like the built-in ralph variant (src/sdk/workflows/builtin/ralph/claude/index.ts:157,171,188).

2. queryWithStructuredOutput reads the wrong field on Claude SDK result messages. At .atomic/workflows/design-driven/claude/index.ts:137 you read msg.output; the SDK exposes the terminal result text on msg.result (see src/sdk/workflows/builtin/ralph/claude/index.ts:64). raw will always be empty, which means hasActionableFindings loses its text-fallback signal and mergeReviewResults has nothing to merge in the raw-text track.

3. Together, these drop all infra-discovery context, all debugger context, and the raw-text side of the review merge for the Claude variant. The review loop still runs, but the reviewers don't see the output they're supposed to.

Architectural / maintainability

4. ~900 lines × 3 nearly-identical workflow files (claude/copilot/opencode). The prompts are the substantive content and are duplicated verbatim; only the SDK glue differs. When a prompt changes, it has to be updated in three places (and this PR already shows the drift risk — the Claude variant has the .output bugs that Copilot/OpenCode don't). Consider extracting prompt builders into a shared module (similar to src/sdk/workflows/builtin/ralph/helpers/prompts.ts that you already reuse for the review/debug prompts) and keep only the SDK-specific transport in each variant.

5. Committing the generated sample site into the root of the Atomic CLI repo is mixing concerns. index.html, src/css/**, src/js/**, preview/**, DESIGN.md, DESIGN_BRIEF.md, spec.txt, plus ~11 MB of PNGs (big-sur-*.png and reference-*.png, sizes up to 3.4 MB) now sit at the repo root — positions normally reserved for the project itself. The bunfig.toml coverage exclusions for src/css/**, src/js/**, preview/** read as a workaround for this layout. Suggest examples/design-driven-portfolio/ (or samples/) so the generated output is clearly demo material and the repo root stays focused on the CLI. Also consider Git LFS or a CDN for the screenshots — binary PNGs in main history are permanent.

6. Hardcoded dev_command default \"bun run preview/server.js\" is leaky — that path only exists because this PR also commits the sample site. For a generalized workflow, the default should be undefined and the scaffold stage should self-describe the dev server (the prompt already asks the agent to "create a dev server setup").

7. componentPlan.result?.phases ?? [] silently skips Stage 5 if the structured output failed. If the agent never called the tool (Copilot) or the schema validation failed (Claude/OpenCode), the workflow quietly proceeds to the QA loop with zero implementation and still burns the tokens for Stage 6–7. Throwing (or at least logging a warning) would surface this early.

8. Parallel component builds rely on a prompt-level "don't touch other files" rule. The LLM is not a reliable lockholder — two parallel stages can race on a shared file and produce corrupt merges. At minimum, post-verify that each stage only modified the files it declared (git diff --name-only after the stage), or serialize when file sets overlap. The files field in ComponentPlan is the natural source of truth for this.

9. Claude variant bypasses s.session for queryWithComponentPlan / queryWithStructuredOutput, calling claudeSdkQuery directly. That's consistent with how the built-in ralph already does structured output, but here you also route the visual-analysis stage through it — a long browser-automation flow, not a one-shot. Those calls won't be tied to the session graph / transcript that the rest of the workflow uses for observability.

Minor

10. CLAUDE.md asks to avoid any/unknown, but these workflows lean heavily on (msg as Record<string, unknown>).structured_output-style casts. Consider a one-time typing helper (e.g. narrowResultMessage(msg)) so each variant doesn't re-do the narrowing inline.

11. name: 'coding-backwards-design' but directory is design-driven. Aligning them would help discovery via atomic workflow -n <name>.

12. dev_command interpolated into prompts without any shape check. Low risk (user supplies it), but worth a note in the workflow description that it is effectively shell-through-LLM.

13. Deleted skills (arrange, frontend-design, normalize, onboard, teach-impeccable) appear to be superseded by the newer impeccable/layout/polish skills — confirmed by the migration note in .agents/skills/impeccable/SKILL.md:13. Worth double-checking no other workflow or doc still /teach-impeccable's them (a quick grep shows only the note itself references them).

14. Copilot variant componentPlanCapture / captureA / captureB are let-vars written from tool handlers. It works, but a deferred promise (or defineTool returning the parsed value) would be more robust than closure mutation — especially since sendAndWait resolving doesn't strictly guarantee the handler has written the closure var.

Tests

15. No unit tests for the new helpers (getAssistantText, extractReview, extractResponseText, queryWithComponentPlan, queryWithStructuredOutput, asAgentCall). Given the runtime narrowing they do, a handful of fixture-driven tests would have caught items (1) and (2) above.

16. The data files under src/js/data/*.test.js are the only tests added — and they test generated sample data, not the workflow itself. The workflow's ReviewResult merging path, component-plan extraction, and tool-call capture are all untested.

What's good

• Stage structure is clear and the prompts are specific and well-tagged with <spec> / <design_reference> blocks.
• The Ralph-style dual-reviewer + debugger + visual-analysis merge is a nice extension of the existing pattern.
• Using captureBranchChangeset and the shared prompts.ts helpers for the review/debug stages is exactly the right reuse.
• Bounded loop with MAX_QA_LOOPS = 5 and an early-exit when there are no actionable findings is the right call.

Overall: the workflow design is solid, but items 1–3 should be fixed before merge (they functionally disable the infra-discovery and debugger feedback loops in the Claude variant), and items 4–5 would make this much more pleasant to maintain long-term.