fix(terminal): keep Herdr views from earlier releases after upgrading (#2634)

Kept Herdr viewer journal entries and follow-view saved tiles store the whole tile attach argv. #2614 changed the writable tile script, and #2464 changed both scripts before it, so views kept from 0.7.5-0.7.9 failed the strict comparison after upgrading: a kept viewer read unconfirmed and a kept follow view stopped adding and closing tiles. earlierTileAttachment accepts a kept argv only when it exactly equals what 0.7.9's or 0.7.5-0.7.8's generator makes for the current session, tmux, server and recorded read-only intent; the entry then moves to the current command. A different target, a changed read-only intent, or any other script (including one sharing the old prefix) still mismatches as before. The earlier generators live in src/domain/terminal/earlier-tile-attach.ts, shared with the tests. The approach follows natefaerber's #2467. No new refusal, error code or setting.

(cherry picked from commit 6a46172df1)

packages/daemon/src/domain/terminal/earlier-tile-attach.ts

@@ -0,0 +1,96 @@
+// The tile commands earlier releases generated, verbatim, so a view kept across an upgrade is recognised only by
+// a command OpenRig actually generated for the same seat. Kept views exist from 0.7.5 on.
+// 0.7.9: tile-attach.ts at 7cabac44 (#2464). 0.7.5 through 0.7.8: tile-attach.ts at 80616b79 (#2133).
+export function tileAttachArgv079(
+  tmux: string, server: readonly string[], session: string, readOnly: boolean,
+  retrySeconds = 30,
+): string[] {
+  // tmux honours ignore-size server-wide: while any client of any session sizes
+  // its window, ignore-size clients size nothing, so a session they alone view
+  // keeps its 80x24 creation size. A writable tile therefore ignores size only
+  // when its session already has a sizing client.
+  const ro = readOnly ? "-r " : "$ro ";
+  const pick = (target: string) => (readOnly ? "" : `pick ${target} "$@"; `);
+  // No apostrophes or control characters in the script: Herdr also stores this
+  // argv as its restart command. All external values stay positional arguments.
+  const script = [
+    'target=$1; shift;',
+    ...(readOnly ? [] : [
+      // Shell builtins only: the provider PATH may hold nothing but tmux.
+      'pick() { s=$1; shift; ro=; for f in $("$@" list-clients -t "$s" -F "#{client_flags}" 2>/dev/null); do',
+      'case ",$f," in *,ignore-size,*) ;; *) ro="-f ignore-size";; esac; done; };',
+    ]),
+    `snapshot=$("$@" display-message -p -t "=$target:" "#{pid} #{session_id}:#{session_name}" 2>/dev/null) || { ${pick('"=$target"')}exec "$@" attach ${ro}-t "=$target"; };`,
+    // display-message may succeed with empty/fallback context for a missing target.
+    `if [ "\${snapshot#*:}" != "$target" ]; then ${pick('"=$target"')}exec "$@" attach ${ro}-t "=$target"; fi;`,
+    'snapshot=${snapshot%%:*};',
+    'node=$("$@" show-environment -t "${snapshot#* }" OPENRIG_NODE_ID 2>/dev/null) || node=;',
+    'case "$node" in OPENRIG_NODE_ID=?*) ;; *) node=;; esac;',
+    'if [ -z "$node" ]; then',
+    'printf "%s\\n" "No managed identity for $target; reopen this view after replacement.";',
+    `${pick('"${snapshot#* }"')}exec "$@" attach ${ro}-t "\${snapshot#* }"; fi;`,
+    'while :; do',
+    `${pick('"${snapshot#* }"')}"$@" attach ${ro}-t "\${snapshot#* }";`,
+    `left=${retrySeconds}; same=0;`,
+    'while [ "$left" -gt 0 ]; do',
+    'next=$("$@" display-message -p -t "=$target:" "#{pid} #{session_id}:#{session_name}" 2>/dev/null) || next=;',
+    'if [ "${next#*:}" = "$target" ]; then next=${next%%:*}; else next=; fi;',
+    'if [ -n "$next" ]; then',
+    'identity=$("$@" show-environment -t "${next#* }" OPENRIG_NODE_ID 2>/dev/null) || identity=;',
+    // A session can disappear between the two metadata reads; unknown waits.
+    'if [ -n "$identity" ]; then',
+    'if [ "$identity" != "$node" ]; then printf "%s\\n" "Seat identity changed for $target; reopen its view."; exit 1; fi;',
+    'if [ "$next" != "$snapshot" ]; then snapshot=$next; break; fi;',
+    // The stop adapter detaches BEFORE killing. Give that kill a chance to
+    // finish; an ordinary Ctrl-b d leaves the original alive and ends the tile.
+    'same=$((same + 1)); if [ "$same" -ge 3 ]; then exit 0; fi;',
+    'else same=0; fi; else same=0; fi;',
+    'sleep 1; left=$((left - 1));',
+    'done;',
+    'if [ "$left" -eq 0 ]; then printf "%s\\n" "No replacement for $target; reopen the view with rig terminal open."; exit 0; fi;',
+    'done',
+  ].join(" ");
+  return ["/bin/sh", "-c", script, "openrig-tile", session, tmux, ...server];
+}
+
+export function tileAttachArgv078(
+  tmux: string, server: readonly string[], session: string, readOnly: boolean,
+  retrySeconds = 30,
+): string[] {
+  const ro = readOnly ? "-r " : "-f ignore-size ";
+  // No apostrophes or control characters in the script: Herdr also stores this
+  // argv as its restart command. All external values stay positional arguments.
+  const script = [
+    'target=$1; shift;',
+    `snapshot=$("$@" display-message -p -t "=$target:" "#{pid} #{session_id}:#{session_name}" 2>/dev/null) || exec "$@" attach ${ro}-t "=$target";`,
+    // display-message may succeed with empty/fallback context for a missing target.
+    `if [ "\${snapshot#*:}" != "$target" ]; then exec "$@" attach ${ro}-t "=$target"; fi;`,
+    'snapshot=${snapshot%%:*};',
+    'node=$("$@" show-environment -t "${snapshot#* }" OPENRIG_NODE_ID 2>/dev/null) || node=;',
+    'case "$node" in OPENRIG_NODE_ID=?*) ;; *) node=;; esac;',
+    'if [ -z "$node" ]; then',
+    'printf "%s\\n" "No managed identity for $target; reopen this view after replacement.";',
+    `exec "$@" attach ${ro}-t "\${snapshot#* }"; fi;`,
+    'while :; do',
+    `"$@" attach ${ro}-t "\${snapshot#* }";`,
+    `left=${retrySeconds}; same=0;`,
+    'while [ "$left" -gt 0 ]; do',
+    'next=$("$@" display-message -p -t "=$target:" "#{pid} #{session_id}:#{session_name}" 2>/dev/null) || next=;',
+    'if [ "${next#*:}" = "$target" ]; then next=${next%%:*}; else next=; fi;',
+    'if [ -n "$next" ]; then',
+    'identity=$("$@" show-environment -t "${next#* }" OPENRIG_NODE_ID 2>/dev/null) || identity=;',
+    // A session can disappear between the two metadata reads; unknown waits.
+    'if [ -n "$identity" ]; then',
+    'if [ "$identity" != "$node" ]; then printf "%s\\n" "Seat identity changed for $target; reopen its view."; exit 1; fi;',
+    'if [ "$next" != "$snapshot" ]; then snapshot=$next; break; fi;',
+    // The stop adapter detaches BEFORE killing. Give that kill a chance to
+    // finish; an ordinary Ctrl-b d leaves the original alive and ends the tile.
+    'same=$((same + 1)); if [ "$same" -ge 3 ]; then exit 0; fi;',
+    'else same=0; fi; else same=0; fi;',
+    'sleep 1; left=$((left - 1));',
+    'done;',
+    'if [ "$left" -eq 0 ]; then printf "%s\\n" "No replacement for $target; reopen the view with rig terminal open."; exit 0; fi;',
+    'done',
+  ].join(" ");
+  return ["/bin/sh", "-c", script, "openrig-tile", session, tmux, ...server];
+}

packages/daemon/src/domain/terminal/herdr-follow-view.ts

@@ -7,6 +7,7 @@ import { HerdrViewer, ViewerCreateNotDispatchedError, findAttributedViewerPanes,
 import { DEFAULT_FOLLOW_POLICY, planFollowingTiles, type FollowControls, type FollowPolicy, type FollowTile, type FollowingStatus } from "./terminal-follow-policy.js";
 import type { ComposedPane, ComposedView, OpenViewResult } from "./terminal-provider.js";
 import type { HerdrTransport } from "./herdr-transport.js";
+import { earlierTileAttachment } from "./tile-attach.js";
 
 export type ReadFollowInput = (target: ComposedPane) => Promise<"empty" | "text" | "unknown">;
 interface Tile extends FollowTile { member: ViewerKey; target: ComposedPane; viewer: ViewerMapping; reason?: string }
@@ -167,6 +168,11 @@ export class HerdrFollowView {
         const currentMember = view.task.members.find(m => keyOf(m.key.hostId, m.key.nodeId) === tile.key);
         if (currentMember?.availability === "available") {
           const target = view.opened.find(p => p.seat === currentMember.seat);
+          // An upgrade can change only the tile script; the kept tile then follows the current target.
+          if (target?.attachArgv && tile.target.attachArgv && target.paneCommand !== tile.target.paneCommand
+            && target.readOnly === tile.target.readOnly && earlierTileAttachment(tile.target.attachArgv, target.attachArgv, target.readOnly)) {
+            tile.target = target;
+          }
           if (!target || target.paneCommand !== tile.target.paneCommand || target.readOnly !== tile.target.readOnly
             || JSON.stringify(target.attachArgv) !== JSON.stringify(tile.target.attachArgv)) {
             throw new Error(`${tile.seat}: Core's current attachment differs from the retained viewer; keeping the previous view unconfirmed.`);

packages/daemon/src/domain/terminal/herdr-viewer.ts

@@ -6,6 +6,7 @@ import path from "node:path";
 import { getDefaultOpenRigPath } from "../../openrig-compat.js";
 import { resolveOnPath } from "../../adapters/tmux-exec.js";
 import type { ComposedPane } from "./terminal-provider.js";
+import { earlierTileAttachment } from "./tile-attach.js";
 
 export interface ViewerKey { hostId: string; nodeId: string }
 export interface ViewerMapping { workspaceId: string; paneId: string; terminalId: string; readOnly: boolean }
@@ -133,6 +134,12 @@ export class HerdrViewer {
       const id = JSON.stringify([key.hostId, key.nodeId]);
       entry = journal.entries[id];
       let created = false;
+      // An upgrade can change only the tile script (argv[2]); the seat it attaches is the same, so the kept
+      // viewer moves to the current script.
+      if (entry && entry.command !== target.paneCommand && entry.readOnly === target.readOnly && Array.isArray(entry.argv)
+        && earlierTileAttachment(entry.argv, target.attachArgv, target.readOnly)) {
+        entry.command = target.paneCommand; entry.argv = target.attachArgv; if (provision) save();
+      }
       if (entry && (entry.command !== target.paneCommand || entry.readOnly !== target.readOnly)) {
         return unconfirmed("The seat attachment changed; the existing viewer must be inspected before reprovisioning.");
       }

packages/daemon/src/domain/terminal/tile-attach.ts

@@ -1,3 +1,5 @@
+import { tileAttachArgv078, tileAttachArgv079 } from "./earlier-tile-attach.js";
+
 // A tile owns its attach process; no daemon watcher or steady-state polling.
 // Session names survive fresh launch, handover and restart. The node identity
 // must survive too: never follow a same-name session belonging to another seat.
@@ -44,3 +46,15 @@ export function tileAttachArgv(
   ].join(" ");
   return ["/bin/sh", "-c", script, "openrig-tile", session, tmux, ...server];
 }
+
+/** Whether a kept tile is the one an earlier release generated for the same seat: exactly the command 0.7.9's, or
+ * 0.7.5-0.7.8's, script makes for the current tile's session, tmux, server and read-only intent. An upgrade changes
+ * only OpenRig's own script; any other difference, an edited script included, is a different attachment. */
+export function earlierTileAttachment(kept: readonly string[], current: readonly string[], readOnly: boolean): boolean {
+  if (current.length < 6) return false;
+  const [session, tmux, ...server] = current.slice(4) as [string, string, ...string[]];
+  return [tileAttachArgv079, tileAttachArgv078].some((make) => {
+    const earlier = make(tmux, server, session, readOnly);
+    return earlier.length === kept.length && earlier.every((word, i) => word === kept[i]);
+  });
+}

packages/daemon/test/herdr-follow-view.test.ts

@@ -11,6 +11,7 @@ import type { ViewerPane } from "../src/domain/terminal/herdr-viewer.js";
 import { inspectComposerInput } from "../src/domain/session-transport.js";
 import type { ComposerSnapshot } from "../src/domain/composer-prompts.js";
 import { codexTwoRowComposer } from "./fixtures/codex-two-row-composer.js";
+import { tileAttachArgv078, tileAttachArgv079 } from "../src/domain/terminal/earlier-tile-attach.js";
 
 const roots: string[] = [];
 afterEach(() => { for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true }); });
@@ -417,3 +418,44 @@ it("keeps a real empty mission workspace enabled through Core with a mission wai
     expect((await f.service.status()).following).toHaveLength(1);
   } finally { await f.service.stop(); }
 });
+
+describe("follow views kept from an earlier release", () => {
+  const quote = (s: string) => "'" + s.replace(/'/g, "'\"'\"'") + "'";
+  it.each([["0.7.9", tileAttachArgv079], ["0.7.8", tileAttachArgv078]] as const)(
+    "a writable follow view kept from %s still adds and closes tiles, and moves to the current script", async (_, make) => {
+      const f = fixture(); expect(await f.follow()).toMatchObject({ ok: true, opened: ["a", "b", "c"] });
+      // Rewrite the saved canvas and the viewer journal as that release left them.
+      const earlier = (seat: string) => make("/usr/bin/tmux", ["-S", "/tmp/follow-seats.sock"], seat, false);
+      const canvasFile = path.join(path.dirname(path.dirname(f.journal())), "canvas.json");
+      const canvas = JSON.parse(readFileSync(canvasFile, "utf8"));
+      for (const tile of canvas.tiles) { tile.target.attachArgv = earlier(tile.seat); tile.target.paneCommand = tile.target.attachArgv.map(quote).join(" "); }
+      writeFileSync(canvasFile, JSON.stringify(canvas));
+      const journal = JSON.parse(readFileSync(f.journal(), "utf8"));
+      for (const [id, entry] of Object.entries<{ argv: string[]; command: string }>(journal.entries)) {
+        entry.argv = earlier(JSON.parse(id)[1]); entry.command = entry.argv.map(quote).join(" ");
+      }
+      writeFileSync(f.journal(), JSON.stringify(journal));
+
+      expect(await f.follow(["a", "b", "c", "d"])).toMatchObject({ ok: true, opened: ["a", "b", "c", "d"] });
+      await f.follow(["a", "b", "d"]); f.advance(4000);
+      expect(await f.follow(["a", "b", "d"])).toMatchObject({ ok: true, opened: ["a", "b", "d"] });
+      expect(f.calls.filter(c => c[1] === "close")).toHaveLength(1);
+      const current = task(["a", "b", "d"]).opened;
+      expect(JSON.parse(readFileSync(canvasFile, "utf8")).tiles.map((t: { target: { paneCommand: string } }) => t.target.paneCommand))
+        .toEqual(current.map(t => t.paneCommand));
+    });
+
+  it("holds a kept follow view whose tile script only shares a tile script's opening, and leaves its saved tiles", async () => {
+    const f = fixture(); expect(await f.follow()).toMatchObject({ ok: true, opened: ["a", "b", "c"] });
+    const canvasFile = path.join(path.dirname(path.dirname(f.journal())), "canvas.json");
+    const canvas = JSON.parse(readFileSync(canvasFile, "utf8"));
+    const tile = canvas.tiles[0];
+    tile.target.attachArgv[2] = 'target=$1; shift; printf "%s\\n" "custom viewer";';
+    tile.target.paneCommand = tile.target.attachArgv.map(quote).join(" ");
+    writeFileSync(canvasFile, JSON.stringify(canvas));
+    const before = f.calls.length;
+    expect(await f.follow(["a", "b", "c", "d"])).toMatchObject({ ok: false, following: { state: "retained" }, error: expect.stringContaining("current attachment differs") });
+    expect(f.calls.slice(before).some(c => ["create", "split", "run", "close"].includes(c[1]!))).toBe(false);
+    expect(JSON.parse(readFileSync(canvasFile, "utf8")).tiles[0].target).toEqual(tile.target);
+  });
+});

packages/daemon/test/herdr-viewer.test.ts

@@ -4,6 +4,7 @@ import { tmpdir } from "node:os";
 import path from "node:path";
 import { HerdrViewer, type HerdrViewerRun } from "../src/domain/terminal/herdr-viewer.js";
 import { tileAttachArgv } from "../src/domain/terminal/tile-attach.js";
+import { tileAttachArgv078, tileAttachArgv079 } from "../src/domain/terminal/earlier-tile-attach.js";
 import { composeView } from "../src/domain/terminal/view-composer.js";
 import { Hono } from "hono";
 import { HerdrAdapter } from "../src/domain/terminal/herdr-adapter.js";
@@ -230,3 +231,70 @@ describe("owned Herdr viewer", () => {
     expect((await send("Bearer fixture-owner-token")).status).toBe(200); expect(calls).toBe(1);
   });
 });
+
+describe("viewers kept from an earlier release", () => {
+  const quote = (s: string) => "'" + s.replace(/'/g, "'\"'\"'") + "'";
+  const id = JSON.stringify([key.hostId, key.nodeId]);
+  const readOnlyTarget = () => composeView("qitem:qitem-root", [{ seat: "a@team", label: "a", tmuxSession: "a@team", host: null, alive: true, readOnly: true }],
+    { resolveHost: () => null, localTmux: "/usr/bin/tmux", localTmuxServer: ["-S", "/tmp/own-tmux.sock"] }).opened[0]!;
+  /** Rewrite the journal as an earlier release left it: that release's script for a seat, with the same tmux and server. */
+  function keepFrom(f: ReturnType<typeof fixture>, make: typeof tileAttachArgv079, readOnly: boolean, session = "a@team") {
+    const file = path.join(f.root, readdirSync(f.root).find(n => n.endsWith(".json"))!);
+    const journal = JSON.parse(readFileSync(file, "utf8"));
+    const entry = journal.entries[id];
+    entry.argv = make("/usr/bin/tmux", ["-S", "/tmp/own-tmux.sock"], session, readOnly);
+    entry.command = entry.argv.map(quote).join(" "); entry.readOnly = readOnly;
+    writeFileSync(file, JSON.stringify(journal));
+    return () => JSON.parse(readFileSync(file, "utf8")).entries[id] as { command: string; argv: string[] };
+  }
+
+  it.each([["0.7.9", tileAttachArgv079], ["0.7.8", tileAttachArgv078]] as const)(
+    "keeps a writable viewer recorded by %s and moves it to the current script", async (_, make) => {
+      const f = fixture(); expect((await f.viewer().ensure(key, f.target)).ok).toBe(true);
+      const stored = keepFrom(f, make, false);
+      expect(stored().command).not.toBe(f.target.paneCommand);
+      expect(await f.viewer().lookup(key, f.target)).toMatchObject({ ok: true });
+      expect(await f.viewer().ensure(key, f.target)).toMatchObject({ ok: true, created: false });
+      expect(stored()).toMatchObject({ command: f.target.paneCommand, argv: f.target.attachArgv });
+      expect(f.calls.filter(a => a[1] === "create")).toHaveLength(1);
+    });
+
+  it.each([["0.7.9", tileAttachArgv079], ["0.7.8", tileAttachArgv078]] as const)(
+    "keeps a read-only viewer recorded by %s", async (_, make) => {
+      const f = fixture(); f.setAttachFlags(["-r"]); const target = readOnlyTarget();
+      expect((await f.viewer().ensure(key, target)).ok).toBe(true);
+      keepFrom(f, make, true);
+      expect(await f.viewer().ensure(key, target)).toMatchObject({ ok: true, created: false });
+      expect(f.calls.filter(a => a[1] === "create")).toHaveLength(1);
+    });
+
+  it("still holds a kept viewer for another seat, for the other mode, or under a command that isn't a tile script", async () => {
+    const f = fixture(); expect((await f.viewer().ensure(key, f.target)).ok).toBe(true);
+    keepFrom(f, tileAttachArgv079, false, "other@team");
+    expect(await f.viewer().ensure(key, f.target)).toMatchObject({ ok: false, code: "viewer_unconfirmed" });
+    keepFrom(f, tileAttachArgv079, true);
+    expect(await f.viewer().ensure(key, f.target)).toMatchObject({ ok: false, code: "viewer_unconfirmed" });
+    const stored = keepFrom(f, tileAttachArgv079, false);
+    const file = path.join(f.root, readdirSync(f.root).find(n => n.endsWith(".json"))!);
+    const journal = JSON.parse(readFileSync(file, "utf8"));
+    journal.entries[id].argv[2] = "exec /usr/bin/tmux attach"; journal.entries[id].command = journal.entries[id].argv.map(quote).join(" ");
+    writeFileSync(file, JSON.stringify(journal));
+    expect(await f.viewer().ensure(key, f.target)).toMatchObject({ ok: false, code: "viewer_unconfirmed" });
+    expect(stored().argv[2]).toBe("exec /usr/bin/tmux attach");
+    expect(f.calls.filter(a => a[1] === "create")).toHaveLength(1);
+  });
+
+  it("holds a kept viewer whose script only shares a tile script's opening, and leaves its saved command", async () => {
+    const f = fixture(); expect((await f.viewer().ensure(key, f.target)).ok).toBe(true);
+    const stored = keepFrom(f, tileAttachArgv079, false);
+    const file = path.join(f.root, readdirSync(f.root).find(n => n.endsWith(".json"))!);
+    const journal = JSON.parse(readFileSync(file, "utf8"));
+    journal.entries[id].argv[2] = 'target=$1; shift; printf "%s\\n" "custom viewer";';
+    journal.entries[id].command = journal.entries[id].argv.map(quote).join(" ");
+    writeFileSync(file, JSON.stringify(journal));
+    const before = stored();
+    expect(await f.viewer().lookup(key, f.target)).toMatchObject({ ok: false, code: "viewer_unconfirmed" });
+    expect(await f.viewer().ensure(key, f.target)).toMatchObject({ ok: false, code: "viewer_unconfirmed" });
+    expect(stored()).toEqual(before);
+  });
+});