fix(herdr-core): mark a finish seen again when a person comes back to its pane

changelog.d/herdr-core-seen-shown-again.md

@@ -0,0 +1,4 @@
+### Fixed
+- On herdr-core, coming back in Ovis to a finish another Herdr client marked unread marks it read
+  for every client again, and one marked unread while it stays on your Ovis screen stays unread
+  even when another client had read it first.

docs/guide.md

@@ -844,7 +844,7 @@ macOS). See [INSTALL.md](../INSTALL.md#2-choose-the-install-method).
   through herdr-core's `herdr.seen` terminal label. A finish you read in Ovis is no longer
   **done** in the Herdr app, and one you looked at in the app is no longer **done** in Ovis; one
   marked unread elsewhere is **done** again (Ovis does not read it for you again while it stays on
-  your screen). A terminal no client has labeled yet shows a turn that ended since Ovis started as
+  your screen; coming back to it later does). A terminal no client has labeled yet shows a turn that ended since Ovis started as
   **done**, until someone reads it. A remote PC's finishes are not shared this way.
 - **Terminal:** the live view streams over the server's own connection to herdr-core, input and
   the key bar work as on herdr, and the terminal takes the size of the viewer that last typed in

server/herdr-core/adapter.test.ts

@@ -4,7 +4,7 @@ import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { HerdrError, herdrRpc, paneGet, paneRead, paneSendKeys, paneSendText, sessionSnapshot, subscribeEvents, type EventFrame, paneClose, tabClose, tabCreate, workspaceClose, workspaceCreate, agentManifests, agentStart, ping } from "../herdr/client.ts";
 import { claudeInputDraft, viewportShowsLive } from "../claude-input.ts";
-import { paintedScreen, startHerdrCoreAdapter } from "./adapter.ts";
+import { paintedScreen, type SeenObserver, startHerdrCoreAdapter } from "./adapter.ts";
 import { SeenMarker } from "./seen-label.ts";
 import { encodeFrame, FRAME_CLOSE, FRAME_DATA, HcConnection, HcError, type HcTransport, makeFrameReader } from "./connection.ts";
 import type { HcTerminal } from "./snapshot.ts";
@@ -123,7 +123,7 @@ for (const method of ["call", "open"] as const) {
     }
   });
 }
-async function setup(terminals: HcTerminal[], extra: Record<string, Handler> = {}) {
+async function setup(terminals: HcTerminal[], extra: Record<string, Handler> = {}, observeSeen?: SeenObserver) {
   const dir = mkdtempSync(join(tmpdir(), "ovis-hc-"));
   const socket = join(dir, "herdr.sock");
   const cores: FakeCore[] = [];
@@ -144,7 +144,7 @@ async function setup(terminals: HcTerminal[], extra: Record<string, Handler> = {
     cores.push(core);
     return core;
   });
-  const adapter = await startHerdrCoreAdapter(hc, socket);
+  const adapter = await startHerdrCoreAdapter(hc, socket, observeSeen);
   cleanups.push(() => { adapter.stop(); hc.close(); rmSync(dir, { recursive: true, force: true }); });
   return { socket, cores, hc };
 }
@@ -391,12 +391,13 @@ it("asks for a fresh snapshot when a report's kind, message or progress changes,
   it("tells a finish past herdr.seen as done on its turn, and a look from any client as idle", async () => {
     let channel = 0;
     const working = { app: "claude-code", state: "working" };
+    const observed: [string, number, string | undefined][] = [];
     const { socket, cores } = await setup([terminal("t1"), terminal("t2")], {
       "events.subscribe": () => {
         channel = cores.at(-1)!.channel();
         return { channel, seq: 1, terminals: [terminal("t1", { status: working, turn: 1, labels: { "herdr.seen": "1" } }), terminal("t2", { status: working, turn: 4 })] };
       },
-    });
+    }, (address, turn, labels) => observed.push([address, turn, labels["herdr.seen"]]));
     const frames: EventFrame[] = [];
     const started = Promise.withResolvers<void>();
     const told = Promise.withResolvers<void>();
@@ -425,14 +426,17 @@ it("asks for a fresh snapshot when a report's kind, message or progress changes,
     await told.promise;
     const shown = (pane: string, status: string) => ({ event: "pane.agent_status_changed", data: { pane_id: pane, agent_status: status, agent: "claude" } });
     expect(frames).toEqual([shown("t1", "done"), shown("t1", "idle"), shown("t1", "done"), shown("t2", "done"), shown("t2", "idle"), shown("t2", "working")]);
+    // what the server's seen marker learns: each terminal's turn and herdr.seen, as they change
+    expect(observed).toEqual([["t1", 1, "1"], ["t2", 4, undefined], ["t1", 2, "1"], ["t1", 2, "2"], ["t1", 2, "2"], ["t1", 2, "1"], ["t2", 5, undefined], ["t2", 5, "5"]]);
   });
 
-  it("marks a terminal seen at its turn, only to raise it or where it is missing, once per turn", async () => {
+  it("marks a terminal seen at its turn, only to raise it or where it is missing, once per turn unless it is shown again", async () => {
     const terminals = [
       terminal("t1", { turn: 3, labels: { "herdr.seen": "1" } }),
       terminal("t2", { turn: 3, labels: { "herdr.seen": "3" } }),
       terminal("t3", { turn: 2 }),
       terminal("t4", { turn: 5, state: "finished" }),
+      terminal("t5", { turn: 3, labels: { "herdr.seen": "3" } }),
     ];
     const written: unknown[] = [];
     const { hc } = await setup(terminals, { "terminal.label": (params) => { written.push(params); return terminals[0]; } });
@@ -441,12 +445,25 @@ it("asks for a fresh snapshot when a report's kind, message or progress changes,
     // another client marks t1 unread while it is still on screen here: this turn is not written again
     terminals[0]!.labels = { "herdr.seen": "2" };
     await marker.mark("t1");
+    // a person coming back to it later raises it again
+    await marker.mark("t1", true);
+    // t2 was read elsewhere first: marked unread while on screen here, it stays unread
+    terminals[1]!.labels = { "herdr.seen": "2" };
+    await marker.mark("t2");
+    // t5 was read elsewhere while idle in front here (Ovis was told of no look): the same
+    marker.observe("t5", 3, { "herdr.seen": "3" });
+    terminals[4]!.labels = { "herdr.seen": "2" };
+    await marker.mark("t5");
+    // and coming back to it raises it again
+    await marker.mark("t5", true);
     // the next turn is
     terminals[0]!.turn = 4;
     await Promise.all([marker.mark("t1"), marker.mark("t1")]);
     expect(written).toEqual([
       { terminal: "t1", labels: { "herdr.seen": "3" } },
       { terminal: "t3", labels: { "herdr.seen": "2" } },
+      { terminal: "t1", labels: { "herdr.seen": "3" } },
+      { terminal: "t5", labels: { "herdr.seen": "3" } },
       { terminal: "t1", labels: { "herdr.seen": "4" } },
     ]);
   });

server/herdr-core/adapter.ts

@@ -82,7 +82,13 @@ export interface HerdrCoreAdapter {
   stop: () => void;
 }
 
-export async function startHerdrCoreAdapter(hc: HcConnection, socketPath: string): Promise<HerdrCoreAdapter> {
+/**
+ * `observeSeen`: told each terminal's turn and labels as the events subscription learns them, so the
+ * server knows a turn some client marked seen before Ovis is told of a look (seen-label.ts SeenMarker).
+ */
+export type SeenObserver = (address: string, turn: number, labels: Record<string, string>) => void;
+
+export async function startHerdrCoreAdapter(hc: HcConnection, socketPath: string, observeSeen?: SeenObserver): Promise<HerdrCoreAdapter> {
   if (existsSync(socketPath)) {
     const live = await new Promise<boolean>((resolve, reject) => {
       const probe = createConnection(socketPath);
@@ -120,7 +126,7 @@ export async function startHerdrCoreAdapter(hc: HcConnection, socketPath: string
             first = false;
             const request = parseRequest(line);
             if (!request) { send(socket, `${JSON.stringify({ id: "", error: { code: "bad_frame", message: "not a herdr request" } })}\n`, true); return; }
-            if (request.method === "events.subscribe") { void subscribe(hc, socket, request, firstTurn); return; }
+            if (request.method === "events.subscribe") { void subscribe(hc, socket, request, firstTurn, observeSeen); return; }
             void answers(request).then((reply) => send(socket, `${JSON.stringify({ id: request.id, ...reply })}\n`, true));
           }, () => socket.end()),
         };
@@ -611,7 +617,7 @@ function labelsOf(labels: unknown): Record<string, string> {
  * subscriber that fell behind on hc (`missed`), or a line to hc that ended, ends this connection:
  * the status collector reopens it and reads what it missed from a fresh snapshot.
  */
-async function subscribe(hc: HcConnection, socket: Socket<ConnectionState>, request: Request, firstTurn: FirstTurn): Promise<void> {
+async function subscribe(hc: HcConnection, socket: Socket<ConnectionState>, request: Request, firstTurn: FirstTurn, observeSeen?: SeenObserver): Promise<void> {
   const subscriptions = Array.isArray(request.params["subscriptions"]) ? request.params["subscriptions"].filter(isRecord) : [];
   const statusPanes = new Set(subscriptions.flatMap((entry) => entry["type"] === STATUS_EVENT && typeof entry["pane_id"] === "string" ? [entry["pane_id"]] : []));
   const structure = subscriptions.some((entry) => typeof entry["type"] === "string" && !NOT_STRUCTURE.has(entry["type"]));
@@ -657,6 +663,7 @@ async function subscribe(hc: HcConnection, socket: Socket<ConnectionState>, requ
     const labels = labelsOf(record["labels"]);
     const first = firstTurn({ address: record["address"], turn });
     known.set(record["address"], { status, turn, first, labels, shown: shownStatusOf(status, turn, labels, first) });
+    observeSeen?.(record["address"], turn, labels);
   };
   const tell = (paneId: string, status: HcStatus | null, shown: AgentStatus) => {
     emit(STATUS_EVENT, { pane_id: paneId, agent_status: shown, agent: agentOfStatus(status) });
@@ -704,6 +711,7 @@ async function subscribe(hc: HcConnection, socket: Socket<ConnectionState>, requ
         const entry = paneId ? known.get(paneId) : undefined;
         if (!entry || typeof event.data?.["turn"] !== "number") return;
         entry.turn = event.data["turn"];
+        observeSeen?.(paneId!, entry.turn, entry.labels);
         retell(paneId!);
         return;
       }
@@ -731,6 +739,7 @@ async function subscribe(hc: HcConnection, socket: Socket<ConnectionState>, requ
         const entry = paneId ? known.get(paneId) : undefined;
         if (entry) {
           entry.labels = labelsOf(event.data?.["labels"]);
+          observeSeen?.(paneId!, entry.turn, entry.labels);
           retell(paneId!);
         }
         lifecycle("layout.updated", "layout_updated");

server/herdr-core/runtime.ts

@@ -2,6 +2,7 @@ import { chmodSync, mkdirSync, statSync } from "node:fs";
 import { join } from "node:path";
 import { startHerdrCoreAdapter, type HerdrCoreAdapter } from "./adapter.ts";
 import { HcConnection, spawnHcConnect } from "./connection.ts";
+import { SeenMarker } from "./seen-label.ts";
 import { existsSync } from "node:fs";
 import { findHc, type FoundHc } from "./discovery.ts";
 
@@ -83,6 +84,14 @@ export function servingCore(): boolean {
   return serving;
 }
 
+let marker: SeenMarker | null = null;
+
+/** The server's one `herdr.seen` writer, which the adapter's events tell what other clients marked seen. */
+export function coreSeenMarker(): SeenMarker {
+  marker ??= new SeenMarker(coreConnection());
+  return marker;
+}
+
 /** The server's one line to hc, which the adapter answers through and live terminals stream on. */
 export function coreConnection(): HcConnection {
   connection ??= new HcConnection(() => spawnHcConnect(hcBin()));
@@ -112,7 +121,7 @@ export async function startRuntime(stateDir: string, find: () => Promise<FoundHc
   if (statSync(stateDir).uid !== process.geteuid?.()) throw new Error("the herdr-core state directory is not owned by this user");
   chmodSync(stateDir, 0o700);
   const hc = coreConnection();
-  const adapter = await startHerdrCoreAdapter(hc, join(stateDir, "herdr-core.sock"));
+  const adapter = await startHerdrCoreAdapter(hc, join(stateDir, "herdr-core.sock"), (address, turn, labels) => coreSeenMarker().observe(address, turn, labels));
   chmodSync(adapter.socketPath, 0o600);
   process.env["HERDR_SOCKET"] = adapter.socketPath;
   serving = true;
@@ -122,6 +131,7 @@ export async function startRuntime(stateDir: string, find: () => Promise<FoundHc
       adapter.stop();
       hc.close();
       connection = null;
+      marker = null;
       serving = false;
       found = null;
       search = null;

server/herdr-core/seen-label.ts

@@ -7,8 +7,10 @@ type Hc = Pick<HcConnection, "call">;
  * Tells every Herdr client that a person looked at a terminal's agent (herdr-app docs/labels.md):
  * `herdr.seen` becomes the terminal's turn, so a finish read in Ovis is read in the Herdr app too.
  * Written only to raise it, or where it is missing so a terminal started elsewhere joins, and once
- * per turn: a pane left on screen does not overrule a "mark unread" from another client. One
- * terminal's writes go in order, so an earlier turn never lands after a later one.
+ * per turn: a pane left on screen does not overrule a "mark unread" from another client. A turn
+ * another client marked seen first counts as written. `again` (a person brought the pane into view)
+ * writes a turn already written: coming back to it raises it again. One terminal's writes go in
+ * order, so an earlier turn never lands after a later one.
  */
 export class SeenMarker {
   /** the turn this server last wrote on each terminal */
@@ -17,21 +19,35 @@ export class SeenMarker {
 
   constructor(private readonly hc: Hc) {}
 
-  mark(address: string): Promise<void> {
-    const run = (this.turns.get(address) ?? Promise.resolve()).then(() => this.write(address));
+  /**
+   * A terminal's turn and labels as the server learned them (adapter.ts events): a turn some client
+   * marked seen counts as written, so a "mark unread" while the pane stays on screen here stands even
+   * when Ovis was never told of a look at that turn (it was idle in front of the person).
+   */
+  observe(address: string, turn: number, labels: Record<string, string>): void {
+    const seen = seenTurnOf(labels);
+    if (seen !== null && seen >= turn && turn > (this.written.get(address) ?? -1)) this.written.set(address, turn);
+  }
+
+  mark(address: string, again = false): Promise<void> {
+    const run = (this.turns.get(address) ?? Promise.resolve()).then(() => this.write(address, again));
     const settled = run.catch(() => {});
     this.turns.set(address, settled);
     void settled.then(() => { if (this.turns.get(address) === settled) this.turns.delete(address); });
     return run;
   }
 
-  private async write(address: string): Promise<void> {
+  private async write(address: string, again: boolean): Promise<void> {
     const record = await this.hc.call("terminal.get", { terminal: address });
     if (record.state !== "live") return;
     const turn = record.turn ?? 0;
-    if (turn <= (this.written.get(address) ?? -1)) return;
+    if (!again && turn <= (this.written.get(address) ?? -1)) return;
     const seen = seenTurnOf(record.labels);
-    if (seen !== null && seen >= turn) return;
+    if (seen !== null && seen >= turn) {
+      // seen here and already marked elsewhere: an unread from another client later still stands
+      this.observe(address, turn, record.labels ?? {});
+      return;
+    }
     await this.hc.call("terminal.label", { terminal: address, labels: { [SEEN_LABEL]: String(turn) } });
     this.written.set(address, turn);
   }

server/index.ts

@@ -13,6 +13,7 @@ import { isLoopbackBind, lanExposed } from "./access.ts";
 import { DeviceStore } from "./devices.ts";
 import { PreferenceStore } from "./preferences.ts";
 import { SeenStore } from "./seen.ts";
+import type { SeenFinish } from "../shared/seen.ts";
 import { TailnetIdentitySource } from "./tailscale.ts";
 import { SHELL_AGENTS, ShellAgentLocator } from "./shell-agent.ts";
 import { AgentDetector, catalogLocator, defaultLoginProbe } from "./agent-catalog-detect.ts";
@@ -22,10 +23,9 @@ import { AGENT_CATALOG, catalogPlatform } from "../shared/agent-catalog.ts";
 import { AgentInstaller } from "./agent-catalog-install.ts";
 import { type BackgroundAgentDeps } from "./background-agent.ts";
 import { herdrSocketPath, ping } from "./herdr/client.ts";
-import { coreConnection, hcBin, NoHerdrCoreError, servingCore, setRuntime, startRuntime } from "./herdr-core/runtime.ts";
+import { coreConnection, coreSeenMarker, hcBin, NoHerdrCoreError, servingCore, setRuntime, startRuntime } from "./herdr-core/runtime.ts";
 import { chooseRuntime, runtimeService, type RuntimeChoice, type RuntimeService } from "./herdr-core/runtime-choice.ts";
 import { bridgePins } from "./herdr-core/pin-label.ts";
-import { SeenMarker } from "./herdr-core/seen-label.ts";
 import { herdrCoreFacts } from "./herdr-core/discovery.ts";
 import { type HerdrCoreFacts, LOCAL_MACHINE } from "../shared/machines.ts";
 import { type AlertTiming, defaultStateDir } from "./push.ts";
@@ -305,14 +305,19 @@ export function createServer(
   // read finishes are kept until this server knows their herdr run ended
   const seen = new SeenStore(stateDir, (pc, epoch) => machines?.finishEnded(pc, epoch) ?? false);
   // on herdr-core a finish read here is read in the Herdr app and its other clients too: only the
-  // pane's finish now, so a read told late never marks a turn that ended since as seen
-  const seenMarker = servingCore() ? new SeenMarker(coreConnection()) : null;
-  if (seenMarker) seen.subscribe((finish) => {
-    if (finish.pc !== LOCAL_MACHINE || machines?.finishEnded(finish.pc, finish.epoch) !== false) return;
+  // pane's finish now, so a read told late never marks a turn that ended since as seen. A pane brought
+  // into view writes it again (shared/seen.ts SeenPresentation); one left on screen does not
+  const seenMarker = servingCore() ? coreSeenMarker() : null;
+  const markSeen = (finish: SeenFinish, again: boolean) => {
+    if (!seenMarker || finish.pc !== LOCAL_MACHINE || machines?.finishEnded(finish.pc, finish.epoch) !== false) return;
     const pane = machines.list().find((machine) => machine.id === LOCAL_MACHINE)?.snapshot?.panes.find((candidate) => candidate.pane_id === finish.pane);
     if (pane?.agent_status !== "done" || pane.finish_id !== finish.id) return;
-    seenMarker.mark(finish.pane).catch((error: unknown) => console.warn(`herdr-core: could not mark ${finish.pane} seen:`, error));
-  });
+    seenMarker.mark(finish.pane, again).catch((error: unknown) => console.warn(`herdr-core: could not mark ${finish.pane} seen:`, error));
+  };
+  if (seenMarker) {
+    seen.subscribe((finish, presented) => { if (presented !== "new") markSeen(finish, false); });
+    seen.onPresented((finish) => markSeen(finish, true));
+  }
   // started after createCore's Node probe (server/pty/sidecar.ts), which can hold the thread up to 3 s: that wait
   // would count against the read's own 2.5 s limit, and a read cut short leaves the owner unknown for 5 minutes
   identityOf(false, null);