From 86784be08520c818b957f479f186d1de28432ce5 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Wed, 4 Nov 2026 16:12:38 -0800 Subject: [PATCH 1/2] feat: ask to choose a design through an approval card The coordinator now asks for the design with choose_design, a native approval, and the existing approval card carries the buttons. The separate Confirm prompt, its client call and its wording are removed. --- apps/hub/src/artifacts/routes.ts | 23 ++-- apps/hub/src/artifacts/service.ts | 107 ++++++++++++------ apps/hub/src/auth/agent-token-bridge.ts | 2 +- apps/hub/src/config/design.ts | 4 - apps/hub/src/config/project-status.ts | 1 + apps/hub/src/db/schema.ts | 4 +- apps/hub/src/projects/deploy-tools.ts | 18 ++- apps/web/src/api/artifacts.ts | 6 - apps/web/src/config.ts | 30 ++--- apps/web/src/pages/workspace-layout.css | 27 ----- .../src/pages/workspace/agent-activity.tsx | 8 +- .../src/pages/workspace/design-concepts.tsx | 104 +---------------- .../src/pages/workspace/document-decision.tsx | 17 ++- apps/web/src/pages/workspace/index.tsx | 40 ------- apps/web/src/pages/workspace/transcript.tsx | 48 ++------ docs/ARCHITECTURE.md | 9 +- docs/IMPLEMENTATION.md | 4 +- docs/PRODUCT.md | 3 +- packages/client/src/client/artifacts.ts | 8 -- packages/client/src/client/shared.ts | 9 -- packages/schemas/src/artifact.ts | 7 +- test/e2e/pipeline.test.ts | 49 +------- .../coordinator-1/exchanges/23/expect.json | 3 +- .../coordinator-1/exchanges/23/response.sse | 8 +- .../coordinator-1/exchanges/24/expect.json | 16 ++- workflows/coordinator/src/prompt.ts | 4 +- 26 files changed, 190 insertions(+), 369 deletions(-) diff --git a/apps/hub/src/artifacts/routes.ts b/apps/hub/src/artifacts/routes.ts index e3fe47a2..0ce04607 100644 --- a/apps/hub/src/artifacts/routes.ts +++ b/apps/hub/src/artifacts/routes.ts @@ -265,7 +265,7 @@ export function mountArtifactRoutes( tags: ["Artifacts"], operationId: "chooseDesign", description: - "The person chooses a filed design_option at the version they saw. The hub files it as the project's design, records the person's approval of exactly that design version and hash, stops the other concepts' designers, and tells the coordinator which design the person chose. Only a person may choose; the coordinator never does.", + "Called by the coordinator's choose_design tool once the person approved its native ask. The hub files the concept's design_option, at the version the person saw, as the project's design, records that approval against exactly that option and design version and hash, and stops the other concepts' designers. Each approval chooses once.", security, requestBody: { required: true, @@ -279,27 +279,22 @@ export function mountArtifactRoutes( "application/json": { schema: resolver(DesignChoiceReceipt) }, }, }, - 403: { - description: "The caller is not a person who manages the project", - }, - 404: { description: "No filed design option with that id" }, + 403: { description: "The caller does not manage the project" }, + 404: { description: "No filed design option for that concept" }, 409: { - description: "The option has a newer version than the one chosen", + description: + "No unused approval of choose_design for that concept, or the option changed while the person decided", }, }, }), - async (c) => { - const current = actorOf(c); - return c.json( + async (c) => + c.json( await service.chooseDesign( - current, + actorOf(c), c.req.param("projectId"), await body(c), - (runId, content) => - mailRunAsCaller(c, nativeFetch, current.tenantId, runId, content), ), - ); - }, + ), ); app.get( "/:projectId/materials/accepted", diff --git a/apps/hub/src/artifacts/service.ts b/apps/hub/src/artifacts/service.ts index 5641ce30..35aa7ab7 100644 --- a/apps/hub/src/artifacts/service.ts +++ b/apps/hub/src/artifacts/service.ts @@ -136,7 +136,6 @@ import { coordinatorTweaksNote, designerTweaksNote, } from "../config/design-tweaks.ts"; -import { designChosenNote } from "../config/design.ts"; import { loadSettings } from "../settings/policy.ts"; import { agentDecidesApprovals, @@ -169,9 +168,9 @@ import { requestErrorMetadata } from "../request-error.ts"; const log = getLogger(["corbits-builder", "hub", "artifacts"]); // The native asks are the human approvals. A request_approval approval covers the documents of -// the kinds it asked about and an instruct_coder approval every filed document. The person approves -// the design by choosing a design option. The app is the coder's own output, filed while it works, -// so no approval covers it. +// the kinds it asked about and an instruct_coder approval every filed document. A choose_design +// approval covers the option the person chose and the design filed from it. The app is the coder's +// own output, filed while it works, so no approval covers it. const APPROVAL_COVERS: Record boolean> = { request_approval: (kind) => kind !== "app" && kind !== "design" && kind !== "design_option", @@ -2205,13 +2204,13 @@ export function createArtifactService( }); }, - // The person's own choice among the filed design options, never the coordinator's: the option - // at the version they saw is filed as the design, and they approve exactly that design version. + // Called by choose_design after the person approved its native ask. The concept's option at the + // version they saw is filed as the design, and that approval covers exactly that design version. + // An approval is used once, so a standing grant never chooses a design without the person. async chooseDesign( actor: Actor, projectId: string, raw: unknown, - mailRun: (runId: string, content: string) => Promise, ): Promise { const input = DesignChoice(raw); if (input instanceof type.errors) @@ -2219,29 +2218,70 @@ export function createArtifactService( issues: input, }); await requireProject(actor, projectId, "manage"); - const [chooser] = await rawDB - .select({ kind: principal.kind }) - .from(principal) - .where(eq(principal.id, actor.principalId)); - if (chooser?.kind !== "user") - throw new ProjectError(403, "design_choice_person_only"); - const option = await getArtifact(db, input.artifactId); - if ( - !option || - option.tenantId !== actor.tenantId || - option.kind !== "design_option" - ) + const option = ( + await listArtifacts(db, actor.tenantId, { + kind: "design_option", + limit: MAX_LIST_LIMIT, + }) + ).rows.find((row) => row.title === input.concept); + const [newest] = option + ? (await listArtifactVersions(db, option.id, { limit: 1 })).versions + : []; + const stored = + option && newest + ? await getArtifactVersion(db, option.id, newest.version) + : undefined; + if (!option || !newest || !stored?.contentSha256) throw new ProjectError(404, "design_option_not_found", { - artifactId: input.artifactId, + concept: input.concept, }); - const [newest] = (await listArtifactVersions(db, option.id, { limit: 1 })) - .versions; - const stored = await getArtifactVersion(db, option.id, input.version); - if (!stored || newest?.version !== input.version) - throw new ProjectError(409, "artifact_ref_stale", { - artifactId: input.artifactId, - version: input.version, + const optionRef: ArtifactRef = { + artifactId: option.id, + kind: "design_option", + version: stored.version, + sha256: stored.contentSha256, + }; + const approvalId = await rawDB.transaction(async (tx) => { + const [granted] = await tx + .select({ id: approval.id, askedAt: approval.createdAt }) + .from(approval) + .where( + and( + eq(approval.tenantId, actor.tenantId), + eq(approval.status, "approved"), + eq(sql`${approval.toolDefinition}->>'name'`, "choose_design"), + eq(sql`${approval.toolArguments}->>'concept'`, input.concept), + ), + ) + .orderBy(desc(approval.resolvedAt)) + .limit(1) + .for("update"); + const [recorded] = granted + ? await tx + .select({ approvalId: artifactApproval.approvalId }) + .from(artifactApproval) + .where(eq(artifactApproval.approvalId, granted.id)) + .limit(1) + : []; + if (!granted || recorded) + throw new ProjectError(409, "approval_not_resolved", { + tool: "choose_design", + }); + // A version filed while the person was deciding is not the one they saw. + if (new Date(newest.createdAt) > granted.askedAt) + throw new ProjectError(409, "artifact_changed_during_approval", { + kind: optionRef.kind, + version: optionRef.version, + }); + await tx.insert(artifactApproval).values({ + approvalId: granted.id, + tenantId: actor.tenantId, + projectId, + tool: "choose_design", + ...optionRef, }); + return granted.id; + }); const [existing] = ( await listArtifacts(db, actor.tenantId, { kind: "design", limit: 1 }) ).rows; @@ -2268,11 +2308,12 @@ export function createArtifactService( version: written.version, sha256: written.contentSha256!, }; - await rawDB.insert(artifactImportApproval).values({ - ...design, + await rawDB.insert(artifactApproval).values({ + approvalId, tenantId: actor.tenantId, projectId, - principalId: actor.principalId, + tool: "choose_design", + ...design, }); // The other concepts' designers would file options nobody compares and hold their slots. const others = await rawDB @@ -2292,12 +2333,6 @@ export function createArtifactService( cancelRun?.(actor.tenantId, runId, "design_concept_not_picked"), ), ); - const coordinator = await activeCoordinator(projectId); - if (coordinator) - await mailRun( - coordinator.runId, - designChosenNote(stored.title, design.version), - ); return { design, concept: stored.title }; }, diff --git a/apps/hub/src/auth/agent-token-bridge.ts b/apps/hub/src/auth/agent-token-bridge.ts index 16aebf3c..856a8377 100644 --- a/apps/hub/src/auth/agent-token-bridge.ts +++ b/apps/hub/src/auth/agent-token-bridge.ts @@ -443,7 +443,7 @@ export function createAgentTokenBridge( ((c.req.method === "GET" && /^(|\/audit|\/artifacts(?:\/[^/]+)?)$/.test(suffix)) || (c.req.method === "POST" && - /^(\/mutations|\/artifacts|\/artifact-approvals|\/design-concepts|\/presenter-assignments)$/.test( + /^(\/mutations|\/artifacts|\/artifact-approvals|\/design-choice|\/design-concepts|\/presenter-assignments)$/.test( suffix, ))); const deployRoute = diff --git a/apps/hub/src/config/design.ts b/apps/hub/src/config/design.ts index 36023219..d20c4943 100644 --- a/apps/hub/src/config/design.ts +++ b/apps/hub/src/config/design.ts @@ -148,7 +148,3 @@ export const designConceptAssignment = (concept: { // The person's own ideas for the look, carried to each designer in their words. export const designLookIdeas = (ideas: string) => `The person's ideas for the look, in their words: ${ideas.trim()}\n\n`; - -// The hub's note to the coordinator once the person chose a design option in the conversation. -export const designChosenNote = (concept: string, version: number) => - `The person chose the ${concept} design concept. It is filed as the design at version ${String(version)}, approved by them. The build plan is written from it next.`; diff --git a/apps/hub/src/config/project-status.ts b/apps/hub/src/config/project-status.ts index 56ca4f2e..beffba5c 100644 --- a/apps/hub/src/config/project-status.ts +++ b/apps/hub/src/config/project-status.ts @@ -19,6 +19,7 @@ const list = (items: readonly string[]) => const APPROVAL: Record = { instruct_coder: "Approve the change for the app builder", design_app: "Approve the design work", + choose_design: "Choose the design to build from", install_packages: "Approve the packages the app builder asked for", }; diff --git a/apps/hub/src/db/schema.ts b/apps/hub/src/db/schema.ts index 31be0802..c1af53fd 100644 --- a/apps/hub/src/db/schema.ts +++ b/apps/hub/src/db/schema.ts @@ -139,8 +139,8 @@ export const artifactApproval = pgTable( ], ); -// Importing a project file and choosing a design option are the person's own -// decisions, so no native approval exists to reference; the row names that person instead. +// Importing a project file is the person's own decision, so no native approval +// exists to reference; the row names that person instead. export const artifactImportApproval = pgTable( "sb_artifact_import_approval", { diff --git a/apps/hub/src/projects/deploy-tools.ts b/apps/hub/src/projects/deploy-tools.ts index d7f152ec..8c61ad81 100644 --- a/apps/hub/src/projects/deploy-tools.ts +++ b/apps/hub/src/projects/deploy-tools.ts @@ -116,6 +116,7 @@ const ReviewArtifact = type({ instruction: Instruction, }); const DeployCoder = type({ "+": "reject", "instruction?": Instruction }); +const ChooseDesign = type({ "+": "reject", concept: "string > 0" }); const InstructCoder = type({ "+": "reject", instruction: Instruction }); const DesignApp = type({ "+": "reject", @@ -172,7 +173,7 @@ const notApproved = (kind: string, version: number) => " version " + String(version) + (kind === "design" - ? "; they approve a design by confirming a design option in the conversation" + ? "; ask them to choose a design option with choose_design" : "; ask them with request_approval"), ); @@ -279,7 +280,7 @@ export function createDeployTools(options?: { { name: "deploy_coder", description: - "Have the coder build the whole app from the filed problem brief, constraints, approaches, product requirements, build plan and design. The documents are the assignment; an optional instruction adds a note to it. It starts once a person approved the current version of each document: the design by confirming a design option in the conversation, the others through request_approval.", + "Have the coder build the whole app from the filed problem brief, constraints, approaches, product requirements, build plan and design. The documents are the assignment; an optional instruction adds a note to it. It starts once a person approved the current version of each document: the design through choose_design, the others through request_approval.", inputSchema: { ...DeployCoder.toJsonSchema() }, }, { @@ -299,6 +300,12 @@ export function createDeployTools(options?: { ).join("\n"), inputSchema: { ...DesignApp.toJsonSchema() }, }, + { + name: "choose_design", + description: + "Ask the person to choose the filed design option of one design concept, by its label, as the design the build plan and app are built from. Ask about the concept they picked, or the one you recommend when they have not said. Once they approve, the option at the version they saw is filed as the design, approved by them, and the other concepts' designers stop.", + inputSchema: { ...ChooseDesign.toJsonSchema() }, + }, { name: "deploy_presenter", description: @@ -319,6 +326,7 @@ export function createDeployTools(options?: { { name: "deploy_coder" }, { name: "instruct_coder", approval: "ask" }, { name: "design_app", approval: "ask" }, + { name: "choose_design", approval: "ask" }, { name: "deploy_presenter" }, ], factory: (env) => ({ @@ -749,6 +757,12 @@ export function createDeployTools(options?: { error: refusalText(outcome.reason), }, ); + } else if (call.name === "choose_design") { + // This handler runs only after the native approval resolved; the hub binds the choice to it. + result = await hub( + projectPath + "/design-choice", + validArguments(ChooseDesign, call.arguments), + ); } else if (call.name === "deploy_presenter") { const input = validArguments(DeployPresenter, call.arguments); const { byKind, approved, preferences } = await listed(); diff --git a/apps/web/src/api/artifacts.ts b/apps/web/src/api/artifacts.ts index 8e1fcb85..5beed6da 100644 --- a/apps/web/src/api/artifacts.ts +++ b/apps/web/src/api/artifacts.ts @@ -3,7 +3,6 @@ import { type PackageZipReviewerQuery, } from "@corbits-builder/client"; import type { - DesignChoice, DesignTweaks, UserManual, } from "@corbits-builder/schemas/artifact"; @@ -195,11 +194,6 @@ export async function saveDesignTweaks(projectId: string, input: DesignTweaks) { return productClient().saveDesignTweaks(tenantId, projectId, input); } -export async function chooseDesign(projectId: string, input: DesignChoice) { - const tenantId = await workspaceTenantId(); - return productClient().chooseDesign(tenantId, projectId, input); -} - /** The file types the coordinator reads and how many and how large they may be. */ export async function acceptedMaterials(projectId: string) { const tenantId = await workspaceTenantId(); diff --git a/apps/web/src/config.ts b/apps/web/src/config.ts index 9b244ff9..ca90658a 100644 --- a/apps/web/src/config.ts +++ b/apps/web/src/config.ts @@ -553,6 +553,10 @@ export const wording = { file_package: () => "filing the approval package", instruct_coder: () => "waiting for you to approve a change for the app builder", + choose_design: (subject?: string) => + subject + ? `waiting for you to choose the ${subject} design` + : "waiting for you to choose a design", exec: () => "rewriting a tool call", } as Record string>, /** What a call returned, in plain words, for a tool whose raw result means nothing to a person. */ @@ -585,17 +589,6 @@ export const wording = { `The designer of your ${label} concept is waiting for its next instruction.`, /** The pane bar's control that shows what a design concept is going for. */ conceptDirection: "What this design mockup is going for", - /** The conversation's ask to confirm the one filed design concept. */ - confirmOne: - "If you like the design and want to move on to preparing a build plan, confirm. Otherwise, tell me what to change.", - /** The same ask when several concepts are filed; each concept's name opens its mockup. */ - confirmSeveral: - "If you like one of the designs and want to move on to preparing a build plan, confirm it. Otherwise, tell me what to change.", - confirm: "Confirm", - confirmConcept: (label: string) => `Confirm the ${label} design`, - confirming: "Confirming…", - chosen: (label: string) => `✓ Using ${label}`, - chooseFailed: "The design could not be chosen.", }, /** Prebuilt looks a person previews, tweaks and picks before any design exists. */ themes: { @@ -809,6 +802,10 @@ export const wording = { question: { design_app: () => "Ready to start the design?", instruct_coder: () => "Send this change to the app builder?", + choose_design: (subject?: string) => + subject + ? `Build from the ${subject} design?` + : "Build from this design?", } as Record string>, document: (document: string) => `Happy with the ${document}?`, /** An ask that names neither a document nor a tool. */ @@ -826,6 +823,8 @@ export const wording = { /** The line under an ask that covers no one document. */ body: { design_app: "If these look right, the design starts from them.", + choose_design: + "If you like it, the build plan is written from it next. Otherwise, tell the coordinator what to change.", } as Record, /** What the card asks the person to approve, so several open cards never blur together. */ approving: { @@ -868,6 +867,7 @@ export const wording = { startNamed: (label: string) => `Start ${label}`, deny: "Not Yet", }, + choose_design: { approve: "Use This Design", deny: "Not Yet" }, } as Record< string, { @@ -902,9 +902,11 @@ export const wording = { approved: (document: string) => `Approved · ${document}`, /** What an approved ask let go ahead, as its decided line names it: "Approved · the design". */ settled: { - design_app: "the design", - instruct_coder: "the change for the app builder", - } as Record, + design_app: () => "the design", + instruct_coder: () => "the change for the app builder", + choose_design: (subject?: string) => + subject ? `the ${subject} design` : "the design", + } as Record string>, /** A declined card's one line: "Not yet · Approaches". */ declined: (documents?: string) => documents === undefined ? "Not yet" : `Not yet · ${documents}`, diff --git a/apps/web/src/pages/workspace-layout.css b/apps/web/src/pages/workspace-layout.css index 793c0be6..0afeddc1 100644 --- a/apps/web/src/pages/workspace-layout.css +++ b/apps/web/src/pages/workspace-layout.css @@ -285,33 +285,6 @@ font-weight: 600; } -.design-choice { - display: flex; - flex-direction: column; - align-items: flex-start; - gap: var(--space-2); - margin: 0; -} - -.design-choice > p { - margin: 0; -} - -.design-choice-options { - display: flex; - flex-direction: column; - gap: var(--space-1); - margin: 0; - padding: 0; - list-style: none; -} - -.design-choice-options li { - display: flex; - align-items: center; - gap: var(--space-2); -} - .document-decision-details summary { color: var(--wb-muted-foreground); font-size: 0.75rem; diff --git a/apps/web/src/pages/workspace/agent-activity.tsx b/apps/web/src/pages/workspace/agent-activity.tsx index d3736137..909e3c2d 100644 --- a/apps/web/src/pages/workspace/agent-activity.tsx +++ b/apps/web/src/pages/workspace/agent-activity.tsx @@ -18,7 +18,12 @@ const BUSY_POLL_MS = 3_000; const RUN_STATE_POLL_MS = 5_000; const text = wording.activity; // The calls that hold until the person answers. -const ASKS = new Set(["instruct_coder", "request_approval", "design_app"]); +const ASKS = new Set([ + "instruct_coder", + "request_approval", + "design_app", + "choose_design", +]); const CHECK_SCRIPT = /\bcheck-app\b/; /** A person sees what the file is, never where it lives: no folders, no internal ids. */ @@ -60,6 +65,7 @@ const SUBJECT_KEYS = [ "query", "kind", "packages", + "concept", ]; /** What a call's arguments say it touches, as the hub's activity reads it. */ diff --git a/apps/web/src/pages/workspace/design-concepts.tsx b/apps/web/src/pages/workspace/design-concepts.tsx index 02c924ae..12e2e68b 100644 --- a/apps/web/src/pages/workspace/design-concepts.tsx +++ b/apps/web/src/pages/workspace/design-concepts.tsx @@ -4,16 +4,12 @@ * concept named after a design theme starts from that theme's sample app, so its draft shows only * once the designer has changed a page of it. */ -import { useState, type CSSProperties } from "react"; -import { Badge, Button } from "@corbits/react-ui"; +import type { CSSProperties } from "react"; import { MenuLabel } from "@corbits/react-ui/ui/menu"; import { Info } from "lucide-react"; -import { toast } from "sonner"; import type { ProjectArtifact } from "@corbits-builder/client"; import type { InFlightRun } from "@corbits-builder/schemas/artifact"; import { wording } from "../../config.ts"; -import { chooseDesign } from "../../api/artifacts.ts"; -import { humanFault } from "./human-fault.ts"; import { DesignPreview, draftBadge, @@ -35,10 +31,6 @@ export type ConceptView = { readonly direction?: string; /** The filed option's design bundle, once its designer filed one. */ readonly content?: string; - /** The filed option at the version the person sees, which they choose by. */ - readonly option?: { readonly artifactId: string; readonly version: number }; - /** The person chose this concept, and its option is the approved design. */ - readonly chosen?: boolean; /** When the concept's designer started, and whether it is mid-turn or parked waiting for mail. */ readonly run?: { readonly startedAt: string; readonly working: boolean }; }; @@ -57,7 +49,6 @@ export function designConcepts( inFlight: readonly InFlightRun[], asked: readonly { readonly label: string; readonly direction: string }[], content: (option: ProjectArtifact) => string | undefined, - chosen?: string, ): ConceptView[] { const labels = new Set([ ...asked.map((concept) => concept.label), @@ -90,16 +81,7 @@ export function designConcepts( working: run.state === "working", }, }), - ...(body === undefined || option === undefined - ? {} - : { - content: body, - option: { - artifactId: option.artifactId, - version: option.version, - }, - }), - ...(label === chosen ? { chosen: true } : {}), + ...(body === undefined ? {} : { content: body }), }; }); } @@ -284,85 +266,3 @@ function ConceptSketching({ ); } - -/** The design the person chose in the conversation, in place of the ask it answered. */ -export const DesignChosen = ({ concept }: { concept: string }) => ( -

- {wording.design.chosen(concept)} -

-); - -/** - * The person's choice of a filed concept, asked in the conversation; the hub files it as the - * approved design and tells the coordinator. - */ -export function DesignChoice({ - projectId, - concepts, - onShowConcept, - onChosen, -}: { - projectId: string; - /** The concepts with a filed option, each confirmable. */ - concepts: readonly ConceptView[]; - onShowConcept: (tab: string) => void; - onChosen: (concept: string) => void; -}) { - const [choosing, setChoosing] = useState(false); - const choose = async (concept: ConceptView) => { - if (concept.option === undefined) return; - setChoosing(true); - try { - const receipt = await chooseDesign(projectId, concept.option); - onChosen(receipt.concept); - } catch (cause) { - toast.error(humanFault(cause, wording.design.chooseFailed)); - } finally { - setChoosing(false); - } - }; - const [only] = concepts; - if (concepts.length === 1 && only !== undefined) - return ( -
-

{wording.design.confirmOne}

- -
- ); - return ( -
-

{wording.design.confirmSeveral}

-
    - {concepts.map((concept) => ( -
  • - - -
    • - ))} -
-
- ); -} diff --git a/apps/web/src/pages/workspace/document-decision.tsx b/apps/web/src/pages/workspace/document-decision.tsx index 8c3925e5..2afeb081 100644 --- a/apps/web/src/pages/workspace/document-decision.tsx +++ b/apps/web/src/pages/workspace/document-decision.tsx @@ -16,6 +16,7 @@ import { } from "@corbits/react-ui"; import { approvalQuestion, shortDocumentName } from "./collaboration.ts"; import { approvalToolPart } from "./conversation-parts.ts"; +import { conceptTab } from "./design-concepts.tsx"; import type { ApprovalOutcome, DocumentApproval } from "./transcript.tsx"; /** At most ten words of a design direction, with parenthetical notes removed. */ @@ -141,7 +142,8 @@ function settledLine(approval: DocumentApproval): string { return text.approved(text.settledUnnamed); const settled = text.settled[approval.toolName]; return text.approved( - settled ?? approvalQuestion(approval.toolName, undefined, approval.subject), + settled?.(approval.subject) ?? + approvalQuestion(approval.toolName, undefined, approval.subject), ); } @@ -178,6 +180,8 @@ export function DocumentDecision({ const instruction = approval.instruction; const tool = approval.toolName ?? ""; const design = tool === "design_app"; + // A design choice names its concept; the card opens that concept's mockup. + const concept = tool === "choose_design" ? approval.subject : undefined; const decide = (decision: ToolApprovalDecision) => { if (decision.outcome === "rejected") onNo(); else onYes(); @@ -288,6 +292,17 @@ export function DocumentDecision({ {text.show(name)} )} + {concept === undefined || onShowDocument === undefined ? null : ( + onShowDocument(conceptTab(concept))} + > + {text.show(wording.design.conceptTitle(concept))} + + )} {instruction !== undefined && FOLDED_INSTRUCTION_ASKS.has(tool) ? (
{text.details} diff --git a/apps/web/src/pages/workspace/index.tsx b/apps/web/src/pages/workspace/index.tsx index 05210787..83dd44dd 100644 --- a/apps/web/src/pages/workspace/index.tsx +++ b/apps/web/src/pages/workspace/index.tsx @@ -139,8 +139,6 @@ import { useUserManual } from "./user-manual.tsx"; import { conceptOfTab, conceptTab, - DesignChoice, - DesignChosen, designConcepts, DESIGN_OPTION, } from "./design-concepts.tsx"; @@ -477,11 +475,6 @@ function ReadyRoom({ null, ); const [paneTab, setPaneTab] = useState<{ value: string } | null>(null); - // The design the person chose here, by the hub's receipt, and when, so the outcome keeps its place. - const [designChoice, setDesignChoice] = useState<{ - concept: string; - at: number; - } | null>(null); const documentPaneRef = useRef(null); const composerRef = useRef(null); // Each committed reply's thinking and tool work, as this session watched it. @@ -572,7 +565,6 @@ function ReadyRoom({ setChangesFor(null); setDocumentChoice(null); setPaneTab(null); - setDesignChoice(null); setMobilePane("conversation"); setSent(openingDraft(projectId, openingMessage)); setSendFailed(false); @@ -1306,36 +1298,7 @@ function ReadyRoom({ ? state.document.content : undefined; }, - design !== undefined && design.approvedVersion === design.version - ? design.title - : undefined, ); - // Filed concepts wait on the person's choice, asked last in the conversation; a choice made here - // keeps its outcome where it was made. - const choosable = concepts.some((concept) => concept.chosen) - ? [] - : concepts.filter((concept) => concept.option !== undefined); - const shownChoice = - designChoice !== null - ? { - at: designChoice.at, - card: , - } - : choosable.length > 0 - ? { - card: ( - { - setDesignChoice({ concept, at: Date.now() }); - onChanged(); - }} - /> - ), - } - : undefined; // The themes join the pane at the point the coordinator may start the design. const designReached = DESIGN_INPUTS.every((kind) => @@ -1717,9 +1680,6 @@ function ReadyRoom({ approvals={shownApprovals} results={liveRun.results} runs={unseenRuns} - {...(shownChoice === undefined || archived - ? {} - : { designChoice: shownChoice })} live={(dispatch) => dispatchLive(dispatch, dispatchRoom)} onDecide={onDecideApproval} onRequestChanges={requestChanges} diff --git a/apps/web/src/pages/workspace/transcript.tsx b/apps/web/src/pages/workspace/transcript.tsx index 856519db..e4b9579d 100644 --- a/apps/web/src/pages/workspace/transcript.tsx +++ b/apps/web/src/pages/workspace/transcript.tsx @@ -218,22 +218,17 @@ export type PlacedDispatch = { readonly dispatch: Dispatch; }; -/** The coordinator's ask to choose a design, placed when it was answered, else last. */ -export type PlacedChoice = { readonly at?: number; readonly card: ReactNode }; - type ThreadItem = | { kind: "turn"; turn: TranscriptTurn; index: number } | { kind: "updates"; group: UpdateGroup } | { kind: "approval"; approval: DocumentApproval } | { kind: "unit"; unit: WorkUnit } - | { kind: "choice"; card: ReactNode } | { kind: "questions"; callId: string; ask: AskQuestions }; type Placed = | { kind: "update"; at: number; update: SpecialistUpdate } | { kind: "approval"; at: number; approval: DocumentApproval } - | { kind: "unit"; at: number; unit: WorkUnit } - | { kind: "choice"; at: number; card: ReactNode }; + | { kind: "unit"; at: number; unit: WorkUnit }; /** * The person's and the coordinator's turns in order, with each hand-off and @@ -247,7 +242,6 @@ function threadItems( threadUpdates: readonly SpecialistUpdate[], approvals: readonly DocumentApproval[], runs: readonly PlacedDispatch[], - choice: PlacedChoice | undefined, results: ReadonlyMap, ): ThreadItem[] { const handoffs = [ @@ -291,15 +285,6 @@ function threadItems( at: unit.at, unit, })), - ...(choice === undefined - ? [] - : [ - { - kind: "choice", - at: choice.at ?? Number.POSITIVE_INFINITY, - card: choice.card, - } satisfies Placed, - ]), ].sort((left, right) => left.at - right.at); const items: ThreadItem[] = []; let next = 0; @@ -327,7 +312,7 @@ function threadItems( items.push({ kind: "turn", turn, index }); }); flushBefore(Number.POSITIVE_INFINITY); - // An approval or choice with no known time still shows, after everything else. + // An approval with no known time still shows, after everything else. for (; next < placed.length; next += 1) { const entry = placed[next]; if (entry !== undefined && entry.kind !== "update") @@ -339,8 +324,7 @@ function threadItems( function placedItem(entry: Exclude): Card { if (entry.kind === "approval") return { kind: "approval", approval: entry.approval }; - if (entry.kind === "unit") return { kind: "unit", unit: entry.unit }; - return { kind: "choice", card: entry.card }; + return { kind: "unit", unit: entry.unit }; } type Card = Exclude; @@ -416,8 +400,8 @@ function threadSections( /** * A piece of work still at work is current, so it sits after the reports and decisions that came - * while it ran rather than at the call that started it. The design choice waits on the person, so it - * stays after those. + * while it ran rather than at the call that started it. Questions wait on the person, so they stay + * after those. */ function liveLast( cards: readonly Card[], @@ -425,12 +409,11 @@ function liveLast( ): Card[] { const isLive = (card: Card) => card.kind === "unit" && live(card.unit.dispatch); - const isChoice = (card: Card) => - card.kind === "choice" || card.kind === "questions"; + const isQuestions = (card: Card) => card.kind === "questions"; return [ - ...cards.filter((card) => !isLive(card) && !isChoice(card)), + ...cards.filter((card) => !isLive(card) && !isQuestions(card)), ...cards.filter(isLive), - ...cards.filter(isChoice), + ...cards.filter(isQuestions), ]; } @@ -602,7 +585,6 @@ export function RoomThread({ approvals = [], results = new Map(), runs = [], - designChoice, live = () => false, onDecide, onRequestChanges, @@ -633,7 +615,6 @@ export function RoomThread({ results?: ReadonlyMap; /** Runs at work with no hand-off in view, placed at their start. */ runs?: readonly PlacedDispatch[]; - designChoice?: PlacedChoice; /** The hand-off's run is still starting or at work. */ live?: (dispatch: Dispatch) => boolean; onDecide?: (decision: { @@ -717,17 +698,6 @@ export function RoomThread({ ); - if (card.kind === "choice") - return ( - - - {headed ? null : ( - - )} - {card.card} - - - ); return renderApproval(card.approval, position, headed); }; const renderUpdates = (group: UpdateGroup, subject: string | undefined) => ( @@ -851,7 +821,7 @@ export function RoomThread({ ); const sections = threadSections( - threadItems(turns, updates, approvals, runs, designChoice, results), + threadItems(turns, updates, approvals, runs, results), results, ); const mapTurns = sections.flatMap( diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index f81100bb..04ea2071 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -84,15 +84,18 @@ assignment and has no read tool, so it works only from what it was given. ### Approvals Approvals are the runtime's own human approvals. A tool that needs a person's -go-ahead (`request_approval`, `design_app`, `instruct_coder`, adding a package) +go-ahead (`request_approval`, `design_app`, `choose_design`, `instruct_coder`, +adding a package) is declared as asking, so the runtime parks the call until a person resolves it. After it resolves, the hub records the exact document versions and hashes it covered, and only those whose hash still matches storage. - Versions and hashes come from the hub, never from model input, so an approval cannot be pointed at a version the person did not see. -- Choosing a design option and importing a project as approved are the - person's own acts; the hub records that person as the approver. +- Choosing a design option is a `choose_design` approval; each approval files + one design, so a standing grant never chooses one for the person. +- Importing a project as approved is the person's own act; the hub records + that person as the approver. - Hand-off tools check approvals before they start the next writer. ### Questions diff --git a/docs/IMPLEMENTATION.md b/docs/IMPLEMENTATION.md index fe5c84d7..898cc944 100644 --- a/docs/IMPLEMENTATION.md +++ b/docs/IMPLEMENTATION.md @@ -72,8 +72,8 @@ own credential: - documents: `read_artifact`, `write_artifact` ([apps/hub/src/artifacts/tools.ts](https://github.com/corbitsdev/corbits-builder/blob/main/apps/hub/src/artifacts/tools.ts)); - hand-offs and decisions: `request_approval`, `ask_questions`, - `deploy_writer`, `review_artifact`, `design_app`, `deploy_coder`, - `instruct_coder` + `deploy_writer`, `review_artifact`, `design_app`, `choose_design`, + `deploy_coder`, `instruct_coder` ([apps/hub/src/projects/deploy-tools.ts](https://github.com/corbitsdev/corbits-builder/blob/main/apps/hub/src/projects/deploy-tools.ts)); - design filing: `file_design_option` ([apps/hub/src/artifacts/design-tools.ts](https://github.com/corbitsdev/corbits-builder/blob/main/apps/hub/src/artifacts/design-tools.ts)); diff --git a/docs/PRODUCT.md b/docs/PRODUCT.md index 137b942d..27a3a391 100644 --- a/docs/PRODUCT.md +++ b/docs/PRODUCT.md @@ -72,7 +72,8 @@ says; there is no fixed sequence of screens. A typical project goes: themes can be clicked through before any design exists. Each concept is a clickable mockup of every screen, with light and dark modes, desktop and phone views, and simple tweaks (colour, background, corners, font). The - person confirms one concept, and that choice is the design approval. + person chooses one concept on an approval card, and that choice is the + design approval. 7. **Build plan.** Written from the chosen approach and design: the stack, the order of work and the acceptance checklist. 8. **Live build.** One coder builds the whole app. The workspace shows the diff --git a/packages/client/src/client/artifacts.ts b/packages/client/src/client/artifacts.ts index e3d74afe..1304422f 100644 --- a/packages/client/src/client/artifacts.ts +++ b/packages/client/src/client/artifacts.ts @@ -26,7 +26,6 @@ export function buildArtifactsMethods( | "downloadApp" | "exportProjectArchive" | "saveDesignTweaks" - | "chooseDesign" | "acceptedMaterials" | "attachMaterials" | "removeMaterial" @@ -157,13 +156,6 @@ export function buildArtifactsMethods( input, ); }, - chooseDesign(tenantId, projectId, input, signal) { - return request( - `${projectPath(tenantId, projectId)}/design-choice`, - signal, - input, - ); - }, acceptedMaterials(tenantId, projectId, signal) { return request( `${projectPath(tenantId, projectId)}/materials/accepted`, diff --git a/packages/client/src/client/shared.ts b/packages/client/src/client/shared.ts index bda6a08b..2318b0d8 100644 --- a/packages/client/src/client/shared.ts +++ b/packages/client/src/client/shared.ts @@ -53,8 +53,6 @@ import type { DecidedApprovalList, DesignDraft, DesignThemeList, - DesignChoice, - DesignChoiceReceipt, DesignTweaks, DesignTweaksReceipt, FailedRun, @@ -524,13 +522,6 @@ export type ProductApiClient = Readonly<{ input: DesignTweaks, signal?: AbortSignal, ): Promise; - /** The person chooses a filed design option at its version; it becomes the approved design. */ - chooseDesign( - tenantId: string, - projectId: string, - input: DesignChoice, - signal?: AbortSignal, - ): Promise; /** The file types the coordinator's model reads; an empty list accepts none. */ acceptedMaterials( tenantId: string, diff --git a/packages/schemas/src/artifact.ts b/packages/schemas/src/artifact.ts index 86af425f..c3115e84 100644 --- a/packages/schemas/src/artifact.ts +++ b/packages/schemas/src/artifact.ts @@ -318,12 +318,11 @@ export const DesignTweaksReceipt = type({ }); export type DesignTweaksReceipt = typeof DesignTweaksReceipt.infer; -// The person's choice of a filed design option at the version they saw; the hub files it as the -// design and records the person's approval of exactly that design version. +// The design concept whose filed option the person approved through choose_design; the hub files +// that option's current version as the design. export const DesignChoice = type({ "+": "reject", - artifactId: "string > 0", - version: "number.integer > 0", + concept: "string > 0", }); export type DesignChoice = typeof DesignChoice.infer; diff --git a/test/e2e/pipeline.test.ts b/test/e2e/pipeline.test.ts index 1a476622..dd787046 100644 --- a/test/e2e/pipeline.test.ts +++ b/test/e2e/pipeline.test.ts @@ -180,11 +180,6 @@ async function withPipelineScenario( sendMessage: (lines: readonly string[]) => Promise; decideArtifactApproval: (step: ArtifactApprovalStep) => Promise; decideToolApproval: (step: ToolApprovalStep) => Promise; - chooseFiledDesignOption: ( - stage: string, - version: number, - presentationSubstring: string, - ) => Promise; assertArtifactStates: ( expected: readonly ExpectedArtifactState[], ) => Promise; @@ -373,40 +368,6 @@ async function withPipelineScenario( step.feedback, ); }, - chooseFiledDesignOption: async ( - stage, - version, - presentationSubstring, - ) => { - const designOption = await waitFor( - stage, - `design_option v${version} and coordinator presentation`, - async () => { - const [{ artifacts }, conv] = await Promise.all([ - client.projectArtifacts(projectTenantId, options.projectId), - client.conversationEvents(projectTenantId, options.projectId), - ]); - const option = findArtifact(artifacts, "design_option", version); - if (!option) return undefined; - const presented = conv.turns?.some( - (turn) => - turn.role === "assistant" && - turn.text.includes(presentationSubstring), - ); - return presented ? option : undefined; - }, - ); - const designReceipt = await client.chooseDesign( - projectTenantId, - options.projectId, - { - artifactId: designOption.artifactId, - version: designOption.version, - }, - ); - expect(designReceipt.design.kind).toBe("design"); - expect(designReceipt.design.version).toBe(version); - }, assertArtifactStates: async (expected) => { const { artifacts, failed } = await client.projectArtifacts( projectTenantId, @@ -553,11 +514,11 @@ describe("Builder E2E pipeline replay", () => { toolName: "design_app", afterApproved: { kind: "product_requirements", version: 1 }, }); - await driver.chooseFiledDesignOption( - "design", - 1, - "The design is filed", - ); + await driver.decideToolApproval({ + stage: "design", + toolName: "choose_design", + afterFiled: { kind: "design_option", version: 1 }, + }); }); await driver.runStage("build", async () => { diff --git a/test/fixtures/pipeline/runs/coordinator-1/exchanges/23/expect.json b/test/fixtures/pipeline/runs/coordinator-1/exchanges/23/expect.json index 1cecaf08..7adb2ae2 100644 --- a/test/fixtures/pipeline/runs/coordinator-1/exchanges/23/expect.json +++ b/test/fixtures/pipeline/runs/coordinator-1/exchanges/23/expect.json @@ -9,7 +9,8 @@ "review_artifact", "design_app", "deploy_coder", - "instruct_coder" + "instruct_coder", + "choose_design" ], "toolResults": [] } diff --git a/test/fixtures/pipeline/runs/coordinator-1/exchanges/23/response.sse b/test/fixtures/pipeline/runs/coordinator-1/exchanges/23/response.sse index a10002c1..2d13a37f 100644 --- a/test/fixtures/pipeline/runs/coordinator-1/exchanges/23/response.sse +++ b/test/fixtures/pipeline/runs/coordinator-1/exchanges/23/response.sse @@ -1,8 +1,12 @@ data: {"choices":[{"index":0,"delta":{"role":"assistant"},"finish_reason":null}]} -data: {"choices":[{"index":0,"delta":{"content":"The design is filed, and it passed the automatic check at desktop and phone sizes. Open it in the workspace and click through it. If you're happy, choose it there, and I'll then write the build plan. If you'd like changes, tell me what to adjust."},"finish_reason":null}]} +data: {"choices":[{"index":0,"delta":{"content":"The design is filed, and it passed the automatic check at desktop and phone sizes. Open it in the workspace and click through it. If you're happy, choose it on the card below, and I'll then write the build plan. If you'd like changes, tell me what to adjust."},"finish_reason":null}]} -data: {"choices":[{"index":0,"delta":{},"finish_reason":"stop"}]} +data: {"choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"id":"coord_choose_design","function":{"name":"choose_design","arguments":""}}]},"finish_reason":null}]} + +data: {"choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":"{\"concept\":\"Adaptive weather-driven\"}"}}]},"finish_reason":null}]} + +data: {"choices":[{"index":0,"delta":{},"finish_reason":"tool_calls"}]} data: {"choices":[],"usage":{"prompt_tokens":120,"completion_tokens":60}} diff --git a/test/fixtures/pipeline/runs/coordinator-1/exchanges/24/expect.json b/test/fixtures/pipeline/runs/coordinator-1/exchanges/24/expect.json index 195d8f68..936e8a76 100644 --- a/test/fixtures/pipeline/runs/coordinator-1/exchanges/24/expect.json +++ b/test/fixtures/pipeline/runs/coordinator-1/exchanges/24/expect.json @@ -1,6 +1,5 @@ { - "lastMessageRole": "user", - "userTextContains": "Adaptive weather-driven", + "lastMessageRole": "tool", "requiredTools": [ "read_artifact", "write_artifact", @@ -9,7 +8,16 @@ "review_artifact", "design_app", "deploy_coder", - "instruct_coder" + "instruct_coder", + "choose_design" ], - "toolResults": [] + "toolResults": [ + { + "callId": "coord_choose_design", + "toolName": "choose_design", + "isError": false, + "keys": ["design", "concept"], + "contains": "Adaptive weather-driven" + } + ] } diff --git a/workflows/coordinator/src/prompt.ts b/workflows/coordinator/src/prompt.ts index b03317a0..da28d402 100644 --- a/workflows/coordinator/src/prompt.ts +++ b/workflows/coordinator/src/prompt.ts @@ -37,7 +37,7 @@ Once a document is good, summarise it for the person and give your view. When it The person makes every decision; your part is a recommendation and a proposal to move on. They approve: - each document, through request_approval. It puts an approval card in front of them, and the card is the question they answer. Whenever filed documents wait on their approval and have no open questions left, cover all of them in one request_approval and tell the person in a sentence or two what is in front of them and what you recommend. Their approval lets the writer of the next document start. - the go-ahead to start the design, when you use design_app. -- the finished design, which they confirm here in the conversation from the design options. +- the finished design, through choose_design, from the design options. - every change you send with instruct_coder. When their preferences let documents go ahead without their approval, tell them about each document instead. @@ -47,7 +47,7 @@ After a project is imported, ask the person what they want to do now: re-plan fr ## Design Once the brief, constraints, approaches and product requirements exist, give the person a quick, plain check-in on where things stand: which documents are drafted, by title, and an invitation to look them over. In the same message ask them two things: whether they want one design you suggest or to explore several different looks, and what ideas they have for the look, such as apps they like, colours or a feel. Once they answer, use design_app with their ideas in their own words: for one suggested design, the single look that best fits their idea and what they said; to explore, as many clearly different looks as the person's preferences ask for, from the catalog in design_app or of your own when the idea calls for it. The instruction states the goal; the designers decide the screens. When the person chose one of the design themes, design_app makes just that theme's concept, with their changes in its direction. -Each concept is filed as a design option the person can click through, and they see the options together once every concept is filed. Then tell them what sets each one apart and which you recommend; the conversation then asks them to confirm the design they want, or to tell you what to change. When they confirm one, the hub tells you which design they chose; it is filed and approved, and the build plan is written from it next. When they want two concepts blended, use design_app for one concept that describes the blend. When none fits, ask what they liked and missed, and start fresh concepts with that in the art direction. +Each concept is filed as a design option the person can click through, and they see the options together once every concept is filed. Then tell them what sets each one apart and which you recommend, and use choose_design for the concept they picked, else the one you recommend; its card asks them to choose it or to tell you what to change. Once they approve, it is filed as the design, and the build plan is written from it next. When they want two concepts blended, use design_app for one concept that describes the blend. When none fits, ask what they liked and missed, and start fresh concepts with that in the art direction. ## Building the app When the brief, constraints, approaches, requirements and build plan all exist, tell the person in your own words that it looks ready to build and that they can ask anything first, and use deploy_coder once for the whole app. The plan's phases order the coder's own work within that one assignment. Send later changes to the running coder with instruct_coder. From 291574385dfbcf8d33c3c2a249f459194407b394 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Wed, 4 Nov 2026 16:40:35 -0800 Subject: [PATCH 2/2] fix: file a design choice atomically for the approved concept A choose_design approval covers the concept, so a look tweak saved while the card is open is filed with the design. The approval lock, the design write and both approval records share one transaction, and a reused approval is refused as approval_already_used. A refused choice tells the coordinator to ask again, and the card says approved only once the design is recorded. The concept link shows only while the card is pending and the concept has a tab. --- apps/hub/src/artifacts/routes.ts | 4 +- apps/hub/src/artifacts/service.ts | 149 +++++------ apps/hub/src/db/schema.ts | 3 +- apps/hub/src/projects/deploy-tools.test.ts | 29 +++ apps/hub/src/projects/deploy-tools.ts | 17 +- apps/web/src/config.ts | 5 + .../src/pages/workspace/document-decision.tsx | 16 +- apps/web/src/pages/workspace/index.tsx | 1 + .../src/pages/workspace/settled-approvals.ts | 19 +- .../src/pages/workspace/transcript.test.tsx | 32 ++- apps/web/src/pages/workspace/transcript.tsx | 6 + docs/ARCHITECTURE.md | 6 +- test/AGENTS.md | 5 +- test/e2e/design-choice.test.ts | 240 ++++++++++++++++++ test/e2e/support/extract.ts | 28 +- 15 files changed, 460 insertions(+), 100 deletions(-) create mode 100644 test/e2e/design-choice.test.ts diff --git a/apps/hub/src/artifacts/routes.ts b/apps/hub/src/artifacts/routes.ts index 0ce04607..25f576e4 100644 --- a/apps/hub/src/artifacts/routes.ts +++ b/apps/hub/src/artifacts/routes.ts @@ -265,7 +265,7 @@ export function mountArtifactRoutes( tags: ["Artifacts"], operationId: "chooseDesign", description: - "Called by the coordinator's choose_design tool once the person approved its native ask. The hub files the concept's design_option, at the version the person saw, as the project's design, records that approval against exactly that option and design version and hash, and stops the other concepts' designers. Each approval chooses once.", + "Called by the coordinator's choose_design tool once the person approved its native ask. The approval covers the concept, so the hub files the concept's newest design_option version, look tweaks the person saved while deciding included, as the project's design, records that approval against exactly that option and design version and hash in the same transaction, and stops the other concepts' designers. Each approval chooses once.", security, requestBody: { required: true, @@ -283,7 +283,7 @@ export function mountArtifactRoutes( 404: { description: "No filed design option for that concept" }, 409: { description: - "No unused approval of choose_design for that concept, or the option changed while the person decided", + "No approval of choose_design for that concept (approval_not_resolved), or it already chose a design (approval_already_used)", }, }, }), diff --git a/apps/hub/src/artifacts/service.ts b/apps/hub/src/artifacts/service.ts index 35aa7ab7..41ce3307 100644 --- a/apps/hub/src/artifacts/service.ts +++ b/apps/hub/src/artifacts/service.ts @@ -168,9 +168,8 @@ import { requestErrorMetadata } from "../request-error.ts"; const log = getLogger(["corbits-builder", "hub", "artifacts"]); // The native asks are the human approvals. A request_approval approval covers the documents of -// the kinds it asked about and an instruct_coder approval every filed document. A choose_design -// approval covers the option the person chose and the design filed from it. The app is the coder's -// own output, filed while it works, so no approval covers it. +// the kinds it asked about and an instruct_coder approval every filed document. The app is the +// coder's own output, filed while it works, so no approval covers it. const APPROVAL_COVERS: Record boolean> = { request_approval: (kind) => kind !== "app" && kind !== "design" && kind !== "design_option", @@ -2204,9 +2203,11 @@ export function createArtifactService( }); }, - // Called by choose_design after the person approved its native ask. The concept's option at the - // version they saw is filed as the design, and that approval covers exactly that design version. - // An approval is used once, so a standing grant never chooses a design without the person. + // Called by choose_design after the person approved its native ask. A choose_design approval + // covers the concept the person chose, so the option's newest version is filed as the design, + // look tweaks they saved while deciding included, and the approval covers the option and the + // design filed from it. An approval is used once, so a standing grant never chooses a design + // without the person. The approval, the design and both records commit together or not at all. async chooseDesign( actor: Actor, projectId: string, @@ -2218,32 +2219,9 @@ export function createArtifactService( issues: input, }); await requireProject(actor, projectId, "manage"); - const option = ( - await listArtifacts(db, actor.tenantId, { - kind: "design_option", - limit: MAX_LIST_LIMIT, - }) - ).rows.find((row) => row.title === input.concept); - const [newest] = option - ? (await listArtifactVersions(db, option.id, { limit: 1 })).versions - : []; - const stored = - option && newest - ? await getArtifactVersion(db, option.id, newest.version) - : undefined; - if (!option || !newest || !stored?.contentSha256) - throw new ProjectError(404, "design_option_not_found", { - concept: input.concept, - }); - const optionRef: ArtifactRef = { - artifactId: option.id, - kind: "design_option", - version: stored.version, - sha256: stored.contentSha256, - }; - const approvalId = await rawDB.transaction(async (tx) => { + const { design, title } = await db.transaction(async (tx) => { const [granted] = await tx - .select({ id: approval.id, askedAt: approval.createdAt }) + .select({ id: approval.id }) .from(approval) .where( and( @@ -2256,64 +2234,75 @@ export function createArtifactService( .orderBy(desc(approval.resolvedAt)) .limit(1) .for("update"); - const [recorded] = granted - ? await tx - .select({ approvalId: artifactApproval.approvalId }) - .from(artifactApproval) - .where(eq(artifactApproval.approvalId, granted.id)) - .limit(1) - : []; - if (!granted || recorded) + if (!granted) throw new ProjectError(409, "approval_not_resolved", { tool: "choose_design", }); - // A version filed while the person was deciding is not the one they saw. - if (new Date(newest.createdAt) > granted.askedAt) - throw new ProjectError(409, "artifact_changed_during_approval", { - kind: optionRef.kind, - version: optionRef.version, + const [recorded] = await tx + .select({ approvalId: artifactApproval.approvalId }) + .from(artifactApproval) + .where(eq(artifactApproval.approvalId, granted.id)) + .limit(1); + if (recorded) + throw new ProjectError(409, "approval_already_used", { + tool: "choose_design", }); - await tx.insert(artifactApproval).values({ - approvalId: granted.id, - tenantId: actor.tenantId, - projectId, - tool: "choose_design", - ...optionRef, - }); - return granted.id; - }); - const [existing] = ( - await listArtifacts(db, actor.tenantId, { kind: "design", limit: 1 }) - ).rows; - const written = existing - ? await writeArtifactVersion(db, { - scope: actor, - artifactId: existing.id, - title: stored.title, - content: stored.content, + const option = ( + await listArtifacts(tx, actor.tenantId, { + kind: "design_option", + limit: MAX_LIST_LIMIT, + }) + ).rows.find((row) => row.title === input.concept); + const stored = option + ? await getArtifactVersion(tx, option.id, option.version) + : undefined; + if (!option || !stored?.contentSha256) + throw new ProjectError(404, "design_option_not_found", { + concept: input.concept, + }); + const [existing] = ( + await listArtifacts(tx, actor.tenantId, { + kind: "design", + limit: 1, }) - : await db.transaction((tx) => - createArtifact(tx, { + ).rows; + const written = existing + ? await writeArtifactVersion(tx, { + scope: actor, + artifactId: existing.id, + title: stored.title, + content: stored.content, + }) + : await createArtifact(tx, { scope: actor, ownerPrincipalId: actor.principalId, kind: "design", title: stored.title, content: stored.content, source: { origin: "manual" }, - }), - ); - const design: ArtifactRef = { - artifactId: "artifactId" in written ? written.artifactId : written.id, - kind: "design", - version: written.version, - sha256: written.contentSha256!, - }; - await rawDB.insert(artifactApproval).values({ - approvalId, - tenantId: actor.tenantId, - projectId, - tool: "choose_design", - ...design, + }); + const design: ArtifactRef = { + artifactId: "artifactId" in written ? written.artifactId : written.id, + kind: "design", + version: written.version, + sha256: written.contentSha256!, + }; + const optionRef: ArtifactRef = { + artifactId: option.id, + kind: "design_option", + version: stored.version, + sha256: stored.contentSha256, + }; + await tx.insert(artifactApproval).values( + [optionRef, design].map((ref) => ({ + approvalId: granted.id, + tenantId: actor.tenantId, + projectId, + tool: "choose_design", + ...ref, + })), + ); + return { design, title: stored.title }; }); // The other concepts' designers would file options nobody compares and hold their slots. const others = await rawDB @@ -2324,7 +2313,7 @@ export function createArtifactService( and( eq(designConcept.tenantId, actor.tenantId), eq(designConcept.projectId, projectId), - ne(designConcept.label, stored.title), + ne(designConcept.label, title), inArray(workflowRun.status, ["deployed", "running"]), ), ); @@ -2333,7 +2322,7 @@ export function createArtifactService( cancelRun?.(actor.tenantId, runId, "design_concept_not_picked"), ), ); - return { design, concept: stored.title }; + return { design, concept: title }; }, approvalRounds: createApprovalRoundService( diff --git a/apps/hub/src/db/schema.ts b/apps/hub/src/db/schema.ts index c1af53fd..01a9e2da 100644 --- a/apps/hub/src/db/schema.ts +++ b/apps/hub/src/db/schema.ts @@ -140,7 +140,8 @@ export const artifactApproval = pgTable( ); // Importing a project file is the person's own decision, so no native approval -// exists to reference; the row names that person instead. +// exists to reference; the row names that person instead. The seed script imports +// its fixture projects the same way, their chosen design included. export const artifactImportApproval = pgTable( "sb_artifact_import_approval", { diff --git a/apps/hub/src/projects/deploy-tools.test.ts b/apps/hub/src/projects/deploy-tools.test.ts index 6e0ec6b4..5e205c35 100644 --- a/apps/hub/src/projects/deploy-tools.test.ts +++ b/apps/hub/src/projects/deploy-tools.test.ts @@ -394,3 +394,32 @@ test("the build plan starts only from the design the person chose", async () => ).toBeUndefined(); expect(deployed).toHaveLength(1); }); + +test("a design choice the hub refuses tells the coordinator to ask the person again", async () => { + const fetch = async () => + Response.json( + { code: "approval_already_used", tool: "choose_design" }, + { status: 409 }, + ); + const capabilities = { + resolve: (name: string) => + name === "credentials" + ? { resolve: async () => ({ kind: "http", fetch }) } + : { send: async () => undefined }, + } as unknown as RuntimeCapabilities; + const result = await createDeployTools({ + tenantId: "tenant", + projectId: "project", + domain: "example.invalid", + workflows, + maxResultBytes: 16384, + })({ capabilities } as never).run( + { id: "call", name: "choose_design", arguments: { concept: "Harbour" } }, + new AbortController().signal, + ); + expect(result.isError).toBe(true); + expect(result.content).toStartWith("approval_already_used"); + expect(result.content).toEndWith( + "no design was filed, so ask the person again with choose_design", + ); +}); diff --git a/apps/hub/src/projects/deploy-tools.ts b/apps/hub/src/projects/deploy-tools.ts index 8c61ad81..f18def9f 100644 --- a/apps/hub/src/projects/deploy-tools.ts +++ b/apps/hub/src/projects/deploy-tools.ts @@ -303,7 +303,7 @@ export function createDeployTools(options?: { { name: "choose_design", description: - "Ask the person to choose the filed design option of one design concept, by its label, as the design the build plan and app are built from. Ask about the concept they picked, or the one you recommend when they have not said. Once they approve, the option at the version they saw is filed as the design, approved by them, and the other concepts' designers stop.", + "Ask the person to choose the filed design option of one design concept, by its label, as the design the build plan and app are built from. Ask about the concept they picked, or the one you recommend when they have not said. Once they approve, the concept's newest option, with any look tweaks they saved, is filed as the design, approved by them, and the other concepts' designers stop.", inputSchema: { ...ChooseDesign.toJsonSchema() }, }, { @@ -759,10 +759,17 @@ export function createDeployTools(options?: { ); } else if (call.name === "choose_design") { // This handler runs only after the native approval resolved; the hub binds the choice to it. - result = await hub( - projectPath + "/design-choice", - validArguments(ChooseDesign, call.arguments), - ); + try { + result = await hub( + projectPath + "/design-choice", + validArguments(ChooseDesign, call.arguments), + ); + } catch (error) { + throw new Refusal( + refusalText(error) + + "; no design was filed, so ask the person again with choose_design", + ); + } } else if (call.name === "deploy_presenter") { const input = validArguments(DeployPresenter, call.arguments); const { byKind, approved, preferences } = await listed(); diff --git a/apps/web/src/config.ts b/apps/web/src/config.ts index ca90658a..33f9e929 100644 --- a/apps/web/src/config.ts +++ b/apps/web/src/config.ts @@ -907,6 +907,11 @@ export const wording = { choose_design: (subject?: string) => subject ? `the ${subject} design` : "the design", } as Record string>, + /** An approved design choice the hub has not filed; a failed one is asked again. */ + choiceNotFiled: (subject?: string) => + subject + ? `Chosen · the ${subject} design, not filed yet` + : "Chosen · the design, not filed yet", /** A declined card's one line: "Not yet · Approaches". */ declined: (documents?: string) => documents === undefined ? "Not yet" : `Not yet · ${documents}`, diff --git a/apps/web/src/pages/workspace/document-decision.tsx b/apps/web/src/pages/workspace/document-decision.tsx index 2afeb081..d69c8f8d 100644 --- a/apps/web/src/pages/workspace/document-decision.tsx +++ b/apps/web/src/pages/workspace/document-decision.tsx @@ -140,6 +140,9 @@ function settledLine(approval: DocumentApproval): string { ); if (approval.toolName === undefined) return text.approved(text.settledUnnamed); + // The choice is made only once the hub files the design; until then the card does not say approved. + if (approval.toolName === "choose_design" && approval.filed !== true) + return text.choiceNotFiled(approval.subject); const settled = text.settled[approval.toolName]; return text.approved( settled?.(approval.subject) ?? @@ -150,6 +153,7 @@ function settledLine(approval: DocumentApproval): string { export function DocumentDecision({ approval, shownDocument, + conceptTabs = [], onShowDocument, disabled = false, error, @@ -160,6 +164,8 @@ export function DocumentDecision({ approval: DocumentApproval; /** The document the reading pane shows, by kind. */ shownDocument?: string | null; + /** The design concepts the reading pane has a tab for, by label. */ + conceptTabs?: readonly string[]; /** Takes the reading pane to that document. */ onShowDocument?: (kind: string) => void; disabled?: boolean; @@ -180,8 +186,14 @@ export function DocumentDecision({ const instruction = approval.instruction; const tool = approval.toolName ?? ""; const design = tool === "design_app"; - // A design choice names its concept; the card opens that concept's mockup. - const concept = tool === "choose_design" ? approval.subject : undefined; + // A pending design choice names its concept; the card opens that concept's mockup while it has a tab. + const concept = + tool === "choose_design" && + approval.outcome === undefined && + approval.subject !== undefined && + conceptTabs.includes(approval.subject) + ? approval.subject + : undefined; const decide = (decision: ToolApprovalDecision) => { if (decision.outcome === "rejected") onNo(); else onYes(); diff --git a/apps/web/src/pages/workspace/index.tsx b/apps/web/src/pages/workspace/index.tsx index 83dd44dd..a5117d1c 100644 --- a/apps/web/src/pages/workspace/index.tsx +++ b/apps/web/src/pages/workspace/index.tsx @@ -1677,6 +1677,7 @@ function ReadyRoom({ }))} onOpenDocument={openDocument} shownDocument={chatFirst ? null : (selected?.kind ?? null)} + conceptTabs={concepts.map((view) => view.label)} approvals={shownApprovals} results={liveRun.results} runs={unseenRuns} diff --git a/apps/web/src/pages/workspace/settled-approvals.ts b/apps/web/src/pages/workspace/settled-approvals.ts index 0ff1db56..78014a5d 100644 --- a/apps/web/src/pages/workspace/settled-approvals.ts +++ b/apps/web/src/pages/workspace/settled-approvals.ts @@ -20,6 +20,9 @@ import type { ApprovalOutcome, DocumentApproval } from "./transcript.tsx"; export type OpenApproval = DocumentApproval & { readonly tenantId: string }; +// The ask whose approval files the design only after it resolves, so its card waits on the record. +const FILES_ON_APPROVAL = "choose_design"; + function settledCard( record: DecidedApproval, artifacts: readonly ProjectArtifact[], @@ -29,7 +32,11 @@ function settledCard( status: record.status, ...(record.scope === null ? {} : { scope: record.scope }), }; - if (decidedHere !== undefined) return { ...decidedHere, outcome }; + const filed = + record.toolName === FILES_ON_APPROVAL + ? { filed: record.documents.some((ref) => ref.kind === "design") } + : {}; + if (decidedHere !== undefined) return { ...decidedHere, outcome, ...filed }; const recorded = artifacts.flatMap((item) => { const approved = record.documents.find((ref) => ref.kind === item.kind); return approved === undefined @@ -44,6 +51,7 @@ function settledCard( ...cardDocuments(ask, record.status === "approved" ? recorded : artifacts), ...ask, outcome, + ...filed, }; } @@ -77,6 +85,15 @@ export function useSettledApprovals( void queryClient.invalidateQueries({ queryKey }); }, [projectId, openKey]); + // A design choice is recorded when its design is filed, after the ask left the pending list. + const designVersion = artifacts.find( + (item) => item.kind === "design", + )?.version; + useEffect(() => { + if (designVersion === undefined) return; + void queryClient.invalidateQueries({ queryKey }); + }, [projectId, designVersion]); + useEffect(() => { if (decided.error !== null) toast.error( diff --git a/apps/web/src/pages/workspace/transcript.test.tsx b/apps/web/src/pages/workspace/transcript.test.tsx index 1ff0f1e7..6d5a12cb 100644 --- a/apps/web/src/pages/workspace/transcript.test.tsx +++ b/apps/web/src/pages/workspace/transcript.test.tsx @@ -18,7 +18,7 @@ import { pendingLiveTurn, waitingOnPerson, } from "./live-run.ts"; -import { RoomThread } from "./transcript.tsx"; +import { RoomThread, type DocumentApproval } from "./transcript.tsx"; function findElement( node: ReactNode, @@ -448,6 +448,36 @@ describe("conversation transcript", () => { expect(html).not.toContain(wording.approval.document("")); }); + test("a design choice links its concept while pending and says approved only once filed", () => { + const card = (approval: DocumentApproval, conceptTabs: string[] = []) => + renderToStaticMarkup( + {}} + onYes={() => {}} + onAlways={() => {}} + onNo={() => {}} + />, + ); + const ask = { + approvalId: "appr-choice", + toolName: "choose_design", + subject: "Harbour", + }; + const link = wording.approval.show(wording.design.conceptTitle("Harbour")); + expect(card(ask, ["Harbour"])).toContain(link); + expect(card(ask)).not.toContain(link); + const approved = { ...ask, outcome: { status: "approved" as const } }; + const unfiled = card(approved, ["Harbour"]); + expect(unfiled).not.toContain(link); + expect(unfiled).not.toContain("Approved"); + expect(unfiled).toContain(wording.approval.choiceNotFiled("Harbour")); + expect(card({ ...approved, filed: true })).toContain( + wording.approval.approved("the Harbour design"), + ); + }); + test("a design approval lists each look and starts one by name", () => { const html = renderToStaticMarkup( void; /** The document the reading pane shows, by kind, so a card can say where it is. */ shownDocument?: string | null; + /** The design concepts the reading pane has a tab for, by label. */ + conceptTabs?: readonly string[]; /** Pending document decisions. Not decided here. */ approvals?: readonly DocumentApproval[]; /** Native results of the coordinator's calls this session saw, by call id. */ @@ -743,6 +748,7 @@ export function RoomThread({ + JSON.stringify({ files: { "index.html": `
${look}
` } }); + +// The design-choice route on real Postgres: each choose_design approval files one design, only +// for the concept it asked about, and covers the concept, so a look tweak the person saves while +// the card is open is filed with it. +test("a design choice files once, for the approved concept, tweaks included", async () => { + const database = { + host: required("E2E_DB_HOST"), + port: Number(required("E2E_DB_PORT")), + username: required("E2E_DB_USER"), + password: required("E2E_DB_PASSWORD"), + }; + const dbName = `sb_choice_${randomUUID().replaceAll("-", "")}`; + const admin = postgres({ + ...database, + database: "postgres", + max: 1, + onnotice: () => undefined, + }); + await admin.unsafe(`create database ${dbName}`); + const directory = await mkdtemp(join(tmpdir(), "sb-choice-")); + const origin = "http://127.0.0.1:1"; + const environment: Record = { + NODE_ENV: "test", + HOST: "127.0.0.1", + // Never bound: requests go straight to the app. + PORT: "1", + DB_HOST: database.host, + DB_PORT: String(database.port), + DB_USER: database.username, + DB_PASSWORD: database.password, + DB_NAME: dbName, + DB_SSL: "false", + BETTER_AUTH_BASE_URL: origin, + BETTER_AUTH_SECRET: randomBytes(32).toString("hex"), + CREDENTIAL_ENCRYPTION_KEY: randomBytes(32).toString("hex"), + PRINCIPAL_KEY_ENCRYPTION_KEY: randomBytes(32).toString("hex"), + HUB_DATA_DIR: join(directory, "hub"), + }; + const handle = createDB({ + ...database, + user: database.username, + database: dbName, + ssl: false, + }); + let runtime: Awaited> | undefined; + try { + await migrateDatabase(environment); + runtime = await createHubRuntime(environment); + // better-auth's sign-up limit is keyed by client IP and outlives a runtime in this process, so + // this test signs up from its own address rather than share the other e2e files' bucket. + const client = "198.51.100.42"; + const call = (path: string, body: object, cookie = "") => + runtime!.app.fetch( + new Request(`${origin}${path}`, { + method: "POST", + headers: { + origin, + "content-type": "application/json", + "x-forwarded-for": client, + cookie, + }, + body: JSON.stringify(body), + }), + ); + const signUp = await call("/api/auth/sign-up/email", { + email: `choice-${dbName}@example.invalid`, + password: randomBytes(16).toString("hex"), + name: "Choice e2e", + }); + expect(signUp.status).toBe(200); + const cookie = signUp.headers + .getSetCookie() + .map((value) => value.split(";")[0]) + .join("; "); + const created = await call( + "/api/tenants", + { name: "Choice e2e", slug: dbName.replaceAll("_", "-") }, + cookie, + ); + expect(created.status).toBe(201); + const tenantId = ((await created.json()) as { id: string }).id; + const [person] = await handle.db + .select({ id: principal.id }) + .from(principal) + .where(and(eq(principal.tenantId, tenantId), eq(principal.kind, "user"))); + const scope = { tenantId, principalId: person!.id }; + await handle.db.execute( + sql`insert into project (id, tenant_id, title) values (${tenantId}, ${tenantId}, 'Choice e2e')`, + ); + const runId = randomUUID(); + await handle.db + .insert(workflowDefinition) + .values({ id: runId, tenantId, name: runId }); + await handle.db.insert(workflowRun).values({ + id: runId, + definitionId: runId, + anchorRunId: runId, + tenantId, + address: `${runId}@${tenantId}.test`, + status: "running", + }); + + const artifactDb = handle.db as unknown as ArtifactDb; + const option = async (concept: string) => + artifactDb.transaction((tx) => + createArtifact(tx, { + scope, + ownerPrincipalId: person!.id, + kind: "design_option", + title: concept, + content: bundle(concept), + source: { origin: "workflow" }, + }), + ); + const harbour = await option("Harbour"); + await option("Meadow"); + const approve = async (concept: string) => { + const id = randomUUID(); + await handle.db.insert(approval).values({ + id, + tenantId, + anchorRunId: runId, + runId, + agentAddress: `${runId}@${tenantId}.test`, + correlationId: id, + toolDefinition: { name: "choose_design" }, + toolArguments: { concept }, + status: "approved", + scope: "once", + resolvedAt: new Date(), + }); + return id; + }; + const choose = async (concept: string) => { + const response = await call( + `/api/tenants/${tenantId}/projects/${tenantId}/design-choice`, + { concept }, + cookie, + ); + return { + status: response.status, + body: (await response.json()) as { + code?: string; + tool?: string; + concept?: string; + design?: { artifactId: string; version: number }; + }, + }; + }; + const recorded = async (approvalId: string) => + ( + await handle.db.execute<{ kind: string; version: number }>( + sql`select kind, version from sb_artifact_approval where approval_id = ${approvalId} order by kind`, + ) + ).map(({ kind, version }) => ({ kind, version })); + + const first = await approve("Harbour"); + const chosen = await choose("Harbour"); + expect(chosen.status).toBe(200); + expect(chosen.body.concept).toBe("Harbour"); + expect(chosen.body.design?.version).toBe(1); + expect(await recorded(first)).toEqual([ + { kind: "design", version: 1 }, + { kind: "design_option", version: 1 }, + ]); + + // The same approval never files a second design. + const reused = await choose("Harbour"); + expect(reused).toEqual({ + status: 409, + body: { code: "approval_already_used", tool: "choose_design" }, + }); + + // An approval of one concept never files another. + const wrong = await choose("Meadow"); + expect(wrong).toEqual({ + status: 409, + body: { code: "approval_not_resolved", tool: "choose_design" }, + }); + + // The person saves a look tweak while the card is open; the approval covers the concept, so + // the tweaked option is what gets filed. + const second = await approve("Harbour"); + await writeArtifactVersion(artifactDb, { + scope, + artifactId: harbour.id, + content: bundle("Harbour, tweaked"), + }); + const tweaked = await choose("Harbour"); + expect(tweaked.status).toBe(200); + expect(tweaked.body.design?.version).toBe(2); + expect(await recorded(second)).toEqual([ + { kind: "design", version: 2 }, + { kind: "design_option", version: 2 }, + ]); + const filed = await getArtifactVersion( + artifactDb, + tweaked.body.design!.artifactId, + 2, + ); + expect(filed?.content).toBe(bundle("Harbour, tweaked")); + } finally { + await runtime?.close(); + await handle.close(); + await admin.unsafe(`drop database if exists ${dbName} with (force)`); + await admin.end(); + await rm(directory, { recursive: true, force: true }); + } +}, 60_000); diff --git a/test/e2e/support/extract.ts b/test/e2e/support/extract.ts index a97ccd7a..838ab33e 100644 --- a/test/e2e/support/extract.ts +++ b/test/e2e/support/extract.ts @@ -602,6 +602,7 @@ export async function extractWeatherAppFixture(options?: { "design_app", "deploy_coder", "instruct_coder", + "choose_design", ]; const coordinatorRun: RunSpec = { @@ -1052,8 +1053,20 @@ export async function extractWeatherAppFixture(options?: { ], }, }, + // The recorded run chose the design through the retired Confirm prompt; the coordinator + // now asks through choose_design's approval card, so its reply names the card. { - text: extractTurnText(coord2[39]), + text: extractTurnText(coord2[39]).replace( + "choose it there", + "choose it on the card below", + ), + toolCalls: [ + { + id: "coord_choose_design", + name: "choose_design", + arguments: { concept: "Adaptive weather-driven" }, + }, + ], expect: { lastMessageRole: "user", userTextContains: "weather-driven design is filed", @@ -1070,10 +1083,17 @@ export async function extractWeatherAppFixture(options?: { }, ], expect: { - lastMessageRole: "user", - userTextContains: "Adaptive weather-driven", + lastMessageRole: "tool", requiredTools: coordinatorTools, - toolResults: [], + toolResults: [ + { + callId: "coord_choose_design", + toolName: "choose_design", + isError: false, + keys: ["design", "concept"], + contains: "Adaptive weather-driven", + }, + ], }, }, {