From b199dc3764cc32f31fd9a471a5e1ad058014f291 Mon Sep 17 00:00:00 2001 From: Pfriedrix Date: Fri, 7 Aug 2026 23:12:26 +0300 Subject: [PATCH] fix(watch): show only the worktrees that are still on disk MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dashboard listed every worktree a daemon had ever run in. A daemon never drops a session from its own list and flushes that list once more as it exits, so its registry file outlives it naming worktrees removed weeks ago — here six sessions, four of them pointing at directories that were gone. Those rows were dead ends: enter on one starts an agent in a directory that is not there. sessions.visible is the rule now, and both read paths go through it — the live snapshot and the fallback the registry provides. The dashboard puts the same test on git's worktree list, which goes on reporting a worktree whose directory was deleted until someone prunes it. A session still running in a deleted worktree is dropped with the rest, and `lcc open --stop-all` is what ends it; nothing produces Status.orphan after that, so it goes too. A registry row reading `unknown` is no longer a match either. It was hiding the status recovered from the hooks for every worktree its daemon had touched, which is precisely when that file is the only thing left able to report one. --- CLAUDE.md | 17 ++++++- README.md | 16 ++++++ src/commands/watch.zig | 111 +++++++++++++++++++++++++++++++++++++++-- src/disk.zig | 38 ++++++++++++++ src/sessions.zig | 83 ++++++++++++++++++++---------- src/watch_state.zig | 8 +-- src/watch_status.zig | 6 +-- src/watch_table.zig | 6 +-- 8 files changed, 239 insertions(+), 46 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 10e2bcc..7cf928e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -12,7 +12,7 @@ library plus CoreFoundation/Security. Run from the repo root: ```bash -zig build test --summary all # unit tests (~3s, 289 at last count) +zig build test --summary all # unit tests (~3s, 293 at last count) zig build # debug binary → zig-out/bin/lcc zig build -Doptimize=ReleaseFast # what PATH should be serving zig build run -- list # run without installing @@ -133,6 +133,21 @@ Do not "simplify" `build.zig`'s separate `test_mod`: reusing the executable's mo improvement and turns every recovered row into an `unknown_session` error. The ids collide across daemons anyway — `next_id` restarts at 1 — which is also why the state file is named for Claude Code's session UUID rather than for either the lcc id or the worktree path. +- **A dashboard row has to name a directory that still exists.** `sessions.visible` is that + rule, and *both* ways the rows are read have to go through it — the live snapshot in + `collect` / `snapshotOnce` and `sessions.resolved` — because the daemon never drops a + session from its own list and flushes that whole list once more as it exits, so its file + outlives it naming every worktree it ever ran in. `collect` puts the same predicate on + `app.worktreeChoices`, not inside it: git keeps listing a worktree whose directory was + deleted (`prunable`), and that is exactly the row `lcc list` shows in red and `lcc remove` + needs in order to clean the entry up. +- **A registry row reading `unknown` is bookkeeping, not a session.** `collect` collapses it to + `null` through `liveMatch` before `rowFor` ever sees it. Leave it a match and the worktree's + hook-recovered status is never consulted — which after a daemon dies is every worktree it + touched, so `watch_state` recovers nothing and the column reads `unknown` with an age + measured from the epoch. Passing `rowFor` the recovered status *and* the dead session id + instead is worse than either: `attachable` goes true and enter asks the daemon for a session + nothing holds. - **A session's hook settings file is per session, not per daemon.** `watch_paths.hooksFor` names it `hooks-.json` and `watch_hooks.settingsJson` bakes that id into every hook command line, so a report says which session it came from. Collapsing them back into diff --git a/README.md b/README.md index 0120906..cbdb921 100644 --- a/README.md +++ b/README.md @@ -123,6 +123,16 @@ A session running in another repository still appears, because an agent working somewhere you are not looking is the one you most need to see. `lcc open xcode` is unchanged and still opens a worktree in Xcode. +What the list never shows is a directory that is not there. A worktree you +removed drops off the moment its directory does — including one `git worktree +prune` has not caught up with yet, which git still lists — and the sessions that +ran in it go with it rather than staying as rows that open nothing. Nothing +cleans up after them otherwise: the background process never drops a session from +its own list and writes that list once more on the way out, so its file goes on +naming every worktree it ever touched until a new one replaces the file. The one +thing you give up is reaching an agent still running in a directory you deleted — +it has no row any more, and `lcc open --stop-all` is what ends it. + `● waiting` is the one that wants you: the agent is blocked on a permission prompt or a question. `◐ active` is a turn in flight, `○ idle` is finished. Those come from Claude Code's own hooks rather than from reading its screen, so @@ -183,6 +193,12 @@ normally clears its file and goes back to `no session`, having nothing left to say. Sessions that died before this shipped left no file behind and stay `no session` until you start them again. +A row the dead process left in the registry does not outrank that file either. It +can only read `unknown` — the one thing that could have said otherwise is gone — +and what the hooks reported is better than that, so the recovered status wins. It +is still not offered for attach: the session behind that row died with its +process, whatever the registry remembers of it. + Sessions also hold the build of `lcc` they started under. After `zig build` the sessions still running can be many commits behind the `lcc` you are typing — older behaviour reached through the same command, and every symptom of it looks diff --git a/src/commands/watch.zig b/src/commands/watch.zig index 4f4d4d6..a493425 100644 --- a/src/commands/watch.zig +++ b/src/commands/watch.zig @@ -76,8 +76,9 @@ fn snapshotOnce(app: app_mod.App, opts: Opts) !void { const outdated = outdatedDaemon(app, app.gpa, exec.selfModified(app.gpa, app.io)); if (live) |list| { - const rows = try app.gpa.alloc(Row, list.len); - for (list, 0..) |s, i| rows[i] = toRow(s, false); + const present = try onDisk(app.io, app.gpa, list); + const rows = try app.gpa.alloc(Row, present.len); + for (present, 0..) |s, i| rows[i] = toRow(s, false); return emit(app, opts, rows, true, outdated, now); } @@ -96,6 +97,19 @@ fn outdatedDaemon(app: app_mod.App, arena: std.mem.Allocator, built: ?i64) bool return sessions.daemonOutdated(sessions.load(arena, app.io, app.environ), built); } +pub fn onDisk( + io: Io, + arena: std.mem.Allocator, + list: []const sessions.Session, +) ![]const sessions.Session { + var out: std.ArrayList(sessions.Session) = .empty; + for (list) |s| { + if (sessions.visible(io, s, true) == null) continue; + try out.append(arena, s); + } + return out.toOwnedSlice(arena); +} + const outdated_warning = "These sessions are running an older build of lcc than this one."; const outdated_hint = "They end on their own 30 minutes after the last one finishes. `lcc open --stop-all` is immediate, but ends them now."; @@ -368,7 +382,7 @@ fn collect(app: app_mod.App, arena: std.mem.Allocator, now: i64) ![]watch_table. var live: []const sessions.Session = &.{}; var stale = false; if (watch_client.snapshot(app) catch null) |list| { - live = list; + live = try onDisk(app.io, arena, list); } else { const state = sessions.load(arena, app.io, app.environ); const resolved = try sessions.resolved(arena, app.io, state, now); @@ -385,8 +399,9 @@ fn collect(app: app_mod.App, arena: std.mem.Allocator, now: i64) ![]watch_table. if (app.repo()) |repo| { for (try app_mod.worktreeChoices(app, repo)) |choice| { + if (!disk.isDirectory(app.io, choice.entry.path)) continue; const branch = choice.entry.branch orelse app_mod.shortHead(choice.entry.head); - const match = findSession(live, choice.entry.path); + const match = liveMatch(findSession(live, choice.entry.path)); const recovered: ?watch_state.Resolved = if (match != null) null else recover: { if (states == null) states = watch_state.load(arena, app.io, app.environ); break :recover watch_state.statusFor( @@ -446,6 +461,11 @@ pub fn rowFor( }; } +pub fn liveMatch(found: ?sessions.Session) ?sessions.Session { + const session = found orelse return null; + return if (session.parsedStatus() == .unknown) null else session; +} + pub fn findSession(list: []const sessions.Session, worktree: []const u8) ?sessions.Session { var fallback: ?sessions.Session = null; for (list) |s| { @@ -637,6 +657,89 @@ test "a live session outranks anything left on disk for the same worktree" { try std.testing.expect(row.attachable()); } +test "a session the daemon is still running in a deleted worktree is dropped too, not just the dead ones" { + const gpa = std.testing.allocator; + const io = std.testing.io; + var arena_state: std.heap.ArenaAllocator = .init(gpa); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + const base = try tmp.dir.realPathFileAlloc(io, ".", arena); + const removed = try std.fs.path.join(arena, &.{ base, "removed" }); + + const present = try onDisk(io, arena, &.{ + .{ .id = "s-here", .worktree = base, .branch = "feature/pe-284", .status = "waiting" }, + .{ .id = "s-gone", .worktree = removed, .branch = "feature/pe-283", .status = "active" }, + .{ .id = "s-done", .worktree = removed, .branch = "feature/pe-286", .status = "exited" }, + }); + + if (present.len != 1) { + std.debug.print( + "{d} of 3 sessions survived a snapshot with two deleted worktrees: the dashboard " ++ + "lists work that has nowhere left to happen, and enter on those rows opens an " ++ + "agent in a directory that is not there.\n", + .{present.len}, + ); + return error.TestExpectedEqual; + } + try std.testing.expectEqualStrings("s-here", present[0].id); + try std.testing.expectEqualStrings("waiting", present[0].status); +} + +test "a row left behind by a dead daemon does not outrank what the hooks reported" { + const gpa = std.testing.allocator; + var arena_state: std.heap.ArenaAllocator = .init(gpa); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + + const leftover: sessions.Session = .{ + .id = "s-00000006", + .worktree = "/w/pe-290", + .branch = "feature/pe-290", + .issue = "PE-290", + .status = "unknown", + .last_activity_at = 1200, + }; + + if (liveMatch(leftover) != null) { + std.debug.print( + "a session the daemon left in the registry still counts as a match: it reports " ++ + "`unknown` for every worktree that daemon ever touched, and the status the hooks " ++ + "wrote to disk is never consulted, which is the whole point of recovering it.\n", + .{}, + ); + return error.TestUnexpectedResult; + } + + const row = rowFor( + arena, + "/w/pe-290", + "feature/pe-290", + liveMatch(leftover), + .{ .status = .waiting, .last_activity_at = 1700 }, + false, + ); + try std.testing.expectEqual(sessions.Status.waiting, row.status.?); + try std.testing.expectEqual(@as(i64, 1700), row.last_activity_at); + + if (row.attachable()) { + std.debug.print( + "the recovered row kept the dead session's id and offers itself for attach: enter " ++ + "asks the daemon for a session nothing holds and comes back unknown_session, " ++ + "instead of starting the work again.\n", + .{}, + ); + return error.TestUnexpectedResult; + } + + var running = leftover; + running.status = "waiting"; + try std.testing.expectEqualStrings("s-00000006", liveMatch(running).?.id); + try std.testing.expect(liveMatch(null) == null); +} + test "a worktree with neither a session nor a report still reads as having none" { const gpa = std.testing.allocator; var arena_state: std.heap.ArenaAllocator = .init(gpa); diff --git a/src/disk.zig b/src/disk.zig index 92ece4b..b7e52f1 100644 --- a/src/disk.zig +++ b/src/disk.zig @@ -38,6 +38,12 @@ pub fn realPath(gpa: std.mem.Allocator, io: Io, target: []const u8) []const u8 { return Io.Dir.cwd().realPathFileAlloc(io, target, gpa) catch target; } +pub fn isDirectory(io: Io, path: []const u8) bool { + if (path.len == 0) return false; + const info = Io.Dir.cwd().statFile(io, path, .{}) catch return false; + return info.kind == .directory; +} + pub fn removeChild(io: Io, parent: []const u8, path: []const u8) !void { const dirname = std.fs.path.dirname(path) orelse return error.RefusingToDelete; const trimmed = std.mem.trimEnd(u8, parent, "/"); @@ -54,6 +60,38 @@ pub fn abbreviate(gpa: std.mem.Allocator, environ: *const std.process.Environ.Ma return std.fmt.allocPrint(gpa, "~{s}", .{path[home.len..]}) catch path; } +test "a worktree is a directory that is there, not a name that used to be one" { + const io = std.testing.io; + const gpa = std.testing.allocator; + + var arena_state: std.heap.ArenaAllocator = .init(gpa); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + const base = try tmp.dir.realPathFileAlloc(io, ".", arena); + + try std.testing.expect(isDirectory(io, base)); + + const gone = try std.fs.path.join(arena, &.{ base, "removed" }); + try std.testing.expect(!isDirectory(io, gone)); + + const file = try std.fs.path.join(arena, &.{ base, "a-file" }); + try Io.Dir.cwd().writeFile(io, .{ .sub_path = file, .data = "" }); + if (isDirectory(io, file)) { + std.debug.print( + "a plain file answered yes: a session whose worktree was replaced by a file of the " ++ + "same name keeps its row, and enter on it starts an agent in a directory that " ++ + "does not exist.\n", + .{}, + ); + return error.TestUnexpectedResult; + } + + try std.testing.expect(!isDirectory(io, "")); +} + test "isInside distinguishes containment from a shared prefix" { const gpa = std.testing.allocator; diff --git a/src/sessions.zig b/src/sessions.zig index 5585481..d4a8242 100644 --- a/src/sessions.zig +++ b/src/sessions.zig @@ -14,7 +14,6 @@ pub const Status = enum { idle, plan, exited, - orphan, unknown, pub fn label(self: Status) []const u8 { @@ -128,37 +127,31 @@ pub fn daemonOutdated(state: State, binary_modified: ?i64) bool { return built > daemon.started_at; } +pub fn visible(io: Io, session: Session, daemon_alive: bool) ?Status { + if (!disk.isDirectory(io, session.worktree)) return null; + return if (daemon_alive) session.parsedStatus() else .unknown; +} + pub fn resolved( gpa: std.mem.Allocator, io: Io, state: State, now: i64, ) ![]const Resolved { - const out = try gpa.alloc(Resolved, state.sessions.len); const daemon_alive = alive(state); const wrote_at = if (state.daemon) |d| d.wrote_at else 0; const age = @max(0, now - wrote_at); - for (state.sessions, 0..) |session, i| { - var status = session.parsedStatus(); - if (!daemon_alive) { - status = .unknown; - } else if (status != .exited and !worktreeExists(io, session.worktree)) { - status = .orphan; - } - out[i] = .{ + var out: std.ArrayList(Resolved) = .empty; + for (state.sessions) |session| { + const status = visible(io, session, daemon_alive) orelse continue; + try out.append(gpa, .{ .session = session, .status = status, .stale = daemon_alive and age > stale_after_seconds, - }; + }); } - return out; -} - -fn worktreeExists(io: Io, worktree: []const u8) bool { - if (worktree.len == 0) return false; - const info = Io.Dir.cwd().statFile(io, worktree, .{}) catch return false; - return info.kind == .directory; + return out.toOwnedSlice(gpa); } pub fn owning(gpa: std.mem.Allocator, state: State, target: []const u8) ?Session { @@ -319,7 +312,7 @@ test "nothing is out of date when there is no daemon to be out of date" { try testing.expect(!daemonOutdated(.{ .daemon = .{ .pid = pid, .started_at = 0 } }, 5_000)); } -test "a worktree that is gone reads as orphan, and the row stays" { +test "a session whose worktree was deleted stops being a row at all" { const gpa = testing.allocator; const io = testing.io; var arena_state: std.heap.ArenaAllocator = .init(gpa); @@ -329,22 +322,62 @@ test "a worktree that is gone reads as orphan, and the row stays" { var tmp = testing.tmpDir(.{}); defer tmp.cleanup(); const base = try tmp.dir.realPathFileAlloc(io, ".", arena); + const removed = try std.fs.path.join(arena, &.{ base, "removed" }); const state: State = .{ .daemon = .{ .pid = 1, .wrote_at = 1000 }, .sessions = &.{ .{ .id = "s-here", .worktree = base, .status = "active" }, - .{ .id = "s-gone", .worktree = try std.fs.path.join(arena, &.{ base, "removed" }), .status = "active" }, - .{ .id = "s-done", .worktree = try std.fs.path.join(arena, &.{ base, "removed" }), .status = "exited" }, - .{ .id = "s-plan", .worktree = try std.fs.path.join(arena, &.{ base, "removed" }), .status = "plan" }, + .{ .id = "s-gone", .worktree = removed, .status = "active" }, + .{ .id = "s-done", .worktree = removed, .status = "exited" }, + .{ .id = "s-plan", .worktree = removed, .status = "plan" }, }, }; const rows = try resolved(arena, io, state, 1001); + if (rows.len != 1) { + std.debug.print( + "{d} rows came back for one surviving worktree: every worktree ever removed keeps a " ++ + "line on the dashboard, so the list stops being what is on disk and starts being " ++ + "everything a daemon once ran.\n", + .{rows.len}, + ); + return error.TestExpectedEqual; + } + try testing.expectEqualStrings("s-here", rows[0].session.id); try testing.expectEqual(Status.active, rows[0].status); - try testing.expectEqual(Status.orphan, rows[1].status); - try testing.expectEqual(Status.exited, rows[2].status); - try testing.expectEqual(Status.orphan, rows[3].status); +} + +test "a deleted worktree is gone from the list whether or not a daemon is still there" { + const gpa = testing.allocator; + const io = testing.io; + var arena_state: std.heap.ArenaAllocator = .init(gpa); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + + var tmp = testing.tmpDir(.{}); + defer tmp.cleanup(); + const base = try tmp.dir.realPathFileAlloc(io, ".", arena); + const removed = try std.fs.path.join(arena, &.{ base, "removed" }); + + const dead: State = .{ + .daemon = .{ .pid = 0x7fff_fffe, .wrote_at = 1000 }, + .sessions = &.{ + .{ .id = "s-gone", .worktree = removed, .status = "waiting" }, + .{ .id = "s-here", .worktree = base, .status = "waiting" }, + }, + }; + try testing.expect(!alive(dead)); + + const rows = try resolved(arena, io, dead, 1001); + try testing.expectEqual(@as(usize, 1), rows.len); + try testing.expectEqualStrings("s-here", rows[0].session.id); + try testing.expectEqual(Status.unknown, rows[0].status); + + try testing.expect(visible(io, .{ .worktree = removed, .status = "waiting" }, true) == null); + try testing.expect(visible(io, .{ .worktree = removed, .status = "waiting" }, false) == null); + try testing.expect(visible(io, .{ .worktree = "", .status = "waiting" }, true) == null); + try testing.expectEqual(Status.waiting, visible(io, .{ .worktree = base, .status = "waiting" }, true).?); } test "plan round-trips as text, like every other status" { diff --git a/src/watch_state.zig b/src/watch_state.zig index ffa9731..85ba0ed 100644 --- a/src/watch_state.zig +++ b/src/watch_state.zig @@ -120,7 +120,7 @@ pub fn load( const file_path = std.fs.path.join(gpa, &.{ base, dirent.name }) catch continue; var record = readAt(gpa, io, file_path) orelse continue; if (record.cwd.len == 0) continue; - if (!worktreeExists(io, record.cwd)) continue; + if (!disk.isDirectory(io, record.cwd)) continue; record.cwd = disk.realPath(gpa, io, record.cwd); out.append(gpa, record) catch continue; @@ -128,11 +128,6 @@ pub fn load( return out.toOwnedSlice(gpa) catch &.{}; } -fn worktreeExists(io: Io, worktree: []const u8) bool { - const info = Io.Dir.cwd().statFile(io, worktree, .{}) catch return false; - return info.kind == .directory; -} - const testing = std.testing; fn testEnviron(arena: std.mem.Allocator, base: []const u8) !std.process.Environ.Map { @@ -182,7 +177,6 @@ test "a recovered status is never one only a live session can be in" { try testing.expect(status != .active); try testing.expect(status != .starting); try testing.expect(status != .exited); - try testing.expect(status != .orphan); try testing.expect(status != .unknown); } } diff --git a/src/watch_status.zig b/src/watch_status.zig index a0e3a6b..b900acc 100644 --- a/src/watch_status.zig +++ b/src/watch_status.zig @@ -33,7 +33,7 @@ pub fn present(current: Status, plan: bool) Status { if (!plan) return current; return switch (current) { .active, .idle => .plan, - .starting, .waiting, .exited, .orphan, .unknown, .plan => current, + .starting, .waiting, .exited, .unknown, .plan => current, }; } @@ -85,7 +85,6 @@ test "every hook event maps to a status a session can actually be in" { for ([_]Event{ .waiting, .active, .idle, .ended }) |event| { const status = apply(.starting, event); try testing.expect(status != .unknown); - try testing.expect(status != .orphan); try testing.expect(status != .plan); } } @@ -97,13 +96,12 @@ test "plan mode replaces working, and never replaces being blocked" { try testing.expectEqual(Status.waiting, present(.waiting, true)); try testing.expectEqual(Status.exited, present(.exited, true)); - try testing.expectEqual(Status.orphan, present(.orphan, true)); try testing.expectEqual(Status.unknown, present(.unknown, true)); try testing.expectEqual(Status.starting, present(.starting, true)); } test "without plan mode, present changes nothing at all" { - for ([_]Status{ .starting, .active, .waiting, .idle, .plan, .exited, .orphan, .unknown }) |status| { + for ([_]Status{ .starting, .active, .waiting, .idle, .plan, .exited, .unknown }) |status| { try testing.expectEqual(status, present(status, false)); } } diff --git a/src/watch_table.zig b/src/watch_table.zig index 089893b..2bfa93d 100644 --- a/src/watch_table.zig +++ b/src/watch_table.zig @@ -18,7 +18,7 @@ pub const Row = struct { pub fn attachable(self: Row) bool { if (self.session_id == null) return false; return switch (self.status orelse return false) { - .starting, .active, .plan, .waiting, .idle, .orphan => true, + .starting, .active, .plan, .waiting, .idle => true, .exited, .unknown => false, }; } @@ -32,7 +32,6 @@ fn glyph(status: ?sessions.Status) []const u8 { .idle => "○", .starting => "◌", .exited => "✗", - .orphan => "⚠", .unknown => "?", }; } @@ -42,7 +41,6 @@ fn paint(status: ?sessions.Status, palette: ui.Palette) []const u8 { .waiting => palette.yellow, .active => palette.green, .plan => palette.cyan, - .orphan => palette.yellow, .exited => palette.red, .idle, .starting, .unknown => palette.dim, }; @@ -357,8 +355,6 @@ test "a row is attachable only when something is actually behind it" { try testing.expect(!row.attachable()); row.status = .exited; try testing.expect(!row.attachable()); - row.status = .orphan; - try testing.expect(row.attachable()); row.status = .plan; try testing.expect(row.attachable());