From ebd48883a5dc288ed76b023c95bbe357f6dc490e Mon Sep 17 00:00:00 2001
From: Tal Weiss <major.tal@gmail.com>
Date: Fri, 31 Jul 2026 13:49:01 +0200
Subject: [PATCH] fix(desktop): require explicit action to mark Inbox read

Signed-off-by: Tal Weiss <major.tal@gmail.com>
---
 desktop/playwright.config.ts                  |   1 +
 .../features/home/lib/inboxSelection.test.mjs |  38 +++-
 .../src/features/home/lib/inboxSelection.ts   |  28 +++
 desktop/src/features/home/ui/HomeView.tsx     |  56 +++--
 desktop/tests/e2e/inbox-explicit-read.spec.ts | 197 ++++++++++++++++++
 5 files changed, 301 insertions(+), 19 deletions(-)
 create mode 100644 desktop/tests/e2e/inbox-explicit-read.spec.ts

diff --git a/desktop/playwright.config.ts b/desktop/playwright.config.ts
index 2b885e3e53..ccb2642e2b 100644
--- a/desktop/playwright.config.ts
+++ b/desktop/playwright.config.ts
@@ -99,6 +99,7 @@ export default defineConfig({
         "**/reaction-order.spec.ts",
         "**/reaction-names.spec.ts",
         "**/inbox-reactions.spec.ts",
+        "**/inbox-explicit-read.spec.ts",
         "**/send-channel-binding.spec.ts",
         "**/project-commit-detail.spec.ts",
         "**/project-inbox.spec.ts",
diff --git a/desktop/src/features/home/lib/inboxSelection.test.mjs b/desktop/src/features/home/lib/inboxSelection.test.mjs
index 84088f743a..4df757bc65 100644
--- a/desktop/src/features/home/lib/inboxSelection.test.mjs
+++ b/desktop/src/features/home/lib/inboxSelection.test.mjs
@@ -1,7 +1,10 @@
 import assert from "node:assert/strict";
 import test from "node:test";
 
-import { resolveInboxFilterSelection } from "./inboxSelection.ts";
+import {
+  resolveInboxCompletionSelection,
+  resolveInboxFilterSelection,
+} from "./inboxSelection.ts";
 
 const items = [
   { conversationId: "first-conversation", id: "first-event" },
@@ -51,3 +54,36 @@ test("empty filter selection clears detail at every width", () => {
     { autoSelectedEventId: null, preserveSelection: false },
   );
 });
+
+test("wide explicit completion selects the next Inbox conversation", () => {
+  assert.equal(
+    resolveInboxCompletionSelection({
+      completedConversationId: "first-conversation",
+      isNarrow: false,
+      items,
+    }),
+    "second-event",
+  );
+});
+
+test("wide explicit completion falls back to the previous final row", () => {
+  assert.equal(
+    resolveInboxCompletionSelection({
+      completedConversationId: "second-conversation",
+      isNarrow: false,
+      items,
+    }),
+    "first-event",
+  );
+});
+
+test("narrow explicit completion returns to the Inbox list", () => {
+  assert.equal(
+    resolveInboxCompletionSelection({
+      completedConversationId: "first-conversation",
+      isNarrow: true,
+      items,
+    }),
+    null,
+  );
+});
diff --git a/desktop/src/features/home/lib/inboxSelection.ts b/desktop/src/features/home/lib/inboxSelection.ts
index c7c6217a2f..3942b32a16 100644
--- a/desktop/src/features/home/lib/inboxSelection.ts
+++ b/desktop/src/features/home/lib/inboxSelection.ts
@@ -1,5 +1,33 @@
 import type { InboxItem } from "@/features/home/lib/inbox";
 
+export function resolveInboxCompletionSelection({
+  completedConversationId,
+  isNarrow,
+  items,
+}: {
+  completedConversationId: string;
+  isNarrow: boolean;
+  items: readonly Pick<InboxItem, "conversationId" | "id">[];
+}): string | null {
+  if (isNarrow) return null;
+
+  const completedIndex = items.findIndex(
+    (item) => item.conversationId === completedConversationId,
+  );
+  if (completedIndex === -1) return items[0]?.id ?? null;
+
+  return (
+    items
+      .slice(completedIndex + 1)
+      .find((item) => item.conversationId !== completedConversationId)?.id ??
+    items
+      .slice(0, completedIndex)
+      .reverse()
+      .find((item) => item.conversationId !== completedConversationId)?.id ??
+    null
+  );
+}
+
 export function resolveInboxFilterSelection({
   isNarrow,
   items,
diff --git a/desktop/src/features/home/ui/HomeView.tsx b/desktop/src/features/home/ui/HomeView.tsx
index 0f7c851643..102b52a921 100644
--- a/desktop/src/features/home/ui/HomeView.tsx
+++ b/desktop/src/features/home/ui/HomeView.tsx
@@ -21,7 +21,10 @@ import {
   filterInboxItems,
   matchesInboxFilter,
 } from "@/features/home/lib/inboxViewHelpers";
-import { resolveInboxFilterSelection } from "@/features/home/lib/inboxSelection";
+import {
+  resolveInboxCompletionSelection,
+  resolveInboxFilterSelection,
+} from "@/features/home/lib/inboxSelection";
 import { useHomeInboxReadState } from "@/features/home/useHomeInboxReadState";
 import { useHomeInboxAutoSelection } from "@/features/home/useHomeInboxAutoSelection";
 import { useHomeInboxContextMessages } from "@/features/home/useHomeInboxContextMessages";
@@ -391,7 +394,8 @@ export function HomeView({
       undoDoneLocal: undoDone,
       undoUnreadLocal: undoUnread,
     });
-  // Resolve selection before filtering so unread-only can retain its active row.
+  // Resolve selection against the complete Inbox so representative changes
+  // can preserve the active conversation across filters.
   const selectedItemFromAll = React.useMemo(
     () =>
       selectedEventId
@@ -414,18 +418,37 @@ export function HomeView({
     return inboxItems.filter(
       (item) =>
         matchesInboxFilter(item, filter, ownedAgentPubkeys) &&
-        (!unreadOnly ||
-          !effectiveDoneSet.has(item.id) ||
-          item.conversationId === selectedConversationId),
+        (!unreadOnly || !effectiveDoneSet.has(item.id)),
     );
-  }, [
-    effectiveDoneSet,
-    filter,
-    inboxItems,
-    ownedAgentPubkeys,
-    selectedConversationId,
-    unreadOnly,
-  ]);
+  }, [effectiveDoneSet, filter, inboxItems, ownedAgentPubkeys, unreadOnly]);
+  const handleMarkItemRead = React.useCallback(
+    (itemId: string) => {
+      const completedItem = findInboxItemByEventId(inboxItems, itemId);
+      if (
+        unreadOnly &&
+        completedItem?.conversationId === selectedConversationId
+      ) {
+        const nextSelectedEventId = resolveInboxCompletionSelection({
+          completedConversationId: completedItem.conversationId,
+          isNarrow: isNarrowHomeViewport,
+          items: filteredItems,
+        });
+        setUnreadBoundary(null);
+        setAutoSelectedEventId(nextSelectedEventId);
+        applyInboxSearchPatch({ item: null }, { replace: true });
+      }
+      markItemRead(itemId);
+    },
+    [
+      applyInboxSearchPatch,
+      filteredItems,
+      inboxItems,
+      isNarrowHomeViewport,
+      markItemRead,
+      selectedConversationId,
+      unreadOnly,
+    ],
+  );
   // A filter change may only retain detail for a conversation that remains
   // visible. The filter handler selects the next valid row in the same update,
   // so the detail pane never renders a stale conversation between states.
@@ -494,9 +517,7 @@ export function HomeView({
       const nextItems = inboxItems.filter(
         (item) =>
           matchesInboxFilter(item, nextFilter, ownedAgentPubkeys) &&
-          (!unreadOnly ||
-            !effectiveDoneSet.has(item.id) ||
-            item.conversationId === selectedConversationId),
+          (!unreadOnly || !effectiveDoneSet.has(item.id)),
       );
       const selection = resolveInboxFilterSelection({
         isNarrow: isNarrowHomeViewport,
@@ -645,7 +666,7 @@ export function HomeView({
               items={filteredItems}
               onDeleteDraft={handleDeleteDraft}
               onFilterChange={handleFilterChange}
-              onMarkRead={markItemRead}
+              onMarkRead={handleMarkItemRead}
               onMarkUnread={markItemUnread}
               onOpenDirect={(item) => {
                 const channelId = item.item.channelId;
@@ -683,7 +704,6 @@ export function HomeView({
                 setSelectedDraftKey(null);
                 setSelectedReminderId(null);
                 handleUserSelectItem(itemId);
-                markItemRead(itemId);
               }}
               onSelectDraft={(draftKey) => {
                 setUnreadBoundary(null);
diff --git a/desktop/tests/e2e/inbox-explicit-read.spec.ts b/desktop/tests/e2e/inbox-explicit-read.spec.ts
new file mode 100644
index 0000000000..1ed74514f9
--- /dev/null
+++ b/desktop/tests/e2e/inbox-explicit-read.spec.ts
@@ -0,0 +1,197 @@
+import { expect, test } from "@playwright/test";
+
+import { installMockBridge, TEST_IDENTITIES } from "../helpers/bridge";
+
+const GENERAL_CHANNEL_ID = "9a1657ac-f7aa-5db0-b632-d8bbeb6dfb50";
+const AGENTS_CHANNEL_ID = "94a444a4-c0a3-5966-ab05-530c6ddc2301";
+
+type MockInboxWindow = Window & {
+  __BUZZ_E2E_PUSH_MOCK_FEED_ITEM__?: (item: {
+    category: "mention";
+    channel_id: string;
+    channel_name: string;
+    channel_type: "stream";
+    content: string;
+    created_at: number;
+    id: string;
+    kind: number;
+    pubkey: string;
+    tags: string[][];
+  }) => unknown;
+  __BUZZ_E2E_SIGNED_EVENTS__?: Array<{
+    content: string;
+    kind: number;
+    tags: string[][];
+  }>;
+};
+
+async function seedUnreadInboxItems(page: import("@playwright/test").Page) {
+  await page.goto("/");
+  await expect(page.getByTestId("home-inbox-list")).toBeVisible();
+  await page.waitForFunction(
+    () =>
+      typeof (window as MockInboxWindow).__BUZZ_E2E_PUSH_MOCK_FEED_ITEM__ ===
+      "function",
+  );
+
+  const ids = {
+    first: "inbox-explicit-read-first",
+    second: "inbox-explicit-read-second",
+  };
+  await page.evaluate(
+    ({ agentsChannelId, currentPubkey, generalChannelId, itemIds, sender }) => {
+      const win = window as MockInboxWindow;
+      const push = win.__BUZZ_E2E_PUSH_MOCK_FEED_ITEM__;
+      if (!push) throw new Error("Mock feed helper is not installed.");
+
+      const createdAt = Math.floor(Date.now() / 1_000) + 10_000;
+      push({
+        category: "mention",
+        channel_id: generalChannelId,
+        channel_name: "general",
+        channel_type: "stream",
+        content: "First unread item for explicit completion",
+        created_at: createdAt + 1,
+        id: itemIds.first,
+        kind: 9,
+        pubkey: sender,
+        tags: [
+          ["h", generalChannelId],
+          ["p", currentPubkey],
+        ],
+      });
+      push({
+        category: "mention",
+        channel_id: agentsChannelId,
+        channel_name: "agents",
+        channel_type: "stream",
+        content: "Second unread item for explicit completion",
+        created_at: createdAt,
+        id: itemIds.second,
+        kind: 9,
+        pubkey: sender,
+        tags: [
+          ["h", agentsChannelId],
+          ["p", currentPubkey],
+        ],
+      });
+      if (win.__BUZZ_E2E_SIGNED_EVENTS__) {
+        win.__BUZZ_E2E_SIGNED_EVENTS__.length = 0;
+      }
+    },
+    {
+      agentsChannelId: AGENTS_CHANNEL_ID,
+      currentPubkey: TEST_IDENTITIES.tyler.pubkey,
+      generalChannelId: GENERAL_CHANNEL_ID,
+      itemIds: ids,
+      sender: TEST_IDENTITIES.alice.pubkey,
+    },
+  );
+
+  await expect(page.getByTestId(`home-inbox-item-${ids.first}`)).toBeVisible();
+  await expect(page.getByTestId(`home-inbox-item-${ids.second}`)).toBeVisible();
+  return ids;
+}
+
+async function setUnreadOnly(
+  page: import("@playwright/test").Page,
+  checked: boolean,
+) {
+  await page.getByTestId("inbox-options-trigger").click();
+  const toggle = page.getByRole("switch", { name: "Show unread only" });
+  if ((await toggle.isChecked()) !== checked) {
+    await toggle.click();
+  }
+  await page.keyboard.press("Escape");
+}
+
+test.beforeEach(async ({ page }) => {
+  await installMockBridge(page);
+});
+
+test("Inbox preview stays unread until the distinct explicit action completes it", async ({
+  page,
+}) => {
+  const ids = await seedUnreadInboxItems(page);
+  const first = page.getByTestId(`home-inbox-item-${ids.first}`);
+  const second = page.getByTestId(`home-inbox-item-${ids.second}`);
+
+  const firstPreview = first.getByRole("button", {
+    name: "Open inbox item from alice",
+  });
+  await firstPreview.focus();
+  await page.keyboard.press("Enter");
+
+  await expect(first).toHaveAttribute("aria-current", "true");
+  await expect(
+    first.getByRole("button", { name: "Mark as read" }),
+  ).toBeVisible();
+  await expect
+    .poll(() =>
+      page.evaluate(
+        () =>
+          (window as MockInboxWindow).__BUZZ_E2E_SIGNED_EVENTS__?.filter(
+            (event) => event.kind === 30_078,
+          ).length ?? 0,
+      ),
+    )
+    .toBe(0);
+
+  await setUnreadOnly(page, true);
+  await expect(first).toBeVisible();
+  await expect(second).toBeVisible();
+
+  const secondPreview = second.getByRole("button", {
+    name: "Open inbox item from alice",
+  });
+  await secondPreview.focus();
+  await page.keyboard.press("Enter");
+  await expect(second).toHaveAttribute("aria-current", "true");
+  await expect(first).toBeVisible();
+  await expect(second).toBeVisible();
+  await expect(
+    second.getByRole("button", { name: "Mark as read" }),
+  ).toBeVisible();
+
+  await firstPreview.focus();
+  await page.keyboard.press("Enter");
+  const markRead = first.getByRole("button", { name: "Mark as read" });
+  await expect(markRead).toBeVisible();
+  await markRead.click();
+
+  await expect(first).toHaveCount(0);
+  await expect(second).toHaveAttribute("aria-current", "true");
+  await expect
+    .poll(
+      () =>
+        page.evaluate(
+          ({ channelId }) => {
+            const events =
+              (window as MockInboxWindow).__BUZZ_E2E_SIGNED_EVENTS__ ?? [];
+            return events.some((event) => {
+              if (event.kind !== 30_078) return false;
+              const payload = JSON.parse(event.content) as {
+                contexts?: Record<string, number>;
+              };
+              return (payload.contexts?.[channelId] ?? 0) > 0;
+            });
+          },
+          { channelId: GENERAL_CHANNEL_ID },
+        ),
+      { timeout: 12_000 },
+    )
+    .toBe(true);
+
+  await setUnreadOnly(page, false);
+  await expect(first).toBeVisible();
+  await first.hover();
+  const markUnread = first.getByRole("button", { name: "Mark unread" });
+  await expect(markUnread).toBeVisible();
+  await markUnread.click();
+  await expect(
+    first.getByRole("button", { name: "Mark as read" }),
+  ).toBeVisible();
+
+  await setUnreadOnly(page, true);
+  await expect(first).toBeVisible();
+});
