From fae3aa7989810979dca841d5d739ed9ed4a5b195 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Mon, 2 Nov 2026 19:27:07 +0000 Subject: [PATCH 1/6] fix(dashboard): Show author photos on transcript messages The conversation list showed participant photos, but transcript messages still drew initials. Message actor identities came from the saved message author and had no photo, and the message avatar drew only initials. Resolve each user Message author Identity when reading an event page and add its photo path. Draw the photo in the message avatar and the speaker card, with initials as the fallback. Co-Authored-By: BYK <126780+BYK@users.noreply.github.com> --- .../conversations/TranscriptMessageShell.tsx | 27 ++++++--- .../conversations/TranscriptMessageView.tsx | 6 ++ .../TranscriptTurnContextView.tsx | 2 +- .../src/api/conversations/event-page.ts | 43 ++++++++++++++- .../src/api/conversations/participants.ts | 31 +++++++++++ .../api/conversations/detail.test.ts | 55 +++++++++++++++++++ 6 files changed, 154 insertions(+), 10 deletions(-) diff --git a/packages/junior-dashboard/src/client/conversations/TranscriptMessageShell.tsx b/packages/junior-dashboard/src/client/conversations/TranscriptMessageShell.tsx index 6602899dcd..cb7c62de77 100644 --- a/packages/junior-dashboard/src/client/conversations/TranscriptMessageShell.tsx +++ b/packages/junior-dashboard/src/client/conversations/TranscriptMessageShell.tsx @@ -1,10 +1,13 @@ import type { ClipboardEventHandler, ReactNode } from "react"; +import { Avatar, AvatarFallback, AvatarImage } from "../components/ui/avatar"; import { transcriptRoleKind } from "../format"; import { cn } from "../styles"; /** Align message text and attachments beside their avatars. */ export function TranscriptMessageShell(props: { actor: string; + /** Junior API path of the author photo. Initials show without it. */ + avatarUrl?: string; children: ReactNode; onCopy?: ClipboardEventHandler; role: string; @@ -36,14 +39,22 @@ export function TranscriptMessageShell(props: { src="/_junior/dashboard/avatar.png" /> ) : ( - - {props.actor - .split(/\s+/) - .map((word) => word[0]) - .slice(0, 2) - .join("") - .toUpperCase()} - + + + + {props.actor + .split(/\s+/) + .map((word) => word[0]) + .slice(0, 2) + .join("") + .toUpperCase()} + + )} ) : null} diff --git a/packages/junior-dashboard/src/client/conversations/TranscriptMessageView.tsx b/packages/junior-dashboard/src/client/conversations/TranscriptMessageView.tsx index 7422ddfb92..5087f77ac7 100644 --- a/packages/junior-dashboard/src/client/conversations/TranscriptMessageView.tsx +++ b/packages/junior-dashboard/src/client/conversations/TranscriptMessageView.tsx @@ -40,6 +40,9 @@ export const TranscriptMessageView = memo( { const selection = event.currentTarget.ownerDocument.getSelection(); if ( @@ -109,6 +112,9 @@ export function RedactedMessageView(props: { - +
{email ? ( diff --git a/packages/junior/src/api/conversations/event-page.ts b/packages/junior/src/api/conversations/event-page.ts index 28bbe9b718..29c18762d3 100644 --- a/packages/junior/src/api/conversations/event-page.ts +++ b/packages/junior/src/api/conversations/event-page.ts @@ -21,6 +21,7 @@ import { conversationReportSourceEventTypes, projectConversationReportEventPage, } from "./events"; +import { readIdentityParticipants } from "./participants"; const conversationEventColumns = getTableColumns(juniorConversationEvents); const MIN_SCAN_SIZE = 64; @@ -192,6 +193,43 @@ async function projectConversationEventRows( }); } +/** + * Add the photo path of each user Message author. A Message without a saved + * author name gets the name of its author Identity. + */ +async function attachMessageAuthorAvatars( + executor: JuniorSqlDatabase, + rows: readonly ConversationEventRow[], + events: ConversationReportEvent[], +): Promise { + const identityIdBySeq = new Map( + rows.flatMap((row) => + row.actorIdentityId ? [[row.seq, row.actorIdentityId] as const] : [], + ), + ); + const authorIdentityId = (event: ConversationReportEvent) => + event.data.type === "message" && event.data.role === "user" + ? identityIdBySeq.get(event.seq) + : undefined; + const participants = await readIdentityParticipants( + executor.db(), + events.flatMap((event) => authorIdentityId(event) ?? []), + ); + return events.map((event) => { + const identityId = authorIdentityId(event); + const author = identityId ? participants.get(identityId) : undefined; + if (!author || event.data.type !== "message") return event; + const saved = event.data.actorIdentity; + const actorIdentity = saved + ? { + ...saved, + ...(author.avatarUrl ? { avatarUrl: author.avatarUrl } : undefined), + } + : author; + return { ...event, data: { ...event.data, actorIdentity } }; + }); +} + /** * Read the latest reporting events before an exclusive boundary. * @@ -235,7 +273,10 @@ export async function readConversationEventPage( beforeSeq = batch.at(-1)!.seq; } - const events = projected.slice(-args.limit); + const page = projected.slice(-args.limit); + const events = args.canExposePayload + ? await attachMessageAuthorAvatars(executor, rows, page) + : page; return { events, ...(projected.length > events.length && events[0] diff --git a/packages/junior/src/api/conversations/participants.ts b/packages/junior/src/api/conversations/participants.ts index 342297d860..df727fffa4 100644 --- a/packages/junior/src/api/conversations/participants.ts +++ b/packages/junior/src/api/conversations/participants.ts @@ -58,6 +58,37 @@ function participantIdentity( return Object.keys(participant).length > 0 ? participant : undefined; } +/** + * Read the actor identity, with its photo path, of each person Identity id. + * The system Identity is not a person, so it is not in the map. + */ +export async function readIdentityParticipants( + db: JuniorDatabase, + identityIds: readonly string[], +): Promise> { + if (identityIds.length === 0) return new Map(); + const rows = await db + .select(identityColumns) + .from(juniorIdentities) + .leftJoin(juniorUsers, eq(juniorUsers.id, juniorIdentities.userId)) + .where( + and( + inArray(juniorIdentities.id, [...new Set(identityIds)]), + ne(juniorIdentities.kind, "system"), + ), + ); + const userAvatarUrls = await readUserAvatarUrls( + db, + rows.flatMap((row) => (row.userId ? [row.userId] : [])), + ); + const participants = new Map(); + for (const row of rows) { + const participant = participantIdentity(row, userAvatarUrls); + if (participant) participants.set(row.identityId, participant); + } + return participants; +} + /** Read actor identities in first-appearance order for each Conversation. */ export async function readConversationParticipants( db: JuniorDatabase, diff --git a/packages/junior/tests/integration/api/conversations/detail.test.ts b/packages/junior/tests/integration/api/conversations/detail.test.ts index 0e9e715951..aaa7cd0cb1 100644 --- a/packages/junior/tests/integration/api/conversations/detail.test.ts +++ b/packages/junior/tests/integration/api/conversations/detail.test.ts @@ -13,6 +13,8 @@ import { readConversationDetail } from "@/api/conversations/detail"; import { appendConversationBrief } from "@/chat/briefs/store"; import { conversationBriefFixture } from "../../../fixtures/conversation-brief"; import { usersInfoOk } from "../../../fixtures/slack/factories/api"; +import { juniorIdentities } from "@/db/schema"; +import { and, eq } from "drizzle-orm"; import { queueSlackApiResponse } from "../../../msw/handlers/slack-api"; describe("conversation detail API", () => { @@ -327,6 +329,59 @@ describe("conversation detail API", () => { ]); }); + it("shows the photo of each message author", async () => { + const conversationId = "slack:C123:author-photos"; + await getConversationStore().recordActivity({ + actor: { + email: "owner@example.com", + fullName: "Owner Person", + platform: "slack", + slackUserId: "UOWNER1", + teamId: "T123", + }, + conversationId, + destination: { channelId: "C123", platform: "slack", teamId: "T123" }, + nowMs: 1, + source: "slack", + title: "Author photos", + visibility: "public", + }); + const [owner] = await getDb() + .update(juniorIdentities) + .set({ avatarUrl: "https://avatars.slack-edge.com/owner_512.png" }) + .where( + and( + eq(juniorIdentities.provider, "slack"), + eq(juniorIdentities.providerSubjectId, "UOWNER1"), + ), + ) + .returning({ id: juniorIdentities.id }); + await getConversationEventStore().append(conversationId, [ + { + createdAtMs: 2, + data: { + authorIdentityId: owner!.id, + messageId: "owner-1", + meta: { author: { fullName: "Owner Person", userId: "UOWNER1" } }, + role: "user", + text: "ship it", + type: "message", + }, + }, + ]); + + const detail = await readConversationDetail(conversationId, { + viewer: testViewer("owner@example.com"), + }); + expect(detail?.events[0]?.data).toMatchObject({ + actorIdentity: { + avatarUrl: `/api/people/avatars/${owner!.id}`, + fullName: "Owner Person", + slackUserId: "UOWNER1", + }, + }); + }); + it("links a viewer-visible source task from a terminal execution", async () => { const { createConfiguredJuniorSqlFixture } = await import("../../../fixtures/sql"); From e5740cfc1ec8f5535af85fe1f78a06a65fee5c02 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Mon, 2 Nov 2026 19:37:05 +0000 Subject: [PATCH 2/6] fix(dashboard): Keep the author name and photo while a send is pending A web send labeled its message with the author email until history arrived, then switched to the name. The transcript label preferred email, and only the durable Message had no email. Label user messages with the name first. Pending rows also had no photo. Add the viewer photo to the local outbox row, and resolve the author photo for pending mailbox rows from their author Identity. Co-Authored-By: BYK <126780+BYK@users.noreply.github.com> --- .../conversations/conversationOutbox.ts | 1 + .../src/client/conversations/queries.ts | 1 + .../junior-dashboard/src/client/format.ts | 5 ++- .../junior-dashboard/tests/format.test.ts | 2 + .../src/api/conversations/pending-messages.ts | 38 +++++++++++++++---- .../conversations/pending-messages.test.ts | 28 +++++++++++++- 6 files changed, 66 insertions(+), 9 deletions(-) diff --git a/packages/junior-dashboard/src/client/conversations/conversationOutbox.ts b/packages/junior-dashboard/src/client/conversations/conversationOutbox.ts index a65c2d11c0..5eb8a6100b 100644 --- a/packages/junior-dashboard/src/client/conversations/conversationOutbox.ts +++ b/packages/junior-dashboard/src/client/conversations/conversationOutbox.ts @@ -163,6 +163,7 @@ function sameActorIdentity( if (left === right) return true; if (!left || !right) return false; return ( + left.avatarUrl === right.avatarUrl && left.email === right.email && left.fullName === right.fullName && left.slackUserId === right.slackUserId && diff --git a/packages/junior-dashboard/src/client/conversations/queries.ts b/packages/junior-dashboard/src/client/conversations/queries.ts index ddf47067b9..e2874cc4c4 100644 --- a/packages/junior-dashboard/src/client/conversations/queries.ts +++ b/packages/junior-dashboard/src/client/conversations/queries.ts @@ -205,6 +205,7 @@ function outboxActorIdentity( ])?.me.user; if (!user?.email) return undefined; return { + ...(user.avatarUrl ? { avatarUrl: user.avatarUrl } : undefined), email: user.email, fullName: user.name || undefined, }; diff --git a/packages/junior-dashboard/src/client/format.ts b/packages/junior-dashboard/src/client/format.ts index 8d5f6e9213..46bb12de20 100644 --- a/packages/junior-dashboard/src/client/format.ts +++ b/packages/junior-dashboard/src/client/format.ts @@ -370,7 +370,10 @@ export function transcriptMessageActorLabel( // The conversation actor started the child agent but did not write its // input, so do not fall back to that actor here. if (message.agentInvocationId) return "Agent invocation"; - return actorLabel(transcriptMessageActor(conversation, message)) ?? "User"; + // Show the person's name, not their email. A web send carries both, and + // its durable Message carries only the name. + const actor = transcriptMessageActor(conversation, message); + return actor?.fullName?.trim() || (actorLabel(actor) ?? "User"); } if (kind === "system") return "System"; if (kind === "tool") return "Tool"; diff --git a/packages/junior-dashboard/tests/format.test.ts b/packages/junior-dashboard/tests/format.test.ts index 9997154229..47770e09e8 100644 --- a/packages/junior-dashboard/tests/format.test.ts +++ b/packages/junior-dashboard/tests/format.test.ts @@ -229,7 +229,9 @@ describe("dashboard conversation formatting", () => { messageId: "user", role: "user", text: "run search", + // A web send carries an email, but the label is the name. actorIdentity: { + email: "taylor@example.com", fullName: "Taylor Chen", slackUserName: "taylor", }, diff --git a/packages/junior/src/api/conversations/pending-messages.ts b/packages/junior/src/api/conversations/pending-messages.ts index a403b5d34f..038cfa2190 100644 --- a/packages/junior/src/api/conversations/pending-messages.ts +++ b/packages/junior/src/api/conversations/pending-messages.ts @@ -18,6 +18,7 @@ import { type ConversationPendingMessagesReport, } from "../schema/conversation"; import { readConversationAccessFromSql } from "./access"; +import { readIdentityParticipants } from "./participants"; import { webActorFromEmail } from "@/chat/conversations/web-input"; import { getWebAuthorization } from "@/chat/conversations/web-authorization"; import { @@ -230,16 +231,39 @@ export async function readConversationPendingMessages( : undefined; const work = await getConversation({ conversationId }); - const projectedMessages = await Promise.all( - (work?.execution.pendingMessages ?? []).map((message) => - projectPendingMessage(message, canExposePayload), + const pendingMessages = work?.execution.pendingMessages ?? []; + const [projectedMessages, authors] = await Promise.all([ + Promise.all( + pendingMessages.map((message) => + projectPendingMessage(message, canExposePayload), + ), ), - ); + canExposePayload + ? readIdentityParticipants( + getDb(), + pendingMessages.flatMap( + (message) => message.input.authorIdentityId ?? [], + ), + ) + : new Map(), + ]); const messages = await attachPendingMentions( conversationId, - projectedMessages.filter((message): message is ConversationPendingMessage => - Boolean(message), - ), + projectedMessages.flatMap((message, index) => { + if (!message) return []; + const identityId = pendingMessages[index]!.input.authorIdentityId; + const avatarUrl = identityId + ? authors.get(identityId)?.avatarUrl + : undefined; + return avatarUrl && message.actorIdentity + ? [ + { + ...message, + actorIdentity: { ...message.actorIdentity, avatarUrl }, + }, + ] + : [message]; + }), ); return conversationPendingMessagesReportSchema.parse({ diff --git a/packages/junior/tests/integration/api/conversations/pending-messages.test.ts b/packages/junior/tests/integration/api/conversations/pending-messages.test.ts index bd79327e8a..4a96164cce 100644 --- a/packages/junior/tests/integration/api/conversations/pending-messages.test.ts +++ b/packages/junior/tests/integration/api/conversations/pending-messages.test.ts @@ -9,7 +9,9 @@ import { appendAndEnqueueWebMessage, createAndEnqueueConversation, } from "@/chat/conversations/web-input"; -import { closeDb, getConversationStore } from "@/chat/db"; +import { closeDb, getConversationStore, getDb } from "@/chat/db"; +import { juniorIdentities } from "@/db/schema"; +import { and, eq, isNotNull } from "drizzle-orm"; import { appendInboundMessage } from "@/chat/task-execution/store"; import { closeConversationFixture, @@ -36,6 +38,29 @@ describe("conversation pending messages API", () => { }, { conversationStore, queue, state }, ); + // The author's Google photo shows before the Message reaches history. + const [author] = await getDb() + .select({ userId: juniorIdentities.userId }) + .from(juniorIdentities) + .where( + and( + eq(juniorIdentities.emailNormalized, actor.email.toLowerCase()), + isNotNull(juniorIdentities.userId), + ), + ) + .limit(1); + await getDb() + .insert(juniorIdentities) + .values({ + id: "identity-google-author", + avatarUrl: "https://lh3.googleusercontent.com/a/author", + createdAt: new Date(1_000), + kind: "user", + provider: "google", + providerSubjectId: "google-author", + updatedAt: new Date(1_000), + userId: author!.userId, + }); const app = createJuniorApi(); const response = await app.request( @@ -50,6 +75,7 @@ describe("conversation pending messages API", () => { expect(report.messages).toEqual([ { actorIdentity: { + avatarUrl: "/api/people/avatars/identity-google-author", email: actor.email, ...(actor.fullName ? { fullName: actor.fullName } : undefined), }, From 50eb65990dcb1f3d6f8b5d201385e37b1e5ba20b Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Mon, 2 Nov 2026 19:43:52 +0000 Subject: [PATCH 3/6] fix(people): Let browsers keep profile photos until they change The photo path did not change when the photo changed, so the one-day browser cache could show an old photo, and every day the browser fetched the photo again in full. Add the version of the provider URL to each photo path. A request with the current version is immutable. Other requests use no-cache with an ETag, and a matching If-None-Match returns 304. Co-Authored-By: BYK <126780+BYK@users.noreply.github.com> --- packages/junior/src/api/people/routes.ts | 28 +++++++++--- packages/junior/src/chat/identities/avatar.ts | 44 +++++++++++++------ packages/junior/tests/fixtures/avatar.ts | 10 +++++ .../tests/integration/api/automations.test.ts | 9 ++-- .../api/conversations/detail.test.ts | 3 +- .../api/conversations/list.test.ts | 5 ++- .../conversations/pending-messages.test.ts | 3 +- .../integration/api/people/avatar.test.ts | 33 ++++++++++++-- 8 files changed, 104 insertions(+), 31 deletions(-) create mode 100644 packages/junior/tests/fixtures/avatar.ts diff --git a/packages/junior/src/api/people/routes.ts b/packages/junior/src/api/people/routes.ts index 5e01ae0a69..1b04ae4bca 100644 --- a/packages/junior/src/api/people/routes.ts +++ b/packages/junior/src/api/people/routes.ts @@ -21,30 +21,46 @@ import { readPeoplePluginReports } from "./plugin-reports"; import { readPeopleProfile } from "./profile"; import { createPersonalSpendReader } from "./spend"; +// A versioned photo path never changes its bytes. `private` keeps photos out +// of shared caches, because the route needs a signed-in viewer. +const AVATAR_IMMUTABLE_CACHE_CONTROL = "private, max-age=31536000, immutable"; + /** Create the HTTP routes owned by the People API. */ export function createPeopleRoutes(): Hono { const app = new Hono(); const readPersonalSpend = createPersonalSpendReader(); // Serve profile photos from Junior, so browsers never load them from - // Slack or Google. The browser keeps each photo for a day. + // Slack or Google. A photo path has the version of its photo, so the + // browser keeps a current versioned photo and checks any other request. app.get( "/avatars/:identityId", requireViewer, validateRequest("param", avatarParamsSchema, "Invalid route parameters."), async (context) => { const { identityId } = context.req.valid("param"); - const image = await readAvatarImage( + const avatar = await readAvatarImage( getDb(), getStateAdapter(), identityId, ); - if (!image) throwApiError(404, "Avatar not found."); - return new Response(Buffer.from(image.data, "base64"), { + if (!avatar) throwApiError(404, "Avatar not found."); + const etag = `"${avatar.version}"`; + const headers = { + "cache-control": + context.req.query("v") === avatar.version + ? AVATAR_IMMUTABLE_CACHE_CONTROL + : "private, no-cache", + etag, + }; + if (context.req.header("if-none-match") === etag) { + return new Response(null, { headers, status: 304 }); + } + return new Response(Buffer.from(avatar.image.data, "base64"), { headers: { - "cache-control": "private, max-age=86400", + ...headers, "content-security-policy": "default-src 'none'; sandbox", - "content-type": image.contentType, + "content-type": avatar.image.contentType, "x-content-type-options": "nosniff", }, }); diff --git a/packages/junior/src/chat/identities/avatar.ts b/packages/junior/src/chat/identities/avatar.ts index 8293ad0013..0d052dc61b 100644 --- a/packages/junior/src/chat/identities/avatar.ts +++ b/packages/junior/src/chat/identities/avatar.ts @@ -36,6 +36,12 @@ interface AvatarImage { data: string; } +/** A saved photo and the version of its provider URL. */ +interface AvatarRead { + image: AvatarImage; + version: string; +} + type AvatarIdentityRow = { avatarUrl: string | null; identityId: string; @@ -64,9 +70,20 @@ export function parseAvatarUrl( : undefined; } +// A new photo has a new provider URL, so the hash of the URL names the photo. +function avatarUrlHash(url: string): string { + return createHash("sha256").update(url).digest("hex"); +} + +// The version names one photo of an Identity. It changes with the photo, so +// browsers can keep a versioned photo path for a long time. +function avatarVersion(url: string): string { + return avatarUrlHash(url).slice(0, 16); +} + // The Junior API path that serves the photo of one Identity. -function avatarPath(identityId: string): string { - return `/api/people/avatars/${encodeURIComponent(identityId)}`; +function avatarPath(identityId: string, avatarUrl: string): string { + return `/api/people/avatars/${encodeURIComponent(identityId)}?v=${avatarVersion(avatarUrl)}`; } /** @@ -80,6 +97,7 @@ export async function readUserAvatarUrls( if (userIds.length === 0) return new Map(); const rows = await db .select({ + avatarUrl: juniorIdentities.avatarUrl, identityId: juniorIdentities.id, provider: juniorIdentities.provider, updatedAt: juniorIdentities.updatedAt, @@ -103,8 +121,8 @@ export async function readUserAvatarUrls( ); const avatars = new Map(); for (const row of rows) { - if (row.userId && !avatars.has(row.userId)) { - avatars.set(row.userId, avatarPath(row.identityId)); + if (row.userId && row.avatarUrl && !avatars.has(row.userId)) { + avatars.set(row.userId, avatarPath(row.identityId, row.avatarUrl)); } } return avatars; @@ -119,7 +137,7 @@ export function identityAvatarUrl( userAvatarUrls: ReadonlyMap, ): string | undefined { if (row.userId) return userAvatarUrls.get(row.userId); - return row.avatarUrl ? avatarPath(row.identityId) : undefined; + return row.avatarUrl ? avatarPath(row.identityId, row.avatarUrl) : undefined; } // Read at most `maxBytes` of the body. Return undefined when it is larger. @@ -213,15 +231,16 @@ async function fetchAvatarImage( } /** - * Read the saved photo of one Identity. Junior fetches it from the provider - * once and keeps it in the state cache for a day. Return undefined when the - * Identity has no photo or the provider does not return one. + * Read the saved photo of one Identity and its version. Junior fetches it + * from the provider once and keeps it in the state cache for a day. Return + * undefined when the Identity has no photo or the provider does not return + * one. */ export async function readAvatarImage( db: JuniorDatabase, state: StateAdapter, identityId: string, -): Promise { +): Promise { const [row] = await db .select({ avatarUrl: juniorIdentities.avatarUrl, @@ -236,9 +255,8 @@ export async function readAvatarImage( ) .limit(1); const url = row ? parseAvatarUrl(row.avatarUrl, row.provider) : undefined; - if (!row || !url) return undefined; - // A new photo has a new URL, so the key changes with the photo. - const key = `avatar:${createHash("sha256").update(url).digest("hex")}`; + if (!row?.avatarUrl || !url) return undefined; + const key = `avatar:${avatarUrlHash(url)}`; const image = await createCache(state, "core").getOrLoad(key, async () => { const loaded = await fetchAvatarImage(url, row.provider); return { @@ -246,5 +264,5 @@ export async function readAvatarImage( ttlMs: loaded ? AVATAR_CACHE_TTL_MS : AVATAR_FAILURE_TTL_MS, }; }); - return image ?? undefined; + return image ? { image, version: avatarVersion(row.avatarUrl) } : undefined; } diff --git a/packages/junior/tests/fixtures/avatar.ts b/packages/junior/tests/fixtures/avatar.ts new file mode 100644 index 0000000000..64446b5bff --- /dev/null +++ b/packages/junior/tests/fixtures/avatar.ts @@ -0,0 +1,10 @@ +import { expect } from "vitest"; + +/** Match the versioned Junior photo path of one Identity. */ +export function avatarPath(identityId: string) { + return expect.stringMatching( + new RegExp( + `^/api/people/avatars/${encodeURIComponent(identityId)}\\?v=[0-9a-f]{16}$`, + ), + ); +} diff --git a/packages/junior/tests/integration/api/automations.test.ts b/packages/junior/tests/integration/api/automations.test.ts index c34640c684..0da6937ca6 100644 --- a/packages/junior/tests/integration/api/automations.test.ts +++ b/packages/junior/tests/integration/api/automations.test.ts @@ -1,4 +1,5 @@ import { eq } from "drizzle-orm"; +import { avatarPath } from "../../fixtures/avatar"; import { Hono } from "hono"; import { describe, expect, test } from "vitest"; import { @@ -375,7 +376,7 @@ describe("Automations API", () => { automations: [ expect.objectContaining({ createdBy: "Aisha Patel", - createdByAvatarUrl: `/api/people/avatars/${otherIdentity!.id}`, + createdByAvatarUrl: avatarPath(otherIdentity!.id), createdByEmail: "aisha@example.com", destination: expect.objectContaining({ label: "#incident-response", @@ -388,7 +389,7 @@ describe("Automations API", () => { }), expect.objectContaining({ createdBy: "Aisha Patel", - createdByAvatarUrl: `/api/people/avatars/${otherIdentity!.id}`, + createdByAvatarUrl: avatarPath(otherIdentity!.id), createdByEmail: "aisha@example.com", destination: expect.objectContaining({ label: "#incident-response", @@ -470,8 +471,8 @@ describe("Automations API", () => { .automations.find( (automation) => automation.id === "event_public_tasks_api", ); - expect(unverifiedCreator?.createdByAvatarUrl).toBe( - `/api/people/avatars/${otherIdentity!.id}`, + expect(unverifiedCreator?.createdByAvatarUrl).toEqual( + avatarPath(otherIdentity!.id), ); expect(unverifiedCreator?.createdByEmail).toBeUndefined(); diff --git a/packages/junior/tests/integration/api/conversations/detail.test.ts b/packages/junior/tests/integration/api/conversations/detail.test.ts index aaa7cd0cb1..2885f8731c 100644 --- a/packages/junior/tests/integration/api/conversations/detail.test.ts +++ b/packages/junior/tests/integration/api/conversations/detail.test.ts @@ -1,4 +1,5 @@ import { afterEach, describe, expect, it } from "vitest"; +import { avatarPath } from "../../../fixtures/avatar"; import { testViewer } from "../../../fixtures/user"; import { createJuniorApi } from "@/api"; import { conversationDetailReportSchema } from "@/api/schema"; @@ -375,7 +376,7 @@ describe("conversation detail API", () => { }); expect(detail?.events[0]?.data).toMatchObject({ actorIdentity: { - avatarUrl: `/api/people/avatars/${owner!.id}`, + avatarUrl: avatarPath(owner!.id), fullName: "Owner Person", slackUserId: "UOWNER1", }, diff --git a/packages/junior/tests/integration/api/conversations/list.test.ts b/packages/junior/tests/integration/api/conversations/list.test.ts index 84d63c506d..bedcd7a85a 100644 --- a/packages/junior/tests/integration/api/conversations/list.test.ts +++ b/packages/junior/tests/integration/api/conversations/list.test.ts @@ -1,4 +1,5 @@ import { createHash } from "node:crypto"; +import { avatarPath } from "../../../fixtures/avatar"; import { describe, expect, test } from "vitest"; import { defineJuniorPlugin } from "@sentry/junior-plugin-api"; import { testViewer } from "../../../fixtures/user"; @@ -831,12 +832,12 @@ describe("conversation list API", () => { isParticipant: true, participants: [ expect.objectContaining({ - avatarUrl: "/api/people/avatars/identity-google-owner", + avatarUrl: avatarPath("identity-google-owner"), email: "owner@example.com", slackUserId: "U-OWNER", }), expect.objectContaining({ - avatarUrl: "/api/people/avatars/identity-participant-slack", + avatarUrl: avatarPath("identity-participant-slack"), email: "participant@example.com", fullName: "Participant", slackUserId: "UPARTICIPANT1", diff --git a/packages/junior/tests/integration/api/conversations/pending-messages.test.ts b/packages/junior/tests/integration/api/conversations/pending-messages.test.ts index 4a96164cce..e59e69419d 100644 --- a/packages/junior/tests/integration/api/conversations/pending-messages.test.ts +++ b/packages/junior/tests/integration/api/conversations/pending-messages.test.ts @@ -1,4 +1,5 @@ import { afterEach, describe, expect, it } from "vitest"; +import { avatarPath } from "../../../fixtures/avatar"; import { Hono } from "hono"; import { createJuniorApi, type JuniorApiVariables } from "@/api"; import { @@ -75,7 +76,7 @@ describe("conversation pending messages API", () => { expect(report.messages).toEqual([ { actorIdentity: { - avatarUrl: "/api/people/avatars/identity-google-author", + avatarUrl: avatarPath("identity-google-author"), email: actor.email, ...(actor.fullName ? { fullName: actor.fullName } : undefined), }, diff --git a/packages/junior/tests/integration/api/people/avatar.test.ts b/packages/junior/tests/integration/api/people/avatar.test.ts index 443335db23..5565f02527 100644 --- a/packages/junior/tests/integration/api/people/avatar.test.ts +++ b/packages/junior/tests/integration/api/people/avatar.test.ts @@ -3,6 +3,7 @@ import { http, HttpResponse } from "msw"; import { describe, expect, test } from "vitest"; import { createJuniorApi, type JuniorApiVariables } from "@/api"; import { migrateSchema } from "@/chat/conversations/sql/migrations"; +import { identityAvatarUrl } from "@/chat/identities/avatar"; import { juniorIdentities } from "@/db/schema"; import { createConfiguredJuniorSqlFixture } from "../../../fixtures/sql"; import { testViewer } from "../../../fixtures/user"; @@ -94,15 +95,39 @@ describe("profile photo API", () => { expect(signedOut.status).toBe(401); const app = signedInApi(); - for (let request = 0; request < 2; request += 1) { - const response = await app.request( - "http://localhost/api/people/avatars/U-PHOTO", - ); + // APIs return the versioned path. The browser keeps that photo, and it + // checks a path without the current version with the ETag. + const versionedPath = identityAvatarUrl( + { + avatarUrl: "https://avatars.slack-edge.com/photo.png", + identityId: "U-PHOTO", + userId: null, + }, + new Map(), + ); + const unversioned = "/api/people/avatars/U-PHOTO"; + const responses = []; + for (const path of [versionedPath, unversioned]) { + const response = await app.request(`http://localhost${path}`); expect(response.status).toBe(200); expect(response.headers.get("content-type")).toBe("image/png"); expect(response.headers.get("x-content-type-options")).toBe("nosniff"); expect(new Uint8Array(await response.arrayBuffer())).toEqual(PNG); + responses.push(response); } + expect(responses[0]!.headers.get("cache-control")).toBe( + "private, max-age=31536000, immutable", + ); + expect(responses[1]!.headers.get("cache-control")).toBe( + "private, no-cache", + ); + const etag = responses[1]!.headers.get("etag"); + expect(etag).toBe(responses[0]!.headers.get("etag")); + const revalidated = await app.request(`http://localhost${unversioned}`, { + headers: { "if-none-match": etag! }, + }); + expect(revalidated.status).toBe(304); + expect(await revalidated.text()).toBe(""); const google = await app.request( "http://localhost/api/people/avatars/G-PHOTO", From 95b7a0cd4d2e6d126af6cb44ea4552bfaac3ea98 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Mon, 2 Nov 2026 22:44:56 +0000 Subject: [PATCH 4/6] ref(dashboard): Refer to message authors by Identity id Messages and queued rows name their author with authorIdentityId. The Conversation participants already carry each person's name and photo, and now list the Identity ids of that person. The client reads the author from there, so the API sends each person once and the event page does no extra author lookup. --- .../conversations/TranscriptMessageView.tsx | 3 +- .../conversations/conversationOutbox.ts | 21 ++++++++ .../client/conversations/eventTranscript.ts | 6 +++ .../src/client/conversations/queries.ts | 16 ++++-- .../junior-dashboard/src/client/format.ts | 16 ++++++ packages/junior-dashboard/src/client/types.ts | 2 + .../tests/conversation-outbox.test.ts | 27 ++++++++++ .../junior-dashboard/tests/format.test.ts | 48 +++++++++++++++++ .../src/api/conversations/event-page.ts | 45 ++++++---------- .../src/api/conversations/participants.ts | 52 ++++--------------- .../src/api/conversations/pending-messages.ts | 37 ++++--------- .../junior/src/api/schema/conversation.ts | 9 ++++ .../api/conversations/detail.test.ts | 16 ++++-- .../conversations/pending-messages.test.ts | 20 ++----- 14 files changed, 195 insertions(+), 123 deletions(-) diff --git a/packages/junior-dashboard/src/client/conversations/TranscriptMessageView.tsx b/packages/junior-dashboard/src/client/conversations/TranscriptMessageView.tsx index 5087f77ac7..10949eb584 100644 --- a/packages/junior-dashboard/src/client/conversations/TranscriptMessageView.tsx +++ b/packages/junior-dashboard/src/client/conversations/TranscriptMessageView.tsx @@ -100,7 +100,8 @@ export const TranscriptMessageView = memo( previous.conversation.conversationId === next.conversation.conversationId && previous.conversation.surface === next.conversation.surface && previous.conversation.canFork === next.conversation.canFork && - previous.conversation.actorIdentity === next.conversation.actorIdentity, + previous.conversation.actorIdentity === next.conversation.actorIdentity && + previous.conversation.participants === next.conversation.participants, ); /** Render a redacted primary chat message without exposing body content. */ diff --git a/packages/junior-dashboard/src/client/conversations/conversationOutbox.ts b/packages/junior-dashboard/src/client/conversations/conversationOutbox.ts index 5eb8a6100b..363ebacd41 100644 --- a/packages/junior-dashboard/src/client/conversations/conversationOutbox.ts +++ b/packages/junior-dashboard/src/client/conversations/conversationOutbox.ts @@ -3,6 +3,7 @@ import type { ConversationPendingMessage, InputImage, } from "@sentry/junior/api/schema"; +import { conversationParticipant } from "../format"; /** Client-owned mailbox row waiting on accept, server visibility, or retry. */ export type ConversationOutboxMessage = { @@ -27,6 +28,26 @@ export type ConversationMailboxMessage = ConversationPendingMessage & { idempotencyKey?: string; }; +/** + * Give each server mailbox row the photo of its author. The row names its + * author by Identity id, and the Conversation participants carry the photo. + */ +export function attachMailboxAuthorAvatars( + messages: readonly ConversationPendingMessage[] | undefined, + participants: readonly ActorIdentity[] | undefined, +): readonly ConversationPendingMessage[] | undefined { + if (!messages || !participants?.length) return messages; + return messages.map((message) => { + const avatarUrl = conversationParticipant( + participants, + message.authorIdentityId, + )?.avatarUrl; + return avatarUrl && message.actorIdentity + ? { ...message, actorIdentity: { ...message.actorIdentity, avatarUrl } } + : message; + }); +} + /** Stable React Query key for one conversation's local send outbox. */ export function conversationOutboxQueryKey(conversationId: string | undefined) { return ["conversation", conversationId, "outbox"] as const; diff --git a/packages/junior-dashboard/src/client/conversations/eventTranscript.ts b/packages/junior-dashboard/src/client/conversations/eventTranscript.ts index 42eb9e2688..fb3d8ede52 100644 --- a/packages/junior-dashboard/src/client/conversations/eventTranscript.ts +++ b/packages/junior-dashboard/src/client/conversations/eventTranscript.ts @@ -93,6 +93,9 @@ export function pendingTranscriptMessage( ...(message.actorIdentity ? { actorIdentity: message.actorIdentity } : undefined), + ...(message.authorIdentityId + ? { authorIdentityId: message.authorIdentityId } + : undefined), ...(message.mentions ? { mentions: message.mentions } : undefined), }; } @@ -222,6 +225,9 @@ export function transcriptMessagesFromEvents( ...(data.actorIdentity ? { actorIdentity: data.actorIdentity } : undefined), + ...(data.authorIdentityId + ? { authorIdentityId: data.authorIdentityId } + : undefined), ...(data.agentInvocationId ? { agentInvocationId: data.agentInvocationId } : undefined), diff --git a/packages/junior-dashboard/src/client/conversations/queries.ts b/packages/junior-dashboard/src/client/conversations/queries.ts index e2874cc4c4..0f30dad749 100644 --- a/packages/junior-dashboard/src/client/conversations/queries.ts +++ b/packages/junior-dashboard/src/client/conversations/queries.ts @@ -40,6 +40,7 @@ import { readDashboardResponse, } from "../http"; import { + attachMailboxAuthorAvatars, conversationOutboxMessageForSubmit, conversationOutboxQueryKey, failConversationOutboxMessage, @@ -820,11 +821,15 @@ export function useConversationData(conversationId: string | undefined) { // Hide stopped rows from both sources, or the outbox copy of a stopped // send would look unseen and come back. const stopped = stoppedMessageIds.data; + const authored = attachMailboxAuthorAvatars( + mailbox?.messages, + detailData?.participants, + ); const server = stopped.length - ? mailbox?.messages.filter( + ? authored?.filter( (message) => !stopped.includes(message.inboundMessageId), ) - : mailbox?.messages; + : authored; const local = stopped.length ? outbox.data.filter((message) => !stopped.includes(message.messageId)) : outbox.data; @@ -844,7 +849,12 @@ export function useConversationData(conversationId: string | undefined) { pendingMessagesRef.current = queued; inlineMessagesRef.current = inline; return { inlineMessages: inline, pendingMessages: queued }; - }, [outbox.data, mailbox?.messages, stoppedMessageIds.data]); + }, [ + outbox.data, + mailbox?.messages, + detailData?.participants, + stoppedMessageIds.data, + ]); const invalidHistoryCursor = isInvalidCursorError(history.error); const shouldRefreshDetail = Boolean( detailData && diff --git a/packages/junior-dashboard/src/client/format.ts b/packages/junior-dashboard/src/client/format.ts index 46bb12de20..64edce7f2d 100644 --- a/packages/junior-dashboard/src/client/format.ts +++ b/packages/junior-dashboard/src/client/format.ts @@ -344,6 +344,17 @@ export type MessageSummary = { total: number; }; +/** Return the Conversation participant who owns one Identity id. */ +export function conversationParticipant( + participants: readonly ActorIdentity[] | undefined, + identityId: string | undefined, +): ActorIdentity | undefined { + if (!identityId) return undefined; + return participants?.find((participant) => + participant.identityIds?.includes(identityId), + ); +} + /** * Return the person who wrote one transcript user message. Child agent input * has no person. Other messages fall back to the conversation actor. @@ -354,6 +365,11 @@ export function transcriptMessageActor( ): ActorIdentity | undefined { if (transcriptRoleKind(message.role) !== "user") return undefined; if (message.agentInvocationId) return undefined; + const author = conversationParticipant( + conversation.participants, + message.authorIdentityId, + ); + if (author) return author; if (actorLabel(message.actorIdentity)) return message.actorIdentity; if (actorLabel(conversation.actorIdentity)) return conversation.actorIdentity; return undefined; diff --git a/packages/junior-dashboard/src/client/types.ts b/packages/junior-dashboard/src/client/types.ts index ce5f414ffa..3a470ab0f4 100644 --- a/packages/junior-dashboard/src/client/types.ts +++ b/packages/junior-dashboard/src/client/types.ts @@ -121,6 +121,8 @@ export type TranscriptViewMessage = { actorIdentity?: ActorIdentity; /** Agent invocation whose input this is. An agent wrote it, not a person. */ agentInvocationId?: string; + /** Identity id of the author. Conversation participants list it. */ + authorIdentityId?: string; contexts?: TranscriptViewTurnContext[]; /** Mailbox delivery mode while the message is still pending history commit. */ delivery?: "defer" | "interrupt"; diff --git a/packages/junior-dashboard/tests/conversation-outbox.test.ts b/packages/junior-dashboard/tests/conversation-outbox.test.ts index 8fb5e8c5ed..867cbc4fd7 100644 --- a/packages/junior-dashboard/tests/conversation-outbox.test.ts +++ b/packages/junior-dashboard/tests/conversation-outbox.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import { + attachMailboxAuthorAvatars, conversationOutboxMessageForSubmit, failConversationOutboxMessage, mailboxMessageFromOutbox, @@ -10,6 +11,32 @@ import { } from "../src/client/conversations/conversationOutbox"; describe("conversation outbox", () => { + it("adds the participant photo to a queued row by author Identity id", () => { + const row = { + actorIdentity: { email: "owner@example.com" }, + authorIdentityId: "identity-web", + createdAt: "2026-01-01T00:00:00.000Z", + delivery: "defer" as const, + inboundMessageId: "inbound-1", + messageId: "message-1", + receivedAt: "2026-01-01T00:00:00.000Z", + role: "user" as const, + source: "web" as const, + }; + + expect( + attachMailboxAuthorAvatars( + [row], + [{ avatarUrl: "/avatar?v=1", identityIds: ["identity-web"] }], + ), + ).toEqual([ + { + ...row, + actorIdentity: { avatarUrl: "/avatar?v=1", email: "owner@example.com" }, + }, + ]); + }); + it("builds a sending row for one submit", () => { expect( conversationOutboxMessageForSubmit({ diff --git a/packages/junior-dashboard/tests/format.test.ts b/packages/junior-dashboard/tests/format.test.ts index 47770e09e8..5b9dabd058 100644 --- a/packages/junior-dashboard/tests/format.test.ts +++ b/packages/junior-dashboard/tests/format.test.ts @@ -32,6 +32,7 @@ import { summarizeModelUsage, summarizeToolCalls, summarizeTurns, + transcriptMessageActor, } from "../src/client/format"; import { formatDuration } from "../src/client/components/Duration"; import type { ConversationTranscript } from "../src/client/types"; @@ -221,6 +222,53 @@ describe("dashboard conversation formatting", () => { expect(formatConversationDuration(conversation!)).toBe("none"); }); + it("reads each message author from the conversation participants", () => { + const owner = { + avatarUrl: "/api/people/identities/identity-web/avatar?v=1", + email: "owner@example.com", + fullName: "Owner Person", + identityIds: ["identity-slack", "identity-web"], + }; + const conversation = transcript( + [ + event(0, { + type: "message", + messageId: "web", + role: "user", + text: "ship it", + authorIdentityId: "identity-web", + actorIdentity: { fullName: "Saved Name" }, + }), + ], + { + actorIdentity: { fullName: "Alice" }, + participants: [{ fullName: "Alice" }, owner], + }, + ); + + expect(summarizeMessages(conversation).items).toEqual([ + { author: "Owner Person", bytes: 7 }, + ]); + expect( + transcriptMessageActor(conversation, { + authorIdentityId: "identity-slack", + parts: [], + role: "user", + sourceSeq: 1, + }), + ).toBe(owner); + // An author who is not a participant yet keeps the saved identity. + expect( + transcriptMessageActor(conversation, { + actorIdentity: { fullName: "New Person" }, + authorIdentityId: "identity-new", + parts: [], + role: "user", + sourceSeq: 2, + }), + ).toEqual({ fullName: "New Person" }); + }); + it("summarizes visible canonical messages and structural tools", () => { const conversation = transcript( [ diff --git a/packages/junior/src/api/conversations/event-page.ts b/packages/junior/src/api/conversations/event-page.ts index 29c18762d3..1396ff6265 100644 --- a/packages/junior/src/api/conversations/event-page.ts +++ b/packages/junior/src/api/conversations/event-page.ts @@ -13,6 +13,7 @@ import { decodeStoredConversationEvent, type ConversationEvent, } from "@/chat/conversations/history"; +import { SYSTEM_IDENTITY_ID } from "@/chat/identities/actor"; import type { JuniorSqlDatabase } from "@/db/db"; import { juniorConversationEvents } from "@/db/schema"; import type { ConversationReportEvent } from "../schema/conversation"; @@ -21,7 +22,6 @@ import { conversationReportSourceEventTypes, projectConversationReportEventPage, } from "./events"; -import { readIdentityParticipants } from "./participants"; const conversationEventColumns = getTableColumns(juniorConversationEvents); const MIN_SCAN_SIZE = 64; @@ -193,40 +193,27 @@ async function projectConversationEventRows( }); } -/** - * Add the photo path of each user Message author. A Message without a saved - * author name gets the name of its author Identity. - */ -async function attachMessageAuthorAvatars( - executor: JuniorSqlDatabase, +// Name the Identity that wrote each user Message. The client reads the name +// and photo of that Identity from the Conversation participants. +function attachMessageAuthorIdentityIds( rows: readonly ConversationEventRow[], events: ConversationReportEvent[], -): Promise { +): ConversationReportEvent[] { const identityIdBySeq = new Map( rows.flatMap((row) => - row.actorIdentityId ? [[row.seq, row.actorIdentityId] as const] : [], + row.actorIdentityId && row.actorIdentityId !== SYSTEM_IDENTITY_ID + ? [[row.seq, row.actorIdentityId] as const] + : [], ), ); - const authorIdentityId = (event: ConversationReportEvent) => - event.data.type === "message" && event.data.role === "user" - ? identityIdBySeq.get(event.seq) - : undefined; - const participants = await readIdentityParticipants( - executor.db(), - events.flatMap((event) => authorIdentityId(event) ?? []), - ); return events.map((event) => { - const identityId = authorIdentityId(event); - const author = identityId ? participants.get(identityId) : undefined; - if (!author || event.data.type !== "message") return event; - const saved = event.data.actorIdentity; - const actorIdentity = saved - ? { - ...saved, - ...(author.avatarUrl ? { avatarUrl: author.avatarUrl } : undefined), - } - : author; - return { ...event, data: { ...event.data, actorIdentity } }; + if (event.data.type !== "message" || event.data.role !== "user") { + return event; + } + const authorIdentityId = identityIdBySeq.get(event.seq); + return authorIdentityId + ? { ...event, data: { ...event.data, authorIdentityId } } + : event; }); } @@ -275,7 +262,7 @@ export async function readConversationEventPage( const page = projected.slice(-args.limit); const events = args.canExposePayload - ? await attachMessageAuthorAvatars(executor, rows, page) + ? attachMessageAuthorIdentityIds(rows, page) : page; return { events, diff --git a/packages/junior/src/api/conversations/participants.ts b/packages/junior/src/api/conversations/participants.ts index df727fffa4..5a4063a465 100644 --- a/packages/junior/src/api/conversations/participants.ts +++ b/packages/junior/src/api/conversations/participants.ts @@ -58,37 +58,6 @@ function participantIdentity( return Object.keys(participant).length > 0 ? participant : undefined; } -/** - * Read the actor identity, with its photo path, of each person Identity id. - * The system Identity is not a person, so it is not in the map. - */ -export async function readIdentityParticipants( - db: JuniorDatabase, - identityIds: readonly string[], -): Promise> { - if (identityIds.length === 0) return new Map(); - const rows = await db - .select(identityColumns) - .from(juniorIdentities) - .leftJoin(juniorUsers, eq(juniorUsers.id, juniorIdentities.userId)) - .where( - and( - inArray(juniorIdentities.id, [...new Set(identityIds)]), - ne(juniorIdentities.kind, "system"), - ), - ); - const userAvatarUrls = await readUserAvatarUrls( - db, - rows.flatMap((row) => (row.userId ? [row.userId] : [])), - ); - const participants = new Map(); - for (const row of rows) { - const participant = participantIdentity(row, userAvatarUrls); - if (participant) participants.set(row.identityId, participant); - } - return participants; -} - /** Read actor identities in first-appearance order for each Conversation. */ export async function readConversationParticipants( db: JuniorDatabase, @@ -149,23 +118,20 @@ export async function readConversationParticipants( row.userId ? [row.userId] : [], ), ); - const ordered = new Map< - string, - { identities: ActorIdentity[]; keys: Set } - >(); + const ordered = new Map>(); const add = (conversationId: string, row: ParticipantIdentityRow) => { const participant = participantIdentity(row, userAvatarUrls); if (!participant) return; - const group = ordered.get(conversationId) ?? { - identities: [], - keys: new Set(), - }; + const group = ordered.get(conversationId) ?? new Map(); + // One person can write from several Identities, such as Slack and web. const key = row.userId ? `user:${row.userId}` : `identity:${row.identityId}`; - if (!group.keys.has(key)) { - group.keys.add(key); - group.identities.push(participant); + const existing = group.get(key); + if (!existing) { + group.set(key, { ...participant, identityIds: [row.identityId] }); + } else if (!existing.identityIds?.includes(row.identityId)) { + existing.identityIds = [...(existing.identityIds ?? []), row.identityId]; } ordered.set(conversationId, group); }; @@ -177,7 +143,7 @@ export async function readConversationParticipants( return new Map( [...ordered].map(([conversationId, group]) => [ conversationId, - group.identities, + [...group.values()], ]), ); } diff --git a/packages/junior/src/api/conversations/pending-messages.ts b/packages/junior/src/api/conversations/pending-messages.ts index 038cfa2190..e4e869284a 100644 --- a/packages/junior/src/api/conversations/pending-messages.ts +++ b/packages/junior/src/api/conversations/pending-messages.ts @@ -18,7 +18,6 @@ import { type ConversationPendingMessagesReport, } from "../schema/conversation"; import { readConversationAccessFromSql } from "./access"; -import { readIdentityParticipants } from "./participants"; import { webActorFromEmail } from "@/chat/conversations/web-input"; import { getWebAuthorization } from "@/chat/conversations/web-authorization"; import { @@ -232,37 +231,21 @@ export async function readConversationPendingMessages( const work = await getConversation({ conversationId }); const pendingMessages = work?.execution.pendingMessages ?? []; - const [projectedMessages, authors] = await Promise.all([ - Promise.all( - pendingMessages.map((message) => - projectPendingMessage(message, canExposePayload), - ), + const projectedMessages = await Promise.all( + pendingMessages.map((message) => + projectPendingMessage(message, canExposePayload), ), - canExposePayload - ? readIdentityParticipants( - getDb(), - pendingMessages.flatMap( - (message) => message.input.authorIdentityId ?? [], - ), - ) - : new Map(), - ]); + ); const messages = await attachPendingMentions( conversationId, projectedMessages.flatMap((message, index) => { if (!message) return []; - const identityId = pendingMessages[index]!.input.authorIdentityId; - const avatarUrl = identityId - ? authors.get(identityId)?.avatarUrl - : undefined; - return avatarUrl && message.actorIdentity - ? [ - { - ...message, - actorIdentity: { ...message.actorIdentity, avatarUrl }, - }, - ] - : [message]; + const authorIdentityId = pendingMessages[index]!.input.authorIdentityId; + return [ + canExposePayload && authorIdentityId + ? { ...message, authorIdentityId } + : message, + ]; }), ); diff --git a/packages/junior/src/api/schema/conversation.ts b/packages/junior/src/api/schema/conversation.ts index dd86936a36..3ccab4cc09 100644 --- a/packages/junior/src/api/schema/conversation.ts +++ b/packages/junior/src/api/schema/conversation.ts @@ -129,6 +129,11 @@ export const actorIdentitySchema = z avatarUrl: z.string().optional(), email: z.string().optional(), fullName: z.string().optional(), + /** + * Identity ids of this participant. A Message names its author with + * `authorIdentityId`, so the client reads the photo from here. + */ + identityIds: z.array(z.string()).optional(), slackUserId: z.string().optional(), slackUserName: z.string().optional(), }) @@ -144,6 +149,8 @@ export const conversationPendingMessageDeliverySchema = z.enum([ export const conversationPendingMessageSchema = z .object({ actorIdentity: actorIdentitySchema.optional(), + /** Identity id of the author. Conversation participants list it. */ + authorIdentityId: z.string().min(1).optional(), createdAt: z.string().datetime(), delivery: conversationPendingMessageDeliverySchema, inboundMessageId: z.string().min(1), @@ -355,6 +362,8 @@ const conversationReportMessageEventDataSchema = z role: z.enum(["assistant", "system", "user"]), source: z.enum(["slack", "web"]).optional(), actorIdentity: actorIdentitySchema.optional(), + /** Identity id of the author. Conversation participants list it. */ + authorIdentityId: z.string().min(1).optional(), /** Agent invocation whose input this is. An agent wrote it, not a person. */ agentInvocationId: z.string().min(1).optional(), eventType: z.string().min(1).optional(), diff --git a/packages/junior/tests/integration/api/conversations/detail.test.ts b/packages/junior/tests/integration/api/conversations/detail.test.ts index 2885f8731c..550a41abbd 100644 --- a/packages/junior/tests/integration/api/conversations/detail.test.ts +++ b/packages/junior/tests/integration/api/conversations/detail.test.ts @@ -330,7 +330,7 @@ describe("conversation detail API", () => { ]); }); - it("shows the photo of each message author", async () => { + it("names the participant who wrote each message", async () => { const conversationId = "slack:C123:author-photos"; await getConversationStore().recordActivity({ actor: { @@ -374,13 +374,21 @@ describe("conversation detail API", () => { const detail = await readConversationDetail(conversationId, { viewer: testViewer("owner@example.com"), }); + // Messages name their author. The participant carries the photo once. expect(detail?.events[0]?.data).toMatchObject({ - actorIdentity: { + authorIdentityId: owner!.id, + }); + expect(detail?.events[0]?.data).not.toHaveProperty( + "actorIdentity.avatarUrl", + ); + expect(detail?.participants).toEqual([ + expect.objectContaining({ avatarUrl: avatarPath(owner!.id), fullName: "Owner Person", + identityIds: [owner!.id], slackUserId: "UOWNER1", - }, - }); + }), + ]); }); it("links a viewer-visible source task from a terminal execution", async () => { diff --git a/packages/junior/tests/integration/api/conversations/pending-messages.test.ts b/packages/junior/tests/integration/api/conversations/pending-messages.test.ts index e59e69419d..6365142926 100644 --- a/packages/junior/tests/integration/api/conversations/pending-messages.test.ts +++ b/packages/junior/tests/integration/api/conversations/pending-messages.test.ts @@ -1,5 +1,4 @@ import { afterEach, describe, expect, it } from "vitest"; -import { avatarPath } from "../../../fixtures/avatar"; import { Hono } from "hono"; import { createJuniorApi, type JuniorApiVariables } from "@/api"; import { @@ -39,9 +38,10 @@ describe("conversation pending messages API", () => { }, { conversationStore, queue, state }, ); - // The author's Google photo shows before the Message reaches history. + // The row names its author, so the client can read the author photo from + // the Conversation participants before the Message reaches history. const [author] = await getDb() - .select({ userId: juniorIdentities.userId }) + .select({ id: juniorIdentities.id }) .from(juniorIdentities) .where( and( @@ -50,18 +50,6 @@ describe("conversation pending messages API", () => { ), ) .limit(1); - await getDb() - .insert(juniorIdentities) - .values({ - id: "identity-google-author", - avatarUrl: "https://lh3.googleusercontent.com/a/author", - createdAt: new Date(1_000), - kind: "user", - provider: "google", - providerSubjectId: "google-author", - updatedAt: new Date(1_000), - userId: author!.userId, - }); const app = createJuniorApi(); const response = await app.request( @@ -76,10 +64,10 @@ describe("conversation pending messages API", () => { expect(report.messages).toEqual([ { actorIdentity: { - avatarUrl: avatarPath("identity-google-author"), email: actor.email, ...(actor.fullName ? { fullName: actor.fullName } : undefined), }, + authorIdentityId: author!.id, createdAt: expect.any(String), delivery: "defer", inboundMessageId: accepted.messageId, From 0f8e94484c4540ed7100b5d451a83897d74ee74c Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Mon, 2 Nov 2026 23:02:02 +0000 Subject: [PATCH 5/6] ref(dashboard): Match message authors by actorIdentityId Event pages read the author Identity id from the event row and send it as actorIdentityId on each user Message. Pending messages do the same. The detail sends authors once, keyed by every Identity id of each person, so the client looks an author up by key. A person who sends from another provider than the one in history still resolves. Move outboxActorIdentity to the outbox module to keep queries.ts under the file length limit. --- .../conversations/TranscriptMessageView.tsx | 2 +- .../conversations/conversationOutbox.ts | 35 ++++-- .../client/conversations/eventTranscript.ts | 8 +- .../src/client/conversations/queries.ts | 34 +----- .../junior-dashboard/src/client/format.ts | 18 +-- packages/junior-dashboard/src/client/types.ts | 4 +- .../tests/conversation-outbox.test.ts | 13 +-- .../junior-dashboard/tests/format.test.ts | 15 ++- .../junior/src/api/conversations/detail.ts | 12 +- .../src/api/conversations/event-page.ts | 60 ++-------- .../junior/src/api/conversations/events.ts | 7 ++ .../src/api/conversations/participants.ts | 110 +++++++++++++----- .../src/api/conversations/pending-messages.ts | 6 +- .../junior/src/api/schema/conversation.ts | 19 +-- .../src/chat/conversations/sql/history.ts | 2 +- .../api/conversations/detail.test.ts | 34 ++++-- .../conversations/pending-messages.test.ts | 4 +- 17 files changed, 195 insertions(+), 188 deletions(-) diff --git a/packages/junior-dashboard/src/client/conversations/TranscriptMessageView.tsx b/packages/junior-dashboard/src/client/conversations/TranscriptMessageView.tsx index 10949eb584..eb31a17801 100644 --- a/packages/junior-dashboard/src/client/conversations/TranscriptMessageView.tsx +++ b/packages/junior-dashboard/src/client/conversations/TranscriptMessageView.tsx @@ -101,7 +101,7 @@ export const TranscriptMessageView = memo( previous.conversation.surface === next.conversation.surface && previous.conversation.canFork === next.conversation.canFork && previous.conversation.actorIdentity === next.conversation.actorIdentity && - previous.conversation.participants === next.conversation.participants, + previous.conversation.authors === next.conversation.authors, ); /** Render a redacted primary chat message without exposing body content. */ diff --git a/packages/junior-dashboard/src/client/conversations/conversationOutbox.ts b/packages/junior-dashboard/src/client/conversations/conversationOutbox.ts index 363ebacd41..7bd601a6e4 100644 --- a/packages/junior-dashboard/src/client/conversations/conversationOutbox.ts +++ b/packages/junior-dashboard/src/client/conversations/conversationOutbox.ts @@ -1,9 +1,11 @@ +import type { QueryClient } from "@tanstack/react-query"; import type { ActorIdentity, + ConversationDetailReport, ConversationPendingMessage, InputImage, } from "@sentry/junior/api/schema"; -import { conversationParticipant } from "../format"; +import type { DashboardCoreData } from "../types"; /** Client-owned mailbox row waiting on accept, server visibility, or retry. */ export type ConversationOutboxMessage = { @@ -30,24 +32,39 @@ export type ConversationMailboxMessage = ConversationPendingMessage & { /** * Give each server mailbox row the photo of its author. The row names its - * author by Identity id, and the Conversation participants carry the photo. + * author by Identity id, and the Conversation `authors` carry the photo. */ -export function attachMailboxAuthorAvatars( +export function withMailboxAuthorPhotos( messages: readonly ConversationPendingMessage[] | undefined, - participants: readonly ActorIdentity[] | undefined, + authors: ConversationDetailReport["authors"], ): readonly ConversationPendingMessage[] | undefined { - if (!messages || !participants?.length) return messages; + if (!messages || !authors) return messages; return messages.map((message) => { - const avatarUrl = conversationParticipant( - participants, - message.authorIdentityId, - )?.avatarUrl; + const avatarUrl = message.actorIdentityId + ? authors[message.actorIdentityId]?.avatarUrl + : undefined; return avatarUrl && message.actorIdentity ? { ...message, actorIdentity: { ...message.actorIdentity, avatarUrl } } : message; }); } +/** Identity of the viewer, for the outbox rows that they send. */ +export function outboxActorIdentity( + queryClient: QueryClient, +): ActorIdentity | undefined { + const user = queryClient.getQueryData([ + "dashboard", + "core", + ])?.me.user; + if (!user?.email) return undefined; + return { + ...(user.avatarUrl ? { avatarUrl: user.avatarUrl } : undefined), + email: user.email, + fullName: user.name || undefined, + }; +} + /** Stable React Query key for one conversation's local send outbox. */ export function conversationOutboxQueryKey(conversationId: string | undefined) { return ["conversation", conversationId, "outbox"] as const; diff --git a/packages/junior-dashboard/src/client/conversations/eventTranscript.ts b/packages/junior-dashboard/src/client/conversations/eventTranscript.ts index fb3d8ede52..89e4af7a46 100644 --- a/packages/junior-dashboard/src/client/conversations/eventTranscript.ts +++ b/packages/junior-dashboard/src/client/conversations/eventTranscript.ts @@ -93,8 +93,8 @@ export function pendingTranscriptMessage( ...(message.actorIdentity ? { actorIdentity: message.actorIdentity } : undefined), - ...(message.authorIdentityId - ? { authorIdentityId: message.authorIdentityId } + ...(message.actorIdentityId + ? { actorIdentityId: message.actorIdentityId } : undefined), ...(message.mentions ? { mentions: message.mentions } : undefined), }; @@ -225,8 +225,8 @@ export function transcriptMessagesFromEvents( ...(data.actorIdentity ? { actorIdentity: data.actorIdentity } : undefined), - ...(data.authorIdentityId - ? { authorIdentityId: data.authorIdentityId } + ...(data.actorIdentityId + ? { actorIdentityId: data.actorIdentityId } : undefined), ...(data.agentInvocationId ? { agentInvocationId: data.agentInvocationId } diff --git a/packages/junior-dashboard/src/client/conversations/queries.ts b/packages/junior-dashboard/src/client/conversations/queries.ts index 0f30dad749..195499c21a 100644 --- a/packages/junior-dashboard/src/client/conversations/queries.ts +++ b/packages/junior-dashboard/src/client/conversations/queries.ts @@ -1,7 +1,6 @@ import { useDeferredValue, useEffect, useMemo, useRef } from "react"; import { queryOptions, - type QueryClient, useInfiniteQuery, useMutation, useMutationState, @@ -9,7 +8,6 @@ import { useQueryClient, } from "@tanstack/react-query"; import type { - ActorIdentity, ConversationDetailReport, ConversationEventPage, ConversationFeed, @@ -30,7 +28,6 @@ import { stopConversationTurnResponseSchema, } from "@sentry/junior/api/schema"; -import type { DashboardCoreData } from "../types"; import { DashboardApiError, del, @@ -40,16 +37,17 @@ import { readDashboardResponse, } from "../http"; import { - attachMailboxAuthorAvatars, conversationOutboxMessageForSubmit, conversationOutboxQueryKey, failConversationOutboxMessage, isInlineOutboxMessage, mergeConversationMailboxMessages, + outboxActorIdentity, acceptConversationOutboxMessage, reuseConversationMailboxMessages, splitInlineMailboxMessages, upsertConversationOutboxMessage, + withMailboxAuthorPhotos, type ConversationMailboxMessage, type ConversationOutboxMessage, } from "./conversationOutbox"; @@ -197,21 +195,6 @@ export function conversationDetailQueryOptions( }); } -function outboxActorIdentity( - queryClient: QueryClient, -): ActorIdentity | undefined { - const user = queryClient.getQueryData([ - "dashboard", - "core", - ])?.me.user; - if (!user?.email) return undefined; - return { - ...(user.avatarUrl ? { avatarUrl: user.avatarUrl } : undefined), - email: user.email, - fullName: user.name || undefined, - }; -} - /** Create one dashboard conversation and refresh the personal feed. */ export function useCreateConversation() { const queryClient = useQueryClient(); @@ -724,6 +707,7 @@ export function useConversationData(conversationId: string | undefined) { ? deferredDetail : detail.data; const mailbox = detailData?.mailbox; + const authors = detailData?.authors; // Accept is not visibility. Keep a local row until either server resource // contains its real Message id. History and mailbox arrive in one render. useEffect(() => { @@ -821,10 +805,7 @@ export function useConversationData(conversationId: string | undefined) { // Hide stopped rows from both sources, or the outbox copy of a stopped // send would look unseen and come back. const stopped = stoppedMessageIds.data; - const authored = attachMailboxAuthorAvatars( - mailbox?.messages, - detailData?.participants, - ); + const authored = withMailboxAuthorPhotos(mailbox?.messages, authors); const server = stopped.length ? authored?.filter( (message) => !stopped.includes(message.inboundMessageId), @@ -849,12 +830,7 @@ export function useConversationData(conversationId: string | undefined) { pendingMessagesRef.current = queued; inlineMessagesRef.current = inline; return { inlineMessages: inline, pendingMessages: queued }; - }, [ - outbox.data, - mailbox?.messages, - detailData?.participants, - stoppedMessageIds.data, - ]); + }, [outbox.data, mailbox?.messages, authors, stoppedMessageIds.data]); const invalidHistoryCursor = isInvalidCursorError(history.error); const shouldRefreshDetail = Boolean( detailData && diff --git a/packages/junior-dashboard/src/client/format.ts b/packages/junior-dashboard/src/client/format.ts index 64edce7f2d..ff8301ca86 100644 --- a/packages/junior-dashboard/src/client/format.ts +++ b/packages/junior-dashboard/src/client/format.ts @@ -344,17 +344,6 @@ export type MessageSummary = { total: number; }; -/** Return the Conversation participant who owns one Identity id. */ -export function conversationParticipant( - participants: readonly ActorIdentity[] | undefined, - identityId: string | undefined, -): ActorIdentity | undefined { - if (!identityId) return undefined; - return participants?.find((participant) => - participant.identityIds?.includes(identityId), - ); -} - /** * Return the person who wrote one transcript user message. Child agent input * has no person. Other messages fall back to the conversation actor. @@ -365,10 +354,9 @@ export function transcriptMessageActor( ): ActorIdentity | undefined { if (transcriptRoleKind(message.role) !== "user") return undefined; if (message.agentInvocationId) return undefined; - const author = conversationParticipant( - conversation.participants, - message.authorIdentityId, - ); + const author = message.actorIdentityId + ? conversation.authors?.[message.actorIdentityId] + : undefined; if (author) return author; if (actorLabel(message.actorIdentity)) return message.actorIdentity; if (actorLabel(conversation.actorIdentity)) return conversation.actorIdentity; diff --git a/packages/junior-dashboard/src/client/types.ts b/packages/junior-dashboard/src/client/types.ts index 3a470ab0f4..1943cad9e8 100644 --- a/packages/junior-dashboard/src/client/types.ts +++ b/packages/junior-dashboard/src/client/types.ts @@ -121,8 +121,8 @@ export type TranscriptViewMessage = { actorIdentity?: ActorIdentity; /** Agent invocation whose input this is. An agent wrote it, not a person. */ agentInvocationId?: string; - /** Identity id of the author. Conversation participants list it. */ - authorIdentityId?: string; + /** Identity id of the author. Conversation `authors` maps it to the person. */ + actorIdentityId?: string; contexts?: TranscriptViewTurnContext[]; /** Mailbox delivery mode while the message is still pending history commit. */ delivery?: "defer" | "interrupt"; diff --git a/packages/junior-dashboard/tests/conversation-outbox.test.ts b/packages/junior-dashboard/tests/conversation-outbox.test.ts index 867cbc4fd7..3f095d4ae8 100644 --- a/packages/junior-dashboard/tests/conversation-outbox.test.ts +++ b/packages/junior-dashboard/tests/conversation-outbox.test.ts @@ -1,20 +1,20 @@ import { describe, expect, it } from "vitest"; import { - attachMailboxAuthorAvatars, conversationOutboxMessageForSubmit, failConversationOutboxMessage, mailboxMessageFromOutbox, mergeConversationMailboxMessages, splitInlineMailboxMessages, upsertConversationOutboxMessage, + withMailboxAuthorPhotos, } from "../src/client/conversations/conversationOutbox"; describe("conversation outbox", () => { - it("adds the participant photo to a queued row by author Identity id", () => { + it("adds the author photo to a queued row by its Identity id", () => { const row = { actorIdentity: { email: "owner@example.com" }, - authorIdentityId: "identity-web", + actorIdentityId: "identity-web", createdAt: "2026-01-01T00:00:00.000Z", delivery: "defer" as const, inboundMessageId: "inbound-1", @@ -25,10 +25,9 @@ describe("conversation outbox", () => { }; expect( - attachMailboxAuthorAvatars( - [row], - [{ avatarUrl: "/avatar?v=1", identityIds: ["identity-web"] }], - ), + withMailboxAuthorPhotos([row], { + "identity-web": { avatarUrl: "/avatar?v=1" }, + }), ).toEqual([ { ...row, diff --git a/packages/junior-dashboard/tests/format.test.ts b/packages/junior-dashboard/tests/format.test.ts index 5b9dabd058..c01700188a 100644 --- a/packages/junior-dashboard/tests/format.test.ts +++ b/packages/junior-dashboard/tests/format.test.ts @@ -222,12 +222,11 @@ describe("dashboard conversation formatting", () => { expect(formatConversationDuration(conversation!)).toBe("none"); }); - it("reads each message author from the conversation participants", () => { + it("reads each message author from the conversation authors", () => { const owner = { - avatarUrl: "/api/people/identities/identity-web/avatar?v=1", + avatarUrl: "/api/people/avatars/identity-web?v=1", email: "owner@example.com", fullName: "Owner Person", - identityIds: ["identity-slack", "identity-web"], }; const conversation = transcript( [ @@ -236,13 +235,13 @@ describe("dashboard conversation formatting", () => { messageId: "web", role: "user", text: "ship it", - authorIdentityId: "identity-web", + actorIdentityId: "identity-web", actorIdentity: { fullName: "Saved Name" }, }), ], { actorIdentity: { fullName: "Alice" }, - participants: [{ fullName: "Alice" }, owner], + authors: { "identity-slack": owner, "identity-web": owner }, }, ); @@ -251,17 +250,17 @@ describe("dashboard conversation formatting", () => { ]); expect( transcriptMessageActor(conversation, { - authorIdentityId: "identity-slack", + actorIdentityId: "identity-slack", parts: [], role: "user", sourceSeq: 1, }), ).toBe(owner); - // An author who is not a participant yet keeps the saved identity. + // An author who is not in the Conversation yet keeps the saved identity. expect( transcriptMessageActor(conversation, { actorIdentity: { fullName: "New Person" }, - authorIdentityId: "identity-new", + actorIdentityId: "identity-new", parts: [], role: "user", sourceSeq: 2, diff --git a/packages/junior/src/api/conversations/detail.ts b/packages/junior/src/api/conversations/detail.ts index e017cf7185..51783949fc 100644 --- a/packages/junior/src/api/conversations/detail.ts +++ b/packages/junior/src/api/conversations/detail.ts @@ -30,7 +30,7 @@ import { readLatestConversationBrief } from "@/chat/briefs/store"; import { readConversationSourceTask } from "@/chat/automations/read"; import { readAutomationTitlesByConversationId } from "@/chat/automations/execution-stats"; import { readConversationArchivedAt } from "./archive"; -import { readConversationParticipants } from "./participants"; +import { readConversationAuthors } from "./participants"; import { readConversationForks } from "./fork"; import { readConversationAgentTree } from "./agent-tree"; @@ -51,6 +51,7 @@ function projectConversationDetail(args: { locationId?: string; modelUsage: NonNullable; participants: NonNullable; + authors: NonNullable; previousSeq?: number; sourceTask?: ConversationDetailReport["sourceTask"]; teamDomainByTeamId?: ReadonlyMap; @@ -88,7 +89,7 @@ function projectConversationDetail(args: { ...(canExposePayload && args.brief ? { brief: args.brief } : undefined), events: args.events, ...(canExposePayload && args.participants.length > 0 - ? { participants: args.participants } + ? { authors: args.authors, participants: args.participants } : undefined), ...(args.previousSeq !== undefined ? { @@ -134,7 +135,7 @@ async function readConversationDetailFromSql( firstHumanMessageTextByConversation, modelUsage, metricsByConversation, - participantsByConversation, + people, sourceTask, teamDomainByTeamId, ] = await Promise.all([ @@ -148,7 +149,7 @@ async function readConversationDetailFromSql( ? readConversationModelUsageFromSql(executor, { conversationId }) : Promise.resolve([]), readConversationTreeMetricsFromSql(getDb(), [conversationId]), - readConversationParticipants(getDb(), [conversationId]), + readConversationAuthors(getDb(), conversationId), readConversationSourceTask({ conversationId, ...(options.viewer ? { viewer: options.viewer } : undefined), @@ -214,7 +215,8 @@ async function readConversationDetailFromSql( firstHumanMessageText: firstHumanMessageTextByConversation.get(conversationId), modelUsage, - participants: participantsByConversation.get(conversationId) ?? [], + authors: people.authors, + participants: people.participants, ...(sourceTask ? { sourceTask } : undefined), teamDomainByTeamId, ...(page.previousSeq === undefined diff --git a/packages/junior/src/api/conversations/event-page.ts b/packages/junior/src/api/conversations/event-page.ts index 1396ff6265..13aac21bdb 100644 --- a/packages/junior/src/api/conversations/event-page.ts +++ b/packages/junior/src/api/conversations/event-page.ts @@ -9,11 +9,8 @@ import { lt, sql, } from "drizzle-orm"; -import { - decodeStoredConversationEvent, - type ConversationEvent, -} from "@/chat/conversations/history"; -import { SYSTEM_IDENTITY_ID } from "@/chat/identities/actor"; +import type { ConversationEvent } from "@/chat/conversations/history"; +import { eventFromRow } from "@/chat/conversations/sql/history"; import type { JuniorSqlDatabase } from "@/db/db"; import { juniorConversationEvents } from "@/db/schema"; import type { ConversationReportEvent } from "../schema/conversation"; @@ -91,22 +88,6 @@ async function readConversationEventRows( .limit(args.limit); } -function decodeConversationEventRow( - row: ConversationEventRow, -): ConversationEvent { - return decodeStoredConversationEvent({ - schemaVersion: row.schemaVersion, - seq: row.seq, - historyVersion: row.historyVersion, - ...(row.idempotencyKey - ? { idempotencyKey: row.idempotencyKey } - : undefined), - createdAtMs: row.createdAt.getTime(), - type: row.type, - payload: row.payload, - }); -} - /** Project stored rows and resolve entity starts that precede the page. */ async function projectConversationEventRows( executor: JuniorSqlDatabase, @@ -117,7 +98,7 @@ async function projectConversationEventRows( }, ): Promise { const events = args.rows - .map(decodeConversationEventRow) + .map(eventFromRow) .sort((left, right) => left.seq - right.seq); // Resolve context before the scanned page, not from today's model config. // Keep the last route boundary and actual call so pages have the same model. @@ -187,33 +168,9 @@ async function projectConversationEventRows( return projectConversationReportEventPage({ canExposePayload: args.canExposePayload, events, - subagentStartEvents: subagentStartRows.map(decodeConversationEventRow), - toolStartEvents: toolStartRows.map(decodeConversationEventRow), - modelContextEvents: modelContextRows.map(decodeConversationEventRow), - }); -} - -// Name the Identity that wrote each user Message. The client reads the name -// and photo of that Identity from the Conversation participants. -function attachMessageAuthorIdentityIds( - rows: readonly ConversationEventRow[], - events: ConversationReportEvent[], -): ConversationReportEvent[] { - const identityIdBySeq = new Map( - rows.flatMap((row) => - row.actorIdentityId && row.actorIdentityId !== SYSTEM_IDENTITY_ID - ? [[row.seq, row.actorIdentityId] as const] - : [], - ), - ); - return events.map((event) => { - if (event.data.type !== "message" || event.data.role !== "user") { - return event; - } - const authorIdentityId = identityIdBySeq.get(event.seq); - return authorIdentityId - ? { ...event, data: { ...event.data, authorIdentityId } } - : event; + subagentStartEvents: subagentStartRows.map(eventFromRow), + toolStartEvents: toolStartRows.map(eventFromRow), + modelContextEvents: modelContextRows.map(eventFromRow), }); } @@ -260,10 +217,7 @@ export async function readConversationEventPage( beforeSeq = batch.at(-1)!.seq; } - const page = projected.slice(-args.limit); - const events = args.canExposePayload - ? attachMessageAuthorIdentityIds(rows, page) - : page; + const events = projected.slice(-args.limit); return { events, ...(projected.length > events.length && events[0] diff --git a/packages/junior/src/api/conversations/events.ts b/packages/junior/src/api/conversations/events.ts index 3cea6213c8..f3007fa170 100644 --- a/packages/junior/src/api/conversations/events.ts +++ b/packages/junior/src/api/conversations/events.ts @@ -3,6 +3,7 @@ import { readMessageAttachments } from "@/chat/attachments/input"; import { readMessageCardRefs } from "@/chat/conversations/cards"; import type { ConversationEvent } from "@/chat/conversations/history"; import { renderJuniorNativeConversationEvent } from "@/chat/conversations/structured-events"; +import { SYSTEM_IDENTITY_ID } from "@/chat/identities/actor"; import { renderPluginConversationEvent } from "@/chat/plugins/conversation-events"; import { buildSentryEventUrl } from "@/chat/sentry-links"; import { z } from "zod"; @@ -357,6 +358,12 @@ function reportEventData(args: { ? { source: data.meta.source } : undefined), ...(actorIdentity ? { actorIdentity } : undefined), + ...(args.canExposePayload && + data.role === "user" && + data.authorIdentityId && + data.authorIdentityId !== SYSTEM_IDENTITY_ID + ? { actorIdentityId: data.authorIdentityId } + : undefined), ...(typeof data.meta?.agentInvocationId === "string" ? { agentInvocationId: data.meta.agentInvocationId } : undefined), diff --git a/packages/junior/src/api/conversations/participants.ts b/packages/junior/src/api/conversations/participants.ts index 5a4063a465..bfa0e94258 100644 --- a/packages/junior/src/api/conversations/participants.ts +++ b/packages/junior/src/api/conversations/participants.ts @@ -58,13 +58,20 @@ function participantIdentity( return Object.keys(participant).length > 0 ? participant : undefined; } -/** Read actor identities in first-appearance order for each Conversation. */ -export async function readConversationParticipants( +type ConversationParticipantRow = ParticipantIdentityRow & { + conversationId: string; +}; + +// One person can write from several Identities, such as Slack and web. +function participantKey(row: { identityId: string; userId: string | null }) { + return row.userId ? `user:${row.userId}` : `identity:${row.identityId}`; +} + +// Read the root actor and each message author, in first-appearance order. +async function readParticipantRows( db: JuniorDatabase, conversationIds: readonly string[], -): Promise> { - if (conversationIds.length === 0) return new Map(); - +): Promise { const [rootRows, eventRows] = await Promise.all([ db .select({ @@ -111,39 +118,84 @@ export async function readConversationParticipants( asc(juniorConversationEvents.seq), ), ]); + return [ + ...rootRows, + ...eventRows.sort((left, right) => left.seq - right.seq), + ]; +} +// Group rows by Conversation, then by person, in first-appearance order. +async function groupParticipants( + db: JuniorDatabase, + rows: readonly ConversationParticipantRow[], +): Promise>> { const userAvatarUrls = await readUserAvatarUrls( db, - [...rootRows, ...eventRows].flatMap((row) => - row.userId ? [row.userId] : [], - ), + rows.flatMap((row) => (row.userId ? [row.userId] : [])), ); - const ordered = new Map>(); - const add = (conversationId: string, row: ParticipantIdentityRow) => { + const groups = new Map>(); + for (const row of rows) { const participant = participantIdentity(row, userAvatarUrls); - if (!participant) return; - const group = ordered.get(conversationId) ?? new Map(); - // One person can write from several Identities, such as Slack and web. - const key = row.userId - ? `user:${row.userId}` - : `identity:${row.identityId}`; - const existing = group.get(key); - if (!existing) { - group.set(key, { ...participant, identityIds: [row.identityId] }); - } else if (!existing.identityIds?.includes(row.identityId)) { - existing.identityIds = [...(existing.identityIds ?? []), row.identityId]; - } - ordered.set(conversationId, group); - }; - - for (const row of rootRows) add(row.conversationId, row); - for (const row of eventRows.sort((left, right) => left.seq - right.seq)) { - add(row.conversationId, row); + if (!participant) continue; + const group = groups.get(row.conversationId) ?? new Map(); + const key = participantKey(row); + if (!group.has(key)) group.set(key, participant); + groups.set(row.conversationId, group); } + return groups; +} + +/** Read actor identities in first-appearance order for each Conversation. */ +export async function readConversationParticipants( + db: JuniorDatabase, + conversationIds: readonly string[], +): Promise> { + if (conversationIds.length === 0) return new Map(); + const groups = await groupParticipants( + db, + await readParticipantRows(db, conversationIds), + ); return new Map( - [...ordered].map(([conversationId, group]) => [ + [...groups].map(([conversationId, group]) => [ conversationId, [...group.values()], ]), ); } + +/** + * Read the participants of one Conversation, and each of them by each of + * their Identity ids. A person is listed under every Identity of their + * user, so a pending Message from another provider resolves too. + */ +export async function readConversationAuthors( + db: JuniorDatabase, + conversationId: string, +): Promise<{ + authors: Record; + participants: ActorIdentity[]; +}> { + const rows = await readParticipantRows(db, [conversationId]); + const userIds = [ + ...new Set(rows.flatMap((row) => (row.userId ? [row.userId] : []))), + ]; + const [groups, userIdentities] = await Promise.all([ + groupParticipants(db, rows), + userIds.length > 0 + ? db + .select({ + identityId: juniorIdentities.id, + userId: juniorIdentities.userId, + }) + .from(juniorIdentities) + .where(inArray(juniorIdentities.userId, userIds)) + : [], + ]); + const group = groups.get(conversationId) ?? new Map(); + const authors: Record = {}; + for (const row of [...rows, ...userIdentities]) { + const participant = group.get(participantKey(row)); + if (participant) authors[row.identityId] = participant; + } + return { authors, participants: [...group.values()] }; +} diff --git a/packages/junior/src/api/conversations/pending-messages.ts b/packages/junior/src/api/conversations/pending-messages.ts index e4e869284a..a890ecbd1c 100644 --- a/packages/junior/src/api/conversations/pending-messages.ts +++ b/packages/junior/src/api/conversations/pending-messages.ts @@ -240,10 +240,10 @@ export async function readConversationPendingMessages( conversationId, projectedMessages.flatMap((message, index) => { if (!message) return []; - const authorIdentityId = pendingMessages[index]!.input.authorIdentityId; + const actorIdentityId = pendingMessages[index]!.input.authorIdentityId; return [ - canExposePayload && authorIdentityId - ? { ...message, authorIdentityId } + canExposePayload && actorIdentityId + ? { ...message, actorIdentityId } : message, ]; }), diff --git a/packages/junior/src/api/schema/conversation.ts b/packages/junior/src/api/schema/conversation.ts index 3ccab4cc09..c0d9f7e582 100644 --- a/packages/junior/src/api/schema/conversation.ts +++ b/packages/junior/src/api/schema/conversation.ts @@ -129,11 +129,6 @@ export const actorIdentitySchema = z avatarUrl: z.string().optional(), email: z.string().optional(), fullName: z.string().optional(), - /** - * Identity ids of this participant. A Message names its author with - * `authorIdentityId`, so the client reads the photo from here. - */ - identityIds: z.array(z.string()).optional(), slackUserId: z.string().optional(), slackUserName: z.string().optional(), }) @@ -149,8 +144,8 @@ export const conversationPendingMessageDeliverySchema = z.enum([ export const conversationPendingMessageSchema = z .object({ actorIdentity: actorIdentitySchema.optional(), - /** Identity id of the author. Conversation participants list it. */ - authorIdentityId: z.string().min(1).optional(), + /** Identity id of the author. Detail `authors` maps it to the person. */ + actorIdentityId: z.string().min(1).optional(), createdAt: z.string().datetime(), delivery: conversationPendingMessageDeliverySchema, inboundMessageId: z.string().min(1), @@ -362,8 +357,8 @@ const conversationReportMessageEventDataSchema = z role: z.enum(["assistant", "system", "user"]), source: z.enum(["slack", "web"]).optional(), actorIdentity: actorIdentitySchema.optional(), - /** Identity id of the author. Conversation participants list it. */ - authorIdentityId: z.string().min(1).optional(), + /** Identity id of the author. Detail `authors` maps it to the person. */ + actorIdentityId: z.string().min(1).optional(), /** Agent invocation whose input this is. An agent wrote it, not a person. */ agentInvocationId: z.string().min(1).optional(), eventType: z.string().min(1).optional(), @@ -861,6 +856,12 @@ export const conversationDetailReportSchema = conversationSummaryReportSchema .strict() .optional(), modelUsage: z.array(conversationModelUsageSchema).optional(), + /** + * Each person who wrote in the Conversation, by each of their Identity + * ids. Messages and pending messages name their author with + * `actorIdentityId`. + */ + authors: z.record(z.string(), actorIdentitySchema).optional(), events: z.array(conversationReportEventSchema), eventHistory: conversationEventHistorySchema, previousCursor: z.string().min(1).optional(), diff --git a/packages/junior/src/chat/conversations/sql/history.ts b/packages/junior/src/chat/conversations/sql/history.ts index 7306e4c24a..50e885eaa5 100644 --- a/packages/junior/src/chat/conversations/sql/history.ts +++ b/packages/junior/src/chat/conversations/sql/history.ts @@ -114,7 +114,7 @@ function insertFromEvent( } /** Parse one physical event row into the storage-compatible domain envelope. */ -function eventFromRow(row: ConversationEventRow): ConversationEvent { +export function eventFromRow(row: ConversationEventRow): ConversationEvent { const payload = row.actorIdentityId && typeof row.payload === "object" && diff --git a/packages/junior/tests/integration/api/conversations/detail.test.ts b/packages/junior/tests/integration/api/conversations/detail.test.ts index 550a41abbd..bd2d2a42d4 100644 --- a/packages/junior/tests/integration/api/conversations/detail.test.ts +++ b/packages/junior/tests/integration/api/conversations/detail.test.ts @@ -356,7 +356,19 @@ describe("conversation detail API", () => { eq(juniorIdentities.providerSubjectId, "UOWNER1"), ), ) - .returning({ id: juniorIdentities.id }); + .returning({ id: juniorIdentities.id, userId: juniorIdentities.userId }); + // The same person can send next from the web before history has it. + await getDb() + .insert(juniorIdentities) + .values({ + id: "identity-owner-google", + createdAt: new Date(1_000), + kind: "user", + provider: "google", + providerSubjectId: "google-owner", + updatedAt: new Date(1_000), + userId: owner!.userId, + }); await getConversationEventStore().append(conversationId, [ { createdAtMs: 2, @@ -374,21 +386,21 @@ describe("conversation detail API", () => { const detail = await readConversationDetail(conversationId, { viewer: testViewer("owner@example.com"), }); - // Messages name their author. The participant carries the photo once. + // Messages name their author. The detail carries each author once. expect(detail?.events[0]?.data).toMatchObject({ - authorIdentityId: owner!.id, + actorIdentityId: owner!.id, }); expect(detail?.events[0]?.data).not.toHaveProperty( "actorIdentity.avatarUrl", ); - expect(detail?.participants).toEqual([ - expect.objectContaining({ - avatarUrl: avatarPath(owner!.id), - fullName: "Owner Person", - identityIds: [owner!.id], - slackUserId: "UOWNER1", - }), - ]); + expect(detail?.authors?.[owner!.id]).toMatchObject({ + avatarUrl: avatarPath(owner!.id), + fullName: "Owner Person", + slackUserId: "UOWNER1", + }); + expect(detail?.authors?.["identity-owner-google"]).toEqual( + detail?.authors?.[owner!.id], + ); }); it("links a viewer-visible source task from a terminal execution", async () => { diff --git a/packages/junior/tests/integration/api/conversations/pending-messages.test.ts b/packages/junior/tests/integration/api/conversations/pending-messages.test.ts index 6365142926..287a75a98d 100644 --- a/packages/junior/tests/integration/api/conversations/pending-messages.test.ts +++ b/packages/junior/tests/integration/api/conversations/pending-messages.test.ts @@ -39,7 +39,7 @@ describe("conversation pending messages API", () => { { conversationStore, queue, state }, ); // The row names its author, so the client can read the author photo from - // the Conversation participants before the Message reaches history. + // the Conversation authors before the Message reaches history. const [author] = await getDb() .select({ id: juniorIdentities.id }) .from(juniorIdentities) @@ -67,7 +67,7 @@ describe("conversation pending messages API", () => { email: actor.email, ...(actor.fullName ? { fullName: actor.fullName } : undefined), }, - authorIdentityId: author!.id, + actorIdentityId: author!.id, createdAt: expect.any(String), delivery: "defer", inboundMessageId: accepted.messageId, From bbbf71c1d8d6eb8311f9bbbe0c27ff136f3a98af Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Mon, 2 Nov 2026 23:15:28 +0000 Subject: [PATCH 6/6] ref(dashboard): Look up queued authors where the queue draws them The queue no longer copies each author photo into every mailbox row. The reply footer passes the Conversation authors to the queue, and each row reads its author by actorIdentityId when it draws the avatar. Live polls keep the authors reference while it does not change, so memoized views do not render again. Co-Authored-By: BYK <126780+BYK@users.noreply.github.com> --- .../client/conversations/ConversationPage.tsx | 3 ++ .../conversations/PendingMailboxStack.tsx | 31 +++++++++++++------ .../conversations/conversationOutbox.ts | 20 ------------ .../src/client/conversations/queries.ts | 9 ++---- .../src/client/conversations/transcript.ts | 14 +++++++-- .../tests/conversation-outbox.test.ts | 26 ---------------- .../tests/conversation-transcript.test.ts | 9 +++++- .../tests/pending-mailbox-stack.test.tsx | 17 ++++++++++ 8 files changed, 65 insertions(+), 64 deletions(-) diff --git a/packages/junior-dashboard/src/client/conversations/ConversationPage.tsx b/packages/junior-dashboard/src/client/conversations/ConversationPage.tsx index 65b88a3d32..ddaa4bec4d 100644 --- a/packages/junior-dashboard/src/client/conversations/ConversationPage.tsx +++ b/packages/junior-dashboard/src/client/conversations/ConversationPage.tsx @@ -522,6 +522,7 @@ export function ConversationPage(props: { // runs without interactive auth, so it has no approval to show. view === "rich" && detail.data?.isParticipant && !agentTree ? ( void; @@ -874,6 +876,7 @@ const ConversationReplyFooter = memo(function ConversationReplyFooter(props: { > ; - } - return ( - - ); + const author = + (actorIdentityId ? props.authors?.[actorIdentityId] : undefined) ?? + props.message.actorIdentity; + if (!author) return ; + return ; } /** Show small previews so the reader can tell which images a queued message has. */ @@ -294,6 +300,7 @@ function PendingAttachmentThumbnails(props: { function PendingRow(props: { actionsDisabled: boolean; + authors?: ConversationDetailReport["authors"]; cancelError: boolean; cancelPending: boolean; canEdit: boolean; @@ -322,7 +329,10 @@ function PendingRow(props: { className="items-center" left={ <> - + {redacted ? null : ( { - const avatarUrl = message.actorIdentityId - ? authors[message.actorIdentityId]?.avatarUrl - : undefined; - return avatarUrl && message.actorIdentity - ? { ...message, actorIdentity: { ...message.actorIdentity, avatarUrl } } - : message; - }); -} - /** Identity of the viewer, for the outbox rows that they send. */ export function outboxActorIdentity( queryClient: QueryClient, diff --git a/packages/junior-dashboard/src/client/conversations/queries.ts b/packages/junior-dashboard/src/client/conversations/queries.ts index 78fc4f80c1..3658adb2c0 100644 --- a/packages/junior-dashboard/src/client/conversations/queries.ts +++ b/packages/junior-dashboard/src/client/conversations/queries.ts @@ -47,7 +47,6 @@ import { reuseConversationMailboxMessages, splitInlineMailboxMessages, upsertConversationOutboxMessage, - withMailboxAuthorPhotos, type ConversationMailboxMessage, type ConversationOutboxMessage, } from "./conversationOutbox"; @@ -727,7 +726,6 @@ export function useConversationData(conversationId: string | undefined) { ? deferredDetail : detail.data; const mailbox = detailData?.mailbox; - const authors = detailData?.authors; // Accept is not visibility. Keep a local row until either server resource // contains its real Message id. History and mailbox arrive in one render. useEffect(() => { @@ -825,12 +823,11 @@ export function useConversationData(conversationId: string | undefined) { // Hide stopped rows from both sources, or the outbox copy of a stopped // send would look unseen and come back. const stopped = stoppedMessageIds.data; - const authored = withMailboxAuthorPhotos(mailbox?.messages, authors); const server = stopped.length - ? authored?.filter( + ? mailbox?.messages.filter( (message) => !stopped.includes(message.inboundMessageId), ) - : authored; + : mailbox?.messages; const local = stopped.length ? outbox.data.filter((message) => !stopped.includes(message.messageId)) : outbox.data; @@ -850,7 +847,7 @@ export function useConversationData(conversationId: string | undefined) { pendingMessagesRef.current = queued; inlineMessagesRef.current = inline; return { inlineMessages: inline, pendingMessages: queued }; - }, [outbox.data, mailbox?.messages, authors, stoppedMessageIds.data]); + }, [outbox.data, mailbox?.messages, stoppedMessageIds.data]); const invalidHistoryCursor = isInvalidCursorError(history.error); const shouldRefreshDetail = Boolean( detailData && diff --git a/packages/junior-dashboard/src/client/conversations/transcript.ts b/packages/junior-dashboard/src/client/conversations/transcript.ts index e209362450..59a4099569 100644 --- a/packages/junior-dashboard/src/client/conversations/transcript.ts +++ b/packages/junior-dashboard/src/client/conversations/transcript.ts @@ -223,8 +223,18 @@ export function reuseConversationEventReferences( JSON.stringify(previous.annotations) === JSON.stringify(next.annotations) ? previous.annotations : next.annotations; - if (events === next.events && annotations === next.annotations) return next; - return { ...next, events, annotations }; + const authors = + JSON.stringify(previous.authors) === JSON.stringify(next.authors) + ? previous.authors + : next.authors; + if ( + events === next.events && + annotations === next.annotations && + authors === next.authors + ) { + return next; + } + return { ...next, events, annotations, authors }; } function sameConversationEventVersion( diff --git a/packages/junior-dashboard/tests/conversation-outbox.test.ts b/packages/junior-dashboard/tests/conversation-outbox.test.ts index 3f095d4ae8..8fb5e8c5ed 100644 --- a/packages/junior-dashboard/tests/conversation-outbox.test.ts +++ b/packages/junior-dashboard/tests/conversation-outbox.test.ts @@ -7,35 +7,9 @@ import { mergeConversationMailboxMessages, splitInlineMailboxMessages, upsertConversationOutboxMessage, - withMailboxAuthorPhotos, } from "../src/client/conversations/conversationOutbox"; describe("conversation outbox", () => { - it("adds the author photo to a queued row by its Identity id", () => { - const row = { - actorIdentity: { email: "owner@example.com" }, - actorIdentityId: "identity-web", - createdAt: "2026-01-01T00:00:00.000Z", - delivery: "defer" as const, - inboundMessageId: "inbound-1", - messageId: "message-1", - receivedAt: "2026-01-01T00:00:00.000Z", - role: "user" as const, - source: "web" as const, - }; - - expect( - withMailboxAuthorPhotos([row], { - "identity-web": { avatarUrl: "/avatar?v=1" }, - }), - ).toEqual([ - { - ...row, - actorIdentity: { avatarUrl: "/avatar?v=1", email: "owner@example.com" }, - }, - ]); - }); - it("builds a sending row for one submit", () => { expect( conversationOutboxMessageForSubmit({ diff --git a/packages/junior-dashboard/tests/conversation-transcript.test.ts b/packages/junior-dashboard/tests/conversation-transcript.test.ts index 4d82bd046e..ea51e6f7fe 100644 --- a/packages/junior-dashboard/tests/conversation-transcript.test.ts +++ b/packages/junior-dashboard/tests/conversation-transcript.test.ts @@ -262,10 +262,15 @@ describe("conversation transcript", () => { }); it("reuses events while keeping fresh poll metadata", () => { - const previous = { ...detail(), annotations: [] }; + const previous = { + ...detail(), + annotations: [], + authors: { "identity-1": { fullName: "Owner Person" } }, + }; const next = { ...previous, annotations: [], + authors: { "identity-1": { fullName: "Owner Person" } }, cumulativeDurationMs: previous.cumulativeDurationMs + 2_000, generatedAt: "2026-07-23T00:00:02.000Z", lastProgressAt: "2026-07-23T00:00:02.000Z", @@ -274,6 +279,8 @@ describe("conversation transcript", () => { const result = reuseConversationEventReferences(previous, next); expect(result.annotations).toBe(previous.annotations); + // Memoized footer and message views compare authors by reference. + expect(result.authors).toBe(previous.authors); expect(result.events).toBe(previous.events); expect(result.generatedAt).toBe("2026-07-23T00:00:02.000Z"); }); diff --git a/packages/junior-dashboard/tests/pending-mailbox-stack.test.tsx b/packages/junior-dashboard/tests/pending-mailbox-stack.test.tsx index cb73f788d4..95a356c0dc 100644 --- a/packages/junior-dashboard/tests/pending-mailbox-stack.test.tsx +++ b/packages/junior-dashboard/tests/pending-mailbox-stack.test.tsx @@ -174,6 +174,23 @@ describe("PendingMailboxStack remove control", () => { expect(html.match(/aria-label="Edit queued message"/g)).toHaveLength(1); }); + it("draws each row author from the Conversation authors", () => { + const html = renderToStaticMarkup( + , + ); + + expect(html).toContain('aria-label="Owner Person"'); + expect(html).not.toContain("owner@example.com"); + }); + it("hides mailbox rows already present in committed history", () => { const html = renderToStaticMarkup(