diff --git a/apps/web/src/shell/app-shell.tsx b/apps/web/src/shell/app-shell.tsx index edcf6c54d..a6b91cf45 100644 --- a/apps/web/src/shell/app-shell.tsx +++ b/apps/web/src/shell/app-shell.tsx @@ -14,6 +14,7 @@ import { canvasColumnAllowed, contextualPanelVisible } from "./breakpoints"; import { useShellFocusRescue } from "./focus-rescue"; import { useScrollReset } from "./use-scroll-reset"; import { + applyChannelPathToCanvas, initialCanvasColumnState, openChannelInCanvas, resolveCanvasVisibility, @@ -38,7 +39,11 @@ export function AppShell({ }) { const navigate = useNavigate(); const layoutMode = useShellLayoutMode(); - const [canvasState, setCanvasState] = useState(initialCanvasColumnState); + // Deep links seed canvas state on first paint (SSR and client) so a `/c/:id` + // URL is not effect-only. Later path changes re-apply through the effect. + const [canvasState, setCanvasState] = useState(() => + applyChannelPathToCanvas(initialCanvasColumnState(), path), + ); const canvasAllowed = canvasColumnAllowed(layoutMode); const canvasOpen = resolveCanvasVisibility(canvasState, canvasAllowed); const frameRef = useRef(null); @@ -52,13 +57,11 @@ export function AppShell({ // here — the toggle only flips open/closed so reopening lands on the // same conversation. useEffect(() => { - const channelId = channelIdFromPath(path); - if (channelId === null) return; - setCanvasState((state) => openChannelInCanvas(state, channelId)); + setCanvasState((state) => applyChannelPathToCanvas(state, path)); }, [path]); const handleChannelChange = (channelId: string) => { - setCanvasState((state) => openChannelInCanvas(state, channelId)); + setCanvasState(openChannelInCanvas(channelId)); if (!isChannelPath(path) || channelIdFromPath(path) !== channelId) { navigate(channelPath(channelId)); } diff --git a/apps/web/src/shell/canvas-column-state.test.ts b/apps/web/src/shell/canvas-column-state.test.ts index 593481376..fb43ec403 100644 --- a/apps/web/src/shell/canvas-column-state.test.ts +++ b/apps/web/src/shell/canvas-column-state.test.ts @@ -1,7 +1,7 @@ import { describe, expect, test } from "bun:test"; import { - closeCanvasColumn, + applyChannelPathToCanvas, initialCanvasColumnState, openChannelInCanvas, resolveCanvasVisibility, @@ -17,24 +17,38 @@ describe("canvas column state", () => { }); test("opening a channel loads it and opens the canvas", () => { - const next = openChannelInCanvas(initialCanvasColumnState(), "ch_1"); - expect(next).toEqual({ open: true, channelId: "ch_1" }); + expect(openChannelInCanvas("ch_1")).toEqual({ + open: true, + channelId: "ch_1", + }); }); test("toggle preserves the loaded channel", () => { - const open = openChannelInCanvas(initialCanvasColumnState(), "ch_1"); + const open = openChannelInCanvas("ch_1"); const closed = toggleCanvasColumn(open); expect(closed).toEqual({ open: false, channelId: "ch_1" }); expect(toggleCanvasColumn(closed)).toEqual(open); }); - test("close drops the channel", () => { - const open = openChannelInCanvas(initialCanvasColumnState(), "ch_1"); - expect(closeCanvasColumn(open)).toEqual({ open: false, channelId: null }); + test("a /c deep link opens the canvas onto that channel", () => { + expect( + applyChannelPathToCanvas(initialCanvasColumnState(), "/c/ch_deep"), + ).toEqual({ open: true, channelId: "ch_deep" }); + expect( + applyChannelPathToCanvas(initialCanvasColumnState(), "/chat/ch_legacy"), + ).toEqual({ open: true, channelId: "ch_legacy" }); + }); + + test("non-channel paths leave canvas state alone", () => { + const open = openChannelInCanvas("ch_1"); + expect(applyChannelPathToCanvas(open, "/agents")).toEqual(open); + expect(applyChannelPathToCanvas(initialCanvasColumnState(), "/")).toEqual( + initialCanvasColumnState(), + ); }); test("visibility is gated by the viewport allow flag", () => { - const open = openChannelInCanvas(initialCanvasColumnState(), "ch_1"); + const open = openChannelInCanvas("ch_1"); expect(resolveCanvasVisibility(open, true)).toBe(true); expect(resolveCanvasVisibility(open, false)).toBe(false); expect(resolveCanvasVisibility(initialCanvasColumnState(), true)).toBe( diff --git a/apps/web/src/shell/canvas-column-state.ts b/apps/web/src/shell/canvas-column-state.ts index e0162d6bc..5ba35ba66 100644 --- a/apps/web/src/shell/canvas-column-state.ts +++ b/apps/web/src/shell/canvas-column-state.ts @@ -1,13 +1,14 @@ -// The canvas column's state as a pure reducer, separate from `breakpoints.ts`'s -// allow/disallow rule — a user's toggle, a channel the user opened into the -// canvas, and the viewport's veto are three independent inputs, and -// `resolveCanvasVisibility` is the one place they combine. +// The canvas column's state as pure transitions, separate from +// `breakpoints.ts`'s allow/disallow rule — a user's toggle, a channel the +// user opened into the canvas, and the viewport's veto are three independent +// inputs, and `resolveCanvasVisibility` is the one place they combine. // // The canvas hosts the channel chat surface (the retired `/chat` page's // `ChatWorkspace`), so its state carries the active channel alongside // open/closed. A deep link (`/c/:channelId`) feeds the same `channelId` from -// the URL in `app-shell.tsx`; this reducer only owns the toggle-and-channel -// shape, never the URL. +// the URL; path→state lives here so the shell and tests share one contract. + +import { channelIdFromPath } from "../channel-path"; export type CanvasColumnState = { readonly open: boolean; @@ -27,19 +28,23 @@ export function toggleCanvasColumn( return { ...state, open: !state.open }; } -/** Open the canvas onto a specific channel (a channel-row click). */ -export function openChannelInCanvas( - _state: CanvasColumnState, - channelId: string, -): CanvasColumnState { +/** Open the canvas onto a specific channel (channel-row click or deep link). */ +export function openChannelInCanvas(channelId: string): CanvasColumnState { return { open: true, channelId }; } -/** Close the canvas and drop the loaded channel. */ -export function closeCanvasColumn( - _state: CanvasColumnState, +/** + * Apply a route path to canvas state. A `/c/:id` (or legacy `/chat/:id`) opens + * the canvas onto that channel; any other path leaves the previous state alone + * so navigating within the rest of the app does not drop an open conversation. + */ +export function applyChannelPathToCanvas( + state: CanvasColumnState, + path: string, ): CanvasColumnState { - return { open: false, channelId: null }; + const channelId = channelIdFromPath(path); + if (channelId === null) return state; + return openChannelInCanvas(channelId); } /** What actually renders: the user's toggle (or a deep-link channel), gated by diff --git a/apps/web/test/routes.test.tsx b/apps/web/test/routes.test.tsx index d5e0fc0ee..f43eda87e 100644 --- a/apps/web/test/routes.test.tsx +++ b/apps/web/test/routes.test.tsx @@ -124,4 +124,11 @@ describe("routes render", () => { expect(markup).toContain("Page not found"); expect(markup).not.toContain('aria-current="page"'); }); + + test("a /c/:channelId deep link opens the canvas on first paint", () => { + // Canvas state is seeded from the path (not effect-only), so static + // markup sees the open column without running useEffect. + const markup = renderApp("/c/ch_deep"); + expect(markup).toMatch(/class="shell-canvas-column"[^>]*data-open="true"/); + }); }); diff --git a/packages/onboarding/src/routes.ts b/packages/onboarding/src/routes.ts index ba929f01c..e0a7bee7f 100644 --- a/packages/onboarding/src/routes.ts +++ b/packages/onboarding/src/routes.ts @@ -75,14 +75,41 @@ export function createOnboardingRoutes( // Optional body: the naming wizard sends `{ name }`; the shell's // membership probe may POST with no body and only wants the read path. // Parse before rate-limiting so the read probe never burns a create slot. - const rawBody: unknown = await c.req.json().catch(() => null); - const body = - rawBody === null - ? undefined - : (() => { - const parsed = ProvisionBody(rawBody); - return parsed instanceof type.errors ? undefined : parsed; - })(); + // Empty body → probe. Present body that is not valid JSON or fails the + // schema → 400 (never silently treated as a probe). + const bodyText = await c.req.text(); + let body: { name?: string } | undefined; + if (bodyText.trim() === "") { + body = undefined; + } else { + let rawBody: unknown; + try { + rawBody = JSON.parse(bodyText) as unknown; + } catch { + return c.json( + { + error: { + code: "bad_request", + message: "Request body must be valid JSON", + }, + }, + 400, + ); + } + const parsed = ProvisionBody(rawBody); + if (parsed instanceof type.errors) { + return c.json( + { + error: { + code: "bad_request", + message: "Invalid provision body", + }, + }, + 400, + ); + } + body = parsed; + } const isCreateAttempt = body?.name !== undefined; // Rate-limit only named creates. The two-step first-login flow is diff --git a/packages/onboarding/test/routes.test.ts b/packages/onboarding/test/routes.test.ts index f7c1565f3..24094d696 100644 --- a/packages/onboarding/test/routes.test.ts +++ b/packages/onboarding/test/routes.test.ts @@ -183,6 +183,72 @@ describe("POST /provision", () => { } }); + test("malformed JSON on /provision is 400, not a silent membership probe", async () => { + const creates: unknown[] = []; + const hub = new Hono(); + hub.get("/api/me/principals", (c) => + c.json({ data: [], nextCursor: null }), + ); + hub.post("/api/tenants", async (c) => { + creates.push(await c.req.json()); + return c.json({ id: "tnt_x" }, 201); + }); + const server = Bun.serve({ port: 0, fetch: hub.fetch }); + try { + const routes = createOnboardingRoutes({ + hubUrl: `http://localhost:${server.port}`, + pushWorkflow: async () => "pushed", + log: () => undefined, + }); + const app = mountAuthenticated(routes); + + const response = await app.request("/provision", { + method: "POST", + headers: { "content-type": "application/json" }, + body: "{not-json", + }); + + expect(response.status).toBe(400); + const body = (await response.json()) as { + error: { code: string; message: string }; + }; + expect(body.error.code).toBe("bad_request"); + expect(creates).toEqual([]); + } finally { + server.stop(true); + } + }); + + test("schema-invalid provision body is 400, not a silent membership probe", async () => { + const hub = new Hono(); + hub.get("/api/me/principals", (c) => + c.json({ data: [], nextCursor: null }), + ); + const server = Bun.serve({ port: 0, fetch: hub.fetch }); + try { + const routes = createOnboardingRoutes({ + hubUrl: `http://localhost:${server.port}`, + pushWorkflow: async () => "pushed", + log: () => undefined, + }); + const app = mountAuthenticated(routes); + + const response = await app.request("/provision", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ name: 12 }), + }); + + expect(response.status).toBe(400); + const body = (await response.json()) as { + error: { code: string; message: string }; + }; + expect(body.error.code).toBe("bad_request"); + } finally { + server.stop(true); + } + }); + test("an anonymous request is rejected before provisioning runs", async () => { const routes = createOnboardingRoutes({ hubUrl: "http://127.0.0.1:0",