fix: pre-1.0 defects — refused surfaces can't save, flow plans survive navigation, real enum values - #84
Merged
Merged
Conversation
… blocked Build names its blocker
Phase F of Composer pre-1.0 readiness. Fail-first: every assertion below fails
on the code as it stands today.
The defect, walked: shadcn project -> Surfaces -> author a root `card` with no
children. The gate strip reads "S1 S2 S3 clean" (no authored rule forbids it)
while the preview reads "emitter refusal: ... Card#root: required prop 'child'
has no value after emission". Save is enabled and succeeds. Afterwards Build
reads "Not ready to build yet: gates not green - 1 error finding", naming
nothing.
Save is gated on S1-S3 only; the emit result the view already computes and
displays does not gate it. The agent's /project/save-example has the same hole
from the other side: it re-lints S1-S3 server-side and never re-emits.
Four new assertions:
1. e2e/composer-surfaces.spec.ts - the refused surface cannot be saved, writes
nothing (across a reload), and saves the moment it can render.
2. e2e/composer-surfaces.spec.ts - a project that ARRIVES with a refused
surface (import is vocabulary, not a build, so it is not emit-gated) makes
Build name the blocker: title, id, the emitter's own reason, and a way to it.
3. apps/agent/src/project.test.ts - save-example refuses an emit-refused
surface with the emitter's words and writes nothing.
4. apps/composer/app/surface-identity.test.ts - blockingFindings(), the pure
reader that turns unresolved error findings into named rows.
FAILING OUTPUT ON MAIN
----------------------
$ npx playwright test --config playwright.composer-smoke.config.ts \
-g "cannot be saved|names the surface that is blocking"
1) e2e/composer-surfaces.spec.ts:165 - a surface the emitter refuses cannot
be saved, and saves as soon as it can render
Error: expect(locator).toBeDisabled() failed
Locator: getByTestId('save-scenario')
Expected: disabled
Received: enabled
Timeout: 5000ms
Call log:
- waiting for getByTestId('save-scenario')
14 x locator resolved to <button class="st-btn" data-testid="save-scenario">Save surface</button>
- unexpected value "enabled"
177 | await expect(page.getByTestId("lint-clean")).toContainText("S1 S2 S3 clean");
178 | await expect(page.getByTestId("preview-refused")).toContainText("required prop 'child' has no value");
> 179 | await expect(page.getByTestId("save-scenario")).toBeDisabled();
2) e2e/composer-surfaces.spec.ts:210 - Build blocked by a finding names the
surface that is blocking it
Error: expect(locator).toContainText(expected) failed
Locator: getByTestId('build-blocker-ex.empty-card')
Expected substring: "A card with nothing in it"
Timeout: 5000ms
Error: element(s) not found
229 | const blocker = page.getByTestId("build-blocker-ex.empty-card");
> 230 | await expect(blocker).toContainText("A card with nothing in it");
2 failed / 1 passed
(the second test reaches "gates not green" in build-not-ready before failing -
the blocked state is genuinely reachable; only the naming is missing.)
$ pnpm --filter agent exec vitest run src/project.test.ts \
-t "rejects a surface the EMITTER refuses"
FAIL src/project.test.ts > accepting a build result (/project/save-example,
fail-closed) > rejects a surface the EMITTER refuses, even though every
S-gate passes
AssertionError: expected 200 to be 422 // Object.is equality
- Expected 422
+ Received 200
471| example: { id: "ex.empty-card", intent: "status-report", ... },
472| });
> 473| expect(status).toBe(422);
Tests 1 failed | 29 skipped (30)
$ pnpm --filter composer exec vitest run app/surface-identity.test.ts
FAIL app/surface-identity.test.ts > blockingFindings - what is blocking the
build, by name (5 tests)
TypeError: (0 , blockingFindings) is not a function
> 111| const rows = blockingFindings(
Tests 5 failed | 10 passed (15)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…blocks a build Two halves of one honest-failure defect, both now asserted (previous commit). SAVE FOLLOWS THE EMITTER, NOT ONLY THE GATES. The Surfaces editor already computes and displays the emit result beside the gate strip; it just did not gate on it. A root Card with no children breaks no authored rule — every S-gate passes — and the emitter still refuses it, so Save wrote a surface the project's own emit then refused from inside the contract. Save is now disabled while the emitter refuses, with the emitter's verbatim reason beside the button (data-testid save-blocked-emit). `should`-level findings are untouched: only refusals and errors block. The agent's /project/save-example had the same hole from the other side — it re-lints S1-S3 server-side and never re-emitted — so accepting a build result could write the same unrenderable surface into a repository contract. It now runs composer-core's projectEmit over the candidate and refuses with 422 and the emitter's own message, writing nothing. Skipped when the project has no profile: nothing to emit against, and discovery is the missing step, not this. A BLOCKED BUILD NAMES ITS BLOCKER. "gates not green — 1 error finding" is true and useless. blockingFindings() (surface-identity.ts) resolves every unresolved error finding's target to the thing it is about — a surface's human title with its canonical id beside it, a component id for coverage, the document otherwise — and Build lists them with the gate's verbatim reason plus a link into Surfaces and Checks. Acknowledged casualties are decisions and never appear, matching gatesSummary. Verification: F1 e2e 2/2 green (were 2/2 red), agent project.test.ts 30 passed (was 29 + 1 red), composer unit 15 passed in surface-identity, composer smoke suite 30 passed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s flow plan
Fail-first. `{view === "build" && <BuildView />}` unmounts the view, so
buildMode, flowPlan and flowBuild — all local useState in BuildView — are
destroyed by any navigation away and back.
The round trip is not hypothetical: the product's own pending-step copy sends
people on it ("build it from Build and accept into this step"). After it, the
plan editor and the per-step rebuild buttons are gone (1 -> 0), per-step
rebuild is unreachable, and the only way forward is to re-plan — which mints a
SECOND flow beside the one the first plan already created.
FAILING OUTPUT ON MAIN
----------------------
$ npx playwright test --config playwright.composer-smoke.config.ts \
-g "leaving Build and coming back"
1) e2e/composer-flows.spec.ts:167 - leaving Build and coming back keeps the
in-progress plan - and never mints a second flow
Error: expect(locator).toBeVisible() failed
Locator: getByTestId('flow-composer')
Expected: visible
Timeout: 5000ms
Error: element(s) not found
182 | // Back exactly as it was left: still in flow mode, the same plan, the same
183 | // frozen drive, and per-step rebuild still reachable.
> 184 | await expect(page.getByTestId("flow-composer")).toBeVisible();
1 failed
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… cannot destroy it
`{view === "build" && <BuildView />}` unmounts the view, and the mode, the
goal, the proposed plan and the plan's per-step build state were all local
useState inside it. Any navigation away and back destroyed them: the plan
editor and the per-step rebuild buttons went 1 -> 0, per-step rebuild became
unreachable, and re-planning — the only way forward — minted a SECOND flow
beside the one the first plan had already created. The product itself invites
that round trip: a pending step reads "build it from Build and accept into
this step".
Smallest change that survives unmount: exactly that state moves to the
existing provider as one `flowComposition` value ({ mode, goal, plan, build })
with a plain setter. The view keeps its own accessors, so every call site
inside BuildView reads unchanged; `planBusy` deliberately stays local — it is
a transient in-flight flag, and the plan it awaits lands in the provider
either way.
Deliberately NOT done: no restructuring of state.tsx (one state + one context
field), no persistence (this is in-progress working state, not project data),
and no change to flow persistence semantics — `saveFlows` still owns what a
flow IS. The composition is cleared in `clearBuildThread`, whose five callers
are all project transitions (load reference, open imported, connect
repository, close project, delete the active project), so a plan can never
leak from one project into another.
Verification: the F2 e2e passes (was red), composer smoke suite 31 passed,
composer typecheck clean.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…num shapes
Fail-first. dspack v0.4 allows an enum prop's `values` to be bare values OR
value descriptor objects ({ value, description }); both are spec-valid, and the
shipped shadcn contract uses the RICH form throughout.
apps/composer/app/views/component-view.tsx:81 renders `p.values.join(", ")`,
which stringifies the objects. Demonstrated against the contract we ship:
$ node -e "...components.button.props..."
variant: [[object Object], [object Object], [object Object], [object Object], [object Object], [object Object]]
size: [[object Object], [object Object], [object Object], [object Object]]
--- enum props on Button: 2
--- rich-enum props across the shipped contract: 27
The same view's "Add prop" path (line 47) writes FLAT strings, so the catalog
ends up holding two shapes for the same idea and the view displays one while
authors write the other.
New unit suite over the pure reader/writer this needs — enumMembers (the
canonical unwrap, mirroring dspack-gen's enumValues(), keeping the description
that reader discards), enumLabel, and parseEnumValues. Covers: the flat form,
the rich form, a mixed list, non-string values, missing/empty/malformed/
non-enum props, a descriptor with no value, the shipped Button props, and the
`join` defect itself.
FAILING OUTPUT ON MAIN
----------------------
$ pnpm --filter composer exec vitest run app/contract-enums.test.ts
FAIL app/contract-enums.test.ts [ app/contract-enums.test.ts ]
Error: Cannot find module './contract-enums' imported from
'/.../apps/composer/app/contract-enums.test.ts'
1| import { describe, expect, it } from "vitest";
2| import contract from "../shadcn-v3-project/shadcn-ui.dspack.json";
3| import { enumLabel, enumMembers, parseEnumValues } from "./contract-en…
| ^
Test Files 1 failed (1)
Tests no tests
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Catalog's component detail joined `p.values` straight into text. Against the contract this product ships — where every enum is the RICH form — that is what a person saw on the page they open to learn the vocabulary: ---- Catalog > button > props (rendered, before) ---- variant enum [[object Object], [object Object], [object Object], [object Object], [object Object], [object Object]] Visual treatment... size enum [[object Object], [object Object], [object Object], [object Object]] Button size. Use icon for... Both shapes are spec-valid, so this is a reader problem, not a contract problem. `enumMembers` (apps/composer/app/contract-enums.ts) is now the one reader: it mirrors dspack-gen's canonical `enumValues()` unwrap and keeps the per-value description that reader discards, because a UI has a use for it. The detail page renders the values as values and hangs each description off a title, so the rich form finally pays for itself. The same file's "Add prop" wrote FLAT strings, so the view displayed one shape and authors wrote another into the same catalog. `parseEnumValues` writes value descriptors — the shape the shipped contract already uses. Neither the contract nor the schema changes: dspack.v0.4 §valueDescriptor requires only `value`. Two more copies of the same unwrap are now gone: the Mapper's local `enumLabel` and the surface editor's inline `.map(v => v.value)`. The surface editor's enum select also stops requiring `Array.isArray(values)` before rendering, and carries the descriptions as titles too — the vocabulary a person browses and the vocabulary they author with are read by one function. Verification: contract-enums unit suite 10 passed (was a module-not-found failure), composer unit 106 passed, composer smoke suite 32 passed including the new end-to-end assertion that the Catalog page and the surface editor agree, composer typecheck clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The ratified vocabulary is Surface / Flow / Step / Example (Example = a reference PROJECT only). Four strings a person reads still carried the retired words: - Projects hub, empty state: "pick a governed design system to begin" — the picker label directly above it already reads "Design system". - Checks, lead: "the contract and its worked surfaces" — a hybrid of the retired "worked examples" and the ratified "surfaces". - Build, scripted-mode notice: "replays a representative example for this context" — "example" now means a reference project, and what is replayed is a surface. - Build, scripted-mode refusal (hosted-build.ts): "replays this intent's own worked example" — now word-for-word the AGENT's twin of the same refusal (apps/agent/src/project.ts), which already used the ratified phrasing. The two doors said the same thing differently. Swept and left alone deliberately: every other user-visible "example" refers to a reference project (the Examples section, "duplicate this example into your projects"), which is the ratified meaning; `scenario` survives only in filenames, component names and data-testids, which the earlier sweep kept on purpose (nothing a person reads); the generated emit report's authoring notes in the contracts package are contract content, out of scope. Verification: composer smoke suite 32 passed, composer typecheck clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… list Follow-up on the blocker panel. Two findings against the same surface are one problem, not two, and they collided on the same data-testid; a contract with many unresolved errors turned the panel into a wall. Rows now group by target (one entry per surface/component, its reasons listed beneath) and cap at six, with an honest "and N more — Checks lists every one" rather than a silent truncation. `blockingFindings` itself is unchanged: it still reports every unresolved error, and the grouping is presentation. Verification: composer smoke suite 32 passed, composer typecheck clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
dspack-studio-composer | 704075c | Aug 12 2026, 05:23 PM |
There was a problem hiding this comment.
Pull request overview
This PR addresses several pre-1.0 correctness and UX defects in Composer and its agent by aligning “save” and “build readiness” behavior with emitter refusals, preserving in-progress flow composition state across navigation, and normalizing how contract enum values are read/written across the UI.
Changes:
- Gate surface saving (browser + agent
/project/save-example) on emitter refusals and surface buildability, not just S-gates. - Preserve “Build a flow” composition state (mode/goal/plan/per-step build state) across navigation by lifting it into the provider and resetting it on project transitions.
- Normalize enum value handling via shared helpers (
enumMembers,parseEnumValues,enumLabel) and improve Build “not ready” UX by naming blocking findings.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| e2e/composer-surfaces.spec.ts | Adds e2e coverage for emit-refused save gating, Build blocker naming/navigation, and enum rendering consistency. |
| e2e/composer-flows.spec.ts | Adds e2e coverage ensuring flow plans survive navigation and don’t mint duplicate flows. |
| apps/composer/app/views/validate-view.tsx | Copy update to remove “worked surfaces” phrasing. |
| apps/composer/app/views/scenario-view.tsx | Disables Save when preview has emitter refusal and uses shared enum reader for enum prop editors. |
| apps/composer/app/views/projects-view.tsx | Copy update to remove “governed” phrasing in empty state. |
| apps/composer/app/views/mapper-view.tsx | Consolidates enum label extraction onto shared enum helper to avoid shape divergence. |
| apps/composer/app/views/component-view.tsx | Writes authored enums using descriptor form and renders enums using shared reader (with per-value descriptions). |
| apps/composer/app/views/build-view.tsx | Lifts flow composition state into provider, improves scripted-mode copy, and lists blocking findings when build isn’t ready. |
| apps/composer/app/surface-identity.ts | Adds blockingFindings utility to map blocking findings to actionable rows by target id/title. |
| apps/composer/app/surface-identity.test.ts | Adds unit tests for blockingFindings behavior and edge cases. |
| apps/composer/app/state.tsx | Adds provider-held flowComposition state and resets it on project transitions. |
| apps/composer/app/hosted-build.ts | Updates scripted-mode error message copy. |
| apps/composer/app/contract-enums.ts | Introduces shared enum reader/writer utilities to handle both spec-valid enum shapes. |
| apps/composer/app/contract-enums.test.ts | Adds unit tests validating enum helpers against both enum shapes and the shipped shadcn contract. |
| apps/composer/app/composer.tsx | Wires BuildView navigation callback so Build blockers can link to Surfaces/Checks. |
| apps/agent/src/project.ts | Adds emit-gating to /project/save-example to refuse emitter-unrenderable surfaces with 422 and no writes. |
| apps/agent/src/project.test.ts | Adds server-side test ensuring emitter-refused surfaces are rejected even when S-gates pass. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase F: the five defects found by walking the product cold and by the new e2e coverage. Correctness and copy only — no contract, governance, intent, A2UI, emit/gen, or runtime change.
F1 — an emit-refused surface could be saved, then silently blocked Build. Save was gated on S1–S3 while the emitter's refusal was displayed beside an enabled button; afterwards Build read "gates not green — 1 error finding" and named nothing. Now: Save is gated on the same emit result the view already computes (verbatim emitter reason shown;
shouldfindings stay non-blocking), and the agent path had the same hole —save-examplere-linted but never re-emitted, returning 200 for a refused surface; it now emits and returns 422, writing nothing. Build's blocked state lists what is blocking it, by surface title + id, with links into Surfaces and Checks.F2 — leaving Build destroyed an in-progress flow plan. The view unmounts, so mode/goal/plan/build were lost; per-step rebuild became unreachable and re-planning minted a second flow. Exactly those four pieces now live in the existing provider and are cleared on project transitions, so a plan can never leak between projects. No restructuring, no persistence — it is working state, not project data.
F3 —
[object Object]in the Catalog. Rich enum values ({value, description}— 27 props across the shipped contract) rendered as[object Object], and the same view's "Add prop" authored flat strings. One reader (enumMembers, mirroring dspack-gen's canonical unwrap) and one matching writer now serve both shapes; two duplicate unwraps elsewhere consolidated onto them.F4/F5 — terminology stragglers: hub empty state and Checks lead, plus two scripted-mode strings.
Fail-first evidence pasted per commit, including the live product output
variant enum [[object Object], …]. Unit 313 → 329, product e2e 28 → 32, agent e2e 49, typecheck clean.🤖 Generated with Claude Code