Merge pull request #259 from httpsphl/fix/unread-survives-restarts

fix(app): the unread mark no longer comes back after a restart

Author: httpsphl

app/src/features/bots/BotView.tsx

@@ -4,7 +4,7 @@ import { useT } from "../../i18n";
 import type { Bot, BotFile, Crew } from "../../lib/protocol.gen";
 import { useWindowVisible } from "../../shell/visibility";
 import { routinesOf } from "../../store/app";
-import { useApi, useApp } from "../../store/context";
+import { useApp } from "../../store/context";
 import { PanelClosing, PanelRestored } from "../../ui/panelMotion";
 import { type Tab, Tabs, tabId } from "../../ui/Tabs";
 import { useStable } from "../../ui/useStable";
@@ -85,14 +85,11 @@ export function BotView({ bot, crew }: { bot: Bot; crew: Crew }) {
   const visible = useWindowVisible();
   const replyAt = useApp((state) => state.replyAt[bot.id]);
   const markSeen = useApp((state) => state.markSeen);
-  const api = useApi();
   useEffect(() => {
     if (visible && replyAt !== undefined) {
       markSeen(bot.id);
-      // A phone connected to this computer stops marking it new (spec 28.12).
-      api.call("chat.read", { botId: bot.id }).catch(() => {});
     }
-  }, [bot.id, visible, replyAt, markSeen, api]);
+  }, [bot.id, visible, replyAt, markSeen]);
   // A browser at rest is open, but the bot is not using it (spec 21.2).
   const browsing = useApp((state) => {
     const browser = state.browsers[bot.id];

app/src/features/crews/unread.test.tsx

@@ -65,6 +65,41 @@ describe("unread replies in the conversation list", () => {
     expect(row("Scout").getAttribute("aria-label")).toBe("Scout, Idle, unread");
   });
 
+  test("what the daemon says was read stays read when the app forgot what it saw", async () => {
+    const fake = new FakeBotloft();
+    const crew = fake.addCrew("Ops");
+    const scout = fake.addBot(crew.id, "Scout");
+    const writer = fake.addBot(crew.id, "Writer");
+    fake.setBotState(scout.id, "idle");
+    fake.setBotState(writer.id, "idle");
+    // Both replied at 5000; the owner read only Writer's, and the app's own
+    // storage (cleared in beforeEach) knows nothing of it.
+    fake.bot(scout.id).lastReplyAt = 5000;
+    fake.bot(writer.id).lastReplyAt = 5000;
+    fake.bot(writer.id).readThrough = 5000;
+    renderApp(fake);
+    await crewOpened("Ops");
+    expect(row("Scout").getAttribute("aria-label")).toBe("Scout, Idle, unread");
+    expect(row("Writer").getAttribute("aria-label")).toBe("Writer, Idle");
+  });
+
+  test("seeing a chat tells the daemon, which keeps it", async () => {
+    const fake = new FakeBotloft();
+    const crew = fake.addCrew("Ops");
+    const scout = fake.addBot(crew.id, "Scout");
+    fake.setBotState(scout.id, "idle");
+    renderApp(fake);
+    await crewOpened("Ops");
+    act(() => {
+      fake.chat.reply(scout.id, "Found three sources.");
+    });
+    openBot("Scout");
+    await screen.findByRole("list", { name: "Messages" });
+    expect(
+      fake.calls.filter((call) => call.method === "chat.read").map((call) => call.params),
+    ).toContainEqual({ botId: scout.id });
+  });
+
   test("replies from before the first start with unread marks count as seen", async () => {
     localStorage.removeItem("botloft.seenSince");
     const fake = new FakeBotloft();

app/src/lib/fakeBots.ts

@@ -46,6 +46,7 @@ export function botHandlers(fake: FakeBotloft): Pick<Handlers, BotMethods> {
         workspace: `C:\\Users\\owner\\Botloft\\${crew.slug}\\${handle}`,
         lastActivity: null,
         lastReplyAt: null,
+        readThrough: null,
         createdAt: fake.now,
         archivedAt: null,
       };

app/src/lib/protocol.gen.ts

@@ -286,6 +286,11 @@ lastActivity: Activity | null,
  * than the owner's last look (spec 15.1).
  */
 lastReplyAt: number | null, 
+/**
+ * Until when the owner read the bot's replies, kept by the daemon so
+ * the mark survives the app losing its storage; `null` if never.
+ */
+readThrough: number | null, 
 /**
  * Unix time in milliseconds.
  */

app/src/store/app.ts

@@ -194,7 +194,7 @@ export function createAppStore(api: BotloftApi): AppStore {
           set({ page, selectedCrewId: null, selectedBotId: null });
         }
       },
-      ...seenActions(get, set),
+      ...seenActions(get, set, api),
       setPanel: (botId, panel) => set((state) => ({ panels: { ...state.panels, [botId]: panel } })),
       putCrew: (crew) => set((state) => withCrew(state, crew)),
       putBot: (bot) => set((state) => withBot(state, bot)),

app/src/store/seen.ts

@@ -4,6 +4,7 @@
 // after it marks the app's icon (`unseenFailures`).
 
 import type { StoreApi } from "zustand/vanilla";
+import type { BotloftApi } from "../lib/api";
 import type { BotId, CrewId, Routine } from "../lib/protocol.gen";
 import type { AppState } from "./app";
 
@@ -79,12 +80,17 @@ export function withReply(
 export function seenActions(
   get: StoreApi<AppState>["getState"],
   set: StoreApi<AppState>["setState"],
+  api: BotloftApi,
 ): Pick<AppState, "markSeen" | "markUnread"> {
   return {
     markSeen: (botId) => {
       const { seenAt, replyAt, markedUnread } = get();
       const next = { ...seenAt, [botId]: Math.max(Date.now(), replyAt[botId] ?? 0) };
       saveSeen(next);
+      // The daemon keeps it too: this app's own storage has gone back to an
+      // older state before, and the mark came back. A phone connected to
+      // this computer also stops marking the chat new (spec 28.12).
+      api.call("chat.read", { botId }).catch(() => {});
       const { [botId]: wasMarked, ...marked } = markedUnread;
       if (wasMarked) {
         saveMarked(marked);
@@ -120,7 +126,8 @@ export function isUnread(state: AppState, botId: BotId): boolean {
     return true;
   }
   const at = state.replyAt[botId];
-  return at !== undefined && at > Math.max(state.seenAt[botId] ?? 0, state.seenSince);
+  const read = state.bots[botId]?.readThrough ?? 0;
+  return at !== undefined && at > Math.max(state.seenAt[botId] ?? 0, state.seenSince, read);
 }
 
 /** How many bots of `crewId` are unread. */
@@ -141,7 +148,8 @@ export function unseenFailures(state: AppState): Routine[] {
     return (
       last?.status === "failed" &&
       routine.botId !== state.selectedBotId &&
-      (last.finishedAt ?? 0) > (state.seenAt[routine.botId] ?? 0)
+      (last.finishedAt ?? 0) >
+        Math.max(state.seenAt[routine.botId] ?? 0, state.bots[routine.botId]?.readThrough ?? 0)
     );
   });
 }

crates/botloft-core/src/protocol/model.rs

@@ -177,6 +177,9 @@ pub struct Bot {
     /// before its first. The app marks the bot unread when this is newer
     /// than the owner's last look (spec 15.1).
     pub last_reply_at: Option<i64>,
+    /// Until when the owner read the bot's replies, kept by the daemon so
+    /// the mark survives the app losing its storage; `null` if never.
+    pub read_through: Option<i64>,
     /// Unix time in milliseconds.
     pub created_at: i64,
     /// Unix time in milliseconds; `null` while the bot is active.

crates/botloft-store/migrations/0029_read_through.sql

@@ -0,0 +1,3 @@
+-- Until when the owner read each bot's replies (Unix ms): the unread mark
+-- of the conversation list survives the app losing its own storage.
+ALTER TABLE bots ADD COLUMN read_through INTEGER;

crates/botloft-store/src/chat.rs

@@ -178,6 +178,27 @@ impl Store {
         Ok(item.and_then(|item| activity(&item.body, item.updated_at)))
     }
 
+    /// The owner read the bot's replies up to `at`: kept, so the unread mark
+    /// does not come back when the app forgets what it saw. Never goes back.
+    pub fn set_read_through(&self, bot: &BotId, at: i64) -> Result<()> {
+        self.conn.prepare_cached(
+            "UPDATE bots SET read_through = ?2              WHERE id = ?1 AND (read_through IS NULL OR read_through < ?2)",
+        )?
+        .execute(params![bot.as_str(), at])?;
+        Ok(())
+    }
+
+    /// Until when the owner read the bot's replies; `None` if never.
+    pub fn read_through(&self, bot: &BotId) -> Result<Option<i64>> {
+        let mut stmt = self
+            .conn
+            .prepare_cached("SELECT read_through FROM bots WHERE id = ?1")?;
+        Ok(stmt
+            .query_row([bot.as_str()], |row| row.get(0))
+            .optional()?
+            .flatten())
+    }
+
     /// When the bot last finished a reply, for the unread mark in the
     /// conversation list (spec 15.1); `None` before its first.
     pub fn last_reply_at(&self, bot: &BotId) -> Result<Option<i64>> {

crates/botloft-store/src/chat/tests.rs

@@ -174,3 +174,16 @@ fn the_last_reply_is_found_behind_newer_items() {
         .expect("insert");
     assert_eq!(fx.store.last_reply_at(bot).expect("reply"), Some(9));
 }
+
+#[test]
+fn what_the_owner_read_is_kept_and_never_goes_back() {
+    let fx = Fixture::new();
+    let bot = &fx.bots[0].id;
+    assert_eq!(fx.store.read_through(bot).expect("none"), None);
+    fx.store.set_read_through(bot, 50).expect("set");
+    assert_eq!(fx.store.read_through(bot).expect("read"), Some(50));
+    fx.store.set_read_through(bot, 20).expect("older");
+    assert_eq!(fx.store.read_through(bot).expect("still"), Some(50));
+    fx.store.set_read_through(bot, 90).expect("newer");
+    assert_eq!(fx.store.read_through(bot).expect("newer"), Some(90));
+}