diff --git a/graphcode-windows/README.md b/graphcode-windows/README.md index 6f459bc7..f61c5ef8 100644 --- a/graphcode-windows/README.md +++ b/graphcode-windows/README.md @@ -514,6 +514,50 @@ The parity row remains **Partial**: shown-dialog accessibility/keyboard/layout and multi-instance behavior, complete live totals, real running-cycle keyboard/window proof, and recoverable deletion still need their own evidence or implementation. +## Per-loop terminal layouts: known limits + +New Tab and Split save the layout's claim on the new session first, then record +that this shell minted it (`\shell-sessions`), then start the session, so +a concurrent cleanup can never end a session the shell is still creating. Only a +session with such a record for the loop being retired is ever ended by the shell. +A free surface slot is checked before the claim is saved, so a full workspace +(32 terminals) fails without writing anything. The limits below are known and +unchanged; none of them can cause a session to be ended. + +- **A failed rollback save is not handled.** When the session start or the native + surface then fails, the claim is taken back in memory and saved again; if that + second save fails it is only dropped, and the on-disk layout keeps a pane that + names either a session that was never created (the start failed) or one that + survived the failure (the native surface failed after the attach was spawned). + The record is already removed, so the claim cannot authorize a kill. A later + successful save of the layout clears it. A restore clears it only after a + successful `zmx ls` shows the session not running; when the listing fails the + pane stays saved and is retried, and a surviving session is re-attached as a + tab with no record. Retiring such a loop moves its layout aside as `.refused` + instead of deleting it. A retry of the save was considered and not built: + `Layout.save` + replaces the file with no compare-and-swap, so a deferred retry could overwrite + a newer layout written by another shell, and loop switches swap the layout and + its path. +- **A pane opened through `openNode` is not recorded as shell-minted.** The only + production caller is opening a quick chat. `attachNode` starts the session + before saving the pane and never writes an ownership record. A session it + creates is therefore never ended by the shell at retire, and a stop between the + start and the save leaves a live session with no claim. A quick chat's session + is named by its graph node and belongs to the daemon, which removes it when the + chat is deleted, so the shell refusing it is intended; no shell-minted tab + reaches this path. `attachNode` is also the re-attach used by a recreate of a + pane that was minted earlier, which keeps its original record. +- **A native surface failure after the attach was started may leave a session.** + New Tab and Split roll back the claim and the record when + `winghostty_host_create_surface_v2` fails, but the attach process is only + killed, and `zmx attach` creates the session on connect, so a session that came + up before the kill can survive with no claim and no record. Whether it does + depends on zmx timing and is not established without a real zmx. + +Coverage is Zig tests against a fake `zmx`, including a test-only forced save +failure; no real zmx, filesystem sharing violation, or live walkthrough is claimed. + ## Worktree discovery processes `WorktreeStatus` runs local Git with explicit `-C` paths and a child-only copy of diff --git a/graphcode-windows/src/App.zig b/graphcode-windows/src/App.zig index 40790da5..4c5785a4 100644 --- a/graphcode-windows/src/App.zig +++ b/graphcode-windows/src/App.zig @@ -15446,6 +15446,143 @@ test "workspace layout: a new tab's layout claim and ownership record exist befo try std.testing.expectEqualStrings("loop-a", try fixture.tmp.dir.readFile(record_name, &buffer)); } +/// How many shell-ownership records the shell has written beside its layouts. +fn shellRecordCount(dir: std.fs.Dir) !usize { + var records = dir.openDir("shell-sessions", .{ .iterate = true }) catch |err| switch (err) { + error.FileNotFound => return 0, + else => return err, + }; + defer records.close(); + var count: usize = 0; + var iterator = records.iterate(); + while (try iterator.next()) |_| count += 1; + return count; +} + +/// How many attach processes the fake zmx has been started for. +fn attachStartCount(dir: std.fs.Dir) !usize { + var entries = try dir.openDir(".", .{ .iterate = true }); + defer entries.close(); + var count: usize = 0; + var iterator = entries.iterate(); + while (try iterator.next()) |entry| { + if (std.mem.startsWith(u8, entry.name, "cwd-graphcode-")) count += 1; + } + return count; +} + +test "workspace layout: New Tab and Split with no free surface slot fail before claiming a session" { + var fixture: LiveTerminalFixture = undefined; + try fixture.init(&.{ .{ .id = "loop-a" } }); + defer fixture.deinit(); + const allocator = std.testing.allocator; + const workspace = &fixture.workspace; + try fixture.setLive(&.{"loop-a"}); + try clickSidebarLoopRow(&fixture.app, fixture.project, "loop-a"); + try fixture.waitFor(LiveTerminalFixture.shows, "loop-a"); + // The loop's attach has recorded itself, so a later count only moves for a new attach. + allocator.free(try fixture.attachDirectory("loop-a")); + const attaches = try attachStartCount(fixture.tmp.dir); + const before = try std.fs.cwd().readFileAlloc(allocator, workspace.layout_path, 1 << 16); + defer allocator.free(before); + + // Every slot is shown or awaiting a launch, as when the user has 32 terminals open. + for (&workspace.launch_waits, 0..) |*wait, index| { + if (workspace.hasSurface(index) or workspace.hasAttach(index) or wait.active()) continue; + wait.begin(try allocator.dupe(u8, "busy"), std.time.milliTimestamp(), 60_000, false); + } + + inline for (.{ "newTab", "splitRight" }) |action| { + // A save that failed now would leave a claim behind; none must even be attempted. + workspace.layout_save_fault = 1; + const failed = if (comptime std.mem.eql(u8, action, "newTab")) + workspace.newTab() + else + workspace.splitFocused(.horizontal); + try std.testing.expectError(error.SurfaceCapacityExceeded, failed); + const after = try std.fs.cwd().readFileAlloc(allocator, workspace.layout_path, 1 << 16); + defer allocator.free(after); + try std.testing.expectEqualStrings(before, after); + try std.testing.expectEqual(@as(?usize, 1), workspace.layout_save_fault); + try std.testing.expectEqual(@as(usize, 1), workspace.tabCount()); + try std.testing.expectEqual(@as(usize, 0), try shellRecordCount(fixture.tmp.dir)); + try std.testing.expectEqual(attaches, try attachStartCount(fixture.tmp.dir)); + } + workspace.layout_save_fault = null; +} + +test "workspace layout: a failed rollback save leaves the claim on disk, never a marker, and no kill ever follows from it" { + var fixture: LiveTerminalFixture = undefined; + try fixture.init(&.{ .{ .id = "loop-a" }, .{ .id = "loop-b" } }); + defer fixture.deinit(); + const allocator = std.testing.allocator; + const WorkspaceLayoutFile = @import("WorkspaceLayout.zig"); + const workspace = &fixture.workspace; + try fixture.setLive(&.{ "loop-a", "loop-b" }); + try clickSidebarLoopRow(&fixture.app, fixture.project, "loop-a"); + try fixture.waitFor(LiveTerminalFixture.shows, "loop-a"); + + // zmx cannot be started, so the new tab's session start fails after its claim is saved, + // and the save that takes the claim back fails as well. + const working_zmx = workspace.zmx_path; + workspace.zmx_path = try allocator.dupe(u8, "no-such-zmx.exe"); + workspace.layout_save_fault = 1; + const failed = workspace.newTab(); + allocator.free(workspace.zmx_path); + workspace.zmx_path = working_zmx; + if (failed) |_| return error.TestExpectedNewTabFailure else |_| {} + try std.testing.expectEqual(@as(?usize, null), workspace.layout_save_fault); + + // Memory is rolled back and no record exists, but the disk still names the pane. + try std.testing.expectEqual(@as(usize, 1), workspace.tabCount()); + try std.testing.expectEqual(@as(usize, 0), try shellRecordCount(fixture.tmp.dir)); + var saved = try WorkspaceLayoutFile.Layout.loadFor(allocator, workspace.layout_path, fixture.project, "loop-a"); + defer saved.deinit(); + try std.testing.expectEqual(@as(usize, 2), saved.tabs.items.len); + const ghost = try allocator.dupe(u8, saved.tabs.items[1].panes.items[0].id); + defer allocator.free(ghost); + try std.testing.expect(!workspace.layout.hasPane(ghost)); + try std.testing.expect(!fixture.ownedMarker(ghost)); + + // Another loop's retirement of this one finds the claim with no record: nothing is + // killed, and the layout is set aside rather than deleted. + try clickSidebarLoopRow(&fixture.app, fixture.project, "loop-b"); + try fixture.waitFor(LiveTerminalFixture.shows, "loop-b"); + try std.testing.expect(fixture.layoutExists("loop-a.json")); + try std.testing.expectEqual(@as(usize, 0), workspace.retireLoop("loop-a", fixture.project)); + fixture.settle(); + try fixture.expectKills(""); + try std.testing.expect(fixture.layoutExists("loop-a.json.refused")); + try std.testing.expect(!fixture.layoutExists("loop-a.json")); +} + +test "workspace layout: restore drops a loop layout's claimed pane whose session was never started" { + var fixture: LiveTerminalFixture = undefined; + try fixture.init(&.{ .{ .id = "loop-a" } }); + defer fixture.deinit(); + const allocator = std.testing.allocator; + const WorkspaceLayoutFile = @import("WorkspaceLayout.zig"); + try fixture.saveLoopLayout(fixture.project, "loop-a", &.{ + .{ .id = "loop-a", .agent = true }, + .{ .id = shell_uuid_one }, + .{ .id = shell_uuid_two }, + }); + try fixture.setLive(&.{ "loop-a", shell_uuid_two }); + + try clickSidebarLoopRow(&fixture.app, fixture.project, "loop-a"); + try fixture.waitFor(LiveTerminalFixture.shows, "loop-a"); + try fixture.waitFor(restoredAliveShell, shell_uuid_two); + fixture.settle(); + try std.testing.expect(fixture.slotFor(shell_uuid_one) == null); + try std.testing.expectError(error.FileNotFound, fixture.tmp.dir.access("cwd-graphcode-" ++ shell_uuid_one ++ ".txt", .{})); + try std.testing.expect(!fixture.workspace.layout.hasPane(shell_uuid_one)); + var persisted = try WorkspaceLayoutFile.Layout.loadFor(allocator, fixture.workspace.layout_path, fixture.project, "loop-a"); + defer persisted.deinit(); + try std.testing.expect(!persisted.hasPane(shell_uuid_one)); + try std.testing.expect(persisted.hasPane(shell_uuid_two)); + try fixture.expectKills(""); +} + 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" } }); diff --git a/graphcode-windows/src/TerminalSurface.zig b/graphcode-windows/src/TerminalSurface.zig index 164ea727..072eec54 100644 --- a/graphcode-windows/src/TerminalSurface.zig +++ b/graphcode-windows/src/TerminalSurface.zig @@ -479,6 +479,9 @@ pub const Workspace = struct { /// session's attach pipe; set by tests, which have no pipe to read. teardown_input_sink: ?*const fn (context: ?*anyopaque, index: usize, bytes: []const u8) void = null, teardown_input_context: ?*anyopaque = null, + /// Set by tests only: the number of saves that still succeed before one fails (once) with + /// `LayoutSaveFaultInjected`, so a compensating save can be made to fail on its own. + layout_save_fault: ?usize = null, layout: WorkspaceLayout.Layout, layout_path: []u8, project_key: []u8, @@ -1590,6 +1593,8 @@ pub const Workspace = struct { /// (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 { + // Checked before the claim is saved: a claim taken back by a failing save is a ghost. + if (self.firstFreeSurfaceSlot(0) == null) return error.SurfaceCapacityExceeded; const surface_id = try self.layout.newSurfaceID(); defer self.allocator.free(surface_id); const initial_grid = try self.workspaceGridSize(); @@ -1620,33 +1625,45 @@ pub const Workspace = struct { return self.createAttachedSurfaceFrom(0, session, initial_grid); } - fn createAttachedSurfaceFrom(self: *Workspace, first_slot: usize, session: []const u8, initial_grid: GridSize) !usize { - for (&self.surfaces, 0..) |*slot, index| { + /// Whether slot `index` can take a new terminal: nothing is shown or attaching in it and + /// no loop launch is waiting for it. The one test creation and its pre-check share. + fn surfaceSlotFree(self: *const Workspace, index: usize) bool { + const slot = &self.surfaces[index]; + return slot.surface == null and slot.attach == null and !self.launch_waits[index].active(); + } + + fn firstFreeSurfaceSlot(self: *const Workspace, first_slot: usize) ?usize { + for (0..self.surfaces.len) |index| { if (index < first_slot) continue; - if (slot.surface != null or slot.attach != null or self.launch_waits[index].active()) continue; - slot.session_name = try self.allocator.dupe(u8, session); - errdefer self.destroySurface(index); - slot.project_path = try self.allocator.dupe(u8, self.project_path); - try self.startSession(index, slot.session_name, initial_grid); - var options = self.surfaceOptions(index); - const result = c.winghostty_host_create_surface_v2( - self.host, - self.parent, - &options, - &self.surfaces[index].surface, - ); - if (result != c.WINGHOSTTY_OK or self.surfaces[index].surface == null) { - self.waitAttach(index); - return error.WinghosttySurfaceCreateFailed; - } + if (self.surfaceSlotFree(index)) return index; + } + return null; + } - self.surfaces[index].destroyed = false; - self.surfaces[index].destroying = false; - clearCells(&self.surfaces[index]); - self.relayout(); - return index; + fn createAttachedSurfaceFrom(self: *Workspace, first_slot: usize, session: []const u8, initial_grid: GridSize) !usize { + const index = self.firstFreeSurfaceSlot(first_slot) orelse return error.SurfaceCapacityExceeded; + const slot = &self.surfaces[index]; + slot.session_name = try self.allocator.dupe(u8, session); + errdefer self.destroySurface(index); + slot.project_path = try self.allocator.dupe(u8, self.project_path); + try self.startSession(index, slot.session_name, initial_grid); + var options = self.surfaceOptions(index); + const result = c.winghostty_host_create_surface_v2( + self.host, + self.parent, + &options, + &self.surfaces[index].surface, + ); + if (result != c.WINGHOSTTY_OK or self.surfaces[index].surface == null) { + self.waitAttach(index); + return error.WinghosttySurfaceCreateFailed; } - return error.SurfaceCapacityExceeded; + + self.surfaces[index].destroyed = false; + self.surfaces[index].destroying = false; + clearCells(&self.surfaces[index]); + self.relayout(); + return index; } /// Re-attaches the layout's panes whose sessions are still running, from slot @@ -1826,6 +1843,7 @@ pub const Workspace = struct { } pub fn splitFocused(self: *Workspace, direction: WorkspaceLayout.Direction) !void { + if (self.firstFreeSurfaceSlot(0) == null) return error.SurfaceCapacityExceeded; const surface_id = try self.layout.newSurfaceID(); defer self.allocator.free(surface_id); const tab = self.layout.selected() orelse return error.NoTabs; @@ -1927,6 +1945,13 @@ pub const Workspace = struct { pub fn persistLayout(self: *Workspace) !void { if (self.persisting_layout or self.layout_path.len == 0) return; + if (self.layout_save_fault) |remaining| { + if (remaining == 0) { + self.layout_save_fault = null; + return error.LayoutSaveFaultInjected; + } + self.layout_save_fault = remaining - 1; + } self.persisting_layout = true; defer self.persisting_layout = false; try self.layout.save(self.layout_path);