From 50a7fe3207727ca0dbd81fb3f09e2b576013990f Mon Sep 17 00:00:00 2001 From: lda Date: Fri, 14 Aug 2026 19:29:39 +0700 Subject: [PATCH] fix: close Task 6 review findings --- .../task-6-report.md | 41 ------- .../authoring/AuthoringPathPicker.test.tsx | 6 +- .../authoring/AuthoringPathPicker.tsx | 8 +- .../authoring/ContextInspector.test.tsx | 32 +++++- .../authoring/DraftWorkbench.test.tsx | 25 +++++ .../SelectedCapabilityInspector.test.tsx | 104 +++++++++++++++++- .../authoring/SelectedCapabilityInspector.tsx | 92 ++++++++++++---- .../authoring/StepInputBindingsForm.test.tsx | 21 ++++ .../authoring/StepInputBindingsForm.tsx | 6 +- .../authoring/StepOutputBindingsForm.test.tsx | 22 ++++ .../authoring/StepOutputBindingsForm.tsx | 6 +- 11 files changed, 288 insertions(+), 75 deletions(-) delete mode 100644 .superpowers/sdd/2026-08-14-workflow-console-contract-graph/task-6-report.md diff --git a/.superpowers/sdd/2026-08-14-workflow-console-contract-graph/task-6-report.md b/.superpowers/sdd/2026-08-14-workflow-console-contract-graph/task-6-report.md deleted file mode 100644 index 86bce259..00000000 --- a/.superpowers/sdd/2026-08-14-workflow-console-contract-graph/task-6-report.md +++ /dev/null @@ -1,41 +0,0 @@ -# Task 6 Report - -## Status - -Complete. The implementation is based on Task 5 commit `eae1a62f`. - -Implementation commit: `92a32df8` (`refactor: use canonical authoring choices`). - -## Implementation - -- Threaded `useAuthoringContract` inventory data through `SelectedCapabilityInspector`. -- Filtered inventory choices by `step_input`, `step_output_source`, and `state_target` use. -- Replaced input target/source datalists, recursive expression path datalists, and output schema-local controls with `AuthoringPathPicker`. -- Preserved canonical input and output binding serializers. Picker values translate canonical inventory paths to the existing local paths before submission. -- Kept unsupported rows and custom or malformed persisted paths editable. Non-catalog values initialize the picker Advanced section without replacing the persisted value. -- Runtime context choices are rendered only when supplied by the inventory; no runtime context field names are hardcoded in production authoring files. -- Preserved existing row diagnostic accessibility through `aria-invalid` and `aria-describedby` on Advanced custom inputs. -- Kept schema helpers required by validation and output preview; removed `workflowSourceSuggestions` as a production source of choices. - -## TDD Evidence - -The required RED command was run before production changes: - -```powershell -pnpm --dir web --filter @lda/console test -- src/workspace/authoring/StepInputBindingsForm.test.tsx src/workspace/authoring/InputExpressionControl.test.tsx src/workspace/authoring/StepOutputBindingsForm.test.tsx src/workspace/authoring/SelectedCapabilityInspector.test.tsx -``` - -RED result: 4 test files failed, 4 tests failed, and 60 tests passed because the forms still rendered the old datalists/selects. - -The tests then covered inventory-backed source/target choices, recursive expression paths, conditional context visibility, canonical submission, custom-path repair, whole-payload path round-tripping, and the selected inspector inventory seam. - -## Verification - -- `pnpm --dir web --filter @lda/console test -- src/workspace/authoring`: 19 files passed, 197 tests passed. -- `pnpm --dir web --filter @lda/console typecheck`: passed. -- `npx react-doctor@latest --verbose --scope changed`: score 87/100; six non-blocking warnings were reported in existing picker/component patterns. -- `git diff --check`: passed before commit. - -## Concerns - -React Doctor reports six non-blocking warnings: mirrored picker state, array lookups, the existing large input form, and empty default props. These patterns were present in the existing authoring seams or are equivalent to the prior suggestion-array defaults; no new blocking diagnostic was reported. diff --git a/web/apps/console/src/workspace/authoring/AuthoringPathPicker.test.tsx b/web/apps/console/src/workspace/authoring/AuthoringPathPicker.test.tsx index bca60d6b..f1f7dfa5 100644 --- a/web/apps/console/src/workspace/authoring/AuthoringPathPicker.test.tsx +++ b/web/apps/console/src/workspace/authoring/AuthoringPathPicker.test.tsx @@ -134,7 +134,7 @@ describe("AuthoringPathPicker", () => { await user.click(screen.getByRole("button", { name: /Title/ })); expect(onChange).toHaveBeenCalledTimes(1); - expect(onChange).toHaveBeenCalledWith("input.title"); + expect(onChange).toHaveBeenCalledWith("input.title", "catalog"); }); it("gives a conditional incompatible option one composed description node", () => { @@ -203,7 +203,7 @@ describe("AuthoringPathPicker", () => { const nested = screen.getByRole("button", { name: /Customer name/ }); nested.focus(); await user.keyboard("{Enter}"); - expect(onChange).toHaveBeenCalledWith("input.customer.name"); + expect(onChange).toHaveBeenCalledWith("input.customer.name", "catalog"); await user.click(screen.getByText("Advanced")); expect(screen.getByRole("textbox", { name: "Custom Source path" })).toBeInTheDocument(); @@ -211,6 +211,6 @@ describe("AuthoringPathPicker", () => { await user.clear(screen.getByRole("textbox", { name: "Custom Source path" })); await user.type(screen.getByRole("textbox", { name: "Custom Source path" }), "context.future"); - expect(onChange).toHaveBeenLastCalledWith("context.future"); + expect(onChange).toHaveBeenLastCalledWith("context.future", "custom"); }); }); diff --git a/web/apps/console/src/workspace/authoring/AuthoringPathPicker.tsx b/web/apps/console/src/workspace/authoring/AuthoringPathPicker.tsx index a7fcac65..14232964 100644 --- a/web/apps/console/src/workspace/authoring/AuthoringPathPicker.tsx +++ b/web/apps/console/src/workspace/authoring/AuthoringPathPicker.tsx @@ -5,11 +5,13 @@ import type { AuthoringPathUse, } from "../domain/authoring-contract-models.js"; +export type AuthoringPathSelection = "catalog" | "custom"; + export type AuthoringPathPickerProps = { readonly options: ReadonlyArray; readonly uses: AuthoringPathUse | ReadonlyArray; readonly value: string; - readonly onChange: (value: string) => void; + readonly onChange: (value: string, selection: AuthoringPathSelection) => void; readonly label: string; readonly allowCustom?: boolean; readonly describedBy?: string | undefined; @@ -106,7 +108,7 @@ export const AuthoringPathPicker = ({ key={option.path} onClick={() => { setCustomValue(option.path); - onChange(option.path); + onChange(option.path, "catalog"); }} type="button" > @@ -138,7 +140,7 @@ export const AuthoringPathPicker = ({ id={customId} onChange={(event) => { setCustomValue(event.target.value); - onChange(event.target.value); + onChange(event.target.value, "custom"); }} type="text" value={customValue} diff --git a/web/apps/console/src/workspace/authoring/ContextInspector.test.tsx b/web/apps/console/src/workspace/authoring/ContextInspector.test.tsx index 63ba41c3..5632594b 100644 --- a/web/apps/console/src/workspace/authoring/ContextInspector.test.tsx +++ b/web/apps/console/src/workspace/authoring/ContextInspector.test.tsx @@ -1,16 +1,46 @@ import { cleanup, fireEvent, render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; -import { afterEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { CapabilityDetail } from "../domain/capability-models.js"; +import type { AuthoringContractInventory } from "../domain/authoring-contract-models.js"; import type { DraftWorkspace } from "../domain/draft-workspace-models.js"; import type { DraftAuthoringController } from "./useDraftAuthoring.js"; import { ContextInspector } from "./ContextInspector.js"; +vi.mock("./useAuthoringContract.js", () => ({ useAuthoringContract: vi.fn() })); + +import { useAuthoringContract } from "./useAuthoringContract.js"; + +const mockedUseAuthoringContract = vi.mocked(useAuthoringContract); + +const emptyInventory: AuthoringContractInventory = { + workspaceId: "draft-report", + revision: 3, + selectedStepId: "read", + readableSources: [], + stepInputTargets: [], + stepOutputSources: [], + stateTargets: [], + workflowOutputTargets: [], + entrySteps: [], + workflowOutcomes: [], + warnings: [], +}; + afterEach(() => { cleanup(); vi.clearAllMocks(); }); +beforeEach(() => { + mockedUseAuthoringContract.mockReturnValue({ + phase: "ready", + inventory: emptyInventory, + message: null, + refresh: vi.fn(), + }); +}); + const draft: DraftWorkspace = { workspaceId: "draft-report", revision: 3, diff --git a/web/apps/console/src/workspace/authoring/DraftWorkbench.test.tsx b/web/apps/console/src/workspace/authoring/DraftWorkbench.test.tsx index e0d675a5..8c270eb2 100644 --- a/web/apps/console/src/workspace/authoring/DraftWorkbench.test.tsx +++ b/web/apps/console/src/workspace/authoring/DraftWorkbench.test.tsx @@ -2,6 +2,7 @@ import { act, cleanup, fireEvent, render, screen, within } from "@testing-librar import userEvent from "@testing-library/user-event"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { CapabilityDetail } from "../domain/capability-models.js"; +import type { AuthoringContractInventory } from "../domain/authoring-contract-models.js"; import type { DraftWorkspace } from "../domain/draft-workspace-models.js"; import { DraftWorkbench } from "./DraftWorkbench.js"; @@ -9,9 +10,27 @@ vi.mock("./useAuthoringCapabilityDetail.js", () => ({ useAuthoringCapabilityDetail: vi.fn(), })); +vi.mock("./useAuthoringContract.js", () => ({ useAuthoringContract: vi.fn() })); + import { useAuthoringCapabilityDetail } from "./useAuthoringCapabilityDetail.js"; +import { useAuthoringContract } from "./useAuthoringContract.js"; const mockedUseAuthoringCapabilityDetail = vi.mocked(useAuthoringCapabilityDetail); +const mockedUseAuthoringContract = vi.mocked(useAuthoringContract); + +const emptyInventory: AuthoringContractInventory = { + workspaceId: "draft-review", + revision: 2, + selectedStepId: "collect", + readableSources: [], + stepInputTargets: [], + stepOutputSources: [], + stateTargets: [], + workflowOutputTargets: [], + entrySteps: [], + workflowOutcomes: [], + warnings: [], +}; const workspace: DraftWorkspace = { workspaceId: "draft-review", @@ -86,6 +105,12 @@ beforeEach(() => { detail: null, message: null, }); + mockedUseAuthoringContract.mockReturnValue({ + phase: "ready", + inventory: emptyInventory, + message: null, + refresh: vi.fn(), + }); }); describe("DraftWorkbench", () => { diff --git a/web/apps/console/src/workspace/authoring/SelectedCapabilityInspector.test.tsx b/web/apps/console/src/workspace/authoring/SelectedCapabilityInspector.test.tsx index b425398b..619c3ea9 100644 --- a/web/apps/console/src/workspace/authoring/SelectedCapabilityInspector.test.tsx +++ b/web/apps/console/src/workspace/authoring/SelectedCapabilityInspector.test.tsx @@ -60,10 +60,18 @@ const inventory: AuthoringContractInventory = { warnings: [], }; +const emptyInventory: AuthoringContractInventory = { + ...inventory, + readableSources: [], + stepInputTargets: [], + stepOutputSources: [], + stateTargets: [], +}; + beforeEach(() => { mockedUseAuthoringContract.mockReturnValue({ - phase: "disconnected", - inventory: null, + phase: "ready", + inventory: emptyInventory, message: null, refresh: vi.fn(), }); @@ -147,6 +155,98 @@ const controllerFor = (workspace: DraftWorkspace): DraftAuthoringController => ( }); describe("SelectedCapabilityInspector", () => { + it("shows loading feedback and gates binding editors without current inventory", async () => { + const user = userEvent.setup(); + mockedUseAuthoringContract.mockReturnValue({ + phase: "loading", + inventory: null, + message: null, + refresh: vi.fn(), + }); + const workspace = draft("read", [], []); + + render( + , + ); + + expect(screen.getByRole("status")).toHaveTextContent("Loading canonical authoring choices..."); + await user.click(screen.getByRole("tab", { name: "Inputs" })); + expect(screen.getByText("Input bindings are unavailable until canonical authoring choices are ready.")).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Save inputs" })).not.toBeInTheDocument(); + }); + + it("shows initial inventory errors with a retry action and gates binding editors", async () => { + const user = userEvent.setup(); + const refresh = vi.fn(); + mockedUseAuthoringContract.mockReturnValue({ + phase: "error", + inventory: null, + message: "Catalog request failed.", + refresh, + }); + const workspace = draft("read", [], []); + + render( + , + ); + + expect(screen.getByRole("alert")).toHaveTextContent("Catalog request failed."); + await user.click(screen.getByRole("button", { name: "Retry canonical authoring choices" })); + expect(refresh).toHaveBeenCalledOnce(); + await user.click(screen.getByRole("tab", { name: "Outputs" })); + expect(screen.getByText("Output bindings are unavailable until canonical authoring choices are ready.")).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Save outputs" })).not.toBeInTheDocument(); + }); + + it("retains binding editors and exposes retry feedback when inventory refresh fails", async () => { + const user = userEvent.setup(); + const refresh = vi.fn(); + mockedUseAuthoringContract.mockReturnValue({ + phase: "error", + inventory, + message: "Refresh failed.", + refresh, + }); + const workspace = draft("read", [], []); + + render( + , + ); + + expect(screen.getByRole("alert")).toHaveTextContent("Refresh failed."); + await user.click(screen.getByRole("button", { name: "Retry canonical authoring choices" })); + expect(refresh).toHaveBeenCalledOnce(); + await user.click(screen.getByRole("tab", { name: "Inputs" })); + expect(screen.getByRole("button", { name: "Save inputs" })).toBeInTheDocument(); + }); + it("threads inventory-backed pickers into the selected step editors", async () => { const user = userEvent.setup(); mockedUseAuthoringContract.mockReturnValue({ diff --git a/web/apps/console/src/workspace/authoring/SelectedCapabilityInspector.tsx b/web/apps/console/src/workspace/authoring/SelectedCapabilityInspector.tsx index 4b8c7e92..8b96deb0 100644 --- a/web/apps/console/src/workspace/authoring/SelectedCapabilityInspector.tsx +++ b/web/apps/console/src/workspace/authoring/SelectedCapabilityInspector.tsx @@ -70,6 +70,41 @@ const inspectorTabs = Object.keys(tabLabels) as InspectorTab[]; const tabPanelId = (tab: InspectorTab): string => `selected-step-panel-${tab}`; const tabId = (tab: InspectorTab): string => `selected-step-tab-${tab}`; +const InventoryStatus = ({ + inventoryAvailable, + message, + phase, + refresh, +}: { + readonly inventoryAvailable: boolean; + readonly message: string | null; + readonly phase: "disconnected" | "idle" | "loading" | "ready" | "error"; + readonly refresh: () => void; +}) => { + if (phase === "loading") { + return

+ {inventoryAvailable + ? "Refreshing canonical authoring choices..." + : "Loading canonical authoring choices..."} +

; + } + if (phase === "error") { + return ( +
+

{message ?? "Canonical authoring choices failed to load."}

+ +
+ ); + } + if (!inventoryAvailable && phase === "disconnected") { + return

Canonical authoring choices are unavailable while disconnected.

; + } + if (!inventoryAvailable) { + return

Waiting for canonical authoring choices...

; + } + return null; +}; + const TabPanel = ({ activeTab, children, @@ -153,6 +188,7 @@ export const SelectedCapabilityInspector = ({ }; const detailReady = capabilityDetailPhase === "ready" && capabilityDetail !== null; const isUnsupported = nodeKind !== undefined && nodeKind !== "use"; + const inventoryAvailable = inventory !== null; return (
@@ -198,6 +234,12 @@ export const SelectedCapabilityInspector = ({ {!detailReady && capabilityDetailPhase === "disconnected" && (

Connect to inspect the capability schema.

)} + {detailReady && ( <> @@ -210,29 +252,37 @@ export const SelectedCapabilityInspector = ({ /> - + {inventoryAvailable ? ( + + ) : ( +

Input bindings are unavailable until canonical authoring choices are ready.

+ )} - + {inventoryAvailable ? ( + + ) : ( +

Output bindings are unavailable until canonical authoring choices are ready.

+ )} )} diff --git a/web/apps/console/src/workspace/authoring/StepInputBindingsForm.test.tsx b/web/apps/console/src/workspace/authoring/StepInputBindingsForm.test.tsx index 3a1f005c..d589bc07 100644 --- a/web/apps/console/src/workspace/authoring/StepInputBindingsForm.test.tsx +++ b/web/apps/console/src/workspace/authoring/StepInputBindingsForm.test.tsx @@ -85,6 +85,27 @@ describe("StepInputBindingsForm", () => { expect(submissions).toEqual([[{ path: "input.request.id", target: "profile.name" }]]); }); + it("preserves a custom target that collides with the canonical step-input namespace", async () => { + const user = userEvent.setup(); + const submissions: ReadonlyArray[] = []; + render( + { submissions.push(value); }} + />, + ); + + const target = await editableCustomInput(user, "Target for row 1"); + await user.clear(target); + await user.type(target, "step_input.text"); + await user.click(screen.getByRole("button", { name: "Save inputs" })); + + expect(submissions).toEqual([[{ path: "input.title", target: "step_input.text" }]]); + }); + it("formats local and graph path objects at whole and nested paths", () => { expect(displayLocalInputPath({ root: "local", parts: [] })).toBe("."); expect(displayLocalInputPath({ root: "local", parts: ["payload", "item"] })).toBe("payload.item"); diff --git a/web/apps/console/src/workspace/authoring/StepInputBindingsForm.tsx b/web/apps/console/src/workspace/authoring/StepInputBindingsForm.tsx index 30f70edc..34a61f3a 100644 --- a/web/apps/console/src/workspace/authoring/StepInputBindingsForm.tsx +++ b/web/apps/console/src/workspace/authoring/StepInputBindingsForm.tsx @@ -481,9 +481,11 @@ const StepInputBindingsFormContent = ({ describedBy={hasIssues ? errorId : undefined} invalid={hasIssues} label={`Target for row ${rowNumber}`} - onChange={(target) => editRow(row.id, (current) => ({ + onChange={(target, selection) => editRow(row.id, (current) => ({ ...current, - target: localPathFromPickerValue(target, targetOptions, "step_input"), + target: selection === "catalog" + ? localPathFromPickerValue(target, targetOptions, "step_input") + : target, }))} options={targetOptions} uses="step_input" diff --git a/web/apps/console/src/workspace/authoring/StepOutputBindingsForm.test.tsx b/web/apps/console/src/workspace/authoring/StepOutputBindingsForm.test.tsx index 167a220b..6adbe9a1 100644 --- a/web/apps/console/src/workspace/authoring/StepOutputBindingsForm.test.tsx +++ b/web/apps/console/src/workspace/authoring/StepOutputBindingsForm.test.tsx @@ -123,6 +123,28 @@ describe("StepOutputBindingsForm", () => { expect(submissions).toEqual([[{ source: "text", target: "state.existing" }]]); }); + it("preserves a custom source that collides with the canonical step-output namespace", async () => { + const user = userEvent.setup(); + const submissions: ReadonlyArray[] = []; + render( + { submissions.push(value); }} + />, + ); + + const source = await editableCustomInput(user, "Source path for output row 1"); + await user.clear(source); + await user.type(source, "step_output.text"); + await user.click(screen.getByRole("button", { name: "Save outputs" })); + + expect(submissions).toEqual([[{ source: "step_output.text", target: "state.existing" }]]); + }); + it("offers capability output sources and existing state targets", () => { render( onEdit(row.id, (current) => ({ + onChange={(source, selection) => onEdit(row.id, (current) => ({ ...current, - sourcePath: localPathFromPickerValue(source, sourceOptions, "step_output"), + sourcePath: selection === "catalog" + ? localPathFromPickerValue(source, sourceOptions, "step_output") + : source, }))} options={sourceOptions} uses="step_output_source"