diff --git a/desktop/playwright.config.ts b/desktop/playwright.config.ts index 7f7ccce4f..4df72c2e8 100644 --- a/desktop/playwright.config.ts +++ b/desktop/playwright.config.ts @@ -45,6 +45,7 @@ export default defineConfig({ "**/identity-archive.spec.ts", "**/identity-archive-hide.spec.ts", "**/relay-connectivity-screenshots.spec.ts", + "**/reply-surfacing-screenshots.spec.ts", "**/unread-pill-screenshots.spec.ts", "**/sidebar-more-unread-overlap.spec.ts", "**/thread-unread-screenshots.spec.ts", diff --git a/desktop/src/features/messages/lib/mergeSurfacedReplies.d.mts b/desktop/src/features/messages/lib/mergeSurfacedReplies.d.mts new file mode 100644 index 000000000..9fd8a8a4a --- /dev/null +++ b/desktop/src/features/messages/lib/mergeSurfacedReplies.d.mts @@ -0,0 +1,21 @@ +/** + * Type declarations for the pure merge in `mergeSurfacedReplies.mjs`. Runtime + * lives in `.mjs` so the (TS-loader-less) `node:test` runner imports it + * directly; this file gives TypeScript callers a typed view. + */ +import type { MainTimelineEntry } from "@/features/messages/lib/threadPanel"; +import type { TimelineMessage } from "@/features/messages/types"; + +/** A root timeline entry, or a pointer to a surfaced nested reply. */ +export type MergedTimelineRow = + | { kind: "entry"; entry: MainTimelineEntry } + | { kind: "surfaced"; message: TimelineMessage }; + +/** + * Interleaves `surfaced` pointers into the root-level `entries`, returning a + * single sequence sorted by `(createdAt, id)` using each row's REAL message id. + */ +export function mergeSurfacedReplies( + entries: MainTimelineEntry[], + surfaced: TimelineMessage[], +): MergedTimelineRow[]; diff --git a/desktop/src/features/messages/lib/mergeSurfacedReplies.mjs b/desktop/src/features/messages/lib/mergeSurfacedReplies.mjs new file mode 100644 index 000000000..274ea727a --- /dev/null +++ b/desktop/src/features/messages/lib/mergeSurfacedReplies.mjs @@ -0,0 +1,41 @@ +/** + * Pure merge that interleaves surfaced reply pointers into the root-level + * timeline. `buildMainTimelineEntries` yields the root entries the channel + * renders, in ascending `createdAt` order; `surfaceReplies` yields the nested + * agent->human replies to surface as pointers at their OWN `createdAt` (the + * blessed "surface it where the eye reads" position). + * + * The two streams are combined into one discriminated sequence sorted by + * `(createdAt, id)` so the existing day-divider grouping loop can run over it + * unchanged. `id` is the REAL message id (a root entry's `message.id`, a + * pointer's surfaced-message `id`), giving a stable tiebreak when a pointer and + * a root share a `createdAt` — deterministic insertion, no re-render flicker. + * This mirrors the `(createdAt, id.localeCompare)` tiebreak already used for + * review comments in `TimelineMessageList`. + * + * Lives in `.mjs` (not `.ts`) so the TS-loader-less test runner imports the + * same source production uses; the sibling `.d.mts` types the discriminant for + * TypeScript callers. + */ + +/** + * @param {{ message: { id: string, createdAt: number } }[]} entries + * @param {{ id: string, createdAt: number }[]} surfaced + */ +export function mergeSurfacedReplies(entries, surfaced) { + const merged = [ + ...entries.map((entry) => ({ kind: "entry", entry })), + ...surfaced.map((message) => ({ kind: "surfaced", message })), + ]; + + const sortId = (row) => + row.kind === "entry" ? row.entry.message.id : row.message.id; + const sortCreatedAt = (row) => + row.kind === "entry" ? row.entry.message.createdAt : row.message.createdAt; + + return merged.sort((left, right) => { + const byCreatedAt = sortCreatedAt(left) - sortCreatedAt(right); + if (byCreatedAt !== 0) return byCreatedAt; + return sortId(left).localeCompare(sortId(right)); + }); +} diff --git a/desktop/src/features/messages/lib/mergeSurfacedReplies.test.mjs b/desktop/src/features/messages/lib/mergeSurfacedReplies.test.mjs new file mode 100644 index 000000000..2c1e9d783 --- /dev/null +++ b/desktop/src/features/messages/lib/mergeSurfacedReplies.test.mjs @@ -0,0 +1,76 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +// Imports the exact source the renderer wires. No inlined copy -> no drift +// between test expectations and production behaviour. +import { mergeSurfacedReplies } from "./mergeSurfacedReplies.mjs"; + +// Minimal shapes: the merge only reads `message.id` and `message.createdAt`. +const entry = (id, createdAt) => ({ + message: { id, createdAt }, + summary: null, +}); +const surfaced = (id, createdAt) => ({ id, createdAt }); + +// Reduce rows to a comparable trace of (kind, id) in result order. +const trace = (rows) => + rows.map((row) => + row.kind === "entry" + ? `entry:${row.entry.message.id}` + : `surfaced:${row.message.id}`, + ); + +test("surfaced reply lands between root entries by its own createdAt", () => { + const rows = mergeSurfacedReplies( + [entry("a", 10), entry("c", 30)], + [surfaced("b", 20)], + ); + assert.deepEqual(trace(rows), ["entry:a", "surfaced:b", "entry:c"]); +}); + +test("equal createdAt tiebreaks on real id so insertion is deterministic", () => { + // Pointer "m" and root "z" share createdAt 20; "m" < "z" by id. + const rows = mergeSurfacedReplies( + [entry("a", 10), entry("z", 20)], + [surfaced("m", 20)], + ); + assert.deepEqual(trace(rows), ["entry:a", "surfaced:m", "entry:z"]); +}); + +test("tiebreak uses the real message id not a synthetic render key", () => { + // If the merge keyed on a "surfaced:" prefix it would sort AFTER "entry" + // rows at the same createdAt (lexically "s" > "e"); keying on the real id + // "b" places it before root "c". + const rows = mergeSurfacedReplies([entry("c", 5)], [surfaced("b", 5)]); + assert.deepEqual(trace(rows), ["surfaced:b", "entry:c"]); +}); + +test("empty surfaced list returns the root entries unchanged in order", () => { + const rows = mergeSurfacedReplies( + [entry("a", 10), entry("b", 20), entry("c", 30)], + [], + ); + assert.deepEqual(trace(rows), ["entry:a", "entry:b", "entry:c"]); +}); + +test("empty entries returns only the surfaced pointers in createdAt order", () => { + const rows = mergeSurfacedReplies( + [], + [surfaced("late", 30), surfaced("early", 10)], + ); + assert.deepEqual(trace(rows), ["surfaced:early", "surfaced:late"]); +}); + +test("multiple surfaced replies interleave across multiple roots", () => { + const rows = mergeSurfacedReplies( + [entry("r1", 10), entry("r2", 40)], + [surfaced("s1", 20), surfaced("s2", 30), surfaced("s3", 50)], + ); + assert.deepEqual(trace(rows), [ + "entry:r1", + "surfaced:s1", + "surfaced:s2", + "entry:r2", + "surfaced:s3", + ]); +}); diff --git a/desktop/src/features/messages/lib/surfaceReplies.d.mts b/desktop/src/features/messages/lib/surfaceReplies.d.mts new file mode 100644 index 000000000..d5c9a0f47 --- /dev/null +++ b/desktop/src/features/messages/lib/surfaceReplies.d.mts @@ -0,0 +1,19 @@ +/** + * Type declarations for the pure projection in `surfaceReplies.mjs`. Runtime + * lives in `.mjs` so the (TS-loader-less) `node:test` runner imports it + * directly; this file gives TypeScript callers a typed view. + */ +import type { TimelineMessage } from "@/features/messages/types"; + +/** + * Returns the nested messages to surface as root-level pointers: agent-authored + * messages that carry a human p-tag and are not already duplicated at root. + * + * @param messages - the built timeline messages (mixed root + nested). + * @param isHuman - classifier for a pubkey; the caller resolves unknown + * pubkeys to `true` (human) so unrecognized authors under-surface. + */ +export function surfaceReplies( + messages: TimelineMessage[], + isHuman: (pubkey: string | undefined) => boolean, +): TimelineMessage[]; diff --git a/desktop/src/features/messages/lib/surfaceReplies.mjs b/desktop/src/features/messages/lib/surfaceReplies.mjs new file mode 100644 index 000000000..4d1ea30ab --- /dev/null +++ b/desktop/src/features/messages/lib/surfaceReplies.mjs @@ -0,0 +1,75 @@ +/** + * Pure projection that pulls agent-authored replies addressed to a human up to + * the thread root. The channel timeline only renders root-level entries + * (`buildMainTimelineEntries` keeps `parentId == null || broadcast`), so an + * agent's reply to a human posted *nested* is invisible where the human reads. + * This function returns the nested messages that should be surfaced as + * lightweight root-level pointers; the real message is never moved or copied — + * the caller (Phase 2) renders a pointer row that links down to it. + * + * Lives in `.mjs` (not `.ts`) so the test runner (`node --test`, no TS loader) + * imports the same source production uses; TypeScript callers get types from + * the sibling `.d.mts`. + */ + +/** + * A message is root-level — and therefore needs no surfacing — under the exact + * condition the timeline uses to render it at root: no parent, or a broadcast + * reply. Inlined (not imported from `threading.ts`) because that module is + * `.ts` and cannot be loaded by the TS-loader-less test runner; the rule is one + * line and must stay identical to `buildMainTimelineEntries`'s filter. + */ +function isRootLevel(message) { + return ( + message.parentId == null || + (message.tags ?? []).some((t) => t[0] === "broadcast" && t[1] === "1") + ); +} + +/** + * Returns the subset of `messages` to surface as root-level pointers. + * + * A nested message surfaces iff ALL hold: + * 1. it is not already root-level (root needs no surfacing); + * 2. its author is an agent — `isHuman(authorPubkey) === false`; + * 3. it carries at least one human p-tag — a `["p", pubkey]` where + * `isHuman(pubkey) === true`. + * + * De-dupe is STRICT and THREAD-SCOPED: a candidate is skipped only when a + * root-level message *in the same thread* has the EXACT SAME body. Author + * identity is irrelevant — an agent's earlier, unrelated root-level post must + * not suppress a genuinely new nested reply. Scoping by thread prevents a root + * "done" in thread A from suppressing a nested "done" in thread B; short common + * bodies collide constantly across a busy channel. The key is + * `${threadId}\u0000${body}` where a message's thread id is `rootId ?? id` (a + * root's own thread id is its `id`, since its `rootId` is null) and `\u0000` + * (NUL) cannot appear in an event id or normal body, so keys never collide. + * + * Empty/whitespace-only bodies carry no content that can "already exist" at + * root, so they are never seeded and never suppress — every such candidate that + * passes the trigger conditions surfaces. + * + * `isHuman` is authoritative as given. The caller resolves unknown + * classification to `true` (human), so an unrecognized author is treated human + * and fails condition (2): the message under-surfaces rather than mis-surfaces. + */ +export function surfaceReplies(messages, isHuman) { + const threadKey = (message) => + `${message.rootId ?? message.id}\u0000${message.body}`; + const rootBodies = new Set( + messages + .filter((message) => isRootLevel(message) && message.body.trim() !== "") + .map(threadKey), + ); + + return messages.filter((message) => { + if (isRootLevel(message)) return false; + if (isHuman(message.pubkey)) return false; + const tagsHuman = (message.tags ?? []).some( + (t) => t[0] === "p" && typeof t[1] === "string" && isHuman(t[1]), + ); + if (!tagsHuman) return false; + if (message.body.trim() === "") return true; + return !rootBodies.has(threadKey(message)); + }); +} diff --git a/desktop/src/features/messages/lib/surfaceReplies.test.mjs b/desktop/src/features/messages/lib/surfaceReplies.test.mjs new file mode 100644 index 000000000..1bf9d7b90 --- /dev/null +++ b/desktop/src/features/messages/lib/surfaceReplies.test.mjs @@ -0,0 +1,214 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +// Imports the exact source the renderer wires in Phase 2. No inlined copy → no +// drift between test expectations and production behaviour. +import { surfaceReplies } from "./surfaceReplies.mjs"; + +const HUMAN = "human-pubkey"; +const HUMAN_2 = "human-pubkey-2"; +const AGENT = "agent-pubkey"; +const AGENT_2 = "agent-pubkey-2"; + +// Caller contract: unknown pubkeys resolve to human (true). Only the known +// agent pubkeys are non-human; everything else (including undefined) is human. +const isHuman = (pubkey) => pubkey !== AGENT && pubkey !== AGENT_2; + +// `rootId` mirrors production: a root-level message has `rootId == null` +// (its own thread id is its `id`); a nested reply carries the thread root's id. +const message = ({ + id, + pubkey, + parentId = null, + rootId = null, + body = "", + tags = [], +}) => ({ + id, + pubkey, + author: pubkey, + parentId, + rootId, + body, + createdAt: 0, + time: "", + depth: parentId == null ? 0 : 1, + tags, +}); + +const p = (pubkey) => ["p", pubkey]; +const ids = (msgs) => msgs.map((m) => m.id); + +test("agent-authored nested message tagging a human surfaces", () => { + const nested = message({ + id: "n1", + pubkey: AGENT, + parentId: "root1", + body: "here is your answer", + tags: [p(HUMAN)], + }); + const out = surfaceReplies( + [message({ id: "root1", pubkey: HUMAN }), nested], + isHuman, + ); + assert.deepEqual(ids(out), ["n1"]); +}); + +test("agent-to-agent nested message (no human p-tag) does not surface", () => { + const nested = message({ + id: "n1", + pubkey: AGENT, + parentId: "root1", + body: "investigating together", + tags: [p(AGENT_2)], + }); + const out = surfaceReplies( + [message({ id: "root1", pubkey: HUMAN }), nested], + isHuman, + ); + assert.deepEqual(out, []); +}); + +test("human-authored nested message tagging a human does not surface", () => { + const nested = message({ + id: "n1", + pubkey: HUMAN, + parentId: "root1", + body: "what do you think", + tags: [p(HUMAN_2)], + }); + const out = surfaceReplies( + [message({ id: "root1", pubkey: AGENT }), nested], + isHuman, + ); + assert.deepEqual(out, []); +}); + +test("nested agent reply whose exact body already exists at root in the same thread is de-duped", () => { + const body = "the deploy is green"; + const out = surfaceReplies( + [ + message({ id: "root1", pubkey: AGENT, body }), + message({ + id: "n1", + pubkey: AGENT, + parentId: "root1", + rootId: "root1", + body, + tags: [p(HUMAN)], + }), + ], + isHuman, + ); + assert.deepEqual(out, []); +}); + +test("near-but-not-identical body still surfaces (STRICT, not loose, de-dupe)", () => { + // Under LOOSE de-dupe (same author already at root) this would be wrongly + // skipped: the agent has an unrelated root post in this thread. STRICT + // compares body, so a genuinely new nested reply to the human still surfaces. + const out = surfaceReplies( + [ + message({ id: "root1", pubkey: AGENT, body: "the deploy is green" }), + message({ + id: "n1", + pubkey: AGENT, + parentId: "root1", + rootId: "root1", + body: "the deploy is green now", + tags: [p(HUMAN)], + }), + ], + isHuman, + ); + assert.deepEqual(ids(out), ["n1"]); +}); + +test("unknown author treated as human under-surfaces (fail-safe)", () => { + // isHuman returns true for the unknown pubkey → fails the agent-author gate. + const nested = message({ + id: "n1", + pubkey: "unknown-pubkey", + parentId: "root1", + body: "ambiguous author", + tags: [p(HUMAN)], + }); + const out = surfaceReplies( + [message({ id: "root1", pubkey: HUMAN }), nested], + isHuman, + ); + assert.deepEqual(out, []); +}); + +test("identical body in a different thread surfaces; same-thread root collision de-dupes", () => { + // Thread A has a root "done"; thread B has a nested agent→human "done". + // Thread-scoped de-dupe must NOT let thread A's root suppress thread B's + // reply — but a matching root in the SAME thread still de-dupes. + const out = surfaceReplies( + [ + message({ id: "A", pubkey: HUMAN, body: "done" }), + message({ + id: "nB", + pubkey: AGENT, + parentId: "B", + rootId: "B", + body: "done", + tags: [p(HUMAN)], + }), + message({ id: "C", pubkey: AGENT, body: "done" }), + message({ + id: "nC", + pubkey: AGENT, + parentId: "C", + rootId: "C", + body: "done", + tags: [p(HUMAN)], + }), + ], + isHuman, + ); + // nB surfaces (no same-thread root "done"); nC de-dupes (root C is "done"). + assert.deepEqual(ids(out), ["nB"]); +}); + +test("broadcast-tagged nested message is treated as root and does not surface", () => { + // Pins the broadcast branch of the inlined isRootLevel: a nested message + // (parentId != null) carrying ["broadcast","1"] is root-level, so it is + // excluded from surfacing. Guards against silent drift from + // buildMainTimelineEntries's canonical filter. + const out = surfaceReplies( + [ + message({ id: "root1", pubkey: HUMAN }), + message({ + id: "n1", + pubkey: AGENT, + parentId: "root1", + rootId: "root1", + body: "broadcast reply to a human", + tags: [["broadcast", "1"], p(HUMAN)], + }), + ], + isHuman, + ); + assert.deepEqual(out, []); +}); + +test("empty-bodied nested agent reply surfaces despite an empty-bodied root", () => { + // An empty body carries no content to "already exist" at root, so it neither + // seeds the de-dupe set nor is suppressed by it. + const out = surfaceReplies( + [ + message({ id: "root1", pubkey: HUMAN, body: "" }), + message({ + id: "n1", + pubkey: AGENT, + parentId: "root1", + rootId: "root1", + body: "", + tags: [p(HUMAN)], + }), + ], + isHuman, + ); + assert.deepEqual(ids(out), ["n1"]); +}); diff --git a/desktop/src/features/messages/ui/SurfacedReplyRow.tsx b/desktop/src/features/messages/ui/SurfacedReplyRow.tsx new file mode 100644 index 000000000..0fbee3354 --- /dev/null +++ b/desktop/src/features/messages/ui/SurfacedReplyRow.tsx @@ -0,0 +1,48 @@ +import type { TimelineMessage } from "@/features/messages/types"; +import { UserAvatar } from "@/shared/ui/UserAvatar"; + +/** + * A read-time pointer that surfaces an agent's nested reply to a human up to + * the root timeline, at the reply's own `createdAt`. The real message is never + * moved or copied; clicking navigates DOWN to it in its thread (open the thread + * at its `rootId`, scroll to and highlight the nested message). + * + * Reuses the visual idiom of `MessageThreadSummaryRow` (rounded pill, avatar, + * muted label) but is a distinct component: a thread summary means + * reply-count/participants and opens a panel rooted at itself, whereas this + * points at one specific reply and navigates down to it. + */ +export function SurfacedReplyRow({ + message, + onNavigate, +}: { + message: TimelineMessage; + onNavigate: (message: TimelineMessage) => void; +}) { + return ( +
+ +
+ ); +} diff --git a/desktop/src/features/messages/ui/TimelineMessageList.tsx b/desktop/src/features/messages/ui/TimelineMessageList.tsx index 8fcee1c8c..6a37170b1 100644 --- a/desktop/src/features/messages/ui/TimelineMessageList.tsx +++ b/desktop/src/features/messages/ui/TimelineMessageList.tsx @@ -1,10 +1,13 @@ import * as React from "react"; +import { useAppNavigation } from "@/app/navigation/useAppNavigation"; import { formatDayHeading, isSameDay, startOfLocalDaySeconds, } from "@/features/messages/lib/dateFormatters"; +import { mergeSurfacedReplies } from "@/features/messages/lib/mergeSurfacedReplies.mjs"; +import { surfaceReplies } from "@/features/messages/lib/surfaceReplies.mjs"; import { buildMainTimelineEntries, shouldRenderUnreadDivider, @@ -22,6 +25,7 @@ import { cn } from "@/shared/lib/cn"; import { DayDivider } from "./DayDivider"; import { MessageRow } from "./MessageRow"; import { MessageThreadSummaryRow } from "./MessageThreadSummaryRow"; +import { SurfacedReplyRow } from "./SurfacedReplyRow"; import { SystemMessageRow } from "./SystemMessageRow"; import { UnreadDivider } from "./UnreadDivider"; @@ -87,12 +91,22 @@ type TimelineMessageRowModel = { type: "message"; }; +type TimelineSurfacedRow = { + key: string; + message: TimelineMessage; + type: "surfaced"; +}; + type TimelineRenderRow = | TimelineDayRow | TimelineUnreadRow - | TimelineMessageRowModel; + | TimelineMessageRowModel + | TimelineSurfacedRow; -type TimelineNonDayRow = TimelineUnreadRow | TimelineMessageRowModel; +type TimelineNonDayRow = + | TimelineUnreadRow + | TimelineMessageRowModel + | TimelineSurfacedRow; type TimelineDayGroup = { key: string; @@ -101,18 +115,31 @@ type TimelineDayGroup = { }; function buildTimelineRenderRows({ + agentPubkeys, firstUnreadMessageId, messages, }: { + agentPubkeys?: ReadonlySet; firstUnreadMessageId: string | null; messages: TimelineMessage[]; }): TimelineRenderRow[] { - const entries = buildMainTimelineEntries(messages); + // Merge nested agent->human replies into the root timeline as pointers at + // their own createdAt. `isHuman` resolves unknown/undefined pubkeys to human + // so unrecognized authors under-surface (the fail-safe matching the pure-core + // contract); surfaceReplies is the single source of "root-level", never + // re-derived here. + const isHuman = (pubkey?: string) => + pubkey == null || !agentPubkeys?.has(pubkey); + const merged = mergeSurfacedReplies( + buildMainTimelineEntries(messages), + surfaceReplies(messages, isHuman), + ); const rows: TimelineRenderRow[] = []; let previousMessage: TimelineMessage | null = null; - for (let i = 0; i < entries.length; i++) { - const { message, summary } = entries[i]; + for (let i = 0; i < merged.length; i++) { + const row = merged[i]; + const message = row.kind === "entry" ? row.entry.message : row.message; const messageRenderKey = message.renderKey ?? message.id; if ( @@ -125,6 +152,19 @@ function buildTimelineRenderRows({ type: "day", }); } + previousMessage = message; + + // Surfaced pointer rows are never a read/unread boundary — their id is a + // nested reply id that can never equal `firstUnreadMessageId` (a top-level + // id) — so they skip the divider check entirely and render as pointers. + if (row.kind === "surfaced") { + rows.push({ + key: `surfaced:${message.id}`, + message, + type: "surfaced", + }); + continue; + } // The unread "New" divider only marks a read/unread boundary when there is // a message above the first unread. When the first unread is the first @@ -137,11 +177,9 @@ function buildTimelineRenderRows({ rows.push({ key: `msg:${messageRenderKey}`, message, - summary, + summary: row.entry.summary, type: "message", }); - - previousMessage = message; } return rows; @@ -203,9 +241,11 @@ export const TimelineMessageList = React.memo(function TimelineMessageList({ threadUnreadCounts, unfollowThreadById, }: TimelineMessageListProps) { + const { goChannel } = useAppNavigation(); const rows = React.useMemo( - () => buildTimelineRenderRows({ firstUnreadMessageId, messages }), - [firstUnreadMessageId, messages], + () => + buildTimelineRenderRows({ agentPubkeys, firstUnreadMessageId, messages }), + [agentPubkeys, firstUnreadMessageId, messages], ); const dayGroups = React.useMemo(() => buildTimelineDayGroups(rows), [rows]); @@ -232,6 +272,7 @@ export const TimelineMessageList = React.memo(function TimelineMessageList({ channelType={channelType} currentPubkey={currentPubkey} followThreadById={followThreadById} + goChannel={goChannel} highlightedMessageId={highlightedMessageId} isFollowingThreadById={isFollowingThreadById} isSendingVideoReviewComment={isSendingVideoReviewComment} @@ -263,6 +304,7 @@ type TimelineRenderRowViewProps = Omit< "firstUnreadMessageId" | "messages" | "personaLookup" > & { allMessages?: TimelineMessage[]; + goChannel: ReturnType["goChannel"]; row: TimelineRenderRow; }; @@ -274,6 +316,7 @@ const TimelineRenderRowView = React.memo(function TimelineRenderRowView({ channelType, currentPubkey, followThreadById, + goChannel, highlightedMessageId = null, isFollowingThreadById, isSendingVideoReviewComment = false, @@ -332,6 +375,27 @@ const TimelineRenderRowView = React.memo(function TimelineRenderRowView({ return ; } + if (row.type === "surfaced") { + return ( +
+ { + if (!channelId) return; + // threadRootId prefetches the thread root event in + // ChannelRouteScreen so the route-target open can find the head + // even when the root is outside the loaded window — the same + // mechanism search hits use to navigate down to a nested reply. + void goChannel(channelId, { + messageId: target.id, + threadRootId: target.rootId, + }); + }} + /> +
+ ); + } + const { message, summary } = row; if (message.kind === KIND_SYSTEM_MESSAGE) { diff --git a/desktop/tests/e2e/relay-connectivity-screenshots.spec.ts b/desktop/tests/e2e/relay-connectivity-screenshots.spec.ts index 5a057d8a2..09562bfd6 100644 --- a/desktop/tests/e2e/relay-connectivity-screenshots.spec.ts +++ b/desktop/tests/e2e/relay-connectivity-screenshots.spec.ts @@ -14,14 +14,19 @@ const MOCK_PUBKEY = "deadbeef".repeat(8); const MOCK_RELAY_URL = "ws://localhost:3000"; const SELF_PROFILE_CACHE_KEY = `buzz-self-profile.v1:${MOCK_RELAY_URL}:${MOCK_PUBKEY}`; +// Wait for mount animations to settle before screenshotting. A cancelled or +// replaced animation's `finished` promise can hang or reject, tearing down the +// evaluate context mid-await (AbortError). Cap each wait and swallow rejections +// so settle() always resolves quickly without a flaky race. async function settle(page: import("@playwright/test").Page) { - await page.evaluate(() => - // Tolerate cancelled animations: a SkeletonReveal animation cancelled - // mid-flight (skeleton → live content swap) rejects `.finished` with an - // AbortError. allSettled lets the animations that DO finish settle instead - // of aborting the whole wait on the first cancel. - Promise.allSettled(document.getAnimations().map((a) => a.finished)), - ); + await page.evaluate(async () => { + const guard = (p: Promise) => + Promise.race([ + p.catch(() => {}), + new Promise((resolve) => setTimeout(resolve, 1000)), + ]); + await Promise.all(document.getAnimations().map((a) => guard(a.finished))); + }); } type ConnectionState = diff --git a/desktop/tests/e2e/reply-surfacing-screenshots.spec.ts b/desktop/tests/e2e/reply-surfacing-screenshots.spec.ts new file mode 100644 index 000000000..3048dbcec --- /dev/null +++ b/desktop/tests/e2e/reply-surfacing-screenshots.spec.ts @@ -0,0 +1,219 @@ +import { expect, type Page, test } from "@playwright/test"; + +import { installMockBridge, TEST_IDENTITIES } from "../helpers/bridge"; + +const SHOTS = "test-results/reply-surfacing"; + +// The mock identity (self) — a human, not in mockAgentPubkeys. +const SELF_PUBKEY = "deadbeef".repeat(8); + +async function waitForMockLiveSubscription(page: Page, channelName: string) { + await expect + .poll(async () => { + return page.evaluate((channelName) => { + return ( + ( + window as Window & { + __BUZZ_E2E_HAS_MOCK_LIVE_SUBSCRIPTION__?: (input: { + channelName: string; + }) => boolean; + } + ).__BUZZ_E2E_HAS_MOCK_LIVE_SUBSCRIPTION__?.({ channelName }) ?? false + ); + }, channelName); + }) + .toBe(true); +} + +type EmitInput = { + channelName: string; + content: string; + parentEventId?: string | null; + pubkey?: string; + mentionPubkeys?: string[]; +}; + +function emitMockMessage(page: Page, input: EmitInput) { + return page.evaluate((input) => { + const emit = ( + window as Window & { + __BUZZ_E2E_EMIT_MOCK_MESSAGE__?: (input: { + channelName: string; + content: string; + parentEventId?: string | null; + pubkey?: string; + mentionPubkeys?: string[]; + }) => { id: string; created_at: number }; + } + ).__BUZZ_E2E_EMIT_MOCK_MESSAGE__; + if (!emit) { + throw new Error("Mock message emitter is unavailable."); + } + return emit(input); + }, input); +} + +test.describe("reply-surfacing screenshots", () => { + test.use({ viewport: { width: 1280, height: 720 } }); + + test("01 — surfaced reply appears in timeline", async ({ page }) => { + await installMockBridge(page); + await page.goto("/"); + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + await waitForMockLiveSubscription(page, "general"); + + // Emit a root message from a human (bob). + const root = await emitMockMessage(page, { + channelName: "general", + content: "Can someone review the latest deploy?", + pubkey: TEST_IDENTITIES.bob.pubkey, + }); + + // Emit a nested reply from an agent (alice) mentioning the self (human). + await emitMockMessage(page, { + channelName: "general", + content: "I reviewed the deploy — looks good, shipping now.", + parentEventId: root.id, + pubkey: TEST_IDENTITIES.alice.pubkey, + mentionPubkeys: [SELF_PUBKEY], + }); + + // Assert the surfaced-reply-row appears. + const surfacedRow = page.getByTestId("surfaced-reply-row"); + await expect(surfacedRow).toBeVisible({ timeout: 5_000 }); + + // Screenshot the timeline area. + const timeline = page.getByTestId("message-timeline"); + await timeline.screenshot({ + path: `${SHOTS}/01-surfaced-reply-in-timeline.png`, + }); + }); + + test("02 — multiple surfaced replies interleave chronologically", async ({ + page, + }) => { + await installMockBridge(page); + await page.goto("/"); + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + await waitForMockLiveSubscription(page, "general"); + + // Emit two root messages from a human. + const root1 = await emitMockMessage(page, { + channelName: "general", + content: "Thread one: design feedback needed", + pubkey: TEST_IDENTITIES.bob.pubkey, + }); + + const root2 = await emitMockMessage(page, { + channelName: "general", + content: "Thread two: backend migration plan", + pubkey: TEST_IDENTITIES.bob.pubkey, + }); + + // Agent replies to both threads, mentioning the human. + await emitMockMessage(page, { + channelName: "general", + content: "Design feedback: the spacing looks off on mobile.", + parentEventId: root1.id, + pubkey: TEST_IDENTITIES.alice.pubkey, + mentionPubkeys: [SELF_PUBKEY], + }); + + await emitMockMessage(page, { + channelName: "general", + content: "Migration plan reviewed — ready to proceed.", + parentEventId: root2.id, + pubkey: TEST_IDENTITIES.alice.pubkey, + mentionPubkeys: [SELF_PUBKEY], + }); + + // Assert multiple surfaced-reply-row elements appear. + const surfacedRows = page.getByTestId("surfaced-reply-row"); + await expect(surfacedRows).toHaveCount(2, { timeout: 5_000 }); + + // Screenshot the timeline. + const timeline = page.getByTestId("message-timeline"); + await timeline.screenshot({ + path: `${SHOTS}/02-multiple-surfaced-replies.png`, + }); + }); + + test("03 — agent-to-agent reply does NOT surface", async ({ page }) => { + await installMockBridge(page); + await page.goto("/"); + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + await waitForMockLiveSubscription(page, "general"); + + // Emit a root message from a human. + const root = await emitMockMessage(page, { + channelName: "general", + content: "Kick off the pipeline when ready", + pubkey: TEST_IDENTITIES.bob.pubkey, + }); + + // Agent replies mentioning ANOTHER agent (charlie) — no human p-tag. + await emitMockMessage(page, { + channelName: "general", + content: "Delegating pipeline kick-off to charlie.", + parentEventId: root.id, + pubkey: TEST_IDENTITIES.alice.pubkey, + mentionPubkeys: [TEST_IDENTITIES.charlie.pubkey], + }); + + // Wait a beat to ensure no surfaced row appears. + await page.waitForTimeout(1_000); + + // Assert NO surfaced-reply-row appears. + const surfacedRows = page.getByTestId("surfaced-reply-row"); + await expect(surfacedRows).toHaveCount(0); + + // Screenshot the timeline showing no pointer row. + const timeline = page.getByTestId("message-timeline"); + await timeline.screenshot({ + path: `${SHOTS}/03-agent-to-agent-no-surface.png`, + }); + }); + + test("04 — click navigates to nested message", async ({ page }) => { + await installMockBridge(page); + await page.goto("/"); + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + await waitForMockLiveSubscription(page, "general"); + + // Emit a root message from a human. + const root = await emitMockMessage(page, { + channelName: "general", + content: "Status update on the release?", + pubkey: TEST_IDENTITIES.bob.pubkey, + }); + + // Agent replies nested, mentioning self. + await emitMockMessage(page, { + channelName: "general", + content: "Release is on track — all checks green.", + parentEventId: root.id, + pubkey: TEST_IDENTITIES.alice.pubkey, + mentionPubkeys: [SELF_PUBKEY], + }); + + // Wait for the surfaced row. + const surfacedRow = page.getByTestId("surfaced-reply-row"); + await expect(surfacedRow).toBeVisible({ timeout: 5_000 }); + + // Click the surfaced reply row. + await surfacedRow.click(); + + // Assert the thread panel opens. + const threadPanel = page.getByTestId("message-thread-panel"); + await expect(threadPanel).toBeVisible({ timeout: 5_000 }); + + // Screenshot the opened thread panel. + await threadPanel.screenshot({ + path: `${SHOTS}/04-click-navigates-to-thread.png`, + }); + }); +});