diff options
Diffstat (limited to 'src/features')
| -rw-r--r-- | src/features/workspaces/adapter/http.test.ts | 30 | ||||
| -rw-r--r-- | src/features/workspaces/adapter/http.ts | 24 | ||||
| -rw-r--r-- | src/features/workspaces/index.ts | 6 | ||||
| -rw-r--r-- | src/features/workspaces/logic/create.test.ts | 50 | ||||
| -rw-r--r-- | src/features/workspaces/logic/create.ts | 64 | ||||
| -rw-r--r-- | src/features/workspaces/store.svelte.ts | 15 | ||||
| -rw-r--r-- | src/features/workspaces/store.test.ts | 61 | ||||
| -rw-r--r-- | src/features/workspaces/ui/WorkspacesHome.svelte | 7 | ||||
| -rw-r--r-- | src/features/workspaces/ui/WorkspacesHome.test.ts | 38 |
9 files changed, 277 insertions, 18 deletions
diff --git a/src/features/workspaces/adapter/http.test.ts b/src/features/workspaces/adapter/http.test.ts index 2e34822..d6409ca 100644 --- a/src/features/workspaces/adapter/http.test.ts +++ b/src/features/workspaces/adapter/http.test.ts @@ -79,15 +79,37 @@ describe("createWorkspaceHttp", () => { expect(result).toEqual({ ok: false, error: "invalid slug" }); }); - it("get returns null on 404", async () => { + it("get reports a missing lookup on 404", async () => { const http = createWorkspaceHttp(BASE, fakeFetch([{ status: 404 }])); - expect(await http.get("nope")).toBeNull(); + expect(await http.get("nope")).toEqual({ kind: "missing" }); }); it("get returns the workspace on 200", async () => { - const ws = { id: "x", title: "X", defaultCwd: "/home", createdAt: 1, lastActivityAt: 2 }; + const ws = { + id: "x", + title: "X", + defaultCwd: "/home", + defaultComputerId: null, + starred: false, + archived: false, + createdAt: 1, + lastActivityAt: 2, + }; const http = createWorkspaceHttp(BASE, fakeFetch([{ body: ws }])); - expect(await http.get("x")).toEqual(ws); + expect(await http.get("x")).toEqual({ kind: "found", workspace: ws }); + }); + + it("get distinguishes a failed response from 404 (not 'missing')", async () => { + const http = createWorkspaceHttp( + BASE, + fakeFetch([{ status: 500, body: { error: "db down" } }]), + ); + expect(await http.get("x")).toEqual({ kind: "failed", error: "db down" }); + }); + + it("get reports a network error as failed (not 'missing')", async () => { + const http = createWorkspaceHttp(BASE, fakeFetch([new Error("network")])); + expect(await http.get("x")).toEqual({ kind: "failed", error: "network" }); }); it("setTitle PUTs the title", async () => { diff --git a/src/features/workspaces/adapter/http.ts b/src/features/workspaces/adapter/http.ts index de95a61..3f7f058 100644 --- a/src/features/workspaces/adapter/http.ts +++ b/src/features/workspaces/adapter/http.ts @@ -9,6 +9,7 @@ import type { WorkspaceListResponse, WorkspaceResponse, } from "@dispatch/transport-contract"; +import type { WorkspaceLookup } from "../logic/create"; /** * Workspace HTTP effects — the injected edge that talks to the backend's @@ -21,7 +22,7 @@ import type { * Endpoints ([email protected]): * - `GET /workspaces` (optional `?archived=true|false|all`, default `false` = active only) → list * - `PUT /workspaces/:id` (create-on-miss, idempotent) → ensure - * - `GET /workspaces/:id` (404 → null) → get + * - `GET /workspaces/:id` (found | 404-missing | failed) → get * - `PUT /workspaces/:id/title` → rename * - `PUT /workspaces/:id/default-cwd` → set/clear default cwd * - `PUT /workspaces/:id/default-computer` → set/clear default computer (SSH handoff #2) @@ -41,7 +42,12 @@ export type WorkspaceArchivedFilter = "true" | "false" | "all"; export interface WorkspaceHttp { list(archived?: WorkspaceArchivedFilter): Promise<readonly WorkspaceEntry[]>; ensure(id: string, body?: EnsureWorkspaceRequest): Promise<WorkspaceResult<Workspace>>; - get(id: string): Promise<Workspace | null>; + /** + * Authoritative existence probe. 404 → `missing`, a network/server failure → + * `failed` (distinct on purpose: the create gate must not treat a broken + * check as "doesn't exist"), 200 → the workspace (`found`). + */ + get(id: string): Promise<WorkspaceLookup>; setTitle(id: string, title: string): Promise<WorkspaceResult<Workspace>>; setDefaultCwd(id: string, defaultCwd: string | null): Promise<WorkspaceResult<Workspace>>; setDefaultComputer(id: string, computerId: string | null): Promise<WorkspaceResult<Workspace>>; @@ -96,13 +102,17 @@ export function createWorkspaceHttp(httpBase: string, fetchImpl: typeof fetch): } }, - async get(id): Promise<Workspace | null> { + async get(id): Promise<WorkspaceLookup> { try { const res = await fetchImpl(`${httpBase}/workspaces/${encodeURIComponent(id)}`); - if (res.status === 404 || !res.ok) return null; - return (await res.json()) as WorkspaceResponse; - } catch { - return null; + if (res.status === 404) return { kind: "missing" }; + if (!res.ok) return { kind: "failed", error: await errText(res) }; + return { kind: "found", workspace: (await res.json()) as WorkspaceResponse }; + } catch (err) { + return { + kind: "failed", + error: err instanceof Error ? err.message : "Workspace lookup failed", + }; } }, diff --git a/src/features/workspaces/index.ts b/src/features/workspaces/index.ts index 1a60bb7..c7f57d5 100644 --- a/src/features/workspaces/index.ts +++ b/src/features/workspaces/index.ts @@ -1,6 +1,12 @@ export type { WorkspaceHttp, WorkspaceResult } from "./adapter/http"; export { createWorkspaceHttp } from "./adapter/http"; export { createTileLayoutHttp, type TileLayoutHttp } from "./adapter/tile-layout-http"; +export type { CreateGate, WorkspaceLookup } from "./logic/create"; +export { + archivedCreateMessage, + checkFailedCreateMessage, + gateCreate, +} from "./logic/create"; export type { DragEmit, DragEvent as DragControllerEvent, diff --git a/src/features/workspaces/logic/create.test.ts b/src/features/workspaces/logic/create.test.ts new file mode 100644 index 0000000..278bcd6 --- /dev/null +++ b/src/features/workspaces/logic/create.test.ts @@ -0,0 +1,50 @@ +import type { Workspace } from "@dispatch/wire"; +import { describe, expect, it } from "vitest"; +import { + archivedCreateMessage, + checkFailedCreateMessage, + gateCreate, + type WorkspaceLookup, +} from "./create"; + +function workspace(overrides: Partial<Workspace> = {}): Workspace { + return { + id: "old", + title: "Old", + defaultCwd: null, + defaultComputerId: null, + starred: false, + archived: false, + createdAt: 1, + lastActivityAt: 2, + ...overrides, + }; +} + +describe("gateCreate", () => { + it("blocks a slug whose workspace is archived, with the can't-create message", () => { + const lookup: WorkspaceLookup = { kind: "found", workspace: workspace({ archived: true }) }; + const gate = gateCreate("old", lookup); + expect(gate).toEqual({ ok: false, error: archivedCreateMessage("old") }); + expect(gate.ok === false && gate.error).toContain( + 'A workspace named "old" already exists and is archived — you can\'t create it.', + ); + }); + + it("proceeds for an existing ACTIVE workspace (ensure is idempotent)", () => { + const lookup: WorkspaceLookup = { kind: "found", workspace: workspace({ archived: false }) }; + expect(gateCreate("old", lookup)).toEqual({ ok: true }); + }); + + it("proceeds for a missing slug", () => { + expect(gateCreate("fresh", { kind: "missing" })).toEqual({ ok: true }); + }); + + it("blocks when the existence probe failed (never guesses)", () => { + const gate = gateCreate("maybe", { kind: "failed", error: "db down" }); + expect(gate).toEqual({ ok: false, error: checkFailedCreateMessage("maybe", "db down") }); + expect(gate.ok === false && gate.error).toContain( + 'Couldn\'t check whether a workspace named "maybe"', + ); + }); +}); diff --git a/src/features/workspaces/logic/create.ts b/src/features/workspaces/logic/create.ts new file mode 100644 index 0000000..c9385e1 --- /dev/null +++ b/src/features/workspaces/logic/create.ts @@ -0,0 +1,64 @@ +/** + * Pure create-gate logic for the New-workspace form — zero DOM, zero effects. + * + * `PUT /workspaces/:id` (ensure) is idempotent, so creating a slug that belongs + * to an ARCHIVED workspace would silently "succeed": the backend returns the + * archived workspace, the active list (which excludes archived) never shows it, + * and the home grid would still place a tile for something invisible. The + * frontend therefore probes `GET /workspaces/:id` first and refuses the create + * when the slug exists and is archived. + */ + +import type { Workspace } from "@dispatch/wire"; + +/** + * The authoritative existence probe's outcome (`GET /workspaces/:id`). The + * adapter maps HTTP status to these kinds; `failed` is deliberately distinct + * from `missing` (404) so a broken check is never mistaken for "no such + * workspace". + */ +export type WorkspaceLookup = + | { readonly kind: "found"; readonly workspace: Workspace } + | { readonly kind: "missing" } + | { readonly kind: "failed"; readonly error: string }; + +/** Whether a create may proceed: `ok` or a user-facing error message. */ +export type CreateGate = { readonly ok: true } | { readonly ok: false; readonly error: string }; + +/** + * The user-facing message when a slug belongs to an archived workspace. It + * states plainly that the create can't happen and points at the existing + * recovery affordance (the Archived workspaces list) without adding UI. + */ +export function archivedCreateMessage(id: string): string { + return `A workspace named "${id}" already exists and is archived — you can't create it. Unarchive it from Archived workspaces instead.`; +} + +/** + * The user-facing message when the existence probe itself failed. Proceeding + * blindly could recreate the silent archived-tile bug, so the create is + * refused with a retryable message instead. + */ +export function checkFailedCreateMessage(id: string, error: string): string { + return `Couldn't check whether a workspace named "${id}" already exists (${error}). Try again.`; +} + +/** + * Decide whether a create may proceed, given the probe outcome. Pure: + * `(id, lookup) → gate`. + * + * - archived slug → blocked with the "can't create" message; + * - missing slug or an ACTIVE existing slug → proceed (`ensure` is idempotent, + * so an active slug keeps today's behavior); + * - a failed probe → blocked too (plan §3.4): never guess, because proceeding + * could ensure an archived workspace's tile. + */ +export function gateCreate(id: string, lookup: WorkspaceLookup): CreateGate { + if (lookup.kind === "failed") { + return { ok: false, error: checkFailedCreateMessage(id, lookup.error) }; + } + if (lookup.kind === "found" && lookup.workspace.archived) { + return { ok: false, error: archivedCreateMessage(id) }; + } + return { ok: true }; +} diff --git a/src/features/workspaces/store.svelte.ts b/src/features/workspaces/store.svelte.ts index 57e34b2..b9b646b 100644 --- a/src/features/workspaces/store.svelte.ts +++ b/src/features/workspaces/store.svelte.ts @@ -1,6 +1,7 @@ import type { EnsureWorkspaceRequest } from "@dispatch/transport-contract"; import type { Workspace, WorkspaceEntry } from "@dispatch/wire"; import type { WorkspaceHttp, WorkspaceResult } from "./adapter/http"; +import { gateCreate } from "./logic/create"; import { applyArchived, applyStarred, sortWorkspaces } from "./logic/view-model"; /** @@ -27,6 +28,14 @@ export interface WorkspaceStore { refreshArchived(): Promise<void>; /** `PUT /workspaces/:id` (create-on-miss). Returns the workspace or an error. */ ensure(id: string, body?: EnsureWorkspaceRequest): Promise<WorkspaceResult<Workspace>>; + /** + * Create a workspace via the New-workspace flow. Probes `GET /workspaces/:id` + * first and refuses a slug that belongs to an ARCHIVED workspace (the + * idempotent `ensure` would otherwise silently resurrect an invisible tile). + * A failed probe is refused too (see `gateCreate`). On success it falls + * through to `ensure`, unchanged. + */ + create(id: string, body?: EnsureWorkspaceRequest): Promise<WorkspaceResult<Workspace>>; /** Rename a workspace (display only; id unchanged). */ rename(id: string, title: string): Promise<WorkspaceResult<Workspace>>; /** Set/clear a workspace's default cwd. */ @@ -102,6 +111,12 @@ export function createWorkspaceStore(http: WorkspaceHttp): WorkspaceStore { } }, + async create(id, body): Promise<WorkspaceResult<Workspace>> { + const gate = gateCreate(id, await http.get(id)); + if (!gate.ok) return { ok: false, error: gate.error }; + return this.ensure(id, body); + }, + async ensure(id, body): Promise<WorkspaceResult<Workspace>> { const result = await http.ensure(id, body); if (result.ok) void this.refresh(); diff --git a/src/features/workspaces/store.test.ts b/src/features/workspaces/store.test.ts index a44d8a6..faec5bf 100644 --- a/src/features/workspaces/store.test.ts +++ b/src/features/workspaces/store.test.ts @@ -1,6 +1,7 @@ import type { Workspace, WorkspaceEntry } from "@dispatch/wire"; import { describe, expect, it, vi } from "vitest"; import type { WorkspaceResult } from "./adapter/http"; +import { archivedCreateMessage, type WorkspaceLookup } from "./logic/create"; import { createWorkspaceStore } from "./store.svelte"; function entry(overrides: Partial<WorkspaceEntry> = {}): WorkspaceEntry { @@ -25,14 +26,23 @@ function fakeHttp(opts: { unstar?: (id: string) => Promise<WorkspaceResult<Workspace>>; archive?: (id: string) => Promise<WorkspaceResult<Workspace>>; unarchive?: (id: string) => Promise<WorkspaceResult<Workspace>>; + get?: (id: string) => Promise<WorkspaceLookup>; + ensure?: (id: string) => Promise<WorkspaceResult<Workspace>>; }) { return { list: vi.fn( async (_archived?: "true" | "false" | "all"): Promise<readonly WorkspaceEntry[]> => opts.list ?? [], ), - ensure: vi.fn(), - get: vi.fn(), + ensure: + opts.ensure ?? + vi.fn( + async (id: string): Promise<WorkspaceResult<Workspace>> => ({ + ok: true, + value: entry({ id }), + }), + ), + get: vi.fn(opts.get ?? (async (): Promise<WorkspaceLookup> => ({ kind: "missing" }))), setTitle: vi.fn(), setDefaultCwd: vi.fn(), setDefaultComputer: vi.fn(), @@ -224,6 +234,53 @@ describe("createWorkspaceStore — setArchived", () => { }); }); +describe("createWorkspaceStore — create (archived-slug gate)", () => { + it("refuses an archived slug with the can't-create message and never calls ensure", async () => { + const http = fakeHttp({ + get: async () => ({ kind: "found", workspace: entry({ id: "old", archived: true }) }), + }); + const store = createWorkspaceStore(http); + + const result = await store.create("old"); + + expect(result).toEqual({ ok: false, error: archivedCreateMessage("old") }); + expect(http.ensure).not.toHaveBeenCalled(); + }); + + it("proceeds for an existing non-archived slug (ensure is called as before)", async () => { + const http = fakeHttp({ + get: async () => ({ kind: "found", workspace: entry({ id: "taken", archived: false }) }), + }); + const store = createWorkspaceStore(http); + + const result = await store.create("taken"); + + expect(result.ok).toBe(true); + expect(http.ensure).toHaveBeenCalledTimes(1); + }); + + it("proceeds for a brand-new slug (404 → missing)", async () => { + const http = fakeHttp({ get: async () => ({ kind: "missing" }) }); + const store = createWorkspaceStore(http); + + const result = await store.create("fresh"); + + expect(result.ok).toBe(true); + expect(http.ensure).toHaveBeenCalledTimes(1); + }); + + it("refuses when the existence check itself failed, without calling ensure (§3.4)", async () => { + const http = fakeHttp({ get: async () => ({ kind: "failed", error: "db down" }) }); + const store = createWorkspaceStore(http); + + const result = await store.create("maybe"); + + expect(result.ok).toBe(false); + if (!result.ok) expect(result.error).toContain("Couldn't check whether"); + expect(http.ensure).not.toHaveBeenCalled(); + }); +}); + describe("createWorkspaceStore — refreshArchived", () => { it("loads the archived list via list('true') without touching the main list", async () => { const http = fakeHttp({ list: [entry({ id: "active", archived: false })] }); diff --git a/src/features/workspaces/ui/WorkspacesHome.svelte b/src/features/workspaces/ui/WorkspacesHome.svelte index 630c67f..65e1593 100644 --- a/src/features/workspaces/ui/WorkspacesHome.svelte +++ b/src/features/workspaces/ui/WorkspacesHome.svelte @@ -215,12 +215,13 @@ if (result.ok) deselect(); } - // ── Create (header popover): create on the backend — do NOT navigate — - // place the new tile in the first empty slot, and go straight into rename. + // ── Create (header popover): check the slug resolves to no archived + // workspace, create on the backend — do NOT navigate — place the new tile in + // the first empty slot, and go straight into rename. async function createWorkspace( slug: string, ): Promise<{ ok: true } | { ok: false; error: string }> { - const result = await store.ensure(slug); + const result = await store.create(slug); if (!result.ok) return { ok: false, error: result.error }; await store.refresh(); // Idempotent: if the list refresh's sync already appended the id, this diff --git a/src/features/workspaces/ui/WorkspacesHome.test.ts b/src/features/workspaces/ui/WorkspacesHome.test.ts index 6316930..d4316b1 100644 --- a/src/features/workspaces/ui/WorkspacesHome.test.ts +++ b/src/features/workspaces/ui/WorkspacesHome.test.ts @@ -5,6 +5,7 @@ import userEvent from "@testing-library/user-event"; import { describe, expect, it, vi } from "vitest"; import type { WorkspaceHttp, WorkspaceResult } from "../adapter/http"; import type { TileLayoutHttp } from "../adapter/tile-layout-http"; +import type { WorkspaceLookup } from "../logic/create"; import { createTileLayoutStore } from "../tile-layout-store.svelte"; import WorkspacesHome from "./WorkspacesHome.svelte"; @@ -24,7 +25,10 @@ function ws(overrides: Partial<WorkspaceEntry> = {}): WorkspaceEntry { } /** A fake WorkspaceHttp edge with a controllable list (no mocks of our modules). */ -function fakeHttp(list: WorkspaceEntry[] = [ws()]): WorkspaceHttp { +function fakeHttp( + list: WorkspaceEntry[] = [ws()], + lookup: (id: string) => Promise<WorkspaceLookup> = async () => ({ kind: "missing" }), +): WorkspaceHttp { return { list: vi.fn(async (): Promise<WorkspaceEntry[]> => list), // Create-on-miss: the backend persists the new workspace, so the next @@ -33,7 +37,7 @@ function fakeHttp(list: WorkspaceEntry[] = [ws()]): WorkspaceHttp { if (!list.some((w) => w.id === id)) list.push(ws({ id, title: id })); return { ok: true, value: ws({ id, title: id }) }; }), - get: vi.fn(), + get: vi.fn(lookup), setTitle: vi.fn(), setDefaultCwd: vi.fn(), setDefaultComputer: vi.fn(), @@ -140,6 +144,36 @@ describe("WorkspacesHome (tile canvas)", () => { expect(store.list.map((w) => w.id)).toContain("fresh-ws"); }); + it("refuses to create an archived slug: message in the popover, form stays open, no tile", async () => { + const user = userEvent.setup(); + const list = [ws({ id: "a", title: "A" })]; + const http = fakeHttp(list, async (id) => ({ + kind: "found", + workspace: ws({ id, title: id, archived: true }), + })); + const store = (await import("../store.svelte")).createWorkspaceStore(http); + render(WorkspacesHome, { + props: { + store, + onNavigate: vi.fn(), + computers: [], + tiles: createTileLayoutStore({ http: memoryLayout() }), + }, + }); + + await screen.findByRole("button", { name: "Open workspace A" }); + await user.click(screen.getByRole("button", { name: "New workspace" })); + await user.type(screen.getByRole("textbox", { name: "New workspace" }), "old-ws{Enter}"); + + expect(await screen.findByText(/already exists and is archived/)).toBeInTheDocument(); + // The popover stays open (the form/input is still mounted). + expect(screen.getByRole("textbox", { name: "New workspace" })).toBeInTheDocument(); + // Nothing was ensured, no tile appeared, and no rename mode was entered. + expect(http.ensure).not.toHaveBeenCalled(); + expect(store.list.map((w) => w.id)).toEqual(["a"]); + expect(screen.queryByLabelText("Workspace title")).toBeNull(); + }); + it("right-clicking a tile opens the action bar; Delete keeps the confirm + store call", async () => { const user = userEvent.setup(); const confirmSpy = vi.spyOn(window, "confirm").mockReturnValue(true); |