fix(surface): a deleted message must not strand a channel's unread state

surface/server/src/event-types.ts

@@ -99,3 +99,34 @@ export const SYNC_CATCHUP_RAW_EVENT_TYPES = [
   ...CATCHUP_RAW_EVENT_TYPES,
   ...SYNC_EVENT_TYPES.filter((type) => !TIMELINE_EVENT_TYPE_SET.has(type)),
 ];
+
+// === the unread rule ===
+
+// Events that make a channel unread. A thread reply counts only when it is
+// broadcast: the agent's answer is an ordinary channel message and marks the
+// channel unread like one, or the very thing you asked for lands below the fold
+// with nothing to say it arrived.
+const UNREAD_EVENT_TYPES = ['message.posted', 'session.spawned', 'session.replied'] as const;
+
+/**
+ * Per-channel `latest_event_id` — the newest event that should draw attention.
+ * Correlates against `c.id`, so it belongs inside a LATERAL over `channels c`.
+ *
+ * ONE definition on purpose. This rule lived as two copies (the channel list and
+ * the push badge) and they silently drifted: the badge never grew the broadcast
+ * clause, so it counted thread replies nobody could see in the feed.
+ *
+ * Deleted messages are excluded. The content is gone, so it must not light a
+ * channel — and a deleted message with no replies left renders no row at all,
+ * which made this counter unreachable: it named an id the client could never
+ * scroll to, so mark-read could never catch up and the channel stayed unread
+ * forever. Whatever id this returns, a client must be able to see and read it.
+ */
+export const CHANNEL_LATEST_EVENT_ID_SQL = `
+       SELECT MAX(e.id) AS latest_event_id
+       FROM events e
+       LEFT JOIN message_state ms ON ms.event_id = e.id
+       WHERE e.channel_id = c.id
+         AND e.type IN ${sqlTypeList(UNREAD_EVENT_TYPES)}
+         AND (e.thread_root_event_id IS NULL OR (e.payload->>'broadcast')::boolean IS TRUE)
+         AND NOT COALESCE(ms.is_deleted, false)`;

surface/server/src/events/read.ts

@@ -1,6 +1,7 @@
 import type { Db, DbClient } from '../db.js';
 import {
   CATCHUP_RAW_EVENT_TYPES as CATCHUP_RAW_EVENT_TYPE_VALUES,
+  CHANNEL_LATEST_EVENT_ID_SQL,
   sqlTypeList,
   SYNC_CATCHUP_RAW_EVENT_TYPES as SYNC_CATCHUP_RAW_EVENT_TYPE_VALUES,
   SYNC_EVENT_TYPES as SYNC_EVENT_TYPE_VALUES,
@@ -476,14 +477,7 @@ export async function listChannelsFor(pool: Db | DbClient, userId: string): Prom
        WHERE m.channel_id = c.id
      ) member_counts ON c.kind IN ('private', 'gdm')
      LEFT JOIN LATERAL (
-       SELECT MAX(e.id) AS latest_event_id
-       FROM events e
-       WHERE e.channel_id = c.id
-         -- The agent's answer is an ordinary channel message, so it marks the
-         -- channel unread like one. Leaving it out would land the very thing
-         -- you asked for below the fold with nothing to say it had arrived.
-         AND e.type IN ('message.posted', 'session.spawned', 'session.replied')
-         AND (e.thread_root_event_id IS NULL OR (e.payload->>'broadcast')::boolean IS TRUE)
+       ${CHANNEL_LATEST_EVENT_ID_SQL}
      ) latest ON true
      -- === mentions-activity additions ===
      LEFT JOIN LATERAL (

surface/server/src/push.ts

@@ -6,6 +6,7 @@ import type { Db } from './db.js';
 import type { WsHub } from './hub.js';
 import type { WireEvent } from './events.js';
 import { config } from './config.js';
+import { CHANNEL_LATEST_EVENT_ID_SQL } from './event-types.js';
 import { memberUserIdsForChannel, resolveDirectMentionUserIds } from './mentions.js';
 import {
   getWebPushSender,
@@ -349,12 +350,7 @@ async function unreadChannelCountFor(pool: Db, userId: string): Promise<number>
      LEFT JOIN channel_mutes mute
        ON mute.channel_id = c.id AND mute.user_id = $1
      LEFT JOIN LATERAL (
-       SELECT MAX(e.id) AS latest_event_id
-       FROM events e
-       WHERE e.channel_id = c.id
-         -- Matches the unread rule in events.ts: an agent's answer is a real
-         -- channel message and counts toward the badge like one.
-         AND e.type IN ('message.posted', 'session.spawned', 'session.replied')
+       ${CHANNEL_LATEST_EVENT_ID_SQL}
      ) latest ON true
      WHERE mute.user_id IS NULL
        AND COALESCE(latest.latest_event_id, 0) > COALESCE(rc.last_read_event_id, 0)

surface/server/test/readCursors.test.ts

@@ -1,3 +1,4 @@
+import { randomUUID } from 'node:crypto';
 import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from 'vitest';
 import type pg from 'pg';
 import { buildApp } from '../src/app.js';
@@ -198,6 +199,39 @@ describe('read cursors', () => {
     expect(channel.latestEventId).toBe(two.id);
   });
 
+  // A deleted message renders no row, so no client can scroll to it to mark it
+  // read. Counting one strands the channel unread forever — every id this
+  // counter reports has to be an id a reader can actually reach.
+  it('does not count a deleted message toward latestEventId', async () => {
+    const { cookie } = await login('alice', 'Alice');
+    const kept = await post(cookie, fx.channelId, 'kept');
+    const doomed = await post(cookie, fx.channelId, 'doomed');
+
+    const removed = await app.inject({
+      method: 'DELETE',
+      url: `/api/messages/${doomed.id}`,
+      headers: { cookie },
+      payload: { opId: randomUUID() },
+    });
+    expect(removed.statusCode).toBe(200);
+
+    const channels = await app.inject({ method: 'GET', url: '/api/channels', headers: { cookie } });
+    expect(channels.statusCode).toBe(200);
+    const channel = channels.json().channels.find((c: any) => c.id === fx.channelId);
+    expect(channel.latestEventId).toBe(kept.id);
+
+    // The whole point: reading the newest surviving message clears the channel.
+    await app.inject({
+      method: 'POST',
+      url: `/api/channels/${fx.channelId}/read`,
+      headers: { cookie },
+      payload: { lastReadEventId: kept.id },
+    });
+    const settled = await app.inject({ method: 'GET', url: '/api/channels', headers: { cookie } });
+    const after = settled.json().channels.find((c: any) => c.id === fx.channelId);
+    expect(after.latestEventId).toBeLessThanOrEqual(after.lastReadEventId);
+  });
+
   it('computes latestEventId from main-timeline-visible events only', async () => {
     const { cookie } = await login('alice', 'Alice');
     const root = await postMessage(pool, {

surface/shared/src/appState.ts

@@ -12,6 +12,7 @@ import {
   markFailed,
   mergeHistory,
   resetToLatest,
+  isRenderableMessage,
   mergeThread,
   removeByClientMsgId,
   rejectLocalOverlay,
@@ -310,7 +311,12 @@ export function newestConfirmedMainEventId(t: ChannelTimeline | undefined): numb
   if (!t) return 0;
   for (let index = t.main.length - 1; index >= 0; index--) {
     const message = t.main[index];
-    if (message?.status === 'confirmed' && typeof message.id === 'number') return message.id;
+    // Skip what never paints: callers compare this against a read cursor to ask
+    // "has that reader seen everything here?", and a deleted trailing message
+    // would answer no forever — nobody can read a row that renders nothing.
+    if (message?.status === 'confirmed' && typeof message.id === 'number' && isRenderableMessage(message)) {
+      return message.id;
+    }
   }
   return 0;
 }

surface/shared/src/timeline.ts

@@ -226,6 +226,20 @@ export function isHumanBroadcastReply(m: ChatMessage): boolean {
   return m.broadcast === true && m.sessionId == null && m.sessionEventType == null;
 }
 
+/**
+ * Does this message produce a row in the feed? A deleted message keeps its
+ * tombstone only to host replies; with none left it renders nothing at all.
+ *
+ * It still lives in `main` (the delete folds onto the posted event rather than
+ * dropping it), so anything that asks "what is the newest message?" must ask
+ * this first. A row that can never paint can never be scrolled to, so keying
+ * mark-read or the unread count off one strands the read cursor permanently —
+ * deleting the newest message in a channel used to leave it unread forever.
+ */
+export function isRenderableMessage(m: ChatMessage): boolean {
+  return !(m.deleted === true && m.replyCount === 0);
+}
+
 export interface ChannelTimeline {
   /** Root messages: confirmed sorted by id asc, then pending/failed in send order. */
   main: ChatMessage[];

surface/shared/src/util.ts

@@ -1,5 +1,5 @@
 import type { Channel } from './api';
-import type { ChatMessage, UserRef } from './timeline';
+import { isRenderableMessage, type ChatMessage, type UserRef } from './timeline';
 
 /** The person on the other side of a DM (yourself, for a self-DM). */
 export function dmPartner(c: Channel, meId: string): UserRef | null {
@@ -186,7 +186,7 @@ export function buildTimelineItems(messages: ChatMessage[]): TimelineItem[] {
   const items: TimelineItem[] = [];
   let prev: ChatMessage | null = null;
   for (const m of messages) {
-    if (m.deleted && m.replyCount === 0) continue;
+    if (!isRenderableMessage(m)) continue;
     const d = new Date(m.createdAt);
     if (!prev || !sameDay(new Date(prev.createdAt), d)) {
       items.push({ kind: 'day', key: `day-${d.toDateString()}`, label: formatDay(m.createdAt) });

surface/web/src/components/Timeline.test.tsx

@@ -85,17 +85,19 @@ function renderTimeline({
   unreadDividerAfterId = 1,
   onReachBottom,
   sessions = {},
+  loaded = true,
 }: {
   messages?: ChatMessage[];
   unreadDividerAfterId?: number | null;
   onReachBottom?: () => void;
   sessions?: Record<string, Session>;
+  loaded?: boolean;
 } = {}) {
-  const renderElement = (nextMessages: ChatMessage[]) => (
+  const renderElement = (nextMessages: ChatMessage[], nextLoaded: boolean) => (
     <ThemeProvider>
       <Timeline
         messages={nextMessages}
-        loaded
+        loaded={nextLoaded}
         hasMoreBefore={false}
         sessions={sessions}
         spectators={{}}
@@ -111,8 +113,12 @@ function renderTimeline({
       />
     </ThemeProvider>
   );
-  const view = render(renderElement(messages));
-  return { ...view, rerenderMessages: (nextMessages: ChatMessage[]) => view.rerender(renderElement(nextMessages)) };
+  const view = render(renderElement(messages, loaded));
+  return {
+    ...view,
+    rerenderMessages: (nextMessages: ChatMessage[], nextLoaded = true) =>
+      view.rerender(renderElement(nextMessages, nextLoaded)),
+  };
 }
 
 function setScrollMetrics(
@@ -404,6 +410,99 @@ describe('Timeline anchored agent answers', () => {
     expect(HTMLElement.prototype.scrollIntoView).toHaveBeenCalled();
     rect.mockRestore();
   });
+
+  // Timeline mounts before the first history page lands (that is what the
+  // skeleton is for), so the "have I seen an answer before?" baseline must not
+  // be taken from that empty first render — every reload would replay the chip
+  // for an answer that arrived days ago.
+  it('does not show the jump chip for the first history page after an empty mount', async () => {
+    const rect = vi.spyOn(HTMLElement.prototype, 'getBoundingClientRect').mockImplementation(function (
+      this: HTMLElement,
+    ) {
+      if (this.getAttribute('role') === 'log') {
+        return { top: 0, bottom: 200, left: 0, right: 300, width: 300, height: 200, x: 0, y: 0, toJSON: vi.fn() };
+      }
+      if (this.getAttribute('data-eid') === '1') {
+        return { top: -120, bottom: -80, left: 0, right: 300, width: 300, height: 40, x: 0, y: -120, toJSON: vi.fn() };
+      }
+      return { top: 20, bottom: 60, left: 0, right: 300, width: 300, height: 40, x: 0, y: 20, toJSON: vi.fn() };
+    });
+    const root = message({ id: 1, text: 'Please ship this carefully' });
+    // The real mount order: the skeleton renders with no history, then the
+    // first page lands with the answer already in it.
+    const view = renderTimeline({ messages: [], loaded: false, unreadDividerAfterId: null });
+
+    view.rerenderMessages([{ ...root, replyCount: 1, lastReplyId: 9 }, answer()], true);
+
+    await act(async () => {});
+    expect(screen.queryByTestId('agent-answer-jump-chip')).toBeNull();
+    rect.mockRestore();
+  });
+});
+
+// A deleted message with no replies renders no row at all (buildTimelineItems
+// skips it), so nothing can scroll to it or mark it read. Every set that drives
+// unread, scroll landing, and mark-read has to agree it isn't there — otherwise
+// deleting the newest message in a channel strands the read cursor forever.
+describe('Timeline deleted tail message', () => {
+  const deletedTail = () => message({ id: 3, text: '', deleted: true, replyCount: 0 });
+
+  function mockRowVisibility(visibleEid: string) {
+    return vi.spyOn(HTMLElement.prototype, 'getBoundingClientRect').mockImplementation(function (this: HTMLElement) {
+      if (this.getAttribute('role') === 'log') {
+        return { top: 0, bottom: 200, left: 0, right: 300, width: 300, height: 200, x: 0, y: 0, toJSON: vi.fn() };
+      }
+      if (this.getAttribute('data-eid') === visibleEid) {
+        return { top: 150, bottom: 190, left: 0, right: 300, width: 300, height: 40, x: 0, y: 150, toJSON: vi.fn() };
+      }
+      return { top: 420, bottom: 460, left: 0, right: 300, width: 300, height: 40, x: 0, y: 420, toJSON: vi.fn() };
+    });
+  }
+
+  it('marks read when the newest rendered row is visible behind a deleted tail', () => {
+    const onReachBottom = vi.fn();
+    const rect = mockRowVisibility('2');
+
+    renderTimeline({
+      messages: [message({ id: 1, text: 'Message 1' }), message({ id: 2, text: 'Message 2' }), deletedTail()],
+      unreadDividerAfterId: 1,
+      onReachBottom,
+    });
+    fireEvent.scroll(screen.getByRole('log', { name: 'Messages' }));
+
+    expect(onReachBottom).toHaveBeenCalled();
+    rect.mockRestore();
+  });
+
+  it('does not count a deleted tail message as unread', () => {
+    renderTimeline({
+      messages: [message({ id: 1, text: 'Message 1' }), message({ id: 2, text: 'Message 2' }), deletedTail()],
+      unreadDividerAfterId: 2,
+    });
+
+    const log = screen.getByRole('log', { name: 'Messages' });
+    setScrollMetrics(log, { scrollHeight: 1000, clientHeight: 200 });
+    log.scrollTop = 0;
+    fireEvent.scroll(log);
+
+    expect(screen.queryByTestId('jump-to-unread')).toBeNull();
+  });
+
+  it('lands at the bottom when the only unread message is a deleted one', () => {
+    const rect = mockRowVisibility('2');
+    Object.defineProperty(HTMLDivElement.prototype, 'scrollHeight', { configurable: true, value: 1000 });
+    Object.defineProperty(HTMLDivElement.prototype, 'clientHeight', { configurable: true, value: 200 });
+
+    renderTimeline({
+      messages: [message({ id: 1, text: 'Message 1' }), message({ id: 2, text: 'Message 2' }), deletedTail()],
+      unreadDividerAfterId: 2,
+    });
+
+    expect(screen.getByRole('log', { name: 'Messages' }).scrollTop).toBe(1000);
+    Reflect.deleteProperty(HTMLDivElement.prototype, 'scrollHeight');
+    Reflect.deleteProperty(HTMLDivElement.prototype, 'clientHeight');
+    rect.mockRestore();
+  });
 });
 
 describe('Timeline human broadcast replies', () => {