From 5d02173aee60ee61c4d5e043f8017aa13229ad56 Mon Sep 17 00:00:00 2001 From: Colin Neilens Date: Fri, 9 Oct 2026 23:20:38 -0700 Subject: [PATCH 1/4] Harden Windows per-loop terminal layout cleanup Only end a shell session this shell provably made (graphcode- plus a lowercase uuid, no node's id, no other layout's pane) and a fresh zmx ls shows running; refuse and log anything else instead of killing it. Retry saved shell tabs after a failed listing and on reopening the open loop. Require two agreeing non-empty snapshots, and layouts older than this shell, before sweeping deleted loops. Keep unusable layouts as .bad, write layouts atomically with a per-writer temporary file. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Colin Neilens --- graphcode-windows/src/App.zig | 455 ++++++++++++++++++-- graphcode-windows/src/TerminalSurface.zig | 394 +++++++++++++++-- graphcode-windows/src/TerminalWorkspace.zig | 1 + graphcode-windows/src/WorkspaceLayout.zig | 253 ++++++++++- graphcode-windows/src/ZmxSession.zig | 87 ++++ 5 files changed, 1082 insertions(+), 108 deletions(-) diff --git a/graphcode-windows/src/App.zig b/graphcode-windows/src/App.zig index d2570972..b48b0d30 100644 --- a/graphcode-windows/src/App.zig +++ b/graphcode-windows/src/App.zig @@ -2072,13 +2072,29 @@ pub const App = struct { /// sessions (the daemon only knows a loop's agent), so the shell reconciles its saved /// loop layouts with each project's graph: a loop that vanished since the previous /// snapshot, or, on the first snapshot of a project, one deleted while the shell was - /// closed. Only layouts saved for this project are considered. + /// closed. Only layouts saved for this project are considered, spelled exactly as the + /// daemon spells it. + /// + /// A snapshot alone is never proof a loop is gone: one cut short by a daemon restart, or + /// a replay of a stale one, lacks loops that still exist. So an empty snapshot proves + /// nothing, a loop must be missing from two successive snapshots before its sessions are + /// ended, and on the first snapshot only layouts older than this shell are considered (a + /// newer one is another shell's, whose loop this snapshot may simply predate). fn endDeletedLoops(self: *App, identity: LoopIdentity) void { const workspace = self.workspace orelse return; const graph = self.model.graphFor(identity.project) orelse return; + if (graph.nodes.items.len == 0) return; + const known: TerminalWorkspace.KnownNodes = .{ .context = graph, .contains = graphNamesNode }; + + const previous = workspace.takeMissingLoops(identity.project) catch return; + defer workspace.freeLoopIds(previous); + for (previous) |id| { + if (!graphHasLoop(graph, id)) _ = workspace.retireLoopGuarded(id, identity.project, known); + } if (identity.ids) |ids| { for (ids) |id| { - if (!graphHasLoop(graph, id)) _ = workspace.retireLoop(id, identity.project); + if (graphHasLoop(graph, id) or containsLoopId(previous, id)) continue; + workspace.markLoopMissing(identity.project, id) catch {}; } return; } @@ -2086,17 +2102,31 @@ pub const App = struct { defer @import("WorkspaceLayout.zig").freeLoopRecords(workspace.allocator, saved); for (saved) |record| { if (!std.mem.eql(u8, record.project, identity.project)) continue; - if (!graphHasLoop(graph, record.loop)) _ = workspace.retireLoop(record.loop, identity.project); + if (record.modified_ns >= workspace.started_ns) continue; + if (graphHasLoop(graph, record.loop) or containsLoopId(previous, record.loop)) continue; + workspace.markLoopMissing(identity.project, record.loop) catch {}; + } + } + + fn containsLoopId(ids: []const []u8, id: []const u8) bool { + for (ids) |candidate| { + if (std.mem.eql(u8, candidate, id)) return true; } + return false; + } + + fn graphNamesNode(context: *const anyopaque, id: []const u8) bool { + const graph: *const GraphModel.GraphSummary = @ptrCast(@alignCast(context)); + return graphHasLoop(graph, id); } - /// Whether `id` still names a loop of `graph`, at its top level or inside a composite. - /// Nested loops are matched by text in their parent's stored subgraph, so any doubt keeps - /// a loop's sessions rather than ending them. + /// Whether `id` still names a loop of `graph`, at its top level or inside a composite, + /// whatever case either is written in. Nested loops are matched by text in their parent's + /// stored subgraph, so any doubt keeps a loop's sessions rather than ending them. fn graphHasLoop(graph: *const GraphModel.GraphSummary, id: []const u8) bool { - if (GraphModel.findNodeIndexByID(graph.nodes.items, id) != null) return true; for (graph.nodes.items) |node| { - if (node.subgraph_json.len != 0 and std.mem.indexOf(u8, node.subgraph_json, id) != null) return true; + if (std.ascii.eqlIgnoreCase(node.id, id)) return true; + if (node.subgraph_json.len != 0 and std.ascii.indexOfIgnoreCase(node.subgraph_json, id) != null) return true; } return false; } @@ -12943,7 +12973,8 @@ test "collapsed loop panel routes native tab and loop-bar clicks to the workspac } /// A `zmx` stand-in for live-terminal tests: `ls` prints `live.txt` (the daemon's running -/// sessions); `attach` records the directory it was started in, then blocks on the +/// sessions), fails while `ls-fail.txt` exists and hangs while `ls-hang.txt` does; +/// `kill` appends its whole argv to `kills.txt`; `attach` records the directory it was started in, then blocks on the /// terminal's input pipe like a real attach until its session is killed. const fake_zmx_script = "@echo off\r\n" ++ @@ -12952,6 +12983,8 @@ const fake_zmx_script = "if \"%~1\"==\"kill\" goto kill\r\n" ++ "exit /b 0\r\n" ++ ":ls\r\n" ++ + "if exist \"%~dp0ls-hang.txt\" ping -n 8 127.0.0.1 >nul\r\n" ++ + "if exist \"%~dp0ls-fail.txt\" exit /b 1\r\n" ++ "if exist \"%~dp0live.txt\" type \"%~dp0live.txt\"\r\n" ++ "exit /b 0\r\n" ++ ":kill\r\n" ++ @@ -12972,6 +13005,8 @@ const bare_zmx_forwarder = const LiveLoop = struct { id: []const u8, state: []const u8 = "idle", worktree: ?[]const u8 = null }; +const SavedPane = struct { id: []const u8, agent: bool = false }; + const TerminalChild = struct { visible: bool, rect: c.RECT }; /// A loop workspace whose terminals are real winghostty surfaces (child windows of the @@ -13238,6 +13273,66 @@ const LiveTerminalFixture = struct { for (0..40) |_| self.tick(); } + /// Turns a fake-zmx behavior on or off by its marker file (`ls-fail.txt`, `ls-hang.txt`). + fn setFlag(self: *LiveTerminalFixture, name: []const u8, on: bool) !void { + if (on) { + try self.tmp.dir.writeFile(.{ .sub_path = name, .data = "on" }); + } else { + self.tmp.dir.deleteFile(name) catch {}; + } + } + + /// Every `zmx kill` line recorded so far, trimmed, is exactly `expected`. + fn expectKills(self: *LiveTerminalFixture, expected: []const u8) !void { + const text = try self.kills(); + defer std.testing.allocator.free(text); + try std.testing.expectEqualStrings(expected, std.mem.trim(u8, text, " \r\n")); + } + + /// The model has not seen the project's graph yet, as at startup or after a daemon restore. + fn forgetGraph(self: *LiveTerminalFixture) !void { + try std.testing.expect(self.app.model.applyLifecycle(.close, self.project)); + try std.testing.expect(self.app.model.graphFor(self.project) == null); + } + + /// Saved layouts count as written before this shell started, as one left by an earlier run. + fn predateLayouts(self: *LiveTerminalFixture) void { + if (@hasField(TerminalWorkspace.Workspace, "started_ns")) self.workspace.started_ns = std.math.maxInt(i128); + } + + /// A graph snapshot for the fixture's project given as raw node JSON. + fn deliverNodes(self: *LiveTerminalFixture, nodes_json: []const u8) !void { + const value = try std.fmt.allocPrint( + std.testing.allocator, + "{{\"version\":2,\"kind\":\"event\",\"sequence\":{d},\"event\":{{\"graphChanged\":{{\"project\":{{\"path\":{f},\"name\":\"Core\"}},\"nodes\":[{s}],\"edges\":[]}}}}}}", + .{ self.sequence, std.json.fmt(self.project, .{}), nodes_json }, + ); + defer std.testing.allocator.free(value); + self.sequence += 1; + self.app.onFrameWithAccessibilityPublish(value, App.publishAccessibility); + // A project with no graph yet is queued for the daemon's subscription; none runs here. + if (self.app.pending_project_path.len != 0) { + std.testing.allocator.free(self.app.pending_project_path); + self.app.pending_project_path = &.{}; + } + } + + /// A loop's layout file as an earlier run saved it. + fn saveLoopLayout(self: *LiveTerminalFixture, project: []const u8, loop: []const u8, panes: []const SavedPane) !void { + const allocator = std.testing.allocator; + var layout = try @import("WorkspaceLayout.zig").Layout.initForLoop(allocator, project, loop); + defer layout.deinit(); + for (panes) |pane| try layout.addTab(pane.id, pane.agent); + const path = try std.fmt.allocPrint(allocator, "{s}\\{s}.json", .{ self.root, loop }); + defer allocator.free(path); + try layout.save(path); + } + + fn layoutExists(self: *LiveTerminalFixture, name: []const u8) bool { + self.tmp.dir.access(name, .{}) catch return false; + return true; + } + fn newTab(self: *LiveTerminalFixture) !void { const workspace = &self.workspace; const bounds = TerminalWorkspace.chromeControlBounds(workspace.layout_origin_x, workspace.layout_origin_y, workspace.layout_width, 0); @@ -14650,6 +14745,29 @@ test "workspace layout: closing a New Tab ends its shell session and never the l try std.testing.expect(!fixture.killed("loop-b")); } +test "workspace layout: closing a tab never ends a session that another loop's layout also names" { + var fixture: LiveTerminalFixture = undefined; + try fixture.init(&.{ .{ .id = "loop-a" }, .{ .id = "loop-b" } }); + defer fixture.deinit(); + const app = &fixture.app; + const workspace = &fixture.workspace; + + try fixture.setLive(&.{ "loop-a", "loop-b" }); + try clickSidebarLoopRow(app, fixture.project, "loop-a"); + try fixture.waitFor(LiveTerminalFixture.shows, "loop-a"); + try fixture.newTab(); + const shell = try fixture.selectedPane(0); + defer std.testing.allocator.free(shell); + try fixture.saveLoopLayout(fixture.project, "loop-b", &.{ .{ .id = "loop-b", .agent = true }, .{ .id = shell } }); + try fixture.setLive(&.{ "loop-a", "loop-b", shell }); + + const shell_tab = TerminalWorkspace.tabBounds(workspace.layout_origin_x, workspace.layout_origin_y, 1); + try nativeClick(app, shell_tab.right - 12, @divTrunc(shell_tab.top + shell_tab.bottom, 2)); + try std.testing.expectEqual(@as(usize, 1), workspace.tabCount()); + fixture.settle(); + try fixture.expectKills(""); +} + test "workspace layout: deleting a loop ends the shell sessions of its own tabs and no other loop's" { var fixture: LiveTerminalFixture = undefined; try fixture.init(&.{ .{ .id = "loop-a" }, .{ .id = "loop-b" } }); @@ -14675,6 +14793,10 @@ test "workspace layout: deleting a loop ends the shell sessions of its own tabs // Loop A is deleted while loop B is the one open. try fixture.setLive(&.{ "loop-b", a_tab, a_split, b_tab }); try fixture.deliver(&.{.{ .id = "loop-b" }}); + fixture.settle(); + try std.testing.expect(!fixture.killed(a_tab)); + // A second snapshot without loop A agrees it is gone. + try fixture.deliver(&.{.{ .id = "loop-b" }}); try fixture.waitFor(LiveTerminalFixture.killed, a_tab); try fixture.waitFor(LiveTerminalFixture.killed, a_split); fixture.settle(); @@ -14845,49 +14967,142 @@ test "workspace layout: a layout saved beside the executable is adopted once, by try std.testing.expectEqualStrings(legacy_before, legacy_after); } -test "workspace layout: a project's first snapshot ends the shells of loops deleted while the shell was closed, and only those" { +const shell_uuid_one = "5e11ba5e-0001-4000-8000-000000000001"; +const shell_uuid_two = "5e11ba5e-0002-4000-8000-000000000002"; +const shell_uuid_three = "5e11ba5e-0003-4000-8000-000000000003"; +const shell_uuid_four = "5e11ba5e-0004-4000-8000-000000000004"; +const shell_uuid_five = "5e11ba5e-0005-4000-8000-000000000005"; +const shell_uuid_six = "5e11ba5e-0006-4000-8000-000000000006"; +const shell_uuid_seven = "5e11ba5e-0007-4000-8000-000000000007"; +const agent_uuid_upper = "AAAA0000-0000-4000-8000-0000000000AA"; + +const node_loop_a = "{\"id\":\"loop-a\",\"title\":\"a\",\"loopType\":\"turnBased\",\"state\":{\"idle\":{}}}"; + +/// The snapshot the daemon sends once its project has loaded: loop A, a composite nesting +/// `nested-loop`, and `upper-loop`. +const nodes_full = node_loop_a ++ "," ++ + "{\"id\":\"group\",\"title\":\"g\",\"loopType\":\"composite\",\"state\":{\"idle\":{}},\"subGraph\":{\"nodes\":[{\"id\":\"nested-loop\",\"title\":\"n\",\"state\":{\"idle\":{}}}],\"edges\":[]}}," ++ + "{\"id\":\"upper-loop\",\"title\":\"u\",\"loopType\":\"turnBased\",\"state\":{\"idle\":{}}}"; + +test "workspace layout: a project's first snapshot ends the shells of loops deleted while the shell was closed, once a second snapshot agrees, and only those" { var fixture: LiveTerminalFixture = undefined; try fixture.init(&.{ .{ .id = "loop-a" } }); defer fixture.deinit(); - const allocator = std.testing.allocator; - const WorkspaceLayoutFile = @import("WorkspaceLayout.zig"); - const Case = struct { loop: []const u8, project: []const u8, shell: []const u8 }; const other_project = "C:\\another-project"; - const cases = [_]Case{ - .{ .loop = "gone-loop", .project = fixture.project, .shell = "gone-shell" }, - .{ .loop = "loop-a", .project = fixture.project, .shell = "a-shell" }, - .{ .loop = "nested-loop", .project = fixture.project, .shell = "nested-shell" }, - .{ .loop = "elsewhere-loop", .project = other_project, .shell = "elsewhere-shell" }, - }; - for (cases) |case| { - var layout = try WorkspaceLayoutFile.Layout.initForLoop(allocator, case.project, case.loop); - defer layout.deinit(); - try layout.addTab(case.loop, true); - try layout.addTab(case.shell, false); - const path = try std.fmt.allocPrint(allocator, "{s}\\{s}.json", .{ fixture.root, case.loop }); - defer allocator.free(path); - try layout.save(path); - } - const nested = try std.fmt.allocPrint( - allocator, - "{{\"version\":2,\"kind\":\"event\",\"sequence\":{d},\"event\":{{\"graphChanged\":{{\"project\":{{\"path\":{f},\"name\":\"Core\"}},\"nodes\":[" ++ - "{{\"id\":\"loop-a\",\"title\":\"a\",\"loopType\":\"turnBased\",\"state\":{{\"idle\":{{}}}}}}," ++ - "{{\"id\":\"group\",\"title\":\"g\",\"loopType\":\"composite\",\"state\":{{\"idle\":{{}}}},\"subGraph\":{{\"nodes\":[{{\"id\":\"nested-loop\",\"title\":\"n\",\"state\":{{\"idle\":{{}}}}}}],\"edges\":[]}}}}" ++ - "],\"edges\":[]}}}}}}", - .{ fixture.sequence, std.json.fmt(fixture.project, .{}) }, - ); - defer allocator.free(nested); - fixture.sequence += 1; - _ = try fixture.app.model.updateFromFrame(nested); + const alias_project = try std.mem.replaceOwned(u8, std.testing.allocator, fixture.project, "\\", "/"); + defer std.testing.allocator.free(alias_project); + + try fixture.saveLoopLayout(fixture.project, "gone-loop", &.{ .{ .id = "gone-loop", .agent = true }, .{ .id = shell_uuid_one } }); + try fixture.saveLoopLayout(fixture.project, "loop-a", &.{ .{ .id = "loop-a", .agent = true }, .{ .id = shell_uuid_two } }); + try fixture.saveLoopLayout(fixture.project, "nested-loop", &.{ .{ .id = "nested-loop", .agent = true }, .{ .id = shell_uuid_three } }); + try fixture.saveLoopLayout(other_project, "elsewhere-loop", &.{ .{ .id = "elsewhere-loop", .agent = true }, .{ .id = shell_uuid_four } }); + try fixture.saveLoopLayout(alias_project, "alias-loop", &.{ .{ .id = "alias-loop", .agent = true }, .{ .id = shell_uuid_five } }); + try fixture.saveLoopLayout(fixture.project, "Upper-Loop", &.{ .{ .id = "Upper-Loop", .agent = true }, .{ .id = shell_uuid_six } }); + try fixture.setLive(&.{ "loop-a", shell_uuid_one, shell_uuid_two, shell_uuid_three, shell_uuid_four, shell_uuid_five, shell_uuid_six }); + fixture.predateLayouts(); + try fixture.forgetGraph(); + + // The first snapshot only records what is missing: one report is not proof. + try fixture.deliverNodes(nodes_full); + fixture.settle(); + try std.testing.expect(!fixture.killed(shell_uuid_one)); + try std.testing.expect(fixture.layoutExists("gone-loop.json")); - fixture.app.endDeletedLoops(.{ .project = @constCast(fixture.project), .ids = null }); - try fixture.waitFor(LiveTerminalFixture.killed, "gone-shell"); + try fixture.deliverNodes(nodes_full); + try fixture.waitFor(LiveTerminalFixture.killed, shell_uuid_one); fixture.settle(); - for ([_][]const u8{ "a-shell", "nested-shell", "elsewhere-shell", "gone-loop", "loop-a" }) |kept| { + for ([_][]const u8{ shell_uuid_two, shell_uuid_three, shell_uuid_four, shell_uuid_five, shell_uuid_six }) |kept| { try std.testing.expect(!fixture.killed(kept)); } - try std.testing.expectError(error.FileNotFound, fixture.tmp.dir.access("gone-loop.json", .{})); - for ([_][]const u8{ "loop-a.json", "nested-loop.json", "elsewhere-loop.json" }) |file| try fixture.tmp.dir.access(file, .{}); + try std.testing.expect(!fixture.layoutExists("gone-loop.json")); + for ([_][]const u8{ "loop-a.json", "nested-loop.json", "elsewhere-loop.json", "alias-loop.json", "Upper-Loop.json" }) |file| { + try std.testing.expect(fixture.layoutExists(file)); + } +} + +test "workspace layout: an empty or partial first snapshot sweeps nothing that the full snapshot still holds" { + var fixture: LiveTerminalFixture = undefined; + try fixture.init(&.{ .{ .id = "loop-a" } }); + defer fixture.deinit(); + try fixture.saveLoopLayout(fixture.project, "loop-a", &.{ .{ .id = "loop-a", .agent = true }, .{ .id = shell_uuid_one } }); + try fixture.saveLoopLayout(fixture.project, "gone-loop", &.{ .{ .id = "gone-loop", .agent = true }, .{ .id = shell_uuid_two } }); + try fixture.setLive(&.{ "loop-a", shell_uuid_one, shell_uuid_two }); + fixture.predateLayouts(); + try fixture.forgetGraph(); + + // A daemon that has not loaded its project reports no loops, however often it says so. + try fixture.deliverNodes(""); + try fixture.deliverNodes(""); + fixture.settle(); + try fixture.expectKills(""); + try std.testing.expect(fixture.layoutExists("loop-a.json")); + try std.testing.expect(fixture.layoutExists("gone-loop.json")); + + // A snapshot cut short by a restart lacks loop A; the next one has it back. + try fixture.forgetGraph(); + try fixture.deliverNodes("{\"id\":\"loop-z\",\"title\":\"z\",\"loopType\":\"turnBased\",\"state\":{\"idle\":{}}}"); + try fixture.deliverNodes(node_loop_a ++ ",{\"id\":\"loop-z\",\"title\":\"z\",\"loopType\":\"turnBased\",\"state\":{\"idle\":{}}}"); + try fixture.waitFor(LiveTerminalFixture.killed, shell_uuid_two); + fixture.settle(); + try std.testing.expect(!fixture.killed(shell_uuid_one)); + try std.testing.expect(fixture.layoutExists("loop-a.json")); +} + +test "workspace layout: a layout saved after this shell started is another shell's and is never swept" { + var fixture: LiveTerminalFixture = undefined; + try fixture.init(&.{ .{ .id = "loop-a" } }); + defer fixture.deinit(); + // Written while this shell runs, as a second shell sharing the support directory would. + try fixture.saveLoopLayout(fixture.project, "other-shells-loop", &.{ .{ .id = "other-shells-loop", .agent = true }, .{ .id = shell_uuid_one } }); + try fixture.setLive(&.{ "loop-a", shell_uuid_one }); + try fixture.forgetGraph(); + + try fixture.deliverNodes(node_loop_a); + try fixture.deliverNodes(node_loop_a); + fixture.settle(); + try fixture.expectKills(""); + try std.testing.expect(fixture.layoutExists("other-shells-loop.json")); +} + +test "workspace layout: a loop deleted after the shell has seen the project is ended on the next snapshot that agrees" { + var fixture: LiveTerminalFixture = undefined; + try fixture.init(&.{ .{ .id = "loop-a" }, .{ .id = "loop-b" } }); + defer fixture.deinit(); + try fixture.saveLoopLayout(fixture.project, "loop-b", &.{ .{ .id = "loop-b", .agent = true }, .{ .id = shell_uuid_one } }); + try fixture.setLive(&.{ "loop-a", "loop-b", shell_uuid_one }); + + try fixture.deliver(&.{.{ .id = "loop-a" }}); + fixture.settle(); + try std.testing.expect(!fixture.killed(shell_uuid_one)); + // A snapshot that brings loop B back (a reconnect's replay) cancels the pending end. + try fixture.deliver(&.{ .{ .id = "loop-a" }, .{ .id = "loop-b" } }); + try fixture.deliver(&.{.{ .id = "loop-a" }}); + try fixture.deliver(&.{.{ .id = "loop-a" }}); + try fixture.waitFor(LiveTerminalFixture.killed, shell_uuid_one); + try std.testing.expect(!fixture.layoutExists("loop-b.json")); +} + +test "workspace layout: a shell session whose uuid is a graph node's id is never ended" { + var fixture: LiveTerminalFixture = undefined; + try fixture.init(&.{ .{ .id = "loop-a" } }); + defer fixture.deinit(); + // Node ids are uuids; the daemon names a loop's session after the uppercase one. + const node_uuid = shell_uuid_seven; + const nodes = node_loop_a ++ ",{\"id\":\"" ++ node_uuid ++ "\",\"title\":\"n\",\"loopType\":\"turnBased\",\"state\":{\"idle\":{}}}"; + try fixture.saveLoopLayout(fixture.project, "gone-loop", &.{ + .{ .id = "gone-loop", .agent = true }, + .{ .id = node_uuid }, + .{ .id = shell_uuid_one }, + }); + try fixture.setLive(&.{ "loop-a", node_uuid, shell_uuid_one }); + fixture.predateLayouts(); + try fixture.forgetGraph(); + + try fixture.deliverNodes(nodes); + try fixture.deliverNodes(nodes); + try fixture.waitFor(LiveTerminalFixture.killed, shell_uuid_one); + fixture.settle(); + try std.testing.expect(!fixture.killed(node_uuid)); } test "workspace layout: a layout that cannot be attributed to the deleted loop ends nothing" { @@ -14913,6 +15128,158 @@ test "workspace layout: a layout that cannot be attributed to the deleted loop e try std.testing.expect(!fixture.killed("foreign-shell")); } +test "workspace layout: deleting a loop ends only live sessions in this shell's own namespace" { + var fixture: LiveTerminalFixture = undefined; + try fixture.init(&.{ .{ .id = "loop-a" } }); + defer fixture.deinit(); + const foreign = "other-" ++ shell_uuid_two; + // A layout from an earlier shell, a stale copy or a hand edit can record anything as a + // plain pane; only a live, lowercase-uuid shell session of this shell's own may be ended. + try fixture.saveLoopLayout(fixture.project, "gone-loop", &.{ + .{ .id = "gone-loop", .agent = true }, + .{ .id = agent_uuid_upper }, + .{ .id = foreign }, + .{ .id = "plain-name" }, + .{ .id = shell_uuid_three }, + .{ .id = shell_uuid_one }, + .{ .id = "graphcode-" ++ shell_uuid_one }, + .{ .id = shell_uuid_four }, + }); + try fixture.saveLoopLayout(fixture.project, "kept-loop", &.{ .{ .id = "kept-loop", .agent = true }, .{ .id = shell_uuid_three } }); + try fixture.setLive(&.{ "loop-a", agent_uuid_upper, foreign, "plain-name", shell_uuid_three, shell_uuid_one }); + + try std.testing.expectEqual(@as(usize, 1), fixture.workspace.retireLoop("gone-loop", fixture.project)); + fixture.settle(); + // One argv, one session: the duplicate name is ended once, the dead one not at all. + try fixture.expectKills("kill graphcode-" ++ shell_uuid_one ++ " --force"); + try std.testing.expect(!fixture.layoutExists("gone-loop.json")); + try std.testing.expect(fixture.layoutExists("gone-loop.json.refused")); + try std.testing.expect(fixture.layoutExists("kept-loop.json")); +} + +test "workspace layout: a loop's shells are ended only after a successful session listing" { + var fixture: LiveTerminalFixture = undefined; + try fixture.init(&.{ .{ .id = "loop-a" } }); + defer fixture.deinit(); + try fixture.saveLoopLayout(fixture.project, "gone-loop", &.{ .{ .id = "gone-loop", .agent = true }, .{ .id = shell_uuid_one } }); + try fixture.setLive(&.{ "loop-a", shell_uuid_one }); + + try fixture.setFlag("ls-fail.txt", true); + try std.testing.expectEqual(@as(usize, 0), fixture.workspace.retireLoop("gone-loop", fixture.project)); + try fixture.setFlag("ls-fail.txt", false); + try fixture.setFlag("ls-hang.txt", true); + try std.testing.expectEqual(@as(usize, 0), fixture.workspace.retireLoop("gone-loop", fixture.project)); + try fixture.setFlag("ls-hang.txt", false); + fixture.settle(); + // Neither a failed nor a timed-out listing ended anything, and the layout is kept to retry. + try fixture.expectKills(""); + try std.testing.expect(fixture.layoutExists("gone-loop.json")); + + try std.testing.expectEqual(@as(usize, 1), fixture.workspace.retireLoop("gone-loop", fixture.project)); + fixture.settle(); + try fixture.expectKills("kill graphcode-" ++ shell_uuid_one ++ " --force"); + try std.testing.expect(!fixture.layoutExists("gone-loop.json")); +} + +test "workspace layout: a saved shell tab is restored by retry after a failed session listing" { + var fixture: LiveTerminalFixture = undefined; + try fixture.init(&.{ .{ .id = "loop-a" }, .{ .id = "loop-b" } }); + defer fixture.deinit(); + const app = &fixture.app; + + try fixture.setLive(&.{ "loop-a", "loop-b" }); + try clickSidebarLoopRow(app, fixture.project, "loop-a"); + try fixture.waitFor(LiveTerminalFixture.shows, "loop-a"); + try fixture.newTab(); + const shell = try fixture.selectedPane(0); + defer std.testing.allocator.free(shell); + try clickSidebarLoopRow(app, fixture.project, "loop-b"); + try fixture.waitFor(LiveTerminalFixture.shows, "loop-b"); + try fixture.setLive(&.{ "loop-a", "loop-b", shell }); + + // The listing fails during the restore the click runs, and works again right after. + try fixture.setFlag("ls-fail.txt", true); + try clickSidebarLoopRow(app, fixture.project, "loop-a"); + try fixture.setFlag("ls-fail.txt", false); + try fixture.waitFor(LiveTerminalFixture.shows, "loop-a"); + try fixture.waitFor(restoredAliveShell, shell); + try std.testing.expect(fixture.workspace.layout.hasPane(shell)); + try fixture.expectKills(""); +} + +test "workspace layout: reopening the open loop restores a shell tab that a failed listing left out" { + var fixture: LiveTerminalFixture = undefined; + try fixture.init(&.{ .{ .id = "loop-a" }, .{ .id = "loop-b" } }); + defer fixture.deinit(); + const app = &fixture.app; + if (@hasField(TerminalWorkspace.Workspace, "restore_retry_limit")) fixture.workspace.restore_retry_limit = 0; + + try fixture.setLive(&.{ "loop-a", "loop-b" }); + try clickSidebarLoopRow(app, fixture.project, "loop-a"); + try fixture.waitFor(LiveTerminalFixture.shows, "loop-a"); + try fixture.newTab(); + const shell = try fixture.selectedPane(0); + defer std.testing.allocator.free(shell); + try clickSidebarLoopRow(app, fixture.project, "loop-b"); + try fixture.waitFor(LiveTerminalFixture.shows, "loop-b"); + try fixture.setLive(&.{ "loop-a", "loop-b", shell }); + + try fixture.setFlag("ls-fail.txt", true); + try clickSidebarLoopRow(app, fixture.project, "loop-a"); + try fixture.setFlag("ls-fail.txt", false); + try fixture.waitFor(LiveTerminalFixture.shows, "loop-a"); + fixture.settle(); + try std.testing.expect(fixture.slotFor(shell) == null); + try std.testing.expect(fixture.workspace.layout.hasPane(shell)); + + try clickSidebarLoopRow(app, fixture.project, "loop-a"); + try fixture.waitFor(restoredAliveShell, shell); + try fixture.expectKills(""); +} + +test "workspace layout: a corrupt, foreign or newer-schema loop layout is kept as .bad and ends nothing" { + var fixture: LiveTerminalFixture = undefined; + try fixture.init(&.{ .{ .id = "loop-a" }, .{ .id = "loop-b" }, .{ .id = "loop-c" } }); + defer fixture.deinit(); + const allocator = std.testing.allocator; + const WorkspaceLayoutFile = @import("WorkspaceLayout.zig"); + const app = &fixture.app; + + const truncated = "{\"schemaVersion\":2,\"project\":\""; + try fixture.tmp.dir.writeFile(.{ .sub_path = "loop-a.json", .data = truncated }); + const newer = try std.fmt.allocPrint(allocator, "{{\"schemaVersion\":3,\"project\":{f},\"loop\":\"loop-b\",\"selectedTab\":0,\"tabs\":[]}}", .{std.json.fmt(fixture.project, .{})}); + defer allocator.free(newer); + try fixture.tmp.dir.writeFile(.{ .sub_path = "loop-b.json", .data = newer }); + try fixture.saveLoopLayout("C:\\another-project", "loop-c", &.{ .{ .id = "loop-c", .agent = true }, .{ .id = shell_uuid_one } }); + const foreign = try fixture.tmp.dir.readFileAlloc(allocator, "loop-c.json", 1 << 16); + defer allocator.free(foreign); + try fixture.setLive(&.{ "loop-a", "loop-b", "loop-c", shell_uuid_one }); + + for ([_][]const u8{ "loop-a", "loop-b", "loop-c" }) |loop| { + try clickSidebarLoopRow(app, fixture.project, loop); + try fixture.waitFor(LiveTerminalFixture.shows, loop); + } + fixture.settle(); + + for ([_]struct { bad: []const u8, original: []const u8 }{ + .{ .bad = "loop-a.json.bad", .original = truncated }, + .{ .bad = "loop-b.json.bad", .original = newer }, + .{ .bad = "loop-c.json.bad", .original = foreign }, + }) |entry| { + const kept = try fixture.tmp.dir.readFileAlloc(allocator, entry.bad, 1 << 16); + defer allocator.free(kept); + try std.testing.expectEqualStrings(entry.original, kept); + } + // Each loop now has a layout of its own that loads, and nothing was ended. + for ([_][]const u8{ "loop-a", "loop-b", "loop-c" }) |loop| { + const path = try std.fmt.allocPrint(allocator, "{s}\\{s}.json", .{ fixture.root, loop }); + defer allocator.free(path); + var loaded = try WorkspaceLayoutFile.Layout.loadFor(allocator, path, fixture.project, loop); + loaded.deinit(); + } + try fixture.expectKills(""); +} + /// Every terminal action must report something other than its failure status. fn expectNoTerminalFailure(app: *App) !void { for ([_][]const u8{ "Unable to open selected loop", "Unable to create tab", "Unable to split workspace" }) |failure| { diff --git a/graphcode-windows/src/TerminalSurface.zig b/graphcode-windows/src/TerminalSurface.zig index 2ce7caaf..1dd2c3f6 100644 --- a/graphcode-windows/src/TerminalSurface.zig +++ b/graphcode-windows/src/TerminalSurface.zig @@ -388,6 +388,37 @@ const KillJob = struct { const kill_wait_ms: i64 = 3_000; +const MissingLoop = struct { + project: []u8, + loop: []u8, +}; + +/// Answers whether an id names a node of the graph the caller holds. A shell session is +/// never ended when its uuid is one, whatever case either is written in. +pub const KnownNodes = struct { + context: *const anyopaque, + contains: *const fn (context: *const anyopaque, id: []const u8) bool, +}; +/// A saved project layout, or null when there is none to use. One whose contents are not a +/// layout of this shell (corrupt, another project's, a newer schema) is moved aside as `.bad` +/// rather than left for the next save to overwrite. +fn loadSavedLayout(allocator: std.mem.Allocator, path: []const u8, project: []const u8) ?WorkspaceLayout.Layout { + return WorkspaceLayout.Layout.load(allocator, path, project) catch |err| { + keepUnusableLayout(allocator, path, err); + return null; + }; +} + +fn keepUnusableLayout(allocator: std.mem.Allocator, path: []const u8, err: anyerror) void { + if (!WorkspaceLayout.isInvalidLayout(err)) return; + const kept = WorkspaceLayout.setAside(allocator, path, "bad"); + std.log.warn("workspace layout {s} is not usable ({s}); {s}", .{ + path, + @errorName(err), + if (kept) "kept as .bad" else "left in place", + }); +} + /// The project key under which a quick chat's layout is saved. It is no project's path, so /// a chat's layout is never mistaken for a graph loop's when loops are reconciled. pub const quick_chat_scope_project = "graphcode://quick-chats"; @@ -454,6 +485,18 @@ pub const Workspace = struct { /// working directory, which for the installed shell is its install `bin` folder. legacy_directory: []u8 = &.{}, kill_jobs: std.ArrayListUnmanaged(KillJob) = .empty, + /// When this shell started. A loop layout saved after that was not left by an earlier + /// run, so it may be another shell's and is never swept as a deleted loop's. Zero (never + /// set) counts every layout as newer. + started_ns: i128 = 0, + /// Slots whose saved shell tab awaits a session listing that proves it live (see + /// `restorePersistedSurfaces`): attaching blind would start a bare shell under its name. + restore_verify: [max_surfaces]bool = [_]bool{false} ** max_surfaces, + restore_attempts: [max_surfaces]u8 = [_]u8{0} ** max_surfaces, + /// Failed listings after which a pending shell tab is left saved until its loop reopens. + restore_retry_limit: u8 = 5, + /// Loops a snapshot reported missing once; a second agreeing one makes them deleted. + missing_loops: std.ArrayListUnmanaged(MissingLoop) = .empty, key_callback: ?WorkspaceKeyCallback = null, key_callback_context: ?*anyopaque = null, layout_origin_x: i32 = 0, @@ -481,6 +524,7 @@ pub const Workspace = struct { .allocator = allocator_, .experimental_vt = experimental_vt, .zmx_path = try allocator_.dupe(u8, std.process.getEnvVarOwned(allocator_, "GRAPHCODE_ZMX") catch "zmx.exe"), + .started_ns = std.time.nanoTimestamp(), .cwd = try allocator_.dupe(u8, std.process.getEnvVarOwned(allocator_, "GRAPHCODE_GATE_CWD") catch "."), .input_queue = .{ .allocator = allocator_ }, .layout = try WorkspaceLayout.Layout.init( @@ -514,10 +558,10 @@ pub const Workspace = struct { } workspace.layout_path = try workspace.layoutPathForProject(workspace.project_key); if (workspace.layout_path.len != 0) { - if (WorkspaceLayout.Layout.load(allocator_, workspace.layout_path, workspace.project_key)) |restored| { + if (loadSavedLayout(allocator_, workspace.layout_path, workspace.project_key)) |restored| { workspace.layout.deinit(); workspace.layout = restored; - } else |_| {} + } } if (c.winghostty_host_initialize(&workspace.host) != c.WINGHOSTTY_OK) { return error.WinghosttyHostInitializeFailed; @@ -535,6 +579,7 @@ pub const Workspace = struct { self.stopInputWorker(); self.cancelAllLaunchWaits(); self.finishKillJobs(); + self.clearMissingLoops(); for (self.surfaces, 0..) |_, index| self.destroySurface(index); for (&self.recreate_sessions) |*session| { if (session.*.len != 0) self.allocator.free(session.*); @@ -583,10 +628,10 @@ pub const Workspace = struct { errdefer self.allocator.free(new_layout_path); var new_layout = try WorkspaceLayout.Layout.init(self.allocator, project); errdefer new_layout.deinit(); - if (WorkspaceLayout.Layout.load(self.allocator, new_layout_path, project)) |restored| { + if (loadSavedLayout(self.allocator, new_layout_path, project)) |restored| { new_layout.deinit(); new_layout = restored; - } else |_| {} + } var old_layout = self.layout; const old_project_key = self.project_key; const old_layout_path = self.layout_path; @@ -626,10 +671,10 @@ pub const Workspace = struct { if (key_changed) { new_layout = try WorkspaceLayout.Layout.init(self.allocator, project_path); errdefer new_layout.deinit(); - if (WorkspaceLayout.Layout.load(self.allocator, new_layout_path, project_path)) |restored| { + if (loadSavedLayout(self.allocator, new_layout_path, project_path)) |restored| { new_layout.deinit(); new_layout = restored; - } else |_| {} + } } else { new_layout = self.layout; } @@ -751,7 +796,12 @@ pub const Workspace = struct { /// as it is. pub fn scopeToLoop(self: *Workspace, loop: []const u8, layout_project: []const u8, launches_agent: bool) !void { if (loop.len == 0) return; - if (std.mem.eql(u8, self.loop_id, loop) and std.mem.eql(u8, self.layout.project_key, layout_project)) return; + if (std.mem.eql(u8, self.loop_id, loop) and std.mem.eql(u8, self.layout.project_key, layout_project)) { + // Reopening the open loop picks up a saved pane an earlier restore could not attach. + self.expediteShellRestores(); + self.restorePersistedSurfaces(loop, 1); + return; + } const root = self.layoutRoot() orelse return; const new_path = try WorkspaceLayout.loopLayoutPath(self.allocator, root, loop); errdefer self.allocator.free(new_path); @@ -780,8 +830,15 @@ pub const Workspace = struct { fn loadLoopLayout(self: *Workspace, path: []const u8, layout_project: []const u8, loop: []const u8) !WorkspaceLayout.Layout { if (WorkspaceLayout.Layout.loadFor(self.allocator, path, layout_project, loop)) |saved| { return saved; - } else |err| if (err == error.FileNotFound) { - if (try self.adoptLegacy(path, layout_project, loop)) |adopted| return adopted; + } else |err| { + if (err == error.FileNotFound) { + if (try self.adoptLegacy(path, layout_project, loop)) |adopted| return adopted; + } else if (err == error.LayoutUnreadable or err == error.OutOfMemory) { + // Not known to be bad, so not replaced: the next save would overwrite it. + return err; + } else { + keepUnusableLayout(self.allocator, path, err); + } } return WorkspaceLayout.Layout.initForLoop(self.allocator, layout_project, loop); } @@ -800,17 +857,35 @@ pub const Workspace = struct { if (std.fs.cwd().access(marker, .{})) |_| return null else |_| {} const legacy_path = try WorkspaceLayout.legacyLayoutPath(self.allocator, self.legacy_directory, self.project_key); defer self.allocator.free(legacy_path); - const adopted = (try WorkspaceLayout.adoptLegacyLayout(self.allocator, legacy_path, self.project_key, loop, layout_project)) orelse return null; - std.fs.cwd().makePath(root) catch {}; - std.fs.cwd().writeFile(.{ .sub_path = marker, .data = loop }) catch {}; + var adopted = (try WorkspaceLayout.adoptLegacyLayout(self.allocator, legacy_path, self.project_key, loop, layout_project)) orelse return null; + // Without the marker every loop of the project would adopt the same shells, so a + // marker that cannot be written means no adoption this time, not a shared one. + const marked = marked: { + std.fs.cwd().makePath(root) catch break :marked false; + std.fs.cwd().writeFile(.{ .sub_path = marker, .data = loop }) catch break :marked false; + break :marked true; + }; + if (!marked) { + adopted.deinit(); + return null; + } return adopted; } - /// Ends the shell sessions saved in a deleted loop's layout and forgets the layout. The - /// sessions ended are exactly the plain-shell panes that layout names; the loop's own - /// session belongs to the daemon and no other loop's is touched. A layout that cannot be - /// read ends nothing. Returns how many sessions were ended. + /// Ends the shell sessions saved in a deleted loop's layout and forgets the layout. Only + /// plain-shell panes are candidates, and only those whose session is one this shell made + /// (`ZmxSession.shellSessionName`) and a fresh `zmx ls` shows running (`endShellSessions`): + /// the loop's own session belongs to the daemon, and no other loop's or user's is touched. + /// A layout that cannot be read ends nothing and stays; so does one whose sessions could + /// not be listed, to be retried by the next sweep. A layout naming a session this shell + /// refused to end is set aside as `.refused` rather than deleted. Returns how many + /// sessions were ended. pub fn retireLoop(self: *Workspace, loop: []const u8, layout_project: []const u8) usize { + return self.retireLoopGuarded(loop, layout_project, null); + } + + /// `retireLoop` that also refuses any session whose uuid names a node of the caller's graph. + pub fn retireLoopGuarded(self: *Workspace, loop: []const u8, layout_project: []const u8, known: ?KnownNodes) usize { const root = self.layoutRoot() orelse return 0; if (loop.len == 0) return 0; const path = WorkspaceLayout.loopLayoutPath(self.allocator, root, loop) catch return 0; @@ -828,13 +903,16 @@ pub const Workspace = struct { if (pane.launches_agent or std.mem.eql(u8, pane.id, loop)) continue; sessions.append(self.allocator, pane.id) catch return 0; }; - const count = sessions.items.len; if (open) { // Detached first, so ending a shell never reads as an exit to re-create. for (self.surfaces, 0..) |_, index| self.destroySurface(index); self.clearAllRecreateState(); } - const started = self.endSessions(sessions.items); + const outcome = self.endShellSessions(sessions.items, .{ + .known = known, + .except_layout = path, + .check_open_layout = !open, + }); if (open) { if (WorkspaceLayout.Layout.init(self.allocator, self.layout.project_key)) |emptied| { var old_layout = self.layout; @@ -847,8 +925,61 @@ pub const Workspace = struct { self.clearLoopScope(); } else |_| {} } - std.fs.cwd().deleteFile(path) catch {}; - return if (started) count else 0; + if (!outcome.listed or !outcome.started) return 0; + if (outcome.refused != 0) { + _ = WorkspaceLayout.setAside(self.allocator, path, "refused"); + } else { + std.fs.cwd().deleteFile(path) catch {}; + } + return outcome.ended; + } + + /// Remembers that a snapshot of `project` lacked `loop`. A loop is only ended once a + /// second snapshot agrees (`takeMissingLoops`): one cut short by a daemon restart or a + /// stale replay must not cost a user's shells. + pub fn markLoopMissing(self: *Workspace, project: []const u8, loop: []const u8) !void { + const owned_project = try self.allocator.dupe(u8, project); + errdefer self.allocator.free(owned_project); + const owned_loop = try self.allocator.dupe(u8, loop); + errdefer self.allocator.free(owned_loop); + try self.missing_loops.append(self.allocator, .{ .project = owned_project, .loop = owned_loop }); + } + + /// The loops an earlier snapshot of `project` reported missing, forgotten as they are + /// handed over; the caller checks them against the new snapshot and frees them with + /// `freeLoopIds`. + pub fn takeMissingLoops(self: *Workspace, project: []const u8) ![][]u8 { + var taken: std.ArrayListUnmanaged([]u8) = .empty; + errdefer { + for (taken.items) |id| self.allocator.free(id); + taken.deinit(self.allocator); + } + var index: usize = 0; + while (index < self.missing_loops.items.len) { + if (!std.mem.eql(u8, self.missing_loops.items[index].project, project)) { + index += 1; + continue; + } + const entry = self.missing_loops.items[index]; + try taken.append(self.allocator, entry.loop); + self.allocator.free(entry.project); + _ = self.missing_loops.orderedRemove(index); + } + return taken.toOwnedSlice(self.allocator); + } + + pub fn freeLoopIds(self: *const Workspace, ids: [][]u8) void { + for (ids) |id| self.allocator.free(id); + self.allocator.free(ids); + } + + fn clearMissingLoops(self: *Workspace) void { + for (self.missing_loops.items) |entry| { + self.allocator.free(entry.project); + self.allocator.free(entry.loop); + } + self.missing_loops.deinit(self.allocator); + self.missing_loops = .empty; } /// The loop-scoped layouts saved beside this workspace's, for reconciling with the graph. @@ -857,33 +988,125 @@ pub const Workspace = struct { return WorkspaceLayout.scanLoopLayouts(self.allocator, root); } - /// `zmx kill --force`, run beside the UI and reaped by `poll`. Never the - /// session of the open loop itself. False when nothing could be started. - fn endSessions(self: *Workspace, sessions: []const []const u8) bool { - if (sessions.len == 0) return true; - const names = self.allocator.alloc([]u8, sessions.len) catch return false; - var named: usize = 0; - errdefer { - for (names[0..named]) |name| self.allocator.free(name); - self.allocator.free(names); + const EndOptions = struct { + known: ?KnownNodes = null, + /// The layout file being retired, which naming a session does not make it shared. + except_layout: []const u8 = "", + /// Whether the layout open in memory is another loop's, and so can share a session. + check_open_layout: bool = true, + }; + + const EndOutcome = struct { + /// False when the sessions could not be listed, so nothing was ended. + listed: bool = true, + /// False when the `zmx kill` could not be started. + started: bool = true, + ended: usize = 0, + refused: usize = 0, + }; + + /// Whether this shell may end the zmx session `name` (already a valid shell session name). + fn mayEndShell(self: *Workspace, name: []const u8, options: EndOptions) bool { + const uuid = name[ZmxSession.prefix.len..]; + if (self.loop_id.len != 0 and std.ascii.eqlIgnoreCase(uuid, self.loop_id)) return false; + if (options.known) |known| { + if (known.contains(known.context, uuid)) return false; } - for (sessions) |session| { - names[named] = ZmxSession.allocName(self.allocator, session) catch return false; - named += 1; + if (options.check_open_layout and (self.layout.hasPane(uuid) or self.layout.hasPane(name))) return false; + if (self.layoutRoot()) |root| { + if (WorkspaceLayout.claimedByOtherLayout(self.allocator, root, options.except_layout, name)) return false; } - const argv = self.allocator.alloc([]const u8, sessions.len + 3) catch return false; - errdefer self.allocator.free(argv); + return true; + } + + /// `zmx kill --force`, run beside the UI and reaped by `poll`, for the panes in + /// `ids` that are provably this shell's own plain shells: a name that is exactly + /// `graphcode-` plus a lowercase uuid (never an agent's uppercase one, a foreign or an + /// unprefixed name), that is no node's id and no other layout's pane, and that a fresh, + /// successful `zmx ls` shows running. Anything else is refused and logged, never ended. + fn endShellSessions(self: *Workspace, ids: []const []const u8, options: EndOptions) EndOutcome { + var outcome: EndOutcome = .{}; + var names: std.ArrayListUnmanaged([]u8) = .empty; + defer { + for (names.items) |name| self.allocator.free(name); + names.deinit(self.allocator); + } + for (ids) |id| { + const name = (ZmxSession.shellSessionName(self.allocator, id) catch { + outcome.started = false; + return outcome; + }) orelse { + std.log.warn("not ending zmx session for pane {s}: not one of this shell's session names", .{id}); + outcome.refused += 1; + continue; + }; + var duplicate = false; + for (names.items) |existing| duplicate = duplicate or std.mem.eql(u8, existing, name); + if (duplicate) { + self.allocator.free(name); + continue; + } + if (!self.mayEndShell(name, options)) { + std.log.warn("not ending zmx session {s}: it names a node or belongs to another layout", .{name}); + self.allocator.free(name); + outcome.refused += 1; + continue; + } + names.append(self.allocator, name) catch { + self.allocator.free(name); + outcome.started = false; + return outcome; + }; + } + if (names.items.len == 0) return outcome; + + var listing = self.sessionListing() orelse { + outcome.listed = false; + return outcome; + }; + defer listing.deinit(self.allocator); + var live: usize = 0; + for (names.items) |name| { + if (LoopLaunchWait.listingShowsLive(listing.items, name)) { + names.items[live] = name; + live += 1; + } else { + self.allocator.free(name); + } + } + names.shrinkRetainingCapacity(live); + if (live == 0) return outcome; + + const argv = self.allocator.alloc([]const u8, live + 3) catch { + outcome.started = false; + return outcome; + }; argv[0] = self.zmxExecutable(); argv[1] = "kill"; - for (names, 0..) |name, index| argv[index + 2] = name; + for (names.items, 0..) |name, index| argv[index + 2] = name; argv[argv.len - 1] = "--force"; var child = ZmxSession.child(self.allocator, argv, self.cwd, .control); - child.spawn() catch return false; - self.kill_jobs.append(self.allocator, .{ .child = child, .argv = argv, .names = names }) catch { + child.spawn() catch { + self.allocator.free(argv); + outcome.started = false; + return outcome; + }; + const owned = names.toOwnedSlice(self.allocator) catch { _ = child.kill() catch {}; - return false; + self.allocator.free(argv); + outcome.started = false; + return outcome; }; - return true; + self.kill_jobs.append(self.allocator, .{ .child = child, .argv = argv, .names = owned }) catch { + _ = child.kill() catch {}; + for (owned) |name| self.allocator.free(name); + self.allocator.free(owned); + self.allocator.free(argv); + outcome.started = false; + return outcome; + }; + outcome.ended = live; + return outcome; } fn pollKillJobs(self: *Workspace) void { @@ -1011,9 +1234,13 @@ pub const Workspace = struct { /// wedged zmx cannot hold the UI thread; null when it could not be read. The caller /// owns the bytes. fn sessionListing(self: *Workspace) ?std.ArrayListUnmanaged(u8) { + return self.sessionListingWithin(restore_probe_timeout_ms); + } + + fn sessionListingWithin(self: *Workspace, timeout_ms: i64) ?std.ArrayListUnmanaged(u8) { var child = self.spawnListing() orelse return null; var output: std.ArrayListUnmanaged(u8) = .empty; - const deadline = nowMilliseconds() + restore_probe_timeout_ms; + const deadline = nowMilliseconds() + timeout_ms; var exit_code: c.DWORD = c.STILL_ACTIVE; while (true) { self.drainListing(&child, &output) catch { @@ -1362,6 +1589,8 @@ pub const Workspace = struct { for (self.layout.tabs.items) |tab| for (tab.panes.items) |pane| { if (count == ids.len) break; if (skip_id.len != 0 and std.mem.eql(u8, pane.id, skip_id)) continue; + // Reopening the open loop reaches here with panes already shown or already queued. + if (self.paneShown(pane.id) or self.restoreQueued(pane.id)) continue; ids[count] = self.allocator.dupe(u8, pane.id) catch continue; agents[count] = pane.launches_agent; count += 1; @@ -1373,9 +1602,10 @@ pub const Workspace = struct { for (ids[0..count], agents[0..count]) |id, launches_agent| { const live: ?bool = if (listing) |value| LoopLaunchWait.listingShowsLive(value.items, id) else null; if (live == null) { - // Unknown is not gone: the pane stays saved. A loop pane retries; a shell is - // never attached blind, since attaching creates the session when it is missing. - if (launches_agent) self.queueRestoreRetry(first_slot, id, error.SessionListingUnavailable, true); + // Unknown is not gone: the pane stays saved. Either kind retries, but a shell + // only re-lists and is never attached blind, since attaching creates the + // session when it is missing. + self.queueRestoreRetry(first_slot, id, error.SessionListingUnavailable, launches_agent); continue; } if (!LoopLaunchWait.restoreKeepsPane(launches_agent, live)) { @@ -1397,13 +1627,34 @@ pub const Workspace = struct { self.syncTopology(); } + fn restoreQueued(self: *const Workspace, id: []const u8) bool { + for (self.recreate_sessions) |session| { + if (session.len != 0 and std.mem.eql(u8, session, id)) return true; + } + return false; + } + + fn restoreFirstSlot(self: *const Workspace) usize { + return if (self.loop_id.len == 0) 0 else 1; + } + + const restore_retry_base_ms: i64 = 1_000; + const restore_retry_probe_timeout_ms: i64 = 500; + fn queueRestoreRetry(self: *Workspace, first_slot: usize, session: []const u8, err: anyerror, daemon_session: bool) void { + // A shell is retried by re-listing, never by a blind attach; with no retries allowed + // it stays saved until its loop is reopened. + if (!daemon_session and self.restore_retry_limit == 0) return; for (self.surfaces, 0..) |slot, index| { if (index < first_slot) continue; if (slot.surface == null and slot.attach == null and self.recreate_sessions[index].len == 0) { self.daemon_sessions[index] = daemon_session; self.recreate_sessions[index] = self.allocator.dupe(u8, session) catch &.{}; - self.recreate_due_ms[index] = nowMilliseconds() + self.recreate_delay_ms[index]; + if (self.recreate_sessions[index].len == 0) return; + self.restore_verify[index] = !daemon_session; + self.restore_attempts[index] = 0; + self.recreate_due_ms[index] = nowMilliseconds() + + if (daemon_session) self.recreate_delay_ms[index] else restore_retry_base_ms; const message = std.fmt.allocPrint(self.allocator, "workspace restore pending: {s}", .{@errorName(err)}) catch return; self.restore_errors[index] = message; return; @@ -1411,6 +1662,51 @@ pub const Workspace = struct { } } + /// One retry of a saved shell tab that could not be attached yet: attach it only once a + /// successful listing shows its session running, drop it once one shows it gone, and + /// leave it saved (to be restored by reopening its loop) after `restore_retry_limit` + /// listings that failed. + fn retryShellRestore(self: *Workspace, index: usize, now: i64) void { + const id = self.allocator.dupe(u8, self.recreate_sessions[index]) catch return; + defer self.allocator.free(id); + var listing = self.sessionListingWithin(restore_retry_probe_timeout_ms) orelse return self.deferShellRestore(index, now); + defer listing.deinit(self.allocator); + if (!LoopLaunchWait.listingShowsLive(listing.items, id)) { + self.clearRecreateSession(index); + self.clearRestoreError(index); + if (self.layout.removePane(id)) self.persistLayout() catch {}; + self.syncTopology(); + return; + } + const initial_grid = self.gridForSession(id) catch return self.deferShellRestore(index, now); + const attached = self.createAttachedSurfaceFrom(self.restoreFirstSlot(), id, initial_grid) catch + return self.deferShellRestore(index, now); + self.daemon_sessions[attached] = false; + self.clearRecreateSession(index); + self.clearRestoreError(index); + self.syncTopology(); + } + + fn deferShellRestore(self: *Workspace, index: usize, now: i64) void { + self.restore_attempts[index] +|= 1; + if (self.restore_attempts[index] >= self.restore_retry_limit) { + self.clearRecreateSession(index); + self.clearRestoreError(index); + return; + } + const doublings: u6 = @intCast(@min(self.restore_attempts[index], 3)); + self.recreate_due_ms[index] = now + (restore_retry_base_ms << doublings); + } + + /// A reopened loop retries its pending shell tabs now, with a fresh budget. + fn expediteShellRestores(self: *Workspace) void { + for (self.recreate_sessions, 0..) |session, index| { + if (session.len == 0 or !self.restore_verify[index]) continue; + self.restore_attempts[index] = 0; + self.recreate_due_ms[index] = 0; + } + } + fn clearRestoreError(self: *Workspace, index: usize) void { if (self.restore_errors[index].len != 0) self.allocator.free(self.restore_errors[index]); self.restore_errors[index] = &.{}; @@ -1419,6 +1715,8 @@ pub const Workspace = struct { fn clearRecreateSession(self: *Workspace, index: usize) void { if (self.recreate_sessions[index].len != 0) self.allocator.free(self.recreate_sessions[index]); self.recreate_sessions[index] = &.{}; + self.restore_verify[index] = false; + self.restore_attempts[index] = 0; } fn clearAllRecreateState(self: *Workspace) void { @@ -1428,6 +1726,8 @@ pub const Workspace = struct { session.* = &.{}; self.recreate_due_ms[index] = 0; self.recreate_delay_ms[index] = 100; + self.restore_verify[index] = false; + self.restore_attempts[index] = 0; } for (&self.restore_errors) |*message| { if (message.*.len != 0) self.allocator.free(message.*); @@ -1527,7 +1827,7 @@ pub const Workspace = struct { // A shell's session is the pane's reason for existing: closing the pane ends it, as // on macOS. The loop's own session belongs to the daemon and ends with the loop. if (!record.launches_agent and !std.mem.eql(u8, record.id, self.loop_id)) - _ = self.endSessions(&.{record.id}); + _ = self.endShellSessions(&.{record.id}, .{}); self.syncTopology(); } @@ -2725,6 +3025,10 @@ pub const Workspace = struct { fn pollRecreates(self: *Workspace) void { const now = nowMilliseconds(); for (self.recreate_sessions, 0..) |session, index| { + if (session.len != 0 and self.restore_verify[index] and now >= self.recreate_due_ms[index]) { + self.retryShellRestore(index, now); + continue; + } if (session.len == 0 or self.surfaces[index].surface != null or now < self.recreate_due_ms[index]) { continue; } diff --git a/graphcode-windows/src/TerminalWorkspace.zig b/graphcode-windows/src/TerminalWorkspace.zig index 732a2000..2a61a909 100644 --- a/graphcode-windows/src/TerminalWorkspace.zig +++ b/graphcode-windows/src/TerminalWorkspace.zig @@ -7,3 +7,4 @@ pub const loopBarLayout = TerminalSurface.loopBarLayout; pub const tabBounds = TerminalSurface.tabBounds; pub const chromeControlBounds = TerminalSurface.chromeControlBounds; pub const quick_chat_scope_project = TerminalSurface.quick_chat_scope_project; +pub const KnownNodes = TerminalSurface.KnownNodes; diff --git a/graphcode-windows/src/WorkspaceLayout.zig b/graphcode-windows/src/WorkspaceLayout.zig index 18ded18a..8db8a468 100644 --- a/graphcode-windows/src/WorkspaceLayout.zig +++ b/graphcode-windows/src/WorkspaceLayout.zig @@ -289,35 +289,48 @@ pub const Layout = struct { if (self.tabs.items.len != 0 and self.selected_tab >= self.tabs.items.len) return error.InvalidTopology; if (self.tabs.items.len != 0) try self.validateTopology(); - const tmp_path = try std.fmt.allocPrint(self.allocator, "{s}.tmp", .{file_path}); + // A name of its own per writer: two shells sharing the support directory must never + // interleave their bytes in one temporary file before it is renamed over the layout. + var nonce: [8]u8 = undefined; + std.crypto.random.bytes(&nonce); + const tmp_path = try std.fmt.allocPrint(self.allocator, "{s}.{x}.tmp", .{ file_path, std.mem.readInt(u64, &nonce, .little) }); defer self.allocator.free(tmp_path); if (std.fs.path.dirname(file_path)) |directory| try std.fs.cwd().makePath(directory); - var file = try std.fs.cwd().createFile(tmp_path, .{ .truncate = true }); - defer file.close(); - var buffer: [4096]u8 = undefined; - var writer = file.writer(&buffer); - try writer.interface.writeAll("{\"schemaVersion\":2,\"project\":"); - try writer.interface.print("{f}", .{std.json.fmt(self.project_key, .{})}); - if (self.loop_key.len != 0) try writer.interface.print(",\"loop\":{f}", .{std.json.fmt(self.loop_key, .{})}); - try writer.interface.print(",\"selectedTab\":{d},\"tabs\":[", .{self.selected_tab}); + { + var file = try std.fs.cwd().createFile(tmp_path, .{ .truncate = true }); + errdefer std.fs.cwd().deleteFile(tmp_path) catch {}; + defer file.close(); + var buffer: [4096]u8 = undefined; + var writer = file.writer(&buffer); + try self.writeJson(&writer.interface); + try writer.interface.flush(); + try file.sync(); + } + errdefer std.fs.cwd().deleteFile(tmp_path) catch {}; + try std.fs.cwd().rename(tmp_path, file_path); + } + + fn writeJson(self: *const Layout, writer: *std.Io.Writer) !void { + try writer.writeAll("{\"schemaVersion\":2,\"project\":"); + try writer.print("{f}", .{std.json.fmt(self.project_key, .{})}); + if (self.loop_key.len != 0) try writer.print(",\"loop\":{f}", .{std.json.fmt(self.loop_key, .{})}); + try writer.print(",\"selectedTab\":{d},\"tabs\":[", .{self.selected_tab}); for (self.tabs.items, 0..) |tab, tab_index| { - if (tab_index != 0) try writer.interface.writeByte(','); - try writer.interface.print( + if (tab_index != 0) try writer.writeByte(','); + try writer.print( "{{\"id\":{d},\"direction\":\"{s}\",\"focused\":{d},\"panes\":[", .{ tab.id, @tagName(tab.split_direction), tab.focused_pane }, ); for (tab.panes.items, 0..) |pane, pane_index| { - if (pane_index != 0) try writer.interface.writeByte(','); - try writer.interface.print( + if (pane_index != 0) try writer.writeByte(','); + try writer.print( "{{\"id\":{f},\"agent\":{s}}}", .{ std.json.fmt(pane.id, .{}), if (pane.launches_agent) "true" else "false" }, ); } - try writer.interface.writeAll("]}"); + try writer.writeAll("]}"); } - try writer.interface.writeAll("]}"); - try writer.interface.flush(); - try std.fs.cwd().rename(tmp_path, file_path); + try writer.writeAll("]}"); } pub fn load( @@ -335,7 +348,11 @@ pub const Layout = struct { expected_project: []const u8, expected_loop: []const u8, ) !Layout { - const data = try std.fs.cwd().readFileAlloc(allocator, file_path, 4 * 1024 * 1024); + const data = std.fs.cwd().readFileAlloc(allocator, file_path, 4 * 1024 * 1024) catch |err| switch (err) { + error.FileNotFound, error.OutOfMemory => return err, + // Locked by another shell, denied, too large: nothing says the layout is bad. + else => return error.LayoutUnreadable, + }; defer allocator.free(data); var parsed = try std.json.parseFromSlice(std.json.Value, allocator, data, .{}); defer parsed.deinit(); @@ -484,6 +501,88 @@ pub fn legacyLayoutPath(allocator: std.mem.Allocator, directory: []const u8, pro return std.fmt.allocPrint(allocator, "{s}\\graphcode-workspace.{s}.json", .{ directory, projectSuffix(project) }); } +/// Whether a failed load means the file's contents are not a usable layout of this shell +/// (corrupt, foreign, or from a newer schema), as opposed to it being unreadable right now. +pub fn isInvalidLayout(err: anyerror) bool { + return switch (err) { + error.FileNotFound, error.LayoutUnreadable, error.OutOfMemory => false, + else => true, + }; +} + +/// Moves a layout file this shell must not use or delete aside as `.`, or +/// `..` when that is taken: renaming over an earlier one would lose it. +/// False when it could not be moved, in which case the file is left where it is. +pub fn setAside(allocator: std.mem.Allocator, file_path: []const u8, suffix: []const u8) bool { + var attempt: usize = 0; + while (attempt < 100) : (attempt += 1) { + const target = if (attempt == 0) + std.fmt.allocPrint(allocator, "{s}.{s}", .{ file_path, suffix }) catch return false + else + std.fmt.allocPrint(allocator, "{s}.{s}.{d}", .{ file_path, suffix, attempt }) catch return false; + defer allocator.free(target); + if (std.fs.cwd().access(target, .{})) |_| continue else |_| {} + std.fs.cwd().rename(file_path, target) catch return false; + return true; + } + return false; +} + +fn sameSession(a: []const u8, b: []const u8) bool { + const prefix = "graphcode-"; + const left = if (std.mem.startsWith(u8, a, prefix)) a[prefix.len..] else a; + const right = if (std.mem.startsWith(u8, b, prefix)) b[prefix.len..] else b; + return std.mem.eql(u8, left, right); +} + +/// Whether any layout file in `root` other than `except_file` names `session` as a pane, +/// whatever its project or loop. A session two layouts both claim is not safely one's to end. +/// Files that cannot be read claim nothing. +pub fn claimedByOtherLayout(allocator: std.mem.Allocator, root: []const u8, except_file: []const u8, session: []const u8) bool { + var directory = std.fs.cwd().openDir(root, .{ .iterate = true }) catch return false; + defer directory.close(); + const except_name = std.fs.path.basename(except_file); + var iterator = directory.iterate(); + var seen: usize = 0; + while (iterator.next() catch null) |entry| { + if (entry.kind != .file or !std.mem.endsWith(u8, entry.name, ".json")) continue; + if (std.ascii.eqlIgnoreCase(entry.name, except_name)) continue; + seen += 1; + if (seen > max_scanned_layouts) break; + const data = directory.readFileAlloc(allocator, entry.name, 4 * 1024 * 1024) catch continue; + defer allocator.free(data); + var parsed = std.json.parseFromSlice(std.json.Value, allocator, data, .{}) catch continue; + defer parsed.deinit(); + const root_object = switch (parsed.value) { + .object => |value| value, + else => continue, + }; + const tabs = switch (root_object.get("tabs") orelse continue) { + .array => |value| value.items, + else => continue, + }; + for (tabs) |tab| { + const tab_object = switch (tab) { + .object => |value| value, + else => continue, + }; + const panes = switch (tab_object.get("panes") orelse continue) { + .array => |value| value.items, + else => continue, + }; + for (panes) |pane| { + const pane_object = switch (pane) { + .object => |value| value, + else => continue, + }; + const id = pane_object.get("id") orelse continue; + if (id == .string and sameSession(id.string, session)) return true; + } + } + } + return false; +} + /// A loop's layout from the project-wide legacy file, for a loop that has none of its own /// yet. The legacy file mixed every loop of the project: it is adopted only when it names /// `loop` as one of its panes, and then without the agent panes of any other loop. Null @@ -522,6 +621,8 @@ pub fn adoptLegacyLayout( pub const LoopRecord = struct { loop: []u8, project: []u8, + /// When the layout file was last written, in nanoseconds since 1970 (0 when unknown). + modified_ns: i128 = 0, }; pub fn freeLoopRecords(allocator: std.mem.Allocator, records: []LoopRecord) void { @@ -569,7 +670,8 @@ pub fn scanLoopLayouts(allocator: std.mem.Allocator, root: []const u8) ![]LoopRe errdefer allocator.free(owned_loop); const owned_project = try allocator.dupe(u8, project.string); errdefer allocator.free(owned_project); - try records.append(allocator, .{ .loop = owned_loop, .project = owned_project }); + const modified_ns: i128 = if (directory.statFile(entry.name)) |stat| stat.mtime else |_| std.math.maxInt(i128); + try records.append(allocator, .{ .loop = owned_loop, .project = owned_project, .modified_ns = modified_ns }); } return records.toOwnedSlice(allocator); } @@ -845,4 +947,117 @@ test "scanning loop layouts reports only loop-scoped files and skips everything const missing = try scanLoopLayouts(std.testing.allocator, "C:\\no\\such\\layouts"); defer freeLoopRecords(std.testing.allocator, missing); try std.testing.expectEqual(@as(usize, 0), missing.len); + try std.testing.expect(records[0].modified_ns > 0); +} + +test "a layout is set aside under a name that never replaces an earlier one" { + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + const root = try tmp.dir.realpathAlloc(std.testing.allocator, "."); + defer std.testing.allocator.free(root); + const path = try std.fmt.allocPrint(std.testing.allocator, "{s}\\loop.json", .{root}); + defer std.testing.allocator.free(path); + + try tmp.dir.writeFile(.{ .sub_path = "loop.json", .data = "first" }); + try std.testing.expect(setAside(std.testing.allocator, path, "bad")); + try tmp.dir.writeFile(.{ .sub_path = "loop.json", .data = "second" }); + try std.testing.expect(setAside(std.testing.allocator, path, "bad")); + try std.testing.expectError(error.FileNotFound, tmp.dir.access("loop.json", .{})); + var buffer: [16]u8 = undefined; + try std.testing.expectEqualStrings("first", try tmp.dir.readFile("loop.json.bad", &buffer)); + try std.testing.expectEqualStrings("second", try tmp.dir.readFile("loop.json.bad.1", &buffer)); + try std.testing.expect(!setAside(std.testing.allocator, path, "bad")); +} + +test "loading tells a missing layout, an unreadable one and an invalid one apart" { + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + const root = try tmp.dir.realpathAlloc(std.testing.allocator, "."); + defer std.testing.allocator.free(root); + const path = try std.fmt.allocPrint(std.testing.allocator, "{s}\\loop.json", .{root}); + defer std.testing.allocator.free(path); + + try std.testing.expectError(error.FileNotFound, Layout.loadFor(std.testing.allocator, path, "project-a", "loop")); + try std.testing.expect(!isInvalidLayout(error.FileNotFound)); + + // A directory where the file should be cannot be read, and says nothing about its contents. + try tmp.dir.makePath("loop.json"); + try std.testing.expectError(error.LayoutUnreadable, Layout.loadFor(std.testing.allocator, path, "project-a", "loop")); + try std.testing.expect(!isInvalidLayout(error.LayoutUnreadable)); + try tmp.dir.deleteDir("loop.json"); + + const invalid = [_][]const u8{ + "{\"schemaVersion\":2,\"project\":\"", + "{\"schemaVersion\":3,\"project\":\"project-a\",\"loop\":\"loop\",\"selectedTab\":0,\"tabs\":[]}", + "{\"schemaVersion\":2,\"project\":\"project-b\",\"loop\":\"loop\",\"selectedTab\":0,\"tabs\":[]}", + "{\"schemaVersion\":2,\"project\":\"project-a\",\"loop\":\"other\",\"selectedTab\":0,\"tabs\":[]}", + "[]", + }; + for (invalid) |data| { + try tmp.dir.writeFile(.{ .sub_path = "loop.json", .data = data }); + if (Layout.loadFor(std.testing.allocator, path, "project-a", "loop")) |value| { + var loaded = value; + loaded.deinit(); + return error.TestExpectedInvalidLayout; + } else |err| try std.testing.expect(isInvalidLayout(err)); + } +} + +test "a pane another layout names is claimed, however the session is spelled" { + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + const root = try tmp.dir.realpathAlloc(std.testing.allocator, "."); + defer std.testing.allocator.free(root); + var other = try Layout.initForLoop(std.testing.allocator, "project-a", "loop-b"); + defer other.deinit(); + try other.addTab("loop-b", true); + try other.addTab("graphcode-shared", false); + const other_path = try loopLayoutPath(std.testing.allocator, root, "loop-b"); + defer std.testing.allocator.free(other_path); + try other.save(other_path); + try tmp.dir.writeFile(.{ .sub_path = "garbage.json", .data = "{ not a layout" }); + const mine = try loopLayoutPath(std.testing.allocator, root, "loop-a"); + defer std.testing.allocator.free(mine); + + try std.testing.expect(claimedByOtherLayout(std.testing.allocator, root, mine, "graphcode-shared")); + try std.testing.expect(claimedByOtherLayout(std.testing.allocator, root, mine, "shared")); + try std.testing.expect(!claimedByOtherLayout(std.testing.allocator, root, mine, "unshared")); + // The layout being retired never makes its own panes shared. + try std.testing.expect(!claimedByOtherLayout(std.testing.allocator, root, other_path, "shared")); + try std.testing.expect(!claimedByOtherLayout(std.testing.allocator, "C:\\no\\such\\layouts", mine, "shared")); +} + +test "saving replaces a layout whole, leaves no temporary file, and keeps the old one when it fails" { + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + const root = try tmp.dir.realpathAlloc(std.testing.allocator, "."); + defer std.testing.allocator.free(root); + const path = try loopLayoutPath(std.testing.allocator, root, "loop-a"); + defer std.testing.allocator.free(path); + + var first = try Layout.initForLoop(std.testing.allocator, "project-a", "loop-a"); + defer first.deinit(); + try first.addTab("loop-a", true); + try first.save(path); + var second = try Layout.initForLoop(std.testing.allocator, "project-a", "loop-a"); + defer second.deinit(); + try second.addTab("loop-a", true); + try second.addTab("shell", false); + try second.save(path); + var loaded = try Layout.loadFor(std.testing.allocator, path, "project-a", "loop-a"); + defer loaded.deinit(); + try std.testing.expect(loaded.hasPane("shell")); + + // A layout that fails validation never touches the file. + second.selected_tab = 9; + try std.testing.expectError(error.InvalidTopology, second.save(path)); + var kept = try Layout.loadFor(std.testing.allocator, path, "project-a", "loop-a"); + kept.deinit(); + + var directory = try tmp.dir.openDir(".", .{ .iterate = true }); + defer directory.close(); + var iterator = directory.iterate(); + while (try iterator.next()) |entry| { + try std.testing.expect(!std.mem.endsWith(u8, entry.name, ".tmp")); + } } diff --git a/graphcode-windows/src/ZmxSession.zig b/graphcode-windows/src/ZmxSession.zig index 2f52bf5e..0c57b28c 100644 --- a/graphcode-windows/src/ZmxSession.zig +++ b/graphcode-windows/src/ZmxSession.zig @@ -12,6 +12,42 @@ pub fn allocName(allocator: std.mem.Allocator, session: []const u8) ![]u8 { return std.fmt.allocPrint(allocator, prefix ++ "{s}", .{session}); } +/// The uuid this shell mints for a tab or split's session (`WorkspaceLayout.newSurfaceID`): +/// lowercase 8-4-4-4-12 hex. The daemon names its agent sessions after the node's uuid in +/// uppercase, so the two namespaces never overlap and a case-sensitive test tells them apart. +pub fn isShellUuid(text: []const u8) bool { + if (text.len != 36) return false; + for (text, 0..) |byte, index| { + if (index == 8 or index == 13 or index == 18 or index == 23) { + if (byte != '-') return false; + } else if (!std.ascii.isDigit(byte) and !(byte >= 'a' and byte <= 'f')) return false; + } + return true; +} + +/// The session name of a pane id when the pane can only be one of this shell's own plain +/// shells: `graphcode-` plus a lowercase uuid, exactly. Null for anything else (an agent's +/// uppercase uuid, a foreign or unprefixed name), which this shell must never end. A gate +/// harness that sets `GRAPHCODE_SHELL_SESSION_PREFIX` makes the shell mint `-` +/// ids (see `WorkspaceLayout.newSurfaceID`), which are accepted under that exact prefix only. +pub fn shellSessionName(allocator: std.mem.Allocator, pane_id: []const u8) !?[]u8 { + const harness = std.process.getEnvVarOwned(allocator, "GRAPHCODE_SHELL_SESSION_PREFIX") catch ""; + defer if (harness.len != 0) allocator.free(harness); + return shellSessionNameWith(allocator, pane_id, harness); +} + +fn shellSessionNameWith(allocator: std.mem.Allocator, pane_id: []const u8, harness_prefix: []const u8) !?[]u8 { + const unprefixed = if (std.mem.startsWith(u8, pane_id, prefix)) pane_id[prefix.len..] else pane_id; + var uuid = unprefixed; + if (harness_prefix.len != 0 and std.mem.startsWith(u8, unprefixed, harness_prefix) and + unprefixed.len > harness_prefix.len and unprefixed[harness_prefix.len] == '-') + { + uuid = unprefixed[harness_prefix.len + 1 ..]; + } + if (!isShellUuid(uuid)) return null; + return try std.fmt.allocPrint(allocator, prefix ++ "{s}", .{unprefixed}); +} + pub const ChildStdio = enum { /// `zmx attach`: the shell writes terminal input and reads terminal output. attach, @@ -67,6 +103,57 @@ test "canonical zmx session names add the ownership prefix exactly once" { ); } +test "only a lowercase uuid under the graphcode prefix names a shell session this shell may end" { + const allocator = std.testing.allocator; + const uuid = "0a1b2c3d-4e5f-4a6b-8c7d-9e0f1a2b3c4d"; + const bare = (try shellSessionName(allocator, uuid)).?; + defer allocator.free(bare); + try std.testing.expectEqualStrings("graphcode-" ++ uuid, bare); + const prefixed = (try shellSessionName(allocator, "graphcode-" ++ uuid)).?; + defer allocator.free(prefixed); + try std.testing.expectEqualStrings("graphcode-" ++ uuid, prefixed); + + for ([_][]const u8{ + "0A1B2C3D-4E5F-4A6B-8C7D-9E0F1A2B3C4D", + "graphcode-0A1B2C3D-4E5F-4A6B-8C7D-9E0F1A2B3C4D", + "0a1b2c3d-4e5f-4a6b-8c7d-9E0F1A2B3C4D", + "other-" ++ uuid, + "Graphcode-" ++ uuid, + "graphcode-graphcode-" ++ uuid, + "alpha", + "graphcode-alpha", + "graphcode-", + "", + uuid ++ "0", + "0a1b2c3d4e5f4a6b8c7d9e0f1a2b3c4d", + "0g1b2c3d-4e5f-4a6b-8c7d-9e0f1a2b3c4d", + }) |not_ours| { + try std.testing.expect((try shellSessionName(allocator, not_ours)) == null); + } +} + +test "a gate harness's session prefix is accepted only as exactly that prefix before a lowercase uuid" { + const allocator = std.testing.allocator; + const uuid = "0a1b2c3d-4e5f-4a6b-8c7d-9e0f1a2b3c4d"; + const ok = (try shellSessionNameWith(allocator, "gs-abc-" ++ uuid, "gs-abc")).?; + defer allocator.free(ok); + try std.testing.expectEqualStrings("graphcode-gs-abc-" ++ uuid, ok); + const prefixed = (try shellSessionNameWith(allocator, "graphcode-gs-abc-" ++ uuid, "gs-abc")).?; + defer allocator.free(prefixed); + try std.testing.expectEqualStrings("graphcode-gs-abc-" ++ uuid, prefixed); + for ([_][]const u8{ + "other-" ++ uuid, + "gs-abc-gs-abc-" ++ uuid, + "gs-abc-0A1B2C3D-4E5F-4A6B-8C7D-9E0F1A2B3C4D", + "gs-abc" ++ uuid, + "gs-abc-alpha", + "gs-abc-", + }) |not_ours| { + try std.testing.expect((try shellSessionNameWith(allocator, not_ours, "gs-abc")) == null); + } + try std.testing.expect((try shellSessionNameWith(allocator, "gs-abc-" ++ uuid, "")) == null); +} + test "zmx listing capture has readable output without stdin or a console" { const listing = child(std.testing.allocator, &.{ "zmx.exe", "ls" }, null, .capture); try std.testing.expectEqual(std.process.Child.StdIo.Ignore, listing.stdin_behavior); From acc4019a6bc4da791a0c32242b0f0a2612c9918e Mon Sep 17 00:00:00 2001 From: Colin Neilens Date: Fri, 9 Oct 2026 23:43:45 -0700 Subject: [PATCH 2/4] Require a recorded owner before ending a shell session Name shape is not provenance: record each tab and split session the shell mints in shell-sessions beside the layouts and end only recorded, live sessions. A layout scan that is cut short or hits an unreadable file now refuses the kill instead of assuming nothing claims the session, and a corrupt layout that cannot be set aside fails the open instead of being overwritten. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Colin Neilens --- graphcode-windows/src/App.zig | 127 ++++++++++++++++- graphcode-windows/src/TerminalSurface.zig | 109 ++++++++++----- graphcode-windows/src/WorkspaceLayout.zig | 158 +++++++++++++++++----- graphcode-windows/src/ZmxSession.zig | 14 +- 4 files changed, 335 insertions(+), 73 deletions(-) diff --git a/graphcode-windows/src/App.zig b/graphcode-windows/src/App.zig index b48b0d30..2e9eb458 100644 --- a/graphcode-windows/src/App.zig +++ b/graphcode-windows/src/App.zig @@ -13333,6 +13333,27 @@ const LiveTerminalFixture = struct { return true; } + /// Records, as the shell does when it mints a tab's session, that these bare uuids are + /// its own shell sessions. + fn markOwned(self: *LiveTerminalFixture, ids: []const []const u8) !void { + try self.tmp.dir.makePath("shell-sessions"); + for (ids) |id| { + const name = try std.fmt.allocPrint(std.testing.allocator, "shell-sessions\\graphcode-{s}", .{id}); + defer std.testing.allocator.free(name); + try self.tmp.dir.writeFile(.{ .sub_path = name, .data = "" }); + } + } + + fn ownedMarker(self: *LiveTerminalFixture, session: []const u8) bool { + var name: [160]u8 = undefined; + const path = std.fmt.bufPrint(&name, "shell-sessions\\graphcode-{s}", .{session}) catch return false; + return self.layoutExists(path); + } + + fn markerGone(self: *LiveTerminalFixture, session: []const u8) bool { + return !self.ownedMarker(session); + } + fn newTab(self: *LiveTerminalFixture) !void { const workspace = &self.workspace; const bounds = TerminalWorkspace.chromeControlBounds(workspace.layout_origin_x, workspace.layout_origin_y, workspace.layout_width, 0); @@ -14998,6 +15019,7 @@ test "workspace layout: a project's first snapshot ends the shells of loops dele try fixture.saveLoopLayout(other_project, "elsewhere-loop", &.{ .{ .id = "elsewhere-loop", .agent = true }, .{ .id = shell_uuid_four } }); try fixture.saveLoopLayout(alias_project, "alias-loop", &.{ .{ .id = "alias-loop", .agent = true }, .{ .id = shell_uuid_five } }); try fixture.saveLoopLayout(fixture.project, "Upper-Loop", &.{ .{ .id = "Upper-Loop", .agent = true }, .{ .id = shell_uuid_six } }); + try fixture.markOwned(&.{ shell_uuid_one, shell_uuid_two, shell_uuid_three, shell_uuid_four, shell_uuid_five, shell_uuid_six }); try fixture.setLive(&.{ "loop-a", shell_uuid_one, shell_uuid_two, shell_uuid_three, shell_uuid_four, shell_uuid_five, shell_uuid_six }); fixture.predateLayouts(); try fixture.forgetGraph(); @@ -15026,6 +15048,7 @@ test "workspace layout: an empty or partial first snapshot sweeps nothing that t defer fixture.deinit(); try fixture.saveLoopLayout(fixture.project, "loop-a", &.{ .{ .id = "loop-a", .agent = true }, .{ .id = shell_uuid_one } }); try fixture.saveLoopLayout(fixture.project, "gone-loop", &.{ .{ .id = "gone-loop", .agent = true }, .{ .id = shell_uuid_two } }); + try fixture.markOwned(&.{ shell_uuid_one, shell_uuid_two }); try fixture.setLive(&.{ "loop-a", shell_uuid_one, shell_uuid_two }); fixture.predateLayouts(); try fixture.forgetGraph(); @@ -15069,6 +15092,7 @@ test "workspace layout: a loop deleted after the shell has seen the project is e try fixture.init(&.{ .{ .id = "loop-a" }, .{ .id = "loop-b" } }); defer fixture.deinit(); try fixture.saveLoopLayout(fixture.project, "loop-b", &.{ .{ .id = "loop-b", .agent = true }, .{ .id = shell_uuid_one } }); + try fixture.markOwned(&.{shell_uuid_one}); try fixture.setLive(&.{ "loop-a", "loop-b", shell_uuid_one }); try fixture.deliver(&.{.{ .id = "loop-a" }}); @@ -15094,6 +15118,7 @@ test "workspace layout: a shell session whose uuid is a graph node's id is never .{ .id = node_uuid }, .{ .id = shell_uuid_one }, }); + try fixture.markOwned(&.{ node_uuid, shell_uuid_one }); try fixture.setLive(&.{ "loop-a", node_uuid, shell_uuid_one }); fixture.predateLayouts(); try fixture.forgetGraph(); @@ -15128,7 +15153,7 @@ test "workspace layout: a layout that cannot be attributed to the deleted loop e try std.testing.expect(!fixture.killed("foreign-shell")); } -test "workspace layout: deleting a loop ends only live sessions in this shell's own namespace" { +test "workspace layout: deleting a loop ends only recorded, live, shell-shaped sessions nothing else names" { var fixture: LiveTerminalFixture = undefined; try fixture.init(&.{ .{ .id = "loop-a" } }); defer fixture.deinit(); @@ -15146,6 +15171,7 @@ test "workspace layout: deleting a loop ends only live sessions in this shell's .{ .id = shell_uuid_four }, }); try fixture.saveLoopLayout(fixture.project, "kept-loop", &.{ .{ .id = "kept-loop", .agent = true }, .{ .id = shell_uuid_three } }); + try fixture.markOwned(&.{ shell_uuid_one, shell_uuid_three, shell_uuid_four }); try fixture.setLive(&.{ "loop-a", agent_uuid_upper, foreign, "plain-name", shell_uuid_three, shell_uuid_one }); try std.testing.expectEqual(@as(usize, 1), fixture.workspace.retireLoop("gone-loop", fixture.project)); @@ -15162,6 +15188,7 @@ test "workspace layout: a loop's shells are ended only after a successful sessio try fixture.init(&.{ .{ .id = "loop-a" } }); defer fixture.deinit(); try fixture.saveLoopLayout(fixture.project, "gone-loop", &.{ .{ .id = "gone-loop", .agent = true }, .{ .id = shell_uuid_one } }); + try fixture.markOwned(&.{shell_uuid_one}); try fixture.setLive(&.{ "loop-a", shell_uuid_one }); try fixture.setFlag("ls-fail.txt", true); @@ -15280,6 +15307,104 @@ test "workspace layout: a corrupt, foreign or newer-schema loop layout is kept a try fixture.expectKills(""); } +test "workspace layout: a live session that only a layout names is never ended without proof the shell made it" { + var fixture: LiveTerminalFixture = undefined; + try fixture.init(&.{ .{ .id = "loop-a" } }); + defer fixture.deinit(); + // A foreign session that happens to look like a shell's: lowercase uuid, live, no node's + // id, named by no other layout. The name shape is not provenance. + try fixture.saveLoopLayout(fixture.project, "gone-loop", &.{ + .{ .id = "gone-loop", .agent = true }, + .{ .id = shell_uuid_one }, + .{ .id = shell_uuid_two }, + }); + try fixture.markOwned(&.{shell_uuid_two}); + try fixture.setLive(&.{ "loop-a", shell_uuid_one, shell_uuid_two }); + + try std.testing.expectEqual(@as(usize, 1), fixture.workspace.retireLoop("gone-loop", fixture.project)); + fixture.settle(); + try fixture.expectKills("kill graphcode-" ++ shell_uuid_two ++ " --force"); + try std.testing.expect(fixture.layoutExists("gone-loop.json.refused")); +} + +test "workspace layout: a tab's session is recorded as the shell's own when minted and forgotten once ended" { + var fixture: LiveTerminalFixture = undefined; + try fixture.init(&.{ .{ .id = "loop-a" } }); + defer fixture.deinit(); + const app = &fixture.app; + const workspace = &fixture.workspace; + + try fixture.setLive(&.{"loop-a"}); + try clickSidebarLoopRow(app, fixture.project, "loop-a"); + try fixture.waitFor(LiveTerminalFixture.shows, "loop-a"); + try fixture.newTab(); + const tab_shell = try fixture.selectedPane(0); + defer std.testing.allocator.free(tab_shell); + try fixture.splitRight(); + const split_shell = try fixture.selectedPane(1); + defer std.testing.allocator.free(split_shell); + try std.testing.expect(fixture.ownedMarker(tab_shell)); + try std.testing.expect(fixture.ownedMarker(split_shell)); + try std.testing.expect(!fixture.ownedMarker("loop-a")); + try fixture.setLive(&.{ "loop-a", tab_shell, split_shell }); + + const shell_tab = TerminalWorkspace.tabBounds(workspace.layout_origin_x, workspace.layout_origin_y, 1); + try nativeClick(app, shell_tab.right - 12, @divTrunc(shell_tab.top + shell_tab.bottom, 2)); + try fixture.waitFor(LiveTerminalFixture.killed, tab_shell); + try fixture.waitFor(LiveTerminalFixture.markerGone, tab_shell); + // Closing the tab ends both of its panes' sessions, and each record goes with its session. + try fixture.waitFor(LiveTerminalFixture.killed, split_shell); + try fixture.waitFor(LiveTerminalFixture.markerGone, split_shell); + try std.testing.expect(!fixture.killed("loop-a")); +} + +test "workspace layout: a layout scan cut short by its cap refuses the kill instead of assuming nothing claims it" { + var fixture: LiveTerminalFixture = undefined; + try fixture.init(&.{ .{ .id = "loop-a" } }); + defer fixture.deinit(); + try fixture.saveLoopLayout(fixture.project, "gone-loop", &.{ .{ .id = "gone-loop", .agent = true }, .{ .id = shell_uuid_one } }); + try fixture.markOwned(&.{shell_uuid_one}); + try fixture.setLive(&.{ "loop-a", shell_uuid_one }); + // Past the scan cap, a layout that names the session sorts after every other file. + for (0..1100) |index| { + var name: [32]u8 = undefined; + try fixture.tmp.dir.writeFile(.{ .sub_path = try std.fmt.bufPrint(&name, "filler-{d:0>4}.json", .{index}), .data = "{\"tabs\":[]}" }); + } + try fixture.tmp.dir.writeFile(.{ + .sub_path = "zzz-claimant.json", + .data = "{\"tabs\":[{\"panes\":[{\"id\":\"" ++ shell_uuid_one ++ "\"}]}]}", + }); + + try std.testing.expectEqual(@as(usize, 0), fixture.workspace.retireLoop("gone-loop", fixture.project)); + fixture.settle(); + try fixture.expectKills(""); +} + +test "workspace layout: a corrupt layout that cannot be set aside is left as it is and its loop is not opened" { + var fixture: LiveTerminalFixture = undefined; + try fixture.init(&.{ .{ .id = "loop-a" } }); + defer fixture.deinit(); + const app = &fixture.app; + const truncated = "{\"schemaVersion\":2,\"project\":\""; + try fixture.tmp.dir.writeFile(.{ .sub_path = "loop-a.json", .data = truncated }); + // Every name the layout could be set aside under is taken. + try fixture.tmp.dir.writeFile(.{ .sub_path = "loop-a.json.bad", .data = "earlier" }); + for (1..100) |attempt| { + var name: [32]u8 = undefined; + try fixture.tmp.dir.writeFile(.{ .sub_path = try std.fmt.bufPrint(&name, "loop-a.json.bad.{d}", .{attempt}), .data = "earlier" }); + } + try fixture.setLive(&.{"loop-a"}); + + try clickSidebarLoopRow(app, fixture.project, "loop-a"); + fixture.settle(); + try std.testing.expectEqualStrings("Unable to open selected loop", app.status()); + var buffer: [128]u8 = undefined; + try std.testing.expectEqualStrings(truncated, try fixture.tmp.dir.readFile("loop-a.json", &buffer)); + try std.testing.expectEqualStrings("earlier", try fixture.tmp.dir.readFile("loop-a.json.bad", &buffer)); + try std.testing.expectEqual(@as(usize, 0), fixture.workspace.loop_id.len); + try fixture.expectKills(""); +} + /// Every terminal action must report something other than its failure status. fn expectNoTerminalFailure(app: *App) !void { for ([_][]const u8{ "Unable to open selected loop", "Unable to create tab", "Unable to split workspace" }) |failure| { diff --git a/graphcode-windows/src/TerminalSurface.zig b/graphcode-windows/src/TerminalSurface.zig index 1dd2c3f6..f807b5de 100644 --- a/graphcode-windows/src/TerminalSurface.zig +++ b/graphcode-windows/src/TerminalSurface.zig @@ -378,11 +378,14 @@ const KillJob = struct { child: std.process.Child, argv: [][]const u8, names: [][]u8, + /// The layout directory whose ownership records name these sessions; empty when none. + owner_root: []u8, fn deinit(self: *KillJob, allocator: std.mem.Allocator) void { for (self.names) |name| allocator.free(name); allocator.free(self.names); allocator.free(self.argv); + allocator.free(self.owner_root); } }; @@ -401,22 +404,22 @@ pub const KnownNodes = struct { }; /// A saved project layout, or null when there is none to use. One whose contents are not a /// layout of this shell (corrupt, another project's, a newer schema) is moved aside as `.bad` -/// rather than left for the next save to overwrite. -fn loadSavedLayout(allocator: std.mem.Allocator, path: []const u8, project: []const u8) ?WorkspaceLayout.Layout { +/// rather than left for the next save to overwrite; when it cannot be moved aside the load +/// fails, since the caller would otherwise write a fresh layout over it. +fn loadSavedLayout(allocator: std.mem.Allocator, path: []const u8, project: []const u8) !?WorkspaceLayout.Layout { return WorkspaceLayout.Layout.load(allocator, path, project) catch |err| { - keepUnusableLayout(allocator, path, err); + try keepUnusableLayout(allocator, path, err); return null; }; } -fn keepUnusableLayout(allocator: std.mem.Allocator, path: []const u8, err: anyerror) void { +fn keepUnusableLayout(allocator: std.mem.Allocator, path: []const u8, err: anyerror) !void { if (!WorkspaceLayout.isInvalidLayout(err)) return; - const kept = WorkspaceLayout.setAside(allocator, path, "bad"); - std.log.warn("workspace layout {s} is not usable ({s}); {s}", .{ - path, - @errorName(err), - if (kept) "kept as .bad" else "left in place", - }); + if (!WorkspaceLayout.setAside(allocator, path, "bad")) { + std.log.warn("workspace layout {s} is not usable ({s}) and could not be set aside; left untouched", .{ path, @errorName(err) }); + return error.LayoutNotPreserved; + } + std.log.warn("workspace layout {s} is not usable ({s}); kept as .bad", .{ path, @errorName(err) }); } /// The project key under which a quick chat's layout is saved. It is no project's path, so @@ -558,9 +561,15 @@ pub const Workspace = struct { } workspace.layout_path = try workspace.layoutPathForProject(workspace.project_key); if (workspace.layout_path.len != 0) { - if (loadSavedLayout(allocator_, workspace.layout_path, workspace.project_key)) |restored| { - workspace.layout.deinit(); - workspace.layout = restored; + if (loadSavedLayout(allocator_, workspace.layout_path, workspace.project_key)) |maybe_restored| { + if (maybe_restored) |restored| { + workspace.layout.deinit(); + workspace.layout = restored; + } + } else |_| { + // Unusable and not preserved: persistence stays off so no save overwrites it. + allocator_.free(workspace.layout_path); + workspace.layout_path = &.{}; } } if (c.winghostty_host_initialize(&workspace.host) != c.WINGHOSTTY_OK) { @@ -628,7 +637,7 @@ pub const Workspace = struct { errdefer self.allocator.free(new_layout_path); var new_layout = try WorkspaceLayout.Layout.init(self.allocator, project); errdefer new_layout.deinit(); - if (loadSavedLayout(self.allocator, new_layout_path, project)) |restored| { + if (try loadSavedLayout(self.allocator, new_layout_path, project)) |restored| { new_layout.deinit(); new_layout = restored; } @@ -671,7 +680,7 @@ pub const Workspace = struct { if (key_changed) { new_layout = try WorkspaceLayout.Layout.init(self.allocator, project_path); errdefer new_layout.deinit(); - if (loadSavedLayout(self.allocator, new_layout_path, project_path)) |restored| { + if (try loadSavedLayout(self.allocator, new_layout_path, project_path)) |restored| { new_layout.deinit(); new_layout = restored; } @@ -837,7 +846,7 @@ pub const Workspace = struct { // Not known to be bad, so not replaced: the next save would overwrite it. return err; } else { - keepUnusableLayout(self.allocator, path, err); + try keepUnusableLayout(self.allocator, path, err); } } return WorkspaceLayout.Layout.initForLoop(self.allocator, layout_project, loop); @@ -1005,25 +1014,33 @@ pub const Workspace = struct { refused: usize = 0, }; - /// Whether this shell may end the zmx session `name` (already a valid shell session name). - fn mayEndShell(self: *Workspace, name: []const u8, options: EndOptions) bool { + /// Why this shell must not end the zmx session `name` (already a valid shell session + /// name), or null when it may. Ownership is proven by the record the shell made when it + /// minted the session, never by the name's shape; a session with no record, one a graph + /// node or another layout also names, and one whose layouts could not all be read are + /// all refused. + fn refuseEnding(self: *Workspace, name: []const u8, options: EndOptions) ?[]const u8 { const uuid = name[ZmxSession.prefix.len..]; - if (self.loop_id.len != 0 and std.ascii.eqlIgnoreCase(uuid, self.loop_id)) return false; + const root = self.layoutRoot() orelse return "no layout directory records which sessions this shell made"; + if (!WorkspaceLayout.isShellOwned(self.allocator, root, name)) return "this shell has no record of having made it"; + if (self.loop_id.len != 0 and std.ascii.eqlIgnoreCase(uuid, self.loop_id)) return "it is the open loop's session"; if (options.known) |known| { - if (known.contains(known.context, uuid)) return false; + if (known.contains(known.context, uuid)) return "it names a node of the graph"; } - if (options.check_open_layout and (self.layout.hasPane(uuid) or self.layout.hasPane(name))) return false; - if (self.layoutRoot()) |root| { - if (WorkspaceLayout.claimedByOtherLayout(self.allocator, root, options.except_layout, name)) return false; - } - return true; + if (options.check_open_layout and (self.layout.hasPane(uuid) or self.layout.hasPane(name))) return "the open layout also names it"; + return switch (WorkspaceLayout.claimedByOtherLayout(self.allocator, root, options.except_layout, name)) { + .unclaimed => null, + .claimed => "another layout also names it", + .unknown => "the other saved layouts could not all be read", + }; } /// `zmx kill --force`, run beside the UI and reaped by `poll`, for the panes in - /// `ids` that are provably this shell's own plain shells: a name that is exactly - /// `graphcode-` plus a lowercase uuid (never an agent's uppercase one, a foreign or an - /// unprefixed name), that is no node's id and no other layout's pane, and that a fresh, - /// successful `zmx ls` shows running. Anything else is refused and logged, never ended. + /// `ids` whose session has the shape of a shell's (exactly `graphcode-` plus a lowercase + /// uuid, never an agent's uppercase one, a foreign or an unprefixed name), that this shell + /// recorded having minted (`refuseEnding`), that is no node's id and no other layout's + /// pane, and that a fresh, successful `zmx ls` shows running. Anything else is refused and + /// logged, never ended. fn endShellSessions(self: *Workspace, ids: []const []const u8, options: EndOptions) EndOutcome { var outcome: EndOutcome = .{}; var names: std.ArrayListUnmanaged([]u8) = .empty; @@ -1046,8 +1063,8 @@ pub const Workspace = struct { self.allocator.free(name); continue; } - if (!self.mayEndShell(name, options)) { - std.log.warn("not ending zmx session {s}: it names a node or belongs to another layout", .{name}); + if (self.refuseEnding(name, options)) |reason| { + std.log.warn("not ending zmx session {s}: {s}", .{ name, reason }); self.allocator.free(name); outcome.refused += 1; continue; @@ -1071,6 +1088,8 @@ pub const Workspace = struct { names.items[live] = name; live += 1; } else { + // Already gone: nothing is left to end, so the record of it goes too. + if (self.layoutRoot()) |root| WorkspaceLayout.forgetShellOwned(self.allocator, root, name); self.allocator.free(name); } } @@ -1097,11 +1116,20 @@ pub const Workspace = struct { outcome.started = false; return outcome; }; - self.kill_jobs.append(self.allocator, .{ .child = child, .argv = argv, .names = owned }) catch { + const owner_root = self.allocator.dupe(u8, self.layoutRoot() orelse "") catch { + _ = child.kill() catch {}; + for (owned) |name| self.allocator.free(name); + self.allocator.free(owned); + self.allocator.free(argv); + outcome.started = false; + return outcome; + }; + self.kill_jobs.append(self.allocator, .{ .child = child, .argv = argv, .names = owned, .owner_root = owner_root }) catch { _ = child.kill() catch {}; for (owned) |name| self.allocator.free(name); self.allocator.free(owned); self.allocator.free(argv); + self.allocator.free(owner_root); outcome.started = false; return outcome; }; @@ -1119,6 +1147,9 @@ pub const Workspace = struct { continue; } _ = job.child.wait() catch {}; + if (exit_code == 0 and job.owner_root.len != 0) { + for (job.names) |name| WorkspaceLayout.forgetShellOwned(self.allocator, job.owner_root, name); + } var finished = self.kill_jobs.orderedRemove(index); finished.deinit(self.allocator); } @@ -1524,9 +1555,22 @@ pub const Workspace = struct { self.syncTopology(); } + /// Records, before any attach can create the session, that the shell minted this tab's + /// session: only a session with such a record is ever ended by the shell (`refuseEnding`). + /// Best effort: a tab whose record could not be written works, but is never ended for it. + fn recordShellOwned(self: *Workspace, surface_id: []const u8) void { + const root = self.layoutRoot() orelse return; + const name = ZmxSession.allocName(self.allocator, surface_id) catch return; + defer self.allocator.free(name); + WorkspaceLayout.markShellOwned(self.allocator, root, name) catch |err| { + std.log.warn("could not record shell session {s} as this shell's own: {s}", .{ name, @errorName(err) }); + }; + } + pub fn newTab(self: *Workspace) !void { const surface_id = try self.layout.newSurfaceID(); defer self.allocator.free(surface_id); + self.recordShellOwned(surface_id); const previous_selected = self.layout.selected_tab; const previous_next_id = self.layout.next_tab_id; const index = try self.createAttachedSurface(surface_id, try self.workspaceGridSize()); @@ -1757,6 +1801,7 @@ pub const Workspace = struct { pub fn splitFocused(self: *Workspace, direction: WorkspaceLayout.Direction) !void { const surface_id = try self.layout.newSurfaceID(); defer self.allocator.free(surface_id); + self.recordShellOwned(surface_id); const tab = self.layout.selected() orelse return error.NoTabs; const previous_focus = tab.focused_pane; const previous_direction = tab.split_direction; diff --git a/graphcode-windows/src/WorkspaceLayout.zig b/graphcode-windows/src/WorkspaceLayout.zig index 8db8a468..55ede8a7 100644 --- a/graphcode-windows/src/WorkspaceLayout.zig +++ b/graphcode-windows/src/WorkspaceLayout.zig @@ -535,52 +535,110 @@ fn sameSession(a: []const u8, b: []const u8) bool { return std.mem.eql(u8, left, right); } +pub const Claim = enum { claimed, unclaimed, unknown }; + /// Whether any layout file in `root` other than `except_file` names `session` as a pane, /// whatever its project or loop. A session two layouts both claim is not safely one's to end. -/// Files that cannot be read claim nothing. -pub fn claimedByOtherLayout(allocator: std.mem.Allocator, root: []const u8, except_file: []const u8, session: []const u8) bool { - var directory = std.fs.cwd().openDir(root, .{ .iterate = true }) catch return false; +/// `unknown` when the scan could not be completed (more files than the cap, a directory or +/// file that could not be read, or one that is not a layout): absence of a claim is only +/// known from a scan that saw every layout. +pub fn claimedByOtherLayout(allocator: std.mem.Allocator, root: []const u8, except_file: []const u8, session: []const u8) Claim { + var directory = std.fs.cwd().openDir(root, .{ .iterate = true }) catch |err| return if (err == error.FileNotFound) .unclaimed else .unknown; defer directory.close(); const except_name = std.fs.path.basename(except_file); var iterator = directory.iterate(); var seen: usize = 0; - while (iterator.next() catch null) |entry| { + var complete = true; + while (true) { + const entry = (iterator.next() catch { + complete = false; + break; + }) orelse break; if (entry.kind != .file or !std.mem.endsWith(u8, entry.name, ".json")) continue; if (std.ascii.eqlIgnoreCase(entry.name, except_name)) continue; seen += 1; - if (seen > max_scanned_layouts) break; - const data = directory.readFileAlloc(allocator, entry.name, 4 * 1024 * 1024) catch continue; - defer allocator.free(data); - var parsed = std.json.parseFromSlice(std.json.Value, allocator, data, .{}) catch continue; - defer parsed.deinit(); - const root_object = switch (parsed.value) { + if (seen > max_scanned_layouts) { + complete = false; + break; + } + switch (fileClaim(allocator, directory, entry.name, session)) { + .claimed => return .claimed, + .unclaimed => {}, + .unknown => complete = false, + } + } + return if (complete) .unclaimed else .unknown; +} + +fn fileClaim(allocator: std.mem.Allocator, directory: std.fs.Dir, name: []const u8, session: []const u8) Claim { + const data = directory.readFileAlloc(allocator, name, 4 * 1024 * 1024) catch |err| { + // A file that vanished mid-scan claims nothing; one that cannot be read might. + return if (err == error.FileNotFound) .unclaimed else .unknown; + }; + defer allocator.free(data); + var parsed = std.json.parseFromSlice(std.json.Value, allocator, data, .{}) catch return .unknown; + defer parsed.deinit(); + const root_object = switch (parsed.value) { + .object => |value| value, + else => return .unknown, + }; + const tabs = switch (root_object.get("tabs") orelse return .unknown) { + .array => |value| value.items, + else => return .unknown, + }; + for (tabs) |tab| { + const tab_object = switch (tab) { .object => |value| value, - else => continue, + else => return .unknown, }; - const tabs = switch (root_object.get("tabs") orelse continue) { + const panes = switch (tab_object.get("panes") orelse return .unknown) { .array => |value| value.items, - else => continue, + else => return .unknown, }; - for (tabs) |tab| { - const tab_object = switch (tab) { + for (panes) |pane| { + const pane_object = switch (pane) { .object => |value| value, - else => continue, - }; - const panes = switch (tab_object.get("panes") orelse continue) { - .array => |value| value.items, - else => continue, + else => return .unknown, }; - for (panes) |pane| { - const pane_object = switch (pane) { - .object => |value| value, - else => continue, - }; - const id = pane_object.get("id") orelse continue; - if (id == .string and sameSession(id.string, session)) return true; - } + const id = pane_object.get("id") orelse return .unknown; + if (id != .string) return .unknown; + if (sameSession(id.string, session)) return .claimed; } } - return false; + return .unclaimed; +} + +pub const owned_sessions_directory_name = "shell-sessions"; + +/// The record that a shell session is this shell's own: a file named for the session in +/// `\shell-sessions`, made when the shell mints the session for a new tab or split and +/// before anything can attach to it. A name's shape cannot say who made the session; only +/// this record, which no layout edit or corruption touches, can. Null when `session` is not +/// a plain file name. +fn ownedSessionPath(allocator: std.mem.Allocator, root: []const u8, session: []const u8) ?[]u8 { + if (!isFileSafeID(session)) return null; + return std.fmt.allocPrint(allocator, "{s}\\{s}\\{s}", .{ root, owned_sessions_directory_name, session }) catch null; +} + +pub fn markShellOwned(allocator: std.mem.Allocator, root: []const u8, session: []const u8) !void { + const path = ownedSessionPath(allocator, root, session) orelse return error.InvalidSessionName; + defer allocator.free(path); + if (std.fs.path.dirname(path)) |directory| try std.fs.cwd().makePath(directory); + var file = try std.fs.cwd().createFile(path, .{ .truncate = true }); + file.close(); +} + +pub fn isShellOwned(allocator: std.mem.Allocator, root: []const u8, session: []const u8) bool { + const path = ownedSessionPath(allocator, root, session) orelse return false; + defer allocator.free(path); + std.fs.cwd().access(path, .{}) catch return false; + return true; +} + +pub fn forgetShellOwned(allocator: std.mem.Allocator, root: []const u8, session: []const u8) void { + const path = ownedSessionPath(allocator, root, session) orelse return; + defer allocator.free(path); + std.fs.cwd().deleteFile(path) catch {}; } /// A loop's layout from the project-wide legacy file, for a loop that has none of its own @@ -1019,12 +1077,44 @@ test "a pane another layout names is claimed, however the session is spelled" { const mine = try loopLayoutPath(std.testing.allocator, root, "loop-a"); defer std.testing.allocator.free(mine); - try std.testing.expect(claimedByOtherLayout(std.testing.allocator, root, mine, "graphcode-shared")); - try std.testing.expect(claimedByOtherLayout(std.testing.allocator, root, mine, "shared")); - try std.testing.expect(!claimedByOtherLayout(std.testing.allocator, root, mine, "unshared")); + try std.testing.expectEqual(Claim.claimed, claimedByOtherLayout(std.testing.allocator, root, mine, "graphcode-shared")); + try std.testing.expectEqual(Claim.claimed, claimedByOtherLayout(std.testing.allocator, root, mine, "shared")); + // An unreadable layout might name anything, so nothing is known to be unclaimed. + try std.testing.expectEqual(Claim.unknown, claimedByOtherLayout(std.testing.allocator, root, mine, "unshared")); + try tmp.dir.deleteFile("garbage.json"); + try std.testing.expectEqual(Claim.unclaimed, claimedByOtherLayout(std.testing.allocator, root, mine, "unshared")); // The layout being retired never makes its own panes shared. - try std.testing.expect(!claimedByOtherLayout(std.testing.allocator, root, other_path, "shared")); - try std.testing.expect(!claimedByOtherLayout(std.testing.allocator, "C:\\no\\such\\layouts", mine, "shared")); + try std.testing.expectEqual(Claim.unclaimed, claimedByOtherLayout(std.testing.allocator, root, other_path, "shared")); + try std.testing.expectEqual(Claim.unclaimed, claimedByOtherLayout(std.testing.allocator, "C:\\no\\such\\layouts", mine, "shared")); +} + +test "a layout scan past its cap is unknown, not unclaimed" { + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + const root = try tmp.dir.realpathAlloc(std.testing.allocator, "."); + defer std.testing.allocator.free(root); + for (0..max_scanned_layouts + 5) |index| { + var name: [32]u8 = undefined; + try tmp.dir.writeFile(.{ .sub_path = try std.fmt.bufPrint(&name, "filler-{d:0>4}.json", .{index}), .data = "{\"tabs\":[]}" }); + } + try tmp.dir.writeFile(.{ .sub_path = "zzz-claimant.json", .data = "{\"tabs\":[{\"panes\":[{\"id\":\"shared\"}]}]}" }); + try std.testing.expectEqual(Claim.unknown, claimedByOtherLayout(std.testing.allocator, root, "", "shared")); +} + +test "a shell session is owned only once the shell has recorded it, and not after it is forgotten" { + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + const root = try tmp.dir.realpathAlloc(std.testing.allocator, "."); + defer std.testing.allocator.free(root); + const name = "graphcode-5e11ba5e-0001-4000-8000-000000000001"; + try std.testing.expect(!isShellOwned(std.testing.allocator, root, name)); + try markShellOwned(std.testing.allocator, root, name); + try std.testing.expect(isShellOwned(std.testing.allocator, root, name)); + try std.testing.expect(!isShellOwned(std.testing.allocator, root, "graphcode-5e11ba5e-0002-4000-8000-000000000002")); + try std.testing.expect(!isShellOwned(std.testing.allocator, root, "..\\escape")); + try std.testing.expectError(error.InvalidSessionName, markShellOwned(std.testing.allocator, root, "..\\escape")); + forgetShellOwned(std.testing.allocator, root, name); + try std.testing.expect(!isShellOwned(std.testing.allocator, root, name)); } test "saving replaces a layout whole, leaves no temporary file, and keeps the old one when it fails" { diff --git a/graphcode-windows/src/ZmxSession.zig b/graphcode-windows/src/ZmxSession.zig index 0c57b28c..76ad3a0d 100644 --- a/graphcode-windows/src/ZmxSession.zig +++ b/graphcode-windows/src/ZmxSession.zig @@ -25,11 +25,13 @@ pub fn isShellUuid(text: []const u8) bool { return true; } -/// The session name of a pane id when the pane can only be one of this shell's own plain -/// shells: `graphcode-` plus a lowercase uuid, exactly. Null for anything else (an agent's -/// uppercase uuid, a foreign or unprefixed name), which this shell must never end. A gate -/// harness that sets `GRAPHCODE_SHELL_SESSION_PREFIX` makes the shell mint `-` -/// ids (see `WorkspaceLayout.newSurfaceID`), which are accepted under that exact prefix only. +/// The session name of a pane id when it has the shape of one of this shell's plain shells: +/// `graphcode-` plus a lowercase uuid, exactly. Null for anything else (an agent's uppercase +/// uuid, a foreign or unprefixed name). The shape is a necessary condition only: it does not +/// say who made the session, which the shell's own record does (`WorkspaceLayout.isShellOwned`). +/// A gate harness that sets `GRAPHCODE_SHELL_SESSION_PREFIX` makes the shell mint +/// `-` ids (see `WorkspaceLayout.newSurfaceID`), which have the shape under +/// that exact prefix only. pub fn shellSessionName(allocator: std.mem.Allocator, pane_id: []const u8) !?[]u8 { const harness = std.process.getEnvVarOwned(allocator, "GRAPHCODE_SHELL_SESSION_PREFIX") catch ""; defer if (harness.len != 0) allocator.free(harness); @@ -103,7 +105,7 @@ test "canonical zmx session names add the ownership prefix exactly once" { ); } -test "only a lowercase uuid under the graphcode prefix names a shell session this shell may end" { +test "only a lowercase uuid under the graphcode prefix has the shape of a shell session name" { const allocator = std.testing.allocator; const uuid = "0a1b2c3d-4e5f-4a6b-8c7d-9e0f1a2b3c4d"; const bare = (try shellSessionName(allocator, uuid)).?; From a341f5db3f067d7e6ae6217d4851c39c8110d71f Mon Sep 17 00:00:00 2001 From: Colin Neilens Date: Sat, 10 Oct 2026 00:28:28 -0700 Subject: [PATCH 3/4] Bind shell-session records to a loop, claim before starting, defer on unreadable scans The layout root is shared by every GraphCode shell of the user, so a record must name the loop it was minted for and only that loop's retirement may use it. New Tab and Split save the layout claim before the record is written and the session started. A scan that could not read every layout now defers the cleanup (layout kept) instead of setting it aside. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Colin Neilens --- graphcode-windows/src/App.zig | 117 ++++++++++++++++++--- graphcode-windows/src/TerminalSurface.zig | 118 ++++++++++++++-------- graphcode-windows/src/WorkspaceLayout.zig | 46 +++++---- 3 files changed, 207 insertions(+), 74 deletions(-) diff --git a/graphcode-windows/src/App.zig b/graphcode-windows/src/App.zig index 2e9eb458..b57ff99f 100644 --- a/graphcode-windows/src/App.zig +++ b/graphcode-windows/src/App.zig @@ -12992,6 +12992,8 @@ const fake_zmx_script = "exit /b 0\r\n" ++ ":attach\r\n" ++ ">\"%~dp0cwd-%~2.txt\" echo %CD%\r\n" ++ + "if exist \"%~dp0loop-a.json\" copy /y \"%~dp0loop-a.json\" \"%~dp0layout-at-attach-%~2.txt\" >nul\r\n" ++ + "if exist \"%~dp0shell-sessions\\%~2\" (echo yes>\"%~dp0marker-at-attach-%~2.txt\") else (echo no>\"%~dp0marker-at-attach-%~2.txt\")\r\n" ++ "pause >nul\r\n" ++ "exit /b 0\r\n"; @@ -13333,14 +13335,14 @@ const LiveTerminalFixture = struct { return true; } - /// Records, as the shell does when it mints a tab's session, that these bare uuids are - /// its own shell sessions. - fn markOwned(self: *LiveTerminalFixture, ids: []const []const u8) !void { + /// Records, as the shell does when it mints a tab's session for a loop, that these bare + /// uuids are shell sessions minted for `loop`. + fn markOwned(self: *LiveTerminalFixture, loop: []const u8, ids: []const []const u8) !void { try self.tmp.dir.makePath("shell-sessions"); for (ids) |id| { const name = try std.fmt.allocPrint(std.testing.allocator, "shell-sessions\\graphcode-{s}", .{id}); defer std.testing.allocator.free(name); - try self.tmp.dir.writeFile(.{ .sub_path = name, .data = "" }); + try self.tmp.dir.writeFile(.{ .sub_path = name, .data = loop }); } } @@ -13354,6 +13356,14 @@ const LiveTerminalFixture = struct { return !self.ownedMarker(session); } + /// What the session's attach process found when it started: the loop-a layout file as saved + /// and whether its ownership record existed. + fn attachSaw(self: *LiveTerminalFixture, session: []const u8) bool { + var name: [160]u8 = undefined; + const path = std.fmt.bufPrint(&name, "marker-at-attach-graphcode-{s}.txt", .{session}) catch return false; + return self.layoutExists(path); + } + fn newTab(self: *LiveTerminalFixture) !void { const workspace = &self.workspace; const bounds = TerminalWorkspace.chromeControlBounds(workspace.layout_origin_x, workspace.layout_origin_y, workspace.layout_width, 0); @@ -15019,7 +15029,12 @@ test "workspace layout: a project's first snapshot ends the shells of loops dele try fixture.saveLoopLayout(other_project, "elsewhere-loop", &.{ .{ .id = "elsewhere-loop", .agent = true }, .{ .id = shell_uuid_four } }); try fixture.saveLoopLayout(alias_project, "alias-loop", &.{ .{ .id = "alias-loop", .agent = true }, .{ .id = shell_uuid_five } }); try fixture.saveLoopLayout(fixture.project, "Upper-Loop", &.{ .{ .id = "Upper-Loop", .agent = true }, .{ .id = shell_uuid_six } }); - try fixture.markOwned(&.{ shell_uuid_one, shell_uuid_two, shell_uuid_three, shell_uuid_four, shell_uuid_five, shell_uuid_six }); + try fixture.markOwned("gone-loop", &.{shell_uuid_one}); + try fixture.markOwned("loop-a", &.{shell_uuid_two}); + try fixture.markOwned("nested-loop", &.{shell_uuid_three}); + try fixture.markOwned("elsewhere-loop", &.{shell_uuid_four}); + try fixture.markOwned("alias-loop", &.{shell_uuid_five}); + try fixture.markOwned("Upper-Loop", &.{shell_uuid_six}); try fixture.setLive(&.{ "loop-a", shell_uuid_one, shell_uuid_two, shell_uuid_three, shell_uuid_four, shell_uuid_five, shell_uuid_six }); fixture.predateLayouts(); try fixture.forgetGraph(); @@ -15048,7 +15063,8 @@ test "workspace layout: an empty or partial first snapshot sweeps nothing that t defer fixture.deinit(); try fixture.saveLoopLayout(fixture.project, "loop-a", &.{ .{ .id = "loop-a", .agent = true }, .{ .id = shell_uuid_one } }); try fixture.saveLoopLayout(fixture.project, "gone-loop", &.{ .{ .id = "gone-loop", .agent = true }, .{ .id = shell_uuid_two } }); - try fixture.markOwned(&.{ shell_uuid_one, shell_uuid_two }); + try fixture.markOwned("loop-a", &.{shell_uuid_one}); + try fixture.markOwned("gone-loop", &.{shell_uuid_two}); try fixture.setLive(&.{ "loop-a", shell_uuid_one, shell_uuid_two }); fixture.predateLayouts(); try fixture.forgetGraph(); @@ -15092,7 +15108,7 @@ test "workspace layout: a loop deleted after the shell has seen the project is e try fixture.init(&.{ .{ .id = "loop-a" }, .{ .id = "loop-b" } }); defer fixture.deinit(); try fixture.saveLoopLayout(fixture.project, "loop-b", &.{ .{ .id = "loop-b", .agent = true }, .{ .id = shell_uuid_one } }); - try fixture.markOwned(&.{shell_uuid_one}); + try fixture.markOwned("loop-b", &.{shell_uuid_one}); try fixture.setLive(&.{ "loop-a", "loop-b", shell_uuid_one }); try fixture.deliver(&.{.{ .id = "loop-a" }}); @@ -15118,7 +15134,7 @@ test "workspace layout: a shell session whose uuid is a graph node's id is never .{ .id = node_uuid }, .{ .id = shell_uuid_one }, }); - try fixture.markOwned(&.{ node_uuid, shell_uuid_one }); + try fixture.markOwned("gone-loop", &.{ node_uuid, shell_uuid_one }); try fixture.setLive(&.{ "loop-a", node_uuid, shell_uuid_one }); fixture.predateLayouts(); try fixture.forgetGraph(); @@ -15171,7 +15187,7 @@ test "workspace layout: deleting a loop ends only recorded, live, shell-shaped s .{ .id = shell_uuid_four }, }); try fixture.saveLoopLayout(fixture.project, "kept-loop", &.{ .{ .id = "kept-loop", .agent = true }, .{ .id = shell_uuid_three } }); - try fixture.markOwned(&.{ shell_uuid_one, shell_uuid_three, shell_uuid_four }); + try fixture.markOwned("gone-loop", &.{ shell_uuid_one, shell_uuid_three, shell_uuid_four }); try fixture.setLive(&.{ "loop-a", agent_uuid_upper, foreign, "plain-name", shell_uuid_three, shell_uuid_one }); try std.testing.expectEqual(@as(usize, 1), fixture.workspace.retireLoop("gone-loop", fixture.project)); @@ -15188,7 +15204,7 @@ test "workspace layout: a loop's shells are ended only after a successful sessio try fixture.init(&.{ .{ .id = "loop-a" } }); defer fixture.deinit(); try fixture.saveLoopLayout(fixture.project, "gone-loop", &.{ .{ .id = "gone-loop", .agent = true }, .{ .id = shell_uuid_one } }); - try fixture.markOwned(&.{shell_uuid_one}); + try fixture.markOwned("gone-loop", &.{shell_uuid_one}); try fixture.setLive(&.{ "loop-a", shell_uuid_one }); try fixture.setFlag("ls-fail.txt", true); @@ -15318,7 +15334,7 @@ test "workspace layout: a live session that only a layout names is never ended w .{ .id = shell_uuid_one }, .{ .id = shell_uuid_two }, }); - try fixture.markOwned(&.{shell_uuid_two}); + try fixture.markOwned("gone-loop", &.{shell_uuid_two}); try fixture.setLive(&.{ "loop-a", shell_uuid_one, shell_uuid_two }); try std.testing.expectEqual(@as(usize, 1), fixture.workspace.retireLoop("gone-loop", fixture.project)); @@ -15363,7 +15379,7 @@ test "workspace layout: a layout scan cut short by its cap refuses the kill inst try fixture.init(&.{ .{ .id = "loop-a" } }); defer fixture.deinit(); try fixture.saveLoopLayout(fixture.project, "gone-loop", &.{ .{ .id = "gone-loop", .agent = true }, .{ .id = shell_uuid_one } }); - try fixture.markOwned(&.{shell_uuid_one}); + try fixture.markOwned("gone-loop", &.{shell_uuid_one}); try fixture.setLive(&.{ "loop-a", shell_uuid_one }); // Past the scan cap, a layout that names the session sorts after every other file. for (0..1100) |index| { @@ -15380,6 +15396,83 @@ test "workspace layout: a layout scan cut short by its cap refuses the kill inst try fixture.expectKills(""); } +test "workspace layout: a session another loop's record names is never ended by a different loop's retirement" { + var fixture: LiveTerminalFixture = undefined; + try fixture.init(&.{ .{ .id = "loop-a" } }); + defer fixture.deinit(); + // The window in a second shell sharing this layout directory: it has recorded a fresh + // session for its own loop-a, while a stale layout of the deleted gone-loop names the uuid. + try fixture.saveLoopLayout(fixture.project, "gone-loop", &.{ .{ .id = "gone-loop", .agent = true }, .{ .id = shell_uuid_one } }); + try fixture.markOwned("loop-a", &.{shell_uuid_one}); + try fixture.setLive(&.{ "loop-a", shell_uuid_one }); + + try std.testing.expectEqual(@as(usize, 0), fixture.workspace.retireLoop("gone-loop", fixture.project)); + fixture.settle(); + try fixture.expectKills(""); + try std.testing.expect(fixture.layoutExists("gone-loop.json.refused")); +} + +test "workspace layout: a new tab's layout claim and ownership record exist before its session starts" { + var fixture: LiveTerminalFixture = undefined; + try fixture.init(&.{ .{ .id = "loop-a" } }); + defer fixture.deinit(); + const app = &fixture.app; + + try fixture.setLive(&.{"loop-a"}); + try clickSidebarLoopRow(app, fixture.project, "loop-a"); + try fixture.waitFor(LiveTerminalFixture.shows, "loop-a"); + try fixture.newTab(); + const tab_shell = try fixture.selectedPane(0); + defer std.testing.allocator.free(tab_shell); + try fixture.splitRight(); + const split_shell = try fixture.selectedPane(1); + defer std.testing.allocator.free(split_shell); + + for ([_][]const u8{ tab_shell, split_shell }) |session| { + try fixture.waitFor(LiveTerminalFixture.attachSaw, session); + const name = try std.fmt.allocPrint(std.testing.allocator, "marker-at-attach-graphcode-{s}.txt", .{session}); + defer std.testing.allocator.free(name); + const marker = try fixture.tmp.dir.readFileAlloc(std.testing.allocator, name, 64); + defer std.testing.allocator.free(marker); + try std.testing.expectEqualStrings("yes", std.mem.trim(u8, marker, " \r\n")); + } + const claim_name = try std.fmt.allocPrint(std.testing.allocator, "layout-at-attach-graphcode-{s}.txt", .{split_shell}); + defer std.testing.allocator.free(claim_name); + const claim = try fixture.tmp.dir.readFileAlloc(std.testing.allocator, claim_name, 1 << 16); + defer std.testing.allocator.free(claim); + try std.testing.expect(std.mem.indexOf(u8, claim, split_shell) != null); + try std.testing.expect(std.mem.indexOf(u8, claim, tab_shell) != null); + // The record names the loop the session was minted for. + const record_name = try std.fmt.allocPrint(std.testing.allocator, "shell-sessions\\graphcode-{s}", .{tab_shell}); + defer std.testing.allocator.free(record_name); + var buffer: [64]u8 = undefined; + try std.testing.expectEqualStrings("loop-a", try fixture.tmp.dir.readFile(record_name, &buffer)); +} + +test "workspace layout: an unreadable unrelated layout defers the cleanup, and the next sweep ends the shell" { + var fixture: LiveTerminalFixture = undefined; + try fixture.init(&.{ .{ .id = "loop-a" } }); + defer fixture.deinit(); + try fixture.saveLoopLayout(fixture.project, "gone-loop", &.{ .{ .id = "gone-loop", .agent = true }, .{ .id = shell_uuid_one } }); + try fixture.markOwned("gone-loop", &.{shell_uuid_one}); + try fixture.setLive(&.{ "loop-a", shell_uuid_one }); + try fixture.tmp.dir.writeFile(.{ .sub_path = "unrelated.json", .data = "{ not a layout" }); + + try std.testing.expectEqual(@as(usize, 0), fixture.workspace.retireLoop("gone-loop", fixture.project)); + fixture.settle(); + try fixture.expectKills(""); + // Kept as it was, so a later sweep can retry; not set aside as a permanent refusal. + try std.testing.expect(fixture.layoutExists("gone-loop.json")); + try std.testing.expect(!fixture.layoutExists("gone-loop.json.refused")); + try std.testing.expect(fixture.ownedMarker(shell_uuid_one)); + + try fixture.tmp.dir.deleteFile("unrelated.json"); + try std.testing.expectEqual(@as(usize, 1), fixture.workspace.retireLoop("gone-loop", fixture.project)); + fixture.settle(); + try fixture.expectKills("kill graphcode-" ++ shell_uuid_one ++ " --force"); + try std.testing.expect(!fixture.layoutExists("gone-loop.json")); +} + test "workspace layout: a corrupt layout that cannot be set aside is left as it is and its loop is not opened" { var fixture: LiveTerminalFixture = undefined; try fixture.init(&.{ .{ .id = "loop-a" } }); diff --git a/graphcode-windows/src/TerminalSurface.zig b/graphcode-windows/src/TerminalSurface.zig index f807b5de..164ea727 100644 --- a/graphcode-windows/src/TerminalSurface.zig +++ b/graphcode-windows/src/TerminalSurface.zig @@ -921,6 +921,7 @@ pub const Workspace = struct { .known = known, .except_layout = path, .check_open_layout = !open, + .owner = loop, }); if (open) { if (WorkspaceLayout.Layout.init(self.allocator, self.layout.project_key)) |emptied| { @@ -934,7 +935,7 @@ pub const Workspace = struct { self.clearLoopScope(); } else |_| {} } - if (!outcome.listed or !outcome.started) return 0; + if (!outcome.listed or !outcome.started or outcome.deferred != 0) return 0; if (outcome.refused != 0) { _ = WorkspaceLayout.setAside(self.allocator, path, "refused"); } else { @@ -1003,6 +1004,8 @@ pub const Workspace = struct { except_layout: []const u8 = "", /// Whether the layout open in memory is another loop's, and so can share a session. check_open_layout: bool = true, + /// The loop whose layout names the sessions: an ownership record must name it too. + owner: []const u8 = "", }; const EndOutcome = struct { @@ -1011,27 +1014,34 @@ pub const Workspace = struct { /// False when the `zmx kill` could not be started. started: bool = true, ended: usize = 0, + /// Entries refused for good: the same answer next time. refused: usize = 0, + /// Entries refused because the other layouts could not all be read: retryable. + deferred: usize = 0, }; + const Refusal = struct { reason: []const u8, retryable: bool = false }; + /// Why this shell must not end the zmx session `name` (already a valid shell session - /// name), or null when it may. Ownership is proven by the record the shell made when it - /// minted the session, never by the name's shape; a session with no record, one a graph - /// node or another layout also names, and one whose layouts could not all be read are - /// all refused. - fn refuseEnding(self: *Workspace, name: []const u8, options: EndOptions) ?[]const u8 { + /// name), or null when it may. Ownership is proven by a record some GraphCode shell made in + /// the shared layout directory for this loop (`WorkspaceLayout.isShellOwnedBy`), never by + /// the name's shape; a session with no such record, one a graph node or another layout + /// also names, and one whose layouts could not all be read are all refused. + fn refuseEnding(self: *Workspace, name: []const u8, options: EndOptions) ?Refusal { const uuid = name[ZmxSession.prefix.len..]; - const root = self.layoutRoot() orelse return "no layout directory records which sessions this shell made"; - if (!WorkspaceLayout.isShellOwned(self.allocator, root, name)) return "this shell has no record of having made it"; - if (self.loop_id.len != 0 and std.ascii.eqlIgnoreCase(uuid, self.loop_id)) return "it is the open loop's session"; + const root = self.layoutRoot() orelse return .{ .reason = "no layout directory records which sessions GraphCode made" }; + if (!WorkspaceLayout.isShellOwnedBy(self.allocator, root, name, options.owner)) + return .{ .reason = "no GraphCode shell recorded it for this loop" }; + if (self.loop_id.len != 0 and std.ascii.eqlIgnoreCase(uuid, self.loop_id)) return .{ .reason = "it is the open loop's session" }; if (options.known) |known| { - if (known.contains(known.context, uuid)) return "it names a node of the graph"; + if (known.contains(known.context, uuid)) return .{ .reason = "it names a node of the graph" }; } - if (options.check_open_layout and (self.layout.hasPane(uuid) or self.layout.hasPane(name))) return "the open layout also names it"; + if (options.check_open_layout and (self.layout.hasPane(uuid) or self.layout.hasPane(name))) + return .{ .reason = "the open layout also names it" }; return switch (WorkspaceLayout.claimedByOtherLayout(self.allocator, root, options.except_layout, name)) { .unclaimed => null, - .claimed => "another layout also names it", - .unknown => "the other saved layouts could not all be read", + .claimed => .{ .reason = "another layout also names it" }, + .unknown => .{ .reason = "the other saved layouts could not all be read", .retryable = true }, }; } @@ -1063,10 +1073,10 @@ pub const Workspace = struct { self.allocator.free(name); continue; } - if (self.refuseEnding(name, options)) |reason| { - std.log.warn("not ending zmx session {s}: {s}", .{ name, reason }); + if (self.refuseEnding(name, options)) |refusal| { + std.log.warn("not ending zmx session {s}: {s}", .{ name, refusal.reason }); self.allocator.free(name); - outcome.refused += 1; + if (refusal.retryable) outcome.deferred += 1 else outcome.refused += 1; continue; } names.append(self.allocator, name) catch { @@ -1556,39 +1566,56 @@ pub const Workspace = struct { } /// Records, before any attach can create the session, that the shell minted this tab's - /// session: only a session with such a record is ever ended by the shell (`refuseEnding`). - /// Best effort: a tab whose record could not be written works, but is never ended for it. + /// session for the open loop: only a session with such a record, naming the loop whose + /// layout is being retired or closed, is ever ended by the shell (`refuseEnding`). Written + /// after the layout claim is saved and before the session starts. Best effort: a tab whose + /// record could not be written works, but is never ended for it. fn recordShellOwned(self: *Workspace, surface_id: []const u8) void { const root = self.layoutRoot() orelse return; const name = ZmxSession.allocName(self.allocator, surface_id) catch return; defer self.allocator.free(name); - WorkspaceLayout.markShellOwned(self.allocator, root, name) catch |err| { - std.log.warn("could not record shell session {s} as this shell's own: {s}", .{ name, @errorName(err) }); + WorkspaceLayout.markShellOwned(self.allocator, root, name, self.loop_id) catch |err| { + std.log.warn("could not record shell session {s} as a GraphCode shell's: {s}", .{ name, @errorName(err) }); }; } + fn forgetShellOwned(self: *Workspace, surface_id: []const u8) void { + const root = self.layoutRoot() orelse return; + const name = ZmxSession.allocName(self.allocator, surface_id) catch return; + defer self.allocator.free(name); + WorkspaceLayout.forgetShellOwned(self.allocator, root, name); + } + + /// The layout claims the new session before it exists: a layout naming it is on disk + /// (visible to any other shell sharing the layout directory) before the record is written + /// and the session started, and any failure afterwards takes the claim back. pub fn newTab(self: *Workspace) !void { const surface_id = try self.layout.newSurfaceID(); defer self.allocator.free(surface_id); - self.recordShellOwned(surface_id); + const initial_grid = try self.workspaceGridSize(); const previous_selected = self.layout.selected_tab; const previous_next_id = self.layout.next_tab_id; - const index = try self.createAttachedSurface(surface_id, try self.workspaceGridSize()); - errdefer self.destroySurface(index); - self.layout.addTab(surface_id, false) catch |err| { - self.destroySurface(index); + try self.layout.addTab(surface_id, false); + self.persistLayout() catch |err| { + self.rollBackNewTab(surface_id, previous_selected, previous_next_id); return err; }; - self.persistLayout() catch |err| { - _ = self.layout.removePane(surface_id); - self.layout.selected_tab = previous_selected; - self.layout.next_tab_id = previous_next_id; - self.destroySurface(index); + self.recordShellOwned(surface_id); + _ = self.createAttachedSurface(surface_id, initial_grid) catch |err| { + self.rollBackNewTab(surface_id, previous_selected, previous_next_id); + self.persistLayout() catch {}; + self.forgetShellOwned(surface_id); return err; }; self.syncTopology(); } + fn rollBackNewTab(self: *Workspace, surface_id: []const u8, previous_selected: usize, previous_next_id: u64) void { + _ = self.layout.removePane(surface_id); + self.layout.selected_tab = previous_selected; + self.layout.next_tab_id = previous_next_id; + } + fn createAttachedSurface(self: *Workspace, session: []const u8, initial_grid: GridSize) !usize { return self.createAttachedSurfaceFrom(0, session, initial_grid); } @@ -1801,28 +1828,33 @@ pub const Workspace = struct { pub fn splitFocused(self: *Workspace, direction: WorkspaceLayout.Direction) !void { const surface_id = try self.layout.newSurfaceID(); defer self.allocator.free(surface_id); - self.recordShellOwned(surface_id); const tab = self.layout.selected() orelse return error.NoTabs; const previous_focus = tab.focused_pane; const previous_direction = tab.split_direction; - const index = try self.createAttachedSurface(surface_id, try self.splitGridSize(direction)); - errdefer self.destroySurface(index); - self.layout.splitFocused(direction, surface_id) catch |err| { - self.destroySurface(index); + const initial_grid = try self.splitGridSize(direction); + try self.layout.splitFocused(direction, surface_id); + self.persistLayout() catch |err| { + self.rollBackSplit(surface_id, previous_focus, previous_direction); return err; }; - self.persistLayout() catch |err| { - _ = self.layout.removePane(surface_id); - if (self.layout.selected()) |current| { - current.focused_pane = previous_focus; - current.split_direction = previous_direction; - } - self.destroySurface(index); + self.recordShellOwned(surface_id); + _ = self.createAttachedSurface(surface_id, initial_grid) catch |err| { + self.rollBackSplit(surface_id, previous_focus, previous_direction); + self.persistLayout() catch {}; + self.forgetShellOwned(surface_id); return err; }; self.syncTopology(); } + fn rollBackSplit(self: *Workspace, surface_id: []const u8, previous_focus: usize, previous_direction: WorkspaceLayout.Direction) void { + _ = self.layout.removePane(surface_id); + if (self.layout.selected()) |current| { + current.focused_pane = previous_focus; + current.split_direction = previous_direction; + } + } + pub fn selectTab(self: *Workspace, index: usize) !void { try self.layout.selectTab(index); try self.persistLayout(); @@ -1872,7 +1904,7 @@ pub const Workspace = struct { // A shell's session is the pane's reason for existing: closing the pane ends it, as // on macOS. The loop's own session belongs to the daemon and ends with the loop. if (!record.launches_agent and !std.mem.eql(u8, record.id, self.loop_id)) - _ = self.endShellSessions(&.{record.id}, .{}); + _ = self.endShellSessions(&.{record.id}, .{ .owner = self.loop_id }); self.syncTopology(); } diff --git a/graphcode-windows/src/WorkspaceLayout.zig b/graphcode-windows/src/WorkspaceLayout.zig index 55ede8a7..724f52c5 100644 --- a/graphcode-windows/src/WorkspaceLayout.zig +++ b/graphcode-windows/src/WorkspaceLayout.zig @@ -610,29 +610,32 @@ fn fileClaim(allocator: std.mem.Allocator, directory: std.fs.Dir, name: []const pub const owned_sessions_directory_name = "shell-sessions"; -/// The record that a shell session is this shell's own: a file named for the session in -/// `\shell-sessions`, made when the shell mints the session for a new tab or split and -/// before anything can attach to it. A name's shape cannot say who made the session; only -/// this record, which no layout edit or corruption touches, can. Null when `session` is not -/// a plain file name. +/// The record that a GraphCode shell minted a shell session for a loop's layout: a file named +/// for the session in `\shell-sessions` whose content is the loop it was minted for (empty +/// when the layout is not loop-scoped). Written after the layout claiming the session is saved +/// and before the session starts. A name's shape cannot say who made the session, and a layout +/// edit or corruption does not touch this record; but the root is shared by every GraphCode +/// shell of the user, so it proves "some GraphCode shell recorded it for that loop", not which +/// shell. Null when `session` is not a plain file name. fn ownedSessionPath(allocator: std.mem.Allocator, root: []const u8, session: []const u8) ?[]u8 { if (!isFileSafeID(session)) return null; return std.fmt.allocPrint(allocator, "{s}\\{s}\\{s}", .{ root, owned_sessions_directory_name, session }) catch null; } -pub fn markShellOwned(allocator: std.mem.Allocator, root: []const u8, session: []const u8) !void { +pub fn markShellOwned(allocator: std.mem.Allocator, root: []const u8, session: []const u8, loop: []const u8) !void { const path = ownedSessionPath(allocator, root, session) orelse return error.InvalidSessionName; defer allocator.free(path); if (std.fs.path.dirname(path)) |directory| try std.fs.cwd().makePath(directory); - var file = try std.fs.cwd().createFile(path, .{ .truncate = true }); - file.close(); + try std.fs.cwd().writeFile(.{ .sub_path = path, .data = loop }); } -pub fn isShellOwned(allocator: std.mem.Allocator, root: []const u8, session: []const u8) bool { +/// Whether a record says `session` was minted for `loop`. +pub fn isShellOwnedBy(allocator: std.mem.Allocator, root: []const u8, session: []const u8, loop: []const u8) bool { const path = ownedSessionPath(allocator, root, session) orelse return false; defer allocator.free(path); - std.fs.cwd().access(path, .{}) catch return false; - return true; + const recorded = std.fs.cwd().readFileAlloc(allocator, path, 4096) catch return false; + defer allocator.free(recorded); + return std.mem.eql(u8, recorded, loop); } pub fn forgetShellOwned(allocator: std.mem.Allocator, root: []const u8, session: []const u8) void { @@ -1101,20 +1104,25 @@ test "a layout scan past its cap is unknown, not unclaimed" { try std.testing.expectEqual(Claim.unknown, claimedByOtherLayout(std.testing.allocator, root, "", "shared")); } -test "a shell session is owned only once the shell has recorded it, and not after it is forgotten" { +test "a shell session is owned only for the loop its record names, and not after it is forgotten" { var tmp = std.testing.tmpDir(.{}); defer tmp.cleanup(); const root = try tmp.dir.realpathAlloc(std.testing.allocator, "."); defer std.testing.allocator.free(root); const name = "graphcode-5e11ba5e-0001-4000-8000-000000000001"; - try std.testing.expect(!isShellOwned(std.testing.allocator, root, name)); - try markShellOwned(std.testing.allocator, root, name); - try std.testing.expect(isShellOwned(std.testing.allocator, root, name)); - try std.testing.expect(!isShellOwned(std.testing.allocator, root, "graphcode-5e11ba5e-0002-4000-8000-000000000002")); - try std.testing.expect(!isShellOwned(std.testing.allocator, root, "..\\escape")); - try std.testing.expectError(error.InvalidSessionName, markShellOwned(std.testing.allocator, root, "..\\escape")); + try std.testing.expect(!isShellOwnedBy(std.testing.allocator, root, name, "loop-a")); + try markShellOwned(std.testing.allocator, root, name, "loop-a"); + try std.testing.expect(isShellOwnedBy(std.testing.allocator, root, name, "loop-a")); + // A record names the loop it was minted for; another loop's retirement does not qualify. + try std.testing.expect(!isShellOwnedBy(std.testing.allocator, root, name, "loop-b")); + try std.testing.expect(!isShellOwnedBy(std.testing.allocator, root, name, "")); + try markShellOwned(std.testing.allocator, root, "graphcode-5e11ba5e-0003-4000-8000-000000000003", ""); + try std.testing.expect(isShellOwnedBy(std.testing.allocator, root, "graphcode-5e11ba5e-0003-4000-8000-000000000003", "")); + try std.testing.expect(!isShellOwnedBy(std.testing.allocator, root, "graphcode-5e11ba5e-0002-4000-8000-000000000002", "loop-a")); + try std.testing.expect(!isShellOwnedBy(std.testing.allocator, root, "..\\escape", "loop-a")); + try std.testing.expectError(error.InvalidSessionName, markShellOwned(std.testing.allocator, root, "..\\escape", "loop-a")); forgetShellOwned(std.testing.allocator, root, name); - try std.testing.expect(!isShellOwned(std.testing.allocator, root, name)); + try std.testing.expect(!isShellOwnedBy(std.testing.allocator, root, name, "loop-a")); } test "saving replaces a layout whole, leaves no temporary file, and keeps the old one when it fails" { From e2aec1bdaffe54b37ced6a33267e8e01ef262564 Mon Sep 17 00:00:00 2001 From: Colin Neilens Date: Sat, 10 Oct 2026 00:59:07 -0700 Subject: [PATCH 4/4] Keep the fake zmx from reading the layout file except in the test that observes it The attach stand-in copied the loop layout on every attach, which could hold the file open while the shell renamed a new layout over it and made a later save fail intermittently. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Colin Neilens --- graphcode-windows/src/App.zig | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/graphcode-windows/src/App.zig b/graphcode-windows/src/App.zig index b57ff99f..71c543af 100644 --- a/graphcode-windows/src/App.zig +++ b/graphcode-windows/src/App.zig @@ -12992,7 +12992,7 @@ const fake_zmx_script = "exit /b 0\r\n" ++ ":attach\r\n" ++ ">\"%~dp0cwd-%~2.txt\" echo %CD%\r\n" ++ - "if exist \"%~dp0loop-a.json\" copy /y \"%~dp0loop-a.json\" \"%~dp0layout-at-attach-%~2.txt\" >nul\r\n" ++ + "if exist \"%~dp0record-attach.txt\" if exist \"%~dp0loop-a.json\" copy /y \"%~dp0loop-a.json\" \"%~dp0layout-at-attach-%~2.txt\" >nul\r\n" ++ "if exist \"%~dp0shell-sessions\\%~2\" (echo yes>\"%~dp0marker-at-attach-%~2.txt\") else (echo no>\"%~dp0marker-at-attach-%~2.txt\")\r\n" ++ "pause >nul\r\n" ++ "exit /b 0\r\n"; @@ -15419,11 +15419,14 @@ test "workspace layout: a new tab's layout claim and ownership record exist befo const app = &fixture.app; try fixture.setLive(&.{"loop-a"}); + try fixture.setFlag("record-attach.txt", true); try clickSidebarLoopRow(app, fixture.project, "loop-a"); try fixture.waitFor(LiveTerminalFixture.shows, "loop-a"); try fixture.newTab(); const tab_shell = try fixture.selectedPane(0); defer std.testing.allocator.free(tab_shell); + // The attach copies the layout file; wait for it so the next save does not race that read. + try fixture.waitFor(LiveTerminalFixture.attachSaw, tab_shell); try fixture.splitRight(); const split_shell = try fixture.selectedPane(1); defer std.testing.allocator.free(split_shell);