Check for a free terminal slot before New Tab and Split claim a session

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 (`<layouts>\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

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" } });

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);