mvschwarz/openrig
1
0 Fork 0
mirror of https://github.com/mvschwarz/openrig.git synced 2026-11-07 05:56:21 -10:00
Code Issues Packages Projects Releases 1 Wiki Activity

fix: preserve original viewer failures in follow notices (#2289)

Browse Source 
A later hold notice for a slice viewer now keeps the first viewer failure next to the current confirmation state, instead of replacing it with a generic not-confirmed message. The first failure is stored per member in the follower's existing presentation journal, bound to the exact attachment, saved before the bounded confirmation reads, cleared only on positive verification or a confirmed tile close, and kept while a departed but uncertain tile is still shown. Old canvases stay readable.
This commit is contained in:
Mike Schwarz authored and GitHub committed 2026-10-29 14:30:57 -07:00
1 parent 957dd83bd1
commit ee41cf610c
2 files changed
+61 -8
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

packages/daemon/src/domain/terminal/herdr-follow-view.ts
+17 -4
View File
@@ -14,6 +14,7 @@ interface Canvas {
version: 1; cwd: string; phase: "creating" | "created" | "starting";
pane?: ViewerPane; policy: FollowPolicy; tiles: Tile[]; events: string[];
closing?: string;
viewerFailures?: Record<string, { target: string; reason: string }>;
}
export interface HerdrFollowDeps {
socketPath: string;
@@ -65,6 +66,13 @@ export class HerdrFollowView {
const notice = (line: string) => {
if (state!.events.at(-1) !== line) state!.events = [...state!.events, line].slice(-60);
};
const viewerFailure = (key: string, target: ComposedPane, failure: { phase?: string; error: string }) => {
const current = `${failure.phase ?? "unknown"}: ${failure.error}`;
const attachment = JSON.stringify([target.paneCommand, target.attachArgv, target.readOnly]);
const failures = state!.viewerFailures ??= {};
if (!failures[key] || failures[key].target !== attachment) failures[key] = { target: attachment, reason: current };
return `current confirmation: ${current}; first failure: ${failures[key]!.reason}`;
};
try {
try {
const saved = JSON.parse(readFileSync(file, "utf8")) as Canvas;
@@ -130,6 +138,9 @@ export class HerdrFollowView {
throw new Error("The explanation strip process is unconfirmed; pane run was not repeated.");
}
const members = view.task.members.map(m => ({ key: keyOf(m.key.hostId, m.key.nodeId), seat: m.seat }));
for (const key of Object.keys(state.viewerFailures ?? {})) {
if (!members.some(member => member.key === key) && !state.tiles.some(tile => tile.key === key)) delete state.viewerFailures![key];
}
const controlsToKeys = (seats: string[] = []) => seats.map(seat => {
const member = members.find(m => m.seat === seat) ?? state!.tiles.find(m => m.seat === seat);
if (!member) throw new Error(`No current or retained member named ${seat}.`);
@@ -165,8 +176,9 @@ export class HerdrFollowView {
try { current = await currentPane(tile.viewer); }
catch (error) { throw new Error(`${tile.seat}: unknown: ${error instanceof Error ? error.message : String(error)}`); }
const verified = await viewers.lookup(tile.member, tile.target);
if (!verified.ok) throw new Error(`${tile.seat}: ${verified.phase ?? "unknown"}: ${verified.error} Keeping the whole view.`);
if (!verified.ok) throw new Error(`${tile.seat}: ${viewerFailure(tile.key, tile.target, verified)} Keeping the whole view.`);
if (JSON.stringify(verified.viewer) !== JSON.stringify(tile.viewer)) throw new Error(`${tile.seat}: Viewer mapping changed; keeping the whole view.`);
delete state.viewerFailures?.[tile.key];
observations.set(tile.key, { focused: current.focused!, input: await this.deps.input?.(tile.target) ?? "unknown" });
}
const plan = planFollowingTiles(state.tiles, members, true, observations, policy, now());
@@ -189,12 +201,13 @@ export class HerdrFollowView {
"--direction", anchor && state!.tiles.length % 2 ? "right" : "down", "--no-focus", "--cwd", cwd]));
return JSON.stringify({ root_pane: split.pane });
});
const originalFailure = !verified.ok ? `${verified.phase ?? "unknown"}: ${verified.error}` : undefined;
if (!verified.ok) { viewerFailure(join.key, target, verified); save(); }
for (let attempt = 0; !verified.ok && attempt < 5; attempt++) {
await new Promise(resolve => setTimeout(resolve, 50));
verified = await viewers.lookup(memberKey, target); // read-only: never repeats a create/run.
}
if (!verified.ok) { notice(`${join.seat}: ${originalFailure}; confirmation: ${verified.phase ?? "unknown"}: ${verified.error}`); continue; }
if (!verified.ok) { notice(`${join.seat}: ${viewerFailure(join.key, target, verified)}`); continue; }
delete state.viewerFailures?.[join.key];
state.tiles.push({ key: join.key, seat: join.seat, target, member: { hostId: member.key.hostId, nodeId: member.key.nodeId }, viewer: verified.viewer });
notice(`${join.seat}: ${join.reason}`); save();
}
@@ -239,7 +252,7 @@ export class HerdrFollowView {
state.closing = tile.key; save();
await this.deps.run(["pane", "close", tile.viewer.paneId]);
if (!await viewers.forgetClosed(tile.member, tile.viewer)) throw new Error("Tile close remains unconfirmed; no second close was attempted.");
state.tiles = state.tiles.filter(t => t.key !== tile.key); delete state.closing; notice(`${tile.seat}: ${leaving.reason}`); save();
state.tiles = state.tiles.filter(t => t.key !== tile.key); delete state.viewerFailures?.[tile.key]; delete state.closing; notice(`${tile.seat}: ${leaving.reason}`); save();
}
for (const retained of plan.retained) {
const tile = state.tiles.find(t => t.key === retained.key);
packages/daemon/test/herdr-follow-view.test.ts
+44 -4
View File
@@ -30,7 +30,7 @@ function fixture() {
const frames = new Map<string, ComposerSnapshot | null>();
let transform = (view: ComposedView) => view;
let lateFocus: { seat: string; reads: number } | undefined;
let clock = 100, lostCreate = false, lostClose = false, readsFailed = false, hideViewers = false;
let clock = 100, lostCreate = false, lostClose = false, readsFailed = false, hideViewers = false, failedViewerRun = false, failedProcessRead = false;
const run = async (args: string[]) => {
calls.push(args);
const command = args.slice(0, 2).join(" ");
@@ -53,12 +53,14 @@ function fixture() {
const selected = panes.get(args[2]!); if (!selected) throw new Error("missing pane");
return JSON.stringify({ result: { pane: selected } });
}
if (command === "pane process-info" && failedProcessRead && panes.get(args[3]!)!.cwd.includes("/viewers/")) { failedProcessRead = false; throw new Error("first viewer inspection failed"); }
if (command === "pane process-info") return JSON.stringify({ result: { process_info: { foreground_processes: [{ argv: panes.get(args[3]!)!.argv }] } } });
if (command === "pane run") {
const selected = panes.get(args[2]!)!;
if (args[3]!.startsWith("/usr/bin/tail")) {
selected.argv = ["/usr/bin/tail", "-n", "30", "-F", path.join(selected.cwd, "..", "events.log")];
} else {
if (failedViewerRun) { failedViewerRun = false; throw new Error("first attach dispatch failed"); }
// The shared composer produces one wrapper per actual seat.
const seat = ["a", "b", "c", "d"].find(s => args[3]!.includes(`'${s}'`))!;
selected.argv = ["/usr/bin/tmux", "-S", "/tmp/follow-seats.sock", "attach", "-f", "ignore-size", "-t", `$${seat}`];
@@ -96,6 +98,9 @@ function fixture() {
};
const exitViewer = (seat: string) => { [...panes.values()].find(p => p.argv.at(-1) === `$${seat}`)!.argv = ["/bin/bash"]; };
return { follow, create, panes, calls, rpc, inputs, frames, exitViewer, journal,
failViewerRun: () => { failedViewerRun = true; },
failProcessRead: () => { failedProcessRead = true; },
confirmViewer: (seat: string) => { const entry = JSON.parse(readFileSync(journal(), "utf8")).entries[JSON.stringify(["self", seat])]; panes.get(entry.pane.pane_id)!.argv = ["/usr/bin/tmux", "-S", "/tmp/follow-seats.sock", "attach", "-f", "ignore-size", "-t", `$${seat}`]; },
hideViewers: () => { hideViewers = true; }, advance: (ms: number) => { clock += ms; },
loseCreate: () => { lostCreate = true; }, loseClose: () => { lostClose = true; }, failReads: () => { readsFailed = true; },
changeTarget: () => { transform = view => ({ ...view, opened: view.opened.map(target => target.seat === "a"
@@ -134,11 +139,46 @@ describe("stable task-following workspace", () => {
expect(held.error).toContain("The viewer process is not confirmed");
await f.follow(); expect(f.calls.slice(count).some(c => ["run", "close", "split"].includes(c[1]!))).toBe(false);
});
it("keeps the first dispatch failure in later notices across refresh and recreation", async () => {
const f = fixture(); await f.follow(); f.focusSeat("b"); f.inputs.set("b", "text"); f.failViewerRun();
const held = await f.follow(["a", "b", "c", "d"]);
expect(held.following!.events.at(-1)).toContain("first failure: starting: Viewer effect is unconfirmed: first attach dispatch failed");
const count = f.calls.length;
const resumed = f.create();
for (let i = 0; i < 2; i++) {
const later = await resumed.follow("qitem:task", task(["a", "b", "c", "d"]), { enabled: true }, async () => true);
expect(later.following!.events.at(-1)).toContain("current confirmation: starting: The viewer process is not confirmed");
expect(later.following!.events.at(-1)).toContain("first failure: starting: Viewer effect is unconfirmed: first attach dispatch failed");
}
expect(f.calls.slice(count).some(c => ["run", "split", "close", "create"].includes(c[1]!))).toBe(false);
expect(f.inputs.get("b")).toBe("text");
expect([...f.panes.values()].find(p => p.argv.at(-1) === "$b")?.focused).toBe(true);
});
it("keeps the original failure when membership drops a still-retained uncertain tile", async () => {
const f = fixture(); await f.follow(); f.exitViewer("a"); f.failProcessRead();
const first = await f.follow();
expect(first.error).toContain("first failure: starting: Viewer effect is unconfirmed: first viewer inspection failed");
const count = f.calls.length;
const later = await f.follow(["b", "c"]);
expect(later).toMatchObject({ ok: false, opened: ["a", "b", "c"] });
expect(later.error).toContain("current confirmation: starting: The viewer process is not confirmed");
expect(later.error).toContain("first failure: starting: Viewer effect is unconfirmed: first viewer inspection failed");
expect(f.calls.slice(count).some(c => ["run", "split", "close", "create"].includes(c[1]!))).toBe(false);
});
it("clears a resolved failure before reporting a later viewer hold", async () => {
const f = fixture(); await f.follow(); f.failViewerRun(); await f.follow(["a", "b", "c", "d"]);
f.confirmViewer("d");
expect(await f.follow(["a", "b", "c", "d"])).toMatchObject({ opened: ["a", "b", "c", "d"] });
f.exitViewer("d");
const held = await f.follow(["a", "b", "c", "d"]);
expect(held.error).toContain("first failure: starting: The viewer process is not confirmed");
expect(held.error).not.toContain("first attach dispatch failed");
});
it("a known refusal before split preserves its reason and allows a later join", async () => {
const f = fixture(); await f.follow(); const count = f.calls.length; let reads = 0;
const held = await f.follow(["a", "b", "c", "d"], {}, true, async () => ++reads !== 3);
expect(held.opened).toEqual(["a", "b", "c"]);
expect(held.following!.events.join("\n")).toContain("d: creating: Membership changed before the split.");
expect(held.following!.events.join("\n")).toContain("first failure: creating: Membership changed before the split.");
expect(f.calls.slice(count).some(c => ["split", "run", "close"].includes(c[1]!))).toBe(false);
expect(await f.follow(["a", "b", "c", "d"])).toMatchObject({ ok: true, opened: ["a", "b", "c", "d"] });
expect(f.calls.slice(count).filter(c => c[1] === "split")).toHaveLength(1);
@@ -151,7 +191,7 @@ describe("stable task-following workspace", () => {
retainedFile = path.join(entry.cwd, "retained-note"); writeFileSync(retainedFile, "keep this file");
return false;
});
expect(held.following!.events.join("\n")).toContain("d: creating: Membership changed before the split.");
expect(held.following!.events.join("\n")).toContain("first failure: creating: Membership changed before the split.");
expect(JSON.parse(readFileSync(f.journal(), "utf8")).entries[JSON.stringify(["self", "d"])]).toBeUndefined();
expect(readFileSync(retainedFile, "utf8")).toBe("keep this file");
expect(await f.follow(["a", "b", "c", "d"])).toMatchObject({ ok: true, opened: ["a", "b", "c", "d"] });
@@ -160,7 +200,7 @@ describe("stable task-following workspace", () => {
it("an uncertain dispatched split keeps its intent even when enumeration is empty", async () => {
const f = fixture(); await f.follow(); f.loseCreate(); f.hideViewers(); const count = f.calls.length;
const held = await f.follow(["a", "b", "c", "d"]);
expect(held.following!.events.join("\n")).toContain("d: creating: Viewer effect is unconfirmed: lost create response");
expect(held.following!.events.join("\n")).toContain("first failure: creating: Viewer effect is unconfirmed: lost create response");
const before = readFileSync(f.journal(), "utf8");
await f.follow(["a", "b", "c", "d"]);
expect(readFileSync(f.journal(), "utf8")).toBe(before);