diff --git a/.superpowers/sdd/2026-08-03-git-workspace-registry/task-9-report.md b/.superpowers/sdd/2026-08-03-git-workspace-registry/task-9-report.md index 7fd1deba..03b48d80 100644 --- a/.superpowers/sdd/2026-08-03-git-workspace-registry/task-9-report.md +++ b/.superpowers/sdd/2026-08-03-git-workspace-registry/task-9-report.md @@ -38,3 +38,36 @@ exit 0 `git diff --check` passed before commit. No workspace secret value, secret-file path, raw diagnostic body, publish call, import flow, or export flow was introduced. + +## Fix round 1 + +### Root causes and fixes + +- The original duplicate proposal appended `-copy` and then truncated at 63 characters. For an + already-maximal ID, truncation could remove the suffix and reproduce the immutable source ID. + The proposal now reserves suffix space and falls back to a distinct `-2` suffix when a maximal + source already ends in `-copy`. +- `dwh.timeout_ms` was rendered as a positive numeric field but was absent from the client + validation map. It now has the same immediate accessible error treatment as other numeric + fields, so a rejected save never reaches the manager’s saved-draft toast. +- Registry status, workspace list, and selected-detail React Query failures were rendered as + loading, empty, or unselected states. Each now has a named alert and a retry control, distinct + from its corresponding loading and empty state. + +### TDD evidence + +- RED: max-length duplication retained the original 63-character ID; the timeout field produced + no alert; and each of the three failed queries had no accessible retry control. +- GREEN: the focused manager/editor tests passed **12/12**, covering a valid changed duplicate + proposal, rejected zero timeout with no save toast, and status/list/detail retry recovery. + +### Verification + +Executed from `frontend/`: + +```text +npx vitest run +50 test files passed, 364 tests passed +npx tsc -b +exit 0 +``` diff --git a/frontend/src/shell/WorkspaceEditor.test.tsx b/frontend/src/shell/WorkspaceEditor.test.tsx index 212df200..a5d00119 100644 --- a/frontend/src/shell/WorkspaceEditor.test.tsx +++ b/frontend/src/shell/WorkspaceEditor.test.tsx @@ -70,3 +70,17 @@ test("uses native closed selects for each workspace enum and embedding provider" expect(screen.getByRole("listbox", { name: "DWH transport" })).toHaveProperty("multiple", true); expect(screen.getByRole("listbox", { name: "Vector transport" })).toHaveProperty("multiple", true); }); + +test("rejects a non-positive DWH timeout without saving a draft", async () => { + const user = userEvent.setup(); + const onSaveDraft = vi.fn(); + render(); + + await user.clear(screen.getByLabelText("DWH timeout (ms)")); + await user.type(screen.getByLabelText("DWH timeout (ms)"), "0"); + await user.click(screen.getByRole("button", { name: "Save draft" })); + + expect(screen.getByRole("alert")).toHaveTextContent("DWH timeout must be a positive whole number"); + expect(screen.getByLabelText("DWH timeout (ms)")).toHaveAttribute("aria-invalid", "true"); + expect(onSaveDraft).not.toHaveBeenCalled(); +}); diff --git a/frontend/src/shell/WorkspaceEditor.tsx b/frontend/src/shell/WorkspaceEditor.tsx index 97b1de12..32b29d41 100644 --- a/frontend/src/shell/WorkspaceEditor.tsx +++ b/frontend/src/shell/WorkspaceEditor.tsx @@ -49,6 +49,8 @@ function validate(workspace: CanonicalWorkspace): FieldErrors { if (!workspace.dwh.supported_transports.length) errors["dwh.transport"] = "Choose at least one DWH transport"; const dwhPort = positiveInteger(workspace.dwh.port, "DWH port", 65_535); if (dwhPort) errors["dwh.port"] = dwhPort; + const dwhTimeout = positiveInteger(workspace.dwh.timeout_ms, "DWH timeout"); + if (dwhTimeout) errors["dwh.timeout"] = dwhTimeout; if (!/^[A-Za-z_][A-Za-z0-9_]*$/.test(workspace.semantic_index.vector_store.database)) errors["vector.database"] = "Use a database identifier"; if (!/^[A-Za-z_][A-Za-z0-9_]*$/.test(workspace.semantic_index.vector_store.schema)) errors["vector.schema"] = "Use a schema identifier"; if (!/^[A-Za-z_][A-Za-z0-9_]*$/.test(workspace.semantic_index.vector_store.collection)) errors["vector.collection"] = "Use a collection identifier"; @@ -175,7 +177,7 @@ export function WorkspaceEditor({ draft, onSaveDraft, onPublish: _onPublish, idL {({ id, describedBy, invalid }) => update((value) => ({ ...value, dwh: { ...value.dwh, port: numberOrUndefined(event.target.value) } }))} />} - + {({ id, describedBy, invalid }) => update((value) => ({ ...value, dwh: { ...value.dwh, timeout_ms: numberOrUndefined(event.target.value) } }))} />} diff --git a/frontend/src/shell/WorkspaceManager.test.tsx b/frontend/src/shell/WorkspaceManager.test.tsx index 58b3f6c6..549134ce 100644 --- a/frontend/src/shell/WorkspaceManager.test.tsx +++ b/frontend/src/shell/WorkspaceManager.test.tsx @@ -72,6 +72,43 @@ test("stages duplicate and delete operations without publishing", async () => { expect(published).toBe(false); }); +test("proposes a different valid ID when duplicating a 63-character workspace ID", async () => { + const user = userEvent.setup(); + const maxId = `w${"a".repeat(62)}`; + const maxWorkspace = { ...workspace, workspace: { ...workspace.workspace, id: maxId } }; + server.use( + http.get("http://localhost:8787/workspaces", () => HttpResponse.json([{ + id: maxId, name: "Maximum", displayName: "Maximum", language: "en", file: `workspaces/${maxId}.yaml`, + revision: { id: maxId, commit: "a".repeat(40), blob: "b".repeat(40), snapshotPath: "/tmp/maximum", state: "operational" }, + }])), + http.get(`http://localhost:8787/workspaces/${maxId}`, () => HttpResponse.json({ + workspace: maxWorkspace, + revision: { id: maxId, commit: "a".repeat(40), blob: "b".repeat(40), snapshotPath: "/tmp/maximum", state: "operational" }, + })), + ); + renderManager(); + + await user.click(await screen.findByRole("button", { name: "Maximum" })); + await user.click(screen.getByRole("button", { name: "Duplicate workspace" })); + + const proposed = screen.getByLabelText("Workspace ID") as HTMLInputElement; + expect(proposed.value).toMatch(/^[a-z][a-z0-9-]{2,62}$/); + expect(proposed).not.toHaveValue(maxId); +}); + +test("does not show a saved-draft toast when manager validation rejects a DWH timeout", async () => { + const user = userEvent.setup(); + renderManager(); + + await user.click(await screen.findByRole("button", { name: "PSD Clinical" })); + await user.clear(screen.getByLabelText("DWH timeout (ms)")); + await user.type(screen.getByLabelText("DWH timeout (ms)"), "0"); + await user.click(screen.getByRole("button", { name: "Save draft" })); + + expect(screen.getByRole("alert")).toHaveTextContent("DWH timeout must be a positive whole number"); + expect(screen.queryByText("Draft saved in this browser.")).not.toBeInTheDocument(); +}); + test("runs validation and installation test with only sanitized messages", async () => { const user = userEvent.setup(); server.use( @@ -90,3 +127,53 @@ test("runs validation and installation test with only sanitized messages", async expect(await screen.findByText("binding_missing: DWH binding is not configured")).toBeVisible(); expect(within(screen.getByTestId("workspace-diagnostics")).queryByText(/password|token|secret/i)).not.toBeInTheDocument(); }); + +test("shows an accessible retry instead of a loading status when the registry status query fails", async () => { + const user = userEvent.setup(); + let calls = 0; + server.use(http.get("http://localhost:8787/workspace-registry/status", () => { + calls += 1; + return calls === 1 ? new HttpResponse(null, { status: 503 }) : HttpResponse.json({ branch: "main", ahead: 0, behind: 0, degraded: false }); + })); + renderManager(); + + expect(await screen.findByRole("alert", { name: "Workspace registry status failed" })).toHaveTextContent("Could not load registry status."); + await user.click(screen.getByRole("button", { name: "Retry registry status" })); + expect(await screen.findByText("main")).toBeVisible(); + expect(calls).toBe(2); +}); + +test("shows an accessible retry instead of an empty list when the workspace list query fails", async () => { + const user = userEvent.setup(); + let calls = 0; + server.use(http.get("http://localhost:8787/workspaces", () => { + calls += 1; + return calls === 1 ? new HttpResponse(null, { status: 503 }) : HttpResponse.json([]); + })); + renderManager(); + + expect(await screen.findByRole("alert", { name: "Workspace list failed" })).toHaveTextContent("Could not load workspaces."); + expect(screen.queryByText("No published workspaces.")).not.toBeInTheDocument(); + await user.click(screen.getByRole("button", { name: "Retry workspace list" })); + expect(await screen.findByText("No published workspaces.")).toBeVisible(); + expect(calls).toBe(2); +}); + +test("shows an accessible retry when the selected workspace detail query fails", async () => { + const user = userEvent.setup(); + let calls = 0; + server.use(http.get("http://localhost:8787/workspaces/psd-clinical", () => { + calls += 1; + return calls === 1 ? new HttpResponse(null, { status: 503 }) : HttpResponse.json({ + workspace, + revision: { id: "psd-clinical", commit: "a".repeat(40), blob: "b".repeat(40), snapshotPath: "/tmp/psd", state: "operational" }, + }); + })); + renderManager(); + + await user.click(await screen.findByRole("button", { name: "PSD Clinical" })); + expect(await screen.findByRole("alert", { name: "Workspace details failed" })).toHaveTextContent("Could not load workspace details."); + await user.click(screen.getByRole("button", { name: "Retry workspace details" })); + expect(await screen.findByRole("heading", { name: "PSD Clinical" })).toBeVisible(); + expect(calls).toBe(2); +}); diff --git a/frontend/src/shell/WorkspaceManager.tsx b/frontend/src/shell/WorkspaceManager.tsx index 27efadbe..f436edd2 100644 --- a/frontend/src/shell/WorkspaceManager.tsx +++ b/frontend/src/shell/WorkspaceManager.tsx @@ -35,7 +35,18 @@ function draftFromRecord(record: WorkspaceRecord): WorkspaceDraft { } function proposedId(id: string): string { - return `${id}-copy`.slice(0, 63); + const copy = `${id.slice(0, 58)}-copy`; + // A max-length source ending in "-copy" would otherwise reproduce itself. + return copy === id ? `${id.slice(0, 61)}-2` : copy; +} + +function QueryError({ name, message, retryLabel, onRetry }: { + name: string; + message: string; + retryLabel: string; + onRetry: () => void; +}) { + return

{message}

; } export function WorkspaceManager({ open, onClose }: { open: boolean; onClose: () => void }) { @@ -44,13 +55,16 @@ export function WorkspaceManager({ open, onClose }: { open: boolean; onClose: () const [notice, setNotice] = useState(); const [diagnostics, setDiagnostics] = useState([]); const [deletionDraft, setDeletionDraft] = useState(); - const { data: status } = useQuery({ queryKey: ["workspace-registry-status"], queryFn: getWorkspaceRegistryStatus, enabled: open }); - const { data: workspaces = [], isLoading: workspacesLoading } = useQuery({ queryKey: ["workspaces"], queryFn: listWorkspaces, enabled: open }); - const { data: record, isLoading: recordLoading } = useQuery({ + const statusQuery = useQuery({ queryKey: ["workspace-registry-status"], queryFn: getWorkspaceRegistryStatus, enabled: open }); + const workspacesQuery = useQuery({ queryKey: ["workspaces"], queryFn: listWorkspaces, enabled: open }); + const detailQuery = useQuery({ queryKey: ["workspace", selectedId], queryFn: () => getWorkspace(selectedId!), enabled: Boolean(open && selectedId && !localDraft), }); + const status = statusQuery.data; + const workspaces = workspacesQuery.data ?? []; + const record = detailQuery.data; const savedDraft = selectedId && !localDraft ? workspaceDrafts.load(selectedId) : undefined; const savedDeletionDraft = selectedId && !deletionDraft ? workspaceDeletionDrafts.load(selectedId) : undefined; @@ -147,26 +161,25 @@ export function WorkspaceManager({ open, onClose }: { open: boolean; onClose: ()
- {!currentDraft && !recordLoading &&

Select a workspace

Review an existing definition or start a browser-only draft.

} - {(currentDraft || recordLoading) && ( + {detailQuery.isError && selectedId && !localDraft ? { void detailQuery.refetch(); }} /> : !currentDraft && !detailQuery.isLoading &&

Select a workspace

Review an existing definition or start a browser-only draft.

} + {!detailQuery.isError && (currentDraft || detailQuery.isLoading) && ( <> - {recordLoading && !currentDraft ?

Loading workspace definition…

: currentDraft && <> + {detailQuery.isLoading && !currentDraft ?

Loading workspace definition…

: currentDraft && <>

Workspace definition