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 : ( ; 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..eb31a17801 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 ( @@ -97,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.authors === next.conversation.authors, ); /** Render a redacted primary chat message without exposing body content. */ @@ -109,6 +113,9 @@ export function RedactedMessageView(props: { - +
{email ? ( diff --git a/packages/junior-dashboard/src/client/conversations/conversationOutbox.ts b/packages/junior-dashboard/src/client/conversations/conversationOutbox.ts index a65c2d11c0..07cc71e8e2 100644 --- a/packages/junior-dashboard/src/client/conversations/conversationOutbox.ts +++ b/packages/junior-dashboard/src/client/conversations/conversationOutbox.ts @@ -1,8 +1,10 @@ +import type { QueryClient } from "@tanstack/react-query"; import type { ActorIdentity, ConversationPendingMessage, InputImage, } from "@sentry/junior/api/schema"; +import type { DashboardCoreData } from "../types"; /** Client-owned mailbox row waiting on accept, server visibility, or retry. */ export type ConversationOutboxMessage = { @@ -27,6 +29,22 @@ export type ConversationMailboxMessage = ConversationPendingMessage & { idempotencyKey?: string; }; +/** 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; @@ -163,6 +181,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/eventTranscript.ts b/packages/junior-dashboard/src/client/conversations/eventTranscript.ts index 42eb9e2688..89e4af7a46 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.actorIdentityId + ? { actorIdentityId: message.actorIdentityId } + : undefined), ...(message.mentions ? { mentions: message.mentions } : undefined), }; } @@ -222,6 +225,9 @@ export function transcriptMessagesFromEvents( ...(data.actorIdentity ? { actorIdentity: data.actorIdentity } : undefined), + ...(data.actorIdentityId + ? { actorIdentityId: data.actorIdentityId } + : 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 6cd2409cb9..3658adb2c0 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, @@ -45,6 +42,7 @@ import { failConversationOutboxMessage, isInlineOutboxMessage, mergeConversationMailboxMessages, + outboxActorIdentity, acceptConversationOutboxMessage, reuseConversationMailboxMessages, splitInlineMailboxMessages, @@ -213,20 +211,6 @@ function keepLatestReadPosition( return { ...next, readPosition: kept }; } -function outboxActorIdentity( - queryClient: QueryClient, -): ActorIdentity | undefined { - const user = queryClient.getQueryData([ - "dashboard", - "core", - ])?.me.user; - if (!user?.email) return undefined; - return { - email: user.email, - fullName: user.name || undefined, - }; -} - /** Create one dashboard conversation and refresh the personal feed. */ export function useCreateConversation() { const queryClient = useQueryClient(); 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/src/client/format.ts b/packages/junior-dashboard/src/client/format.ts index 8d5f6e9213..ff8301ca86 100644 --- a/packages/junior-dashboard/src/client/format.ts +++ b/packages/junior-dashboard/src/client/format.ts @@ -354,6 +354,10 @@ export function transcriptMessageActor( ): ActorIdentity | undefined { if (transcriptRoleKind(message.role) !== "user") return undefined; if (message.agentInvocationId) return undefined; + 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; return undefined; @@ -370,7 +374,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/src/client/types.ts b/packages/junior-dashboard/src/client/types.ts index ce5f414ffa..1943cad9e8 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 `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-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/format.test.ts b/packages/junior-dashboard/tests/format.test.ts index 9997154229..c01700188a 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,52 @@ describe("dashboard conversation formatting", () => { expect(formatConversationDuration(conversation!)).toBe("none"); }); + it("reads each message author from the conversation authors", () => { + const owner = { + avatarUrl: "/api/people/avatars/identity-web?v=1", + email: "owner@example.com", + fullName: "Owner Person", + }; + const conversation = transcript( + [ + event(0, { + type: "message", + messageId: "web", + role: "user", + text: "ship it", + actorIdentityId: "identity-web", + actorIdentity: { fullName: "Saved Name" }, + }), + ], + { + actorIdentity: { fullName: "Alice" }, + authors: { "identity-slack": owner, "identity-web": owner }, + }, + ); + + expect(summarizeMessages(conversation).items).toEqual([ + { author: "Owner Person", bytes: 7 }, + ]); + expect( + transcriptMessageActor(conversation, { + actorIdentityId: "identity-slack", + parts: [], + role: "user", + sourceSeq: 1, + }), + ).toBe(owner); + // An author who is not in the Conversation yet keeps the saved identity. + expect( + transcriptMessageActor(conversation, { + actorIdentity: { fullName: "New Person" }, + actorIdentityId: "identity-new", + parts: [], + role: "user", + sourceSeq: 2, + }), + ).toEqual({ fullName: "New Person" }); + }); + it("summarizes visible canonical messages and structural tools", () => { const conversation = transcript( [ @@ -229,7 +276,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-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( ; participants: NonNullable; + authors: NonNullable; previousSeq?: number; sourceTask?: ConversationDetailReport["sourceTask"]; teamDomainByTeamId?: ReadonlyMap; @@ -89,7 +90,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 ? { @@ -135,7 +136,7 @@ async function readConversationDetailFromSql( firstHumanMessageTextByConversation, modelUsage, metricsByConversation, - participantsByConversation, + people, sourceTask, teamDomainByTeamId, ] = await Promise.all([ @@ -149,7 +150,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), @@ -215,7 +216,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 28bbe9b718..13aac21bdb 100644 --- a/packages/junior/src/api/conversations/event-page.ts +++ b/packages/junior/src/api/conversations/event-page.ts @@ -9,10 +9,8 @@ import { lt, sql, } from "drizzle-orm"; -import { - decodeStoredConversationEvent, - type ConversationEvent, -} from "@/chat/conversations/history"; +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"; @@ -90,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, @@ -116,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. @@ -186,9 +168,9 @@ async function projectConversationEventRows( return projectConversationReportEventPage({ canExposePayload: args.canExposePayload, events, - subagentStartEvents: subagentStartRows.map(decodeConversationEventRow), - toolStartEvents: toolStartRows.map(decodeConversationEventRow), - modelContextEvents: modelContextRows.map(decodeConversationEventRow), + subagentStartEvents: subagentStartRows.map(eventFromRow), + toolStartEvents: toolStartRows.map(eventFromRow), + modelContextEvents: modelContextRows.map(eventFromRow), }); } 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 342297d860..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,42 +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< - string, - { identities: ActorIdentity[]; keys: Set } - >(); - 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) ?? { - identities: [], - keys: new Set(), - }; - const key = row.userId - ? `user:${row.userId}` - : `identity:${row.identityId}`; - if (!group.keys.has(key)) { - group.keys.add(key); - group.identities.push(participant); - } - 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.identities, + [...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 a403b5d34f..a890ecbd1c 100644 --- a/packages/junior/src/api/conversations/pending-messages.ts +++ b/packages/junior/src/api/conversations/pending-messages.ts @@ -230,16 +230,23 @@ export async function readConversationPendingMessages( : undefined; const work = await getConversation({ conversationId }); + const pendingMessages = work?.execution.pendingMessages ?? []; const projectedMessages = await Promise.all( - (work?.execution.pendingMessages ?? []).map((message) => + pendingMessages.map((message) => projectPendingMessage(message, canExposePayload), ), ); const messages = await attachPendingMentions( conversationId, - projectedMessages.filter((message): message is ConversationPendingMessage => - Boolean(message), - ), + projectedMessages.flatMap((message, index) => { + if (!message) return []; + const actorIdentityId = pendingMessages[index]!.input.authorIdentityId; + return [ + canExposePayload && actorIdentityId + ? { ...message, actorIdentityId } + : message, + ]; + }), ); return conversationPendingMessagesReportSchema.parse({ 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/api/schema/conversation.ts b/packages/junior/src/api/schema/conversation.ts index 5a30c7b83a..0d98f64df4 100644 --- a/packages/junior/src/api/schema/conversation.ts +++ b/packages/junior/src/api/schema/conversation.ts @@ -145,6 +145,8 @@ export const conversationPendingMessageDeliverySchema = z.enum([ export const conversationPendingMessageSchema = z .object({ actorIdentity: actorIdentitySchema.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), @@ -356,6 +358,8 @@ const conversationReportMessageEventDataSchema = z role: z.enum(["assistant", "system", "user"]), source: z.enum(["slack", "web"]).optional(), actorIdentity: actorIdentitySchema.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(), @@ -855,6 +859,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/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 0e9e715951..bd2d2a42d4 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"; @@ -13,6 +14,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 +330,79 @@ describe("conversation detail API", () => { ]); }); + it("names the participant who wrote each message", 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, 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, + 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"), + }); + // Messages name their author. The detail carries each author once. + expect(detail?.events[0]?.data).toMatchObject({ + actorIdentityId: owner!.id, + }); + expect(detail?.events[0]?.data).not.toHaveProperty( + "actorIdentity.avatarUrl", + ); + 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 () => { const { createConfiguredJuniorSqlFixture } = await import("../../../fixtures/sql"); 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 bd79327e8a..287a75a98d 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,18 @@ describe("conversation pending messages API", () => { }, { conversationStore, queue, state }, ); + // The row names its author, so the client can read the author photo from + // the Conversation authors before the Message reaches history. + const [author] = await getDb() + .select({ id: juniorIdentities.id }) + .from(juniorIdentities) + .where( + and( + eq(juniorIdentities.emailNormalized, actor.email.toLowerCase()), + isNotNull(juniorIdentities.userId), + ), + ) + .limit(1); const app = createJuniorApi(); const response = await app.request( @@ -53,6 +67,7 @@ describe("conversation pending messages API", () => { email: actor.email, ...(actor.fullName ? { fullName: actor.fullName } : undefined), }, + actorIdentityId: author!.id, createdAt: expect.any(String), delivery: "defer", inboundMessageId: accepted.messageId, 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",