diff --git a/packages/control-plane/src/session/ws-client-mapping-repository.test.ts b/packages/control-plane/src/session/ws-client-mapping-repository.test.ts index 55a2fad9a..d92caa1b1 100644 --- a/packages/control-plane/src/session/ws-client-mapping-repository.test.ts +++ b/packages/control-plane/src/session/ws-client-mapping-repository.test.ts @@ -35,7 +35,16 @@ describe("WsClientMappingRepository", () => { }); it("restores a mapping with joined participant data", () => { - mock.setRows([{ participant_id: "p-1", client_id: "client-1", user_id: "user-1" }]); + mock.setRows([ + { + participant_id: "p-1", + client_id: "client-1", + user_id: "user-1", + canonical_user_id: null, + scm_name: null, + scm_login: null, + }, + ]); expect(repository.getWsClientMapping("ws-1")).toMatchObject({ participant_id: "p-1", client_id: "client-1", @@ -48,6 +57,25 @@ describe("WsClientMappingRepository", () => { expect(repository.getWsClientMapping("unknown")).toBeNull(); }); + it("returns null for a malformed persisted mapping", () => { + mock.setRows([{ participant_id: "p-1", client_id: 42, user_id: "user-1" }]); + expect(repository.getWsClientMapping("ws-1")).toBeNull(); + }); + + it("accepts nullable participant profile fields", () => { + mock.setRows([ + { + participant_id: "p-1", + client_id: "client-1", + user_id: "user-1", + canonical_user_id: null, + scm_name: null, + scm_login: null, + }, + ]); + expect(repository.getWsClientMapping("ws-1")?.scm_name).toBeNull(); + }); + it("checks whether a mapping exists", () => { expect(repository.hasWsClientMapping("unknown")).toBe(false); mock.setRows([{ participant_id: "p-1" }]); diff --git a/packages/control-plane/src/session/ws-client-mapping-repository.ts b/packages/control-plane/src/session/ws-client-mapping-repository.ts index 18fb6bbb9..5d60569b8 100644 --- a/packages/control-plane/src/session/ws-client-mapping-repository.ts +++ b/packages/control-plane/src/session/ws-client-mapping-repository.ts @@ -1,16 +1,18 @@ +import { z } from "zod"; import type { SqlStorage } from "./sql-storage"; /** WS client mapping result for hibernation recovery. */ -export interface WsClientMappingResult { - participant_id: string; - client_id: string; - user_id: string; - canonical_user_id?: string | null; - scm_name: string | null; - scm_login: string | null; - /** Dormant legacy column may still be present on older mapping fixtures. */ - auth_name?: string | null; -} +const wsClientMappingResultSchema = z.object({ + participant_id: z.string(), + client_id: z.string(), + user_id: z.string(), + canonical_user_id: z.string().nullable().optional(), + scm_name: z.string().nullable(), + scm_login: z.string().nullable(), + auth_name: z.string().nullable().optional(), +}); + +export type WsClientMappingResult = z.infer; /** Data for a WS client mapping. */ export interface WsClientMappingData { @@ -45,7 +47,8 @@ export class WsClientMappingRepository { WHERE m.ws_id = ?`, wsId ); - return (result.toArray() as WsClientMappingResult[])[0] ?? null; + const parsed = wsClientMappingResultSchema.safeParse(result.toArray()[0]); + return parsed.success ? parsed.data : null; } hasWsClientMapping(wsId: string): boolean { diff --git a/packages/shared/src/completion/extractor.test.ts b/packages/shared/src/completion/extractor.test.ts index 69e1f13c1..7c626c2eb 100644 --- a/packages/shared/src/completion/extractor.test.ts +++ b/packages/shared/src/completion/extractor.test.ts @@ -17,6 +17,12 @@ describe("completion artifact type narrowing", () => { null ); }); + + it("ignores malformed metadata while preserving artifact labels", () => { + expect( + toEventArtifactInfo({ artifactType: "branch", metadata: ["main"], url: "/branches/main" }) + ).toEqual({ type: "branch", url: "/branches/main", label: "Branch: branch" }); + }); }); describe("buildAgentResponseFromEvents", () => { diff --git a/packages/shared/src/completion/extractor.ts b/packages/shared/src/completion/extractor.ts index 6ddc7de68..80f2ab50c 100644 --- a/packages/shared/src/completion/extractor.ts +++ b/packages/shared/src/completion/extractor.ts @@ -28,6 +28,12 @@ export type { ControlPlaneFetcher }; /** Server-side limit for the events API. */ const EVENTS_PAGE_LIMIT = 200; +function recordOrUndefined(value: unknown): Record | undefined { + return value && typeof value === "object" && !Array.isArray(value) + ? (value as Record) + : undefined; +} + export interface BuildAgentResponseOptions { defaultSuccess?: boolean; } @@ -329,7 +335,7 @@ function isArtifactInEventRange( */ export function summarizeToolCall(data: Record): ToolCallSummary { const tool = String(data.tool ?? "Unknown"); - const args = (data.args ?? {}) as Record; + const args = recordOrUndefined(data.args) ?? {}; switch (tool) { case "Read": @@ -355,12 +361,12 @@ export function summarizeToolCall(data: Record): ToolCallSummar export function getArtifactLabel(data: Record): string { const type = String(data.artifactType ?? "artifact"); if (type === "pr") { - const metadata = data.metadata as Record | undefined; + const metadata = recordOrUndefined(data.metadata); const prNum = metadata?.number; return prNum ? `PR #${prNum}` : "Pull Request"; } if (type === "branch") { - const metadata = data.metadata as Record | undefined; + const metadata = recordOrUndefined(data.metadata); return `Branch: ${metadata?.name ?? "branch"}`; } return type; @@ -406,10 +412,7 @@ export function toEventMediaArtifactInfo(data: Record): MediaAr const id = typeof data.artifactId === "string" ? data.artifactId.trim() : ""; if (!id) return null; - const metadata = - data.metadata && typeof data.metadata === "object" && !Array.isArray(data.metadata) - ? (data.metadata as Record) - : undefined; + const metadata = recordOrUndefined(data.metadata); const mimeType = typeof metadata?.mimeType === "string" ? metadata.mimeType : undefined; const sizeBytes = typeof metadata?.sizeBytes === "number" && diff --git a/packages/web/src/app/api/sessions/[id]/sandbox-access/route.test.ts b/packages/web/src/app/api/sessions/[id]/sandbox-access/route.test.ts index 1fa8e0e9c..98e16ba31 100644 --- a/packages/web/src/app/api/sessions/[id]/sandbox-access/route.test.ts +++ b/packages/web/src/app/api/sessions/[id]/sandbox-access/route.test.ts @@ -47,6 +47,17 @@ describe("sandbox access BFF", () => { await expect(response.json()).resolves.toEqual({ error: "Sandbox access changed; retry" }); }); + it("preserves malformed conflicts instead of treating them as sandbox unavailable", async () => { + vi.mocked(controlPlaneUserFetch).mockResolvedValue(Response.json(["bad"], { status: 409 })); + + const response = await GET({} as Request, { + params: Promise.resolve({ id: "session-1" }), + }); + + expect(response.status).toBe(409); + await expect(response.json()).resolves.toEqual(["bad"]); + }); + it("preserves unexpected control-plane errors", async () => { vi.mocked(controlPlaneUserFetch).mockResolvedValue( Response.json({ error: "Session not found" }, { status: 404 }) diff --git a/packages/web/src/app/api/sessions/[id]/sandbox-access/route.ts b/packages/web/src/app/api/sessions/[id]/sandbox-access/route.ts index 16bba9176..a4a732383 100644 --- a/packages/web/src/app/api/sessions/[id]/sandbox-access/route.ts +++ b/packages/web/src/app/api/sessions/[id]/sandbox-access/route.ts @@ -2,6 +2,16 @@ import { NextResponse } from "next/server"; import { getServerAuthSession } from "@/lib/server-auth-session"; import { controlPlaneUserFetch } from "@/lib/control-plane"; +function hasSandboxUnavailableError(value: unknown): boolean { + return ( + value !== null && + typeof value === "object" && + !Array.isArray(value) && + "error" in value && + value.error === "Sandbox access is unavailable" + ); +} + export async function GET(_request: Request, { params }: { params: Promise<{ id: string }> }) { const session = await getServerAuthSession(); if (!session?.user) { @@ -18,12 +28,12 @@ export async function GET(_request: Request, { params }: { params: Promise<{ id: ); const conflict = response.status === 409 - ? ((await response + ? await response .clone() .json() - .catch(() => null)) as { error?: unknown } | null) + .catch(() => null) : null; - if (conflict?.error === "Sandbox access is unavailable") { + if (hasSandboxUnavailableError(conflict)) { await response.body?.cancel(); return new Response(null, { status: 204,