diff --git a/doc/TOASTS.md b/doc/TOASTS.md index f2e0489fb00..fc984f1a640 100644 --- a/doc/TOASTS.md +++ b/doc/TOASTS.md @@ -63,6 +63,10 @@ never persisted or published on the shared company event bus. Authorized events still reach cache handling and component subscribers when eligibility is false. Lookup failure is logged but does not discard the authorized event. Existing task/run privacy checks and payload redaction remain in force. +Task activity titles, fallback text, and links use the authorized task identifier. +If the optional identifier lookup fails, the event still arrives and uses `Task` +unless the browser already has the identifier. Permission-check failures do not +use this fallback. When an authenticated comment event omits the comment ID, the visible task thread refreshes through its authorized HTTP read instead of trying to hydrate the comment. Closed sockets cannot deliver stale notifications after account/company changes. diff --git a/server/src/__tests__/live-events-toast.test.ts b/server/src/__tests__/live-events-toast.test.ts index e82af433d73..c877b70f090 100644 --- a/server/src/__tests__/live-events-toast.test.ts +++ b/server/src/__tests__/live-events-toast.test.ts @@ -29,7 +29,7 @@ function database(onPredicate: (kind: "where" | "join", condition: SQL) => void innerJoin: (_table: unknown, condition: SQL) => { onPredicate("join", condition); return query; }, where: (condition: SQL) => { onPredicate("where", condition); return query; }, limit: async () => { - if (table === "issues") return [{ id: "task" }]; + if (table === "issues") return [{ id: "task", identifier: "PAP-1234" }]; if (table === "company_memberships") return fields.name ? [{ name: "Alex Example", image: "/alex.png" }] : [{ id: "membership" }]; if (table === "agents") return [{ name: "Coder", appearance: { schemaVersion: 1, characterVersion: "cap-v1", paletteId: "bubblegum-sky" } }]; return []; @@ -39,6 +39,60 @@ function database(onPredicate: (kind: "where" | "join", condition: SQL) => void } }; } describe("authenticated activity notifications", () => { + it.each(["local_trusted", "authenticated"] as const)("keeps authorized notifications when optional identifier reads fail (%s)", async deploymentMode => { + const db = database(); + const select = db.select; + const identifierRead = vi.fn(); + db.select = fields => { + if (fields.identifier) { + identifierRead(); + if (identifierRead.mock.calls.length <= 3) throw new Error("Identifier lookup unavailable"); + } + return select(fields); + }; + const server = createServer(); + setupLiveEventsWebSocketServer(server, db as never, { deploymentMode, + resolveCloudActor: async () => ({ userId: "viewer", companyIds: ["company"] }) }); + server.listen(0, "127.0.0.1"); + await once(server, "listening"); + const socket = new WebSocket(`ws://127.0.0.1:${(server.address() as { port: number }).port}/api/companies/company/events/ws`); + try { + await once(socket, "open"); + const messages: Array<{ payload: Record }> = []; + const delivered = new Promise(resolve => socket.on("message", data => { + const event = JSON.parse(data.toString()); + messages.push(event); + if (event.payload.entityId === "after-failure") resolve(); + })); + const activity = (entityId: string, actorType: string) => publishLiveEvent({ + companyId: "company", type: "activity.logged", payload: { + action: "issue.updated", entityType: "issue", entityId, actorType, + actorId: actorType === "user" ? "viewer" : actorType === "agent" ? "agent" : null, + issueIdentifier: "SPOOF-1", details: { secret: "private task details" }, + }, + }); + if (deploymentMode === "authenticated") activity("hidden", "system"); + for (const actorType of ["system", "agent", "user"]) { + activity("visible", actorType); + } + activity("after-failure", "system"); + await delivered; + expect(messages).toHaveLength(4); + expect(identifierRead).toHaveBeenCalledTimes(4); + for (const [index, actorType] of ["system", "agent", "user"].entries()) { + const event = messages[index]; + expect(event.payload).toMatchObject({ entityId: "visible", actorType, issueIdentifier: null }); + if (actorType === "agent") expect(event.payload.actorName).toBe("Coder"); + if (actorType === "user") expect(event.payload.actorName).toBe("Alex Example"); + if (deploymentMode === "authenticated") expect(event.payload).not.toHaveProperty("details"); + } + expect(messages[3].payload.issueIdentifier).toBe("PAP-1234"); + } finally { + socket.terminate(); + await new Promise(resolve => server.close(() => resolve())); + } + }); + it("preserves only an authorized handoff task identity for empty case-panel refresh", async () => { const server = createServer(); setupLiveEventsWebSocketServer(server, database() as never, { deploymentMode: "authenticated", @@ -88,6 +142,7 @@ describe("authenticated activity notifications", () => { } }); const [data] = await delivery; expect(JSON.parse(data.toString()).toastEligible).toBe(true); + expect(JSON.parse(data.toString()).payload.issueIdentifier).toBe("PAP-1234"); expect(taskToastEligible).toHaveBeenLastCalledWith(expect.anything(), expect.anything(), "local-board"); } finally { socket.terminate(); @@ -123,6 +178,8 @@ describe("authenticated activity notifications", () => { expect(b.toastEligible).toBe(false); expect(a.payload.entityId).toBe(entityId); expect(b.payload.entityId).toBe(entityId); + expect(a.payload.issueIdentifier).toBe("PAP-1234"); + expect(b.payload.issueIdentifier).toBe("PAP-1234"); expect(original).not.toHaveProperty("toastEligible"); } } finally { @@ -132,9 +189,9 @@ describe("authenticated activity notifications", () => { } }); - it("keeps actor identity after authorization and excludes hidden tasks and private details", async () => { + it.each(["authenticated", "local_trusted"] as const)("keeps task and actor identity for %s without exposing hidden tasks or copying private details", async (deploymentMode) => { const server = createServer(); - setupLiveEventsWebSocketServer(server, database() as never, { deploymentMode: "authenticated", + setupLiveEventsWebSocketServer(server, database() as never, { deploymentMode, resolveCloudActor: async () => ({ userId: "viewer", companyIds: ["company"] }) }); server.listen(0, "127.0.0.1"); await once(server, "listening"); @@ -145,19 +202,21 @@ describe("authenticated activity notifications", () => { socket.on("message", (data) => messages.push(JSON.parse(data.toString()))); const activity = (entityId: string, actorType = "user") => publishLiveEvent({ companyId: "company", type: "activity.logged", payload: { action: "issue.created", entityType: "issue", entityId, actorType, actorId: actorType === "user" ? "viewer" : "agent", + issueIdentifier: "SPOOF-1", details: { bodySnippet: "private comment", secret: "not for the socket" }, } }); - activity("hidden"); + if (deploymentMode === "authenticated") activity("hidden"); const first = once(socket, "message"); activity("visible"); await first; expect(messages).toHaveLength(1); - expect(messages[0].payload).toEqual({ action: "issue.created", entityType: "issue", entityId: "visible", actorType: "user", actorId: "viewer", actorName: "Alex Example", actorImage: "/alex.png" }); + expect(messages[0].payload).toMatchObject({ action: "issue.created", entityType: "issue", entityId: "visible", issueIdentifier: "PAP-1234", actorType: "user", actorId: "viewer", actorName: "Alex Example", actorImage: "/alex.png" }); + if (deploymentMode === "authenticated") expect(messages[0].payload).not.toHaveProperty("details"); const second = once(socket, "message"); activity("visible", "agent"); await second; - expect(messages[1].payload).toMatchObject({ actorName: "Coder", actorType: "agent", actorId: "agent", actorAppearance: { paletteId: "bubblegum-sky" } }); - expect(messages[1].payload).not.toHaveProperty("details"); + expect(messages[1].payload).toMatchObject({ issueIdentifier: "PAP-1234", actorName: "Coder", actorType: "agent", actorId: "agent", actorAppearance: { paletteId: "bubblegum-sky" } }); + if (deploymentMode === "authenticated") expect(messages[1].payload).not.toHaveProperty("details"); } finally { socket.terminate(); await new Promise((resolve) => server.close(() => resolve())); @@ -167,6 +226,20 @@ describe("authenticated activity notifications", () => { describe("activity profile query boundaries", () => { + it("reads the task identifier within its company even when there is no actor", async () => { + const predicates: { sql: string; params: unknown[] }[] = []; + const db = database((_kind, condition) => { + const { sql, params } = new PgDialect().sqlToQuery(condition); + predicates.push({ sql, params }); + }); + expect(await activityToastIdentity(db as never, "company-A", { + action: "issue.updated", entityType: "issue", entityId: "task-A", actorType: "system", + })).toMatchObject({ issueIdentifier: "PAP-1234" }); + expect(predicates).toEqual([ + { sql: '("issues"."company_id" = $1 and "issues"."id" = $2)', params: ["company-A", "task-A"] }, + ]); + }); + it("requires the exact company, human actor, and active membership before reading a person profile", async () => { const predicates: { kind: string; sql: string; params: unknown[] }[] = []; const db = database((kind, condition) => { diff --git a/server/src/realtime/activity-toast-identity.ts b/server/src/realtime/activity-toast-identity.ts index 414f46e199f..b845c0630c5 100644 --- a/server/src/realtime/activity-toast-identity.ts +++ b/server/src/realtime/activity-toast-identity.ts @@ -1,5 +1,6 @@ import { and, eq } from "drizzle-orm"; -import { agents, authUsers, companyMemberships, type Db } from "@paperclipai/db"; +import { agents, authUsers, companyMemberships, issues, type Db } from "@paperclipai/db"; +import { logger } from "../middleware/logger.js"; const TOAST_ACTIONS = new Set(["issue.created", "issue.updated", "issue.comment_added"]); @@ -11,8 +12,26 @@ export async function activityToastIdentity( ) { const actorType = typeof payload.actorType === "string" ? payload.actorType : null; const actorId = typeof payload.actorId === "string" ? payload.actorId : null; - const identity = { actorType, actorId }; - if (!actorId || !TOAST_ACTIONS.has(String(payload.action))) return identity; + const actorIdentity = { actorType, actorId }; + if (!TOAST_ACTIONS.has(String(payload.action))) return actorIdentity; + + const issueId = payload.entityType === "issue" && typeof payload.entityId === "string" + ? payload.entityId : null; + let issueIdentifier: string | null = null; + if (issueId) { + try { + const [issue] = await db + .select({ identifier: issues.identifier }) + .from(issues) + .where(and(eq(issues.companyId, companyId), eq(issues.id, issueId))) + .limit(1); + issueIdentifier = issue?.identifier ?? null; + } catch (err) { + logger.warn({ err, companyId, issueId }, "task toast identifier lookup failed"); + } + } + const identity = { ...actorIdentity, ...(issueId ? { issueIdentifier } : {}) }; + if (!actorId) return identity; if (actorType === "agent") { const [agent] = await db diff --git a/ui/src/context/LiveUpdatesProvider.test.ts b/ui/src/context/LiveUpdatesProvider.test.ts index d45d2244b75..e7d3603e003 100644 --- a/ui/src/context/LiveUpdatesProvider.test.ts +++ b/ui/src/context/LiveUpdatesProvider.test.ts @@ -2085,7 +2085,31 @@ describe("task subtree notification context", () => { describe("activity toast attribution", () => { const build = __liveUpdatesTestUtils.buildActivityToast; - const payload = { action: "issue.created", entityType: "issue", entityId: "task-1", actorType: "user", actorId: "user-1", actorName: "Alex Example", actorImage: "/alex.png" }; + const payload = { action: "issue.created", entityType: "issue", entityId: "task-1", issueIdentifier: "PAP-1234", actorType: "user", actorId: "user-1", actorName: "Alex Example", actorImage: "/alex.png" }; + it.each([ + ["issue.created", "Alex Example created PAP-1234"], + ["issue.updated", "Alex Example updated PAP-1234"], + ["issue.comment_added", "Alex Example commented on PAP-1234"], + ])("uses the server task identifier for %s with an empty cache and no activity details", (action, title) => { + const toast = build(new QueryClient(), "company", { ...payload, action }, { userId: "other-user", agentId: null }); + expect(toast?.title).toBe(title); + expect(toast?.action).toEqual({ label: "View PAP-1234", href: "/issues/PAP-1234" }); + if (action === "issue.updated") expect(toast?.body).toBe("PAP-1234"); + expect(JSON.stringify(toast)).not.toContain("Task task-1"); + }); + it("uses a generic task label when the identifier is unavailable", () => { + const toast = build(new QueryClient(), "company", { ...payload, action: "issue.updated", issueIdentifier: undefined }, { userId: "other-user", agentId: null }); + expect(toast?.title).toBe("Alex Example updated Task"); + expect(toast?.body).toBe("Task"); + expect(toast?.action).toEqual({ label: "View Task", href: "/issues/task-1" }); + }); + it("resolves cached task titles by the server identifier", () => { + const queryClient = new QueryClient(); + queryClient.setQueryData(queryKeys.issues.detail("PAP-1234"), { id: "task-1", identifier: "PAP-1234", title: "Fix task toasts" }); + const toast = build(queryClient, "company", { ...payload, action: "issue.updated" }, { userId: "other-user", agentId: null }); + expect(toast?.title).toBe("Alex Example updated PAP-1234"); + expect(toast?.body).toBe("Fix task toasts"); + }); it.each(["issue.created", "issue.updated", "issue.comment_added"])("hides the viewer's own %s", (action) => { expect(build(new QueryClient(), "company", { ...payload, action }, { userId: "user-1", agentId: null })).toBeNull(); }); @@ -2094,18 +2118,18 @@ describe("activity toast attribution", () => { }); it("uses the real name and photo with an empty directory cache", () => { const toast = build(new QueryClient(), "company", payload, { userId: "other-user", agentId: null }); - expect(toast?.title).toBe("Alex Example created Task task-1"); + expect(toast?.title).toBe("Alex Example created PAP-1234"); expect(toast?.actor).toMatchObject({ type: "user", name: "Alex Example", image: "/alex.png" }); }); it("uses the agent's custom avatar with an empty agent cache", () => { const appearance = { schemaVersion: 1, characterVersion: "cap-v1", paletteId: "bubblegum-sky", customAvatarAssetId: "11111111-1111-4111-8111-111111111111" }; const toast = build(new QueryClient(), "company", { ...payload, actorType: "agent", actorId: "agent-1", actorName: "Coder", actorAppearance: appearance }, { userId: "user-1", agentId: null }); - expect(toast?.title).toBe("Coder created Task task-1"); + expect(toast?.title).toBe("Coder created PAP-1234"); expect(toast?.actor?.appearance).toEqual(appearance); }); it("keeps system activity distinct from people", () => { const toast = build(new QueryClient(), "company", { ...payload, actorType: "system", actorName: undefined }, { userId: "user-1", agentId: null }); - expect(toast?.title).toBe("System created Task task-1"); + expect(toast?.title).toBe("System created PAP-1234"); expect(toast?.actor).toBeUndefined(); }); }); diff --git a/ui/src/context/LiveUpdatesProvider.tsx b/ui/src/context/LiveUpdatesProvider.tsx index e66fc151b08..d099b7e6d6f 100644 --- a/ui/src/context/LiveUpdatesProvider.tsx +++ b/ui/src/context/LiveUpdatesProvider.tsx @@ -375,11 +375,11 @@ function resolveIssueToastContext( issueRefs.some((ref) => issue.id === ref || issue.identifier === ref), ); const cachedIssue = detailIssue ?? listIssue ?? null; - const ref = + const identifier = readString(details?.identifier) ?? readString(details?.issueIdentifier) ?? - cachedIssue?.identifier ?? - `Task ${shortId(issueId)}`; + cachedIssue?.identifier; + const ref = identifier ?? "Task"; const title = readString(details?.title) ?? readString(details?.issueTitle) ?? @@ -389,7 +389,7 @@ function resolveIssueToastContext( ref, title, label: title ? `${ref} - ${truncate(title, 72)}` : ref, - href: `/issues/${cachedIssue?.identifier ?? issueId}`, + href: `/issues/${identifier ?? issueId}`, }; } @@ -1064,7 +1064,10 @@ function buildActivityToast( queryClient, companyId, entityId, - details, + { + ...details, + identifier: readString(payload.issueIdentifier) ?? readString(details?.identifier), + }, ); const actor = readString(readString(payload.actorName)?.trim()) ?? resolveActorLabel(queryClient, companyId, actorType, actorId); const cachedAgent = actorType === "agent" && actorId