diff --git a/README.md b/README.md index 8d7a103..60628e4 100644 --- a/README.md +++ b/README.md @@ -104,6 +104,7 @@ already there, starting one if not. ISSUE STATUS BRANCH AGE ❯ PE-256 ● waiting feature/pe-256-app-hangs-on-launch 4s PE-270 ◐ active feature/pe-270-crash-in-mapview 12s + PE-301 ◈ plan feature/pe-301-widget-refresh 31s PE-9 · no session feature/pe-9-unrelated — ↑↓ move · enter opens · n new issue · x kill · q quit @@ -127,6 +128,13 @@ 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 a new Claude Code release cannot quietly make them wrong. +`◈ plan` is a turn in flight too, in Claude Code's plan mode — it is researching +and writing a plan, and has not been approved to touch any files yet. Since +`planMode` defaults to on, every session starts there and leaves when you +approve the plan, so the marker going away is the useful half of it. A session +that is *blocked* on that approval reads `● waiting`, not `◈ plan`: the prompt +in front of you outranks the mode behind it. + Enter attaches, and from there the terminal is entirely Claude Code's — lcc writes nothing of its own over it. **`^\` returns to the dashboard** without touching the session. `^C` still reaches the agent, which is the point of not diff --git a/src/commands/watch.zig b/src/commands/watch.zig index f59af5f..0ba3540 100644 --- a/src/commands/watch.zig +++ b/src/commands/watch.zig @@ -660,7 +660,7 @@ pub fn hook(app: app_mod.App, opts: HookOpts) !void { // `opts.socket`, not the environment: the daemon baked its own socket into // this command line, and the environment here is the session's — whichever // shell started it. See `watch_client.connectAt`. - watch_client.report(app, opts.socket, payload.cwd, payload.session_id, event); + watch_client.report(app, opts.socket, payload.cwd, payload.session_id, event, payload.permission_mode); } test "the --json keys name sessions, never the process behind them" { diff --git a/src/daemon.zig b/src/daemon.zig index d8d0d0f..cc78738 100644 --- a/src/daemon.zig +++ b/src/daemon.zig @@ -639,7 +639,7 @@ fn handleFrame(loop: *Loop, client: *Client, frame: wire.Frame, at: i64) void { // session it did not start — ignored rather than an error, so a // stale hook config cannot make anything fail. const session = loop.findByWorktree(body.cwd) orelse return; - if (session.note(event, at)) loop.dirty = true; + if (session.note(event, body.permission_mode, at)) loop.dirty = true; }, .attach => attachClient(loop, client, frame), .detach => detachClient(loop, client), @@ -702,7 +702,10 @@ fn registerSession(loop: *Loop, client: *Client, frame: wire.Frame, at: i64) voi loop.sendControl(client, .registered, wire.Registered{ .session_id = id, .pid = @intCast(session.pid), - .status = @tagName(session.status), + // `shown`, like every other status that leaves this process. It is + // always `starting` here, but a second spelling of "what a reader is + // told" is how the two answers drift apart later. + .status = @tagName(session.shown()), .started_at = session.started_at, }); } @@ -1251,6 +1254,25 @@ test "a registered session runs, echoes, and its output survives a reconnect" { const after = try conn.recv(.snapshot, &b); const after_view = try wire.parse(wire.Snapshot, arena, after); try testing.expectEqualStrings("waiting", after_view.sessions[0].status); + + // And the mode rides the same frame. The unit tests prove the payload + // parses and prove the projection; only this proves the field survives + // the socket and reaches the session the cwd names. + try conn.send(arena, .hook, wire.Hook{ + .cwd = base, + .session_id = "irrelevant", + .event = "active", + .permission_mode = "plan", + }); + waited = 5_000; + while (waited > 0) : (waited -= 100) { + try conn.send(arena, .list, .{}); + const snap = try conn.recv(.snapshot, &b); + const view = try wire.parse(wire.Snapshot, arena, snap); + if (std.mem.eql(u8, view.sessions[0].status, "plan")) break; + io.sleep(.fromMilliseconds(100), .awake) catch {}; + } + try testing.expect(waited > 0); } // Everything above is gone — both connections closed, as if the terminal @@ -1392,13 +1414,13 @@ test "a hook reports to the socket it was handed, not to the one its environment // Without a socket there is nothing to fall back to but that environment, // and the report is lost. Asserted so the fallback stays a fallback: if this // ever starts landing, the test above it has stopped proving anything. - watch_client.report(hook_app, null, base, "s", "waiting"); + watch_client.report(hook_app, null, base, "s", "waiting", ""); io.sleep(.fromMilliseconds(300), .awake) catch {}; try testing.expectEqualStrings("starting", try statusNow(&conn, arena, &b)); // Handed the daemon's own socket, exactly as `watch_hooks.settingsJson` // writes it, the same report lands. - watch_client.report(hook_app, socket_path, base, "s", "waiting"); + watch_client.report(hook_app, socket_path, base, "s", "waiting", ""); var waited: i32 = 5_000; while (waited > 0) : (waited -= 100) { if (std.mem.eql(u8, try statusNow(&conn, arena, &b), "waiting")) break; diff --git a/src/sessions.zig b/src/sessions.zig index 687c82c..a14c777 100644 --- a/src/sessions.zig +++ b/src/sessions.zig @@ -27,6 +27,17 @@ pub const Status = enum { active, waiting, idle, + /// Working, but still in Claude Code's plan mode — it has not been approved + /// to touch files yet. + /// + /// A *mode* rather than a point in the lifecycle, and it sits in this enum + /// anyway because the column has one slot and this is the more useful thing + /// to put in it: `lcc start` launches every session in plan mode, so the + /// question a row has to answer is not "is a turn in flight" — it nearly + /// always is — but "has this one been let loose yet". It displaces `active` + /// and `idle` only. See `watch_status.present` for what it must never + /// displace and why. + plan, exited, /// The worktree is gone from disk but the agent is still running in it. orphan, @@ -461,6 +472,11 @@ test "a worktree that is gone reads as orphan, and the row stays" { // An exited session's worktree being gone is ordinary cleanup, not // an orphan — there is no agent left to be stranded. .{ .id = "s-done", .worktree = try std.fs.path.join(arena, &.{ base, "removed" }), .status = "exited" }, + // Still planning, and the directory it was planning in is gone. The + // reader's verdict has to win over the daemon's here as much as it + // does for `active` — an agent stranded in a deleted worktree is + // stranded whatever mode it is in. + .{ .id = "s-plan", .worktree = try std.fs.path.join(arena, &.{ base, "removed" }), .status = "plan" }, }, }; @@ -468,6 +484,15 @@ test "a worktree that is gone reads as orphan, and the row stays" { 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 "plan round-trips as text, like every other status" { + // Stored as TEXT, so the tag order stays an implementation detail — adding + // `plan` in the middle of the enum must not repaint rows already on disk. + const s: Session = .{ .status = "plan" }; + try testing.expectEqual(Status.plan, s.parsedStatus()); + try testing.expectEqualStrings("plan", Status.plan.label()); } test "owning matches the worktree and what is inside it, never a sibling prefix" { diff --git a/src/watch_client.zig b/src/watch_client.zig index 61bf16e..339551b 100644 --- a/src/watch_client.zig +++ b/src/watch_client.zig @@ -245,6 +245,7 @@ pub fn report( cwd: []const u8, session_id: []const u8, event: []const u8, + permission_mode: []const u8, ) void { const opened = if (socket) |path| connectAt(app, .control, path) @@ -256,6 +257,7 @@ pub fn report( .cwd = cwd, .session_id = session_id, .event = event, + .permission_mode = permission_mode, }) catch {}; } diff --git a/src/watch_hooks.zig b/src/watch_hooks.zig index 4a0909f..8ea093e 100644 --- a/src/watch_hooks.zig +++ b/src/watch_hooks.zig @@ -20,6 +20,19 @@ //! notification's payload to work out which kind it was. A field lcc never //! reads is a field that cannot be renamed out from under it. //! +//! `permission_mode` is the one exception, and it is one because Claude Code +//! offers no way to make it the rule: matchers select on a notification's type +//! and on tool names, never on the mode, so plan mode cannot be baked into a +//! command line the way every other state above is. It is read out of the +//! payload instead — from `PreToolUse`, `UserPromptSubmit` and `Stop`, the +//! three of these events that carry it. +//! +//! What keeps that from becoming the fragility the rule exists to avoid: an +//! absent or empty value is a **no-op**, never a clear. A renamed or dropped +//! field freezes the last mode reported rather than silently deciding every +//! session has left plan mode, and a value this build has never heard of reads +//! as "not plan" — the direction `Session.parsedStatus` already fails in. +//! //! Installed through `claude --settings `, which loads *additional* //! settings and merges hook entries rather than replacing them. So lcc writes //! nothing to `~/.claude/settings.json`, nothing into the repo, and the hooks @@ -163,8 +176,26 @@ pub const Payload = struct { session_id: []const u8 = "", hook_event_name: []const u8 = "", transcript_path: []const u8 = "", + /// Claude Code's own permission mode. Empty on the events that do not carry + /// it — `Notification`, `SubagentStart`, `SessionEnd` — which is why the + /// daemon holds the last one rather than re-deriving it per event. + permission_mode: []const u8 = "", }; +/// The one mode lcc distinguishes. +/// +/// The others (`default`, `acceptEdits`, `bypassPermissions`, `dontAsk`, +/// `auto`) all present as the lifecycle status. Plan mode is worth a row of its +/// own because it ends: `lcc start` launches in it and approving the plan +/// leaves it, so the marker appearing and going away is the signal. A badge +/// that every session wore for its whole life would not be. +pub const plan_mode = "plan"; + +/// Whether a reported mode means the session is still planning. +pub fn isPlan(permission_mode: []const u8) bool { + return std.mem.eql(u8, permission_mode, plan_mode); +} + pub fn parsePayload(gpa: std.mem.Allocator, raw: []const u8) ?Payload { return std.json.parseFromSliceLeaky(Payload, gpa, raw, .{ .ignore_unknown_fields = true, @@ -177,6 +208,7 @@ pub const Report = struct { cwd: []const u8, session_id: []const u8, event: []const u8, + permission_mode: []const u8 = "", }; const testing = std.testing; @@ -274,20 +306,30 @@ test "a hook payload yields the worktree, and a broken one yields nothing" { defer arena_state.deinit(); const arena = arena_state.allocator(); + // Captured verbatim from Claude Code 2.1.223, only the paths shortened. + // Written out rather than reduced to the four fields lcc reads, because + // what this has to prove is that the *real* shape parses — including + // `effort` and `tool_input`, which are nested objects rather than the + // scalar unknowns a hand-written fixture would have contained. const raw = - \\{"session_id":"abc123","transcript_path":"/h/.claude/projects/x/t.jsonl", - \\ "cwd":"/r/.lcc/worktrees/pe-256","permission_mode":"plan", - \\ "hook_event_name":"Notification","message":"Claude needs your permission"} + \\{"session_id":"669f68ae","transcript_path":"/h/.claude/projects/x/t.jsonl", + \\ "cwd":"/r/.lcc/worktrees/pe-256","prompt_id":"551136fb", + \\ "permission_mode":"plan","effort":{"level":"xhigh"}, + \\ "hook_event_name":"PreToolUse","tool_name":"Bash", + \\ "tool_input":{"command":"echo hi","description":"Print hi"}, + \\ "tool_use_id":"toolu_016wh833eFmoMGNzHJyvZ3Ay"} ; const payload = parsePayload(arena, raw).?; // cwd is the worktree, which is the key the daemon already files sessions // under — no correlation table, no session-id mapping to keep in sync. try testing.expectEqualStrings("/r/.lcc/worktrees/pe-256", payload.cwd); - try testing.expectEqualStrings("abc123", payload.session_id); + try testing.expectEqualStrings("669f68ae", payload.session_id); + try testing.expectEqualStrings("PreToolUse", payload.hook_event_name); - // Fields lcc does not read must not be able to break it: `permission_mode` - // and `message` are ignored rather than rejected. - try testing.expectEqualStrings("Notification", payload.hook_event_name); + // The one field read out of the payload rather than baked into a matcher, + // because Claude Code offers no matcher that selects on it. See the header. + try testing.expectEqualStrings("plan", payload.permission_mode); + try testing.expect(isPlan(payload.permission_mode)); // Malformed input is a dropped update, never a crash in a hook that runs on // every turn of every session. @@ -298,6 +340,66 @@ test "a hook payload yields the worktree, and a broken one yields nothing" { // Claude Code that drops a key costs one update rather than all of them. const sparse = parsePayload(arena, "{}").?; try testing.expectEqualStrings("", sparse.cwd); + // And an absent mode reads as absent, not as "left plan mode". The daemon + // holds the last one it was told; a renamed field must cost the update + // rather than silently clearing every session's plan marker. + try testing.expectEqualStrings("", sparse.permission_mode); + try testing.expect(!isPlan(sparse.permission_mode)); +} + +test "the events that report no mode really report none" { + const gpa = testing.allocator; + var arena_state: std.heap.ArenaAllocator = .init(gpa); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + + // Half the events lcc registers carry no `permission_mode` — measured, and + // the reason `watch_session.Session` holds the last one rather than reading + // it per event. This payload is a `Notification`, which is *also* the one + // event whose absent mode would do the most damage if read as a clear: it + // fires on the permission prompt at the end of a plan, so a session would + // drop its plan marker at the exact moment the marker was earned. + const raw = + \\{"session_id":"abc123","transcript_path":"/h/.claude/projects/x/t.jsonl", + \\ "cwd":"/r/.lcc/worktrees/pe-256","hook_event_name":"Notification", + \\ "notification_type":"permission_prompt", + \\ "message":"Claude needs your permission"} + ; + const payload = parsePayload(arena, raw).?; + try testing.expectEqualStrings("/r/.lcc/worktrees/pe-256", payload.cwd); + try testing.expectEqualStrings("", payload.permission_mode); +} + +test "plan is the only mode lcc distinguishes" { + try testing.expect(isPlan("plan")); + // The other five Claude Code reports all mean the same thing to a row: the + // lifecycle status, unchanged. Plan mode earns a marker because it *ends* — + // one that every session wore for its whole life would say nothing. + for ([_][]const u8{ "default", "acceptEdits", "bypassPermissions", "dontAsk", "auto" }) |mode| { + try testing.expect(!isPlan(mode)); + } + // Case matters, and a near miss is not a match: reading `Plan` as plan mode + // would be a guess, and guessing is what the hook design exists to avoid. + try testing.expect(!isPlan("Plan")); + try testing.expect(!isPlan("")); +} + +test "plan mode needs no hook of its own" { + const gpa = testing.allocator; + var arena_state: std.heap.ArenaAllocator = .init(gpa); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + + // The mode rides on events lcc already registers, so the settings file is + // byte-for-byte what it was. Pinned because the obvious "fix" for a future + // bug here is to add a hook — and there is no hook event that reports a + // mode change, so one would fire on something else and mean nothing. + const body = try settingsJson(arena, "/opt/homebrew/bin/lcc", "/s.sock"); + try testing.expect(std.mem.indexOf(u8, body, "permission_mode") == null); + try testing.expect(std.mem.indexOf(u8, body, "--event plan") == null); + inline for (@typeInfo(Events).@"struct".fields) |field| { + try testing.expect(std.mem.indexOf(u8, body, "\"" ++ field.name ++ "\"") != null); + } } test "event names round-trip through the command line" { diff --git a/src/watch_session.zig b/src/watch_session.zig index 3c79165..2ca9783 100644 --- a/src/watch_session.zig +++ b/src/watch_session.zig @@ -42,6 +42,13 @@ pub const Session = struct { started_at: i64, /// When the last hook event arrived — what the time-based decay measures. last_event_at: i64, + /// Claude Code's plan mode, as last reported. + /// + /// Held rather than read per event, because half the events lcc registers + /// carry no `permission_mode` at all: without this a `Notification` would + /// look like leaving plan mode, when a permission prompt is the one moment + /// the mode certainly has not changed. + plan: bool = false, exit: ?pty.Exit = null, input: [input_capacity]u8 = undefined, @@ -98,9 +105,36 @@ pub const Session = struct { self.master_open = false; } + /// The status a reader is shown: the lifecycle status and plan mode + /// together. + /// + /// Everything that leaves the daemon goes through this. `status` on its own + /// is the lifecycle — what the transition function and the decay operate on + /// — and reporting it raw is what would leave a row saying `active` for a + /// session that has not been allowed near a file yet. + pub fn shown(self: Session) sessions.Status { + return watch_status.present(self.status, self.plan); + } + + /// Both setters key `status_at` and `dirty` off `shown`, not off the field + /// they write. Two things fall out of that. The old "the same status again + /// is not a change" guard still holds — writing an unchanged field cannot + /// change what it composes to. And a change no reader can see costs no + /// write: an `active` session in plan mode decaying to `idle` still reads + /// `plan`, and rewriting the registry for it would be a file write per + /// session per quarter hour to record nothing. fn setStatus(self: *Session, status: sessions.Status, now: i64) void { - if (self.status == status) return; + const before = self.shown(); self.status = status; + if (self.shown() == before) return; + self.status_at = now; + self.dirty = true; + } + + fn setPlan(self: *Session, plan: bool, now: i64) void { + const before = self.shown(); + self.plan = plan; + if (self.shown() == before) return; self.status_at = now; self.dirty = true; } @@ -166,19 +200,25 @@ pub const Session = struct { } /// A hook event. Returns true when the registry needs rewriting. - pub fn note(self: *Session, event: watch_hooks.Event, now: i64) bool { - const before = self.status; + /// + /// An empty `permission_mode` leaves plan mode alone rather than clearing + /// it — most of the events lcc registers carry no mode, and treating their + /// silence as "no longer planning" would flip the row back on the first + /// `Notification` of every plan. + pub fn note(self: *Session, event: watch_hooks.Event, permission_mode: []const u8, now: i64) bool { + const before = self.shown(); self.last_event_at = now; + if (permission_mode.len > 0) self.setPlan(watch_hooks.isPlan(permission_mode), now); self.setStatus(watch_status.apply(self.status, event), now); - return before != self.status; + return before != self.shown(); } /// Time-based decay, run on the coarse tick. Separate from `note` so a /// status only ages when nothing has been reported. pub fn tick(self: *Session, now: i64) bool { - const before = self.status; + const before = self.shown(); self.setStatus(watch_status.resolve(self.status, self.last_event_at, now), now); - return before != self.status; + return before != self.shown(); } /// `waitpid(WNOHANG)`. Returns true once the child has been reaped. @@ -219,7 +259,7 @@ pub const Session = struct { .issue = self.issue, .repo_root = self.repo_root, .pid = @intCast(self.pid), - .status = @tagName(self.status), + .status = @tagName(self.shown()), .status_at = self.status_at, .started_at = self.started_at, .last_activity_at = self.last_event_at, @@ -339,7 +379,7 @@ test "a status change marks the registry dirty; repeating one does not" { var s = stubSession(&scratch); s.dirty = false; - try testing.expect(s.note(.waiting, 1010)); + try testing.expect(s.note(.waiting, "", 1010)); try testing.expectEqual(sessions.Status.waiting, s.status); try testing.expectEqual(@as(i64, 1010), s.status_at); try testing.expect(s.dirty); @@ -347,7 +387,7 @@ test "a status change marks the registry dirty; repeating one does not" { // The same status again is not a change. Without this, PreToolUse on every // tool call would rewrite the registry file continuously through a long turn. s.dirty = false; - try testing.expect(!s.note(.waiting, 1020)); + try testing.expect(!s.note(.waiting, "", 1020)); try testing.expect(!s.dirty); // But the event still counts as activity, or the decay would fire mid-turn. try testing.expectEqual(@as(i64, 1020), s.last_event_at); @@ -374,7 +414,7 @@ test "the registry entry carries the status as text" { var scratch = try ring.Ring.init(gpa, 64); defer scratch.deinit(gpa); var s = stubSession(&scratch); - _ = s.note(.active, 1010); + _ = s.note(.active, "", 1010); const row = s.entry(); try testing.expectEqualStrings("active", row.status); @@ -390,7 +430,7 @@ test "a decayed session goes idle, and a waiting one never does" { defer scratch.deinit(gpa); var s = stubSession(&scratch); - _ = s.note(.active, 1000); + _ = s.note(.active, "", 1000); try testing.expect(!s.tick(1000 + watch_status.active_decay_seconds)); try testing.expect(s.tick(1000 + watch_status.active_decay_seconds + 1)); try testing.expectEqual(sessions.Status.idle, s.status); @@ -398,7 +438,78 @@ test "a decayed session goes idle, and a waiting one never does" { // An unanswered permission prompt is still unanswered a day later, and // ageing it into `idle` would hide it exactly when the user has been away // longest. - _ = s.note(.waiting, 2000); + _ = s.note(.waiting, "", 2000); try testing.expect(!s.tick(2000 + watch_status.active_decay_seconds * 100)); try testing.expectEqual(sessions.Status.waiting, s.status); } + +test "a session reports plan mode, and keeps reporting it through events that omit it" { + const gpa = testing.allocator; + var scratch = try ring.Ring.init(gpa, 64); + defer scratch.deinit(gpa); + var s = stubSession(&scratch); + + // PreToolUse carries the mode, so the very first tool call in a planning + // turn is enough — nothing has to wait for the user to type anything. + try testing.expect(s.note(.active, "plan", 1010)); + try testing.expectEqualStrings("plan", s.entry().status); + // The lifecycle underneath is untouched: `plan` is what the row shows, not + // a state the transition function or the decay ever has to reason about. + try testing.expectEqual(sessions.Status.active, s.status); + + // SubagentStart carries no `permission_mode` at all. Reading its silence as + // "no longer planning" would drop the row back to `active` every time the + // agent handed work to a subagent — which during a plan is constantly. + try testing.expect(!s.note(.active, "", 1020)); + try testing.expectEqualStrings("plan", s.entry().status); + + // Neither does Notification, and a permission prompt is the one moment the + // mode certainly has not changed. `waiting` outranks plan mode here: it is + // the only status that means "go here now", and the prompt at the end of a + // plan is exactly when that must not be painted over. + try testing.expect(s.note(.waiting, "", 1030)); + try testing.expectEqualStrings("waiting", s.entry().status); + try testing.expect(s.plan); + + // Approving the plan is not an event of its own — the next hook that + // carries a mode simply reports a different one, and that is what ends it. + try testing.expect(s.note(.active, "default", 1040)); + try testing.expectEqualStrings("active", s.entry().status); + try testing.expect(!s.plan); +} + +test "a mode this build has never heard of is not plan mode" { + const gpa = testing.allocator; + var scratch = try ring.Ring.init(gpa, 64); + defer scratch.deinit(gpa); + var s = stubSession(&scratch); + + // Claude Code has six modes today and may have seven tomorrow. An unknown + // one reads as "not planning", which is the safe direction: the row loses a + // marker rather than claiming an agent cannot touch files when it can. + _ = s.note(.active, "plan", 1000); + _ = s.note(.active, "some-mode-from-2027", 1010); + try testing.expectEqualStrings("active", s.entry().status); +} + +test "a change no reader can see costs no registry write" { + const gpa = testing.allocator; + var scratch = try ring.Ring.init(gpa, 64); + defer scratch.deinit(gpa); + var s = stubSession(&scratch); + + _ = s.note(.active, "plan", 1000); + s.dirty = false; + + // The lifecycle decays `active` to `idle` underneath, but both compose to + // `plan`, so the row is unchanged. Reporting this as dirty would be a file + // write per session per quarter hour to record nothing. + try testing.expect(!s.tick(1000 + watch_status.active_decay_seconds + 1)); + try testing.expect(!s.dirty); + try testing.expectEqual(sessions.Status.idle, s.status); + try testing.expectEqualStrings("plan", s.entry().status); + + // And leaving plan mode then shows the decay that already happened. + try testing.expect(s.note(.idle, "acceptEdits", 2000)); + try testing.expectEqualStrings("idle", s.entry().status); +} diff --git a/src/watch_status.zig b/src/watch_status.zig index ce3cc24..43685c1 100644 --- a/src/watch_status.zig +++ b/src/watch_status.zig @@ -6,6 +6,14 @@ //! //! The daemon owns `exited` — `waitpid` answers it, not a hook, because a //! process that died in a way that skipped `SessionEnd` still died. +//! +//! Plan mode is *projected* here rather than applied. It is a mode, not a +//! transition: nothing fires when a plan is approved — the next hook that +//! carries a `permission_mode` simply reports a different one. Modelling it as +//! a state would put a "which mode am I in" question inside `apply` and a +//! "does this decay" question inside `decay`, to describe something neither of +//! them drives. So the session keeps its lifecycle status, carries plan mode +//! beside it, and `present` decides what the two of them add up to. const std = @import("std"); const sessions = @import("sessions.zig"); @@ -58,6 +66,31 @@ pub fn resolve(current: Status, last_event_at: i64, now: i64) Status { return decay(current, now - last_event_at); } +/// The lifecycle status and plan mode, combined into the one thing a row shows. +/// +/// Only `active` and `idle` give way. The rest outrank plan mode, each for its +/// own reason: +/// +/// `waiting` because it is the only status that means "go here now", and the +/// moment plan mode matters most — the approval prompt at the end of a plan — +/// *is* a permission prompt. Showing `plan` there would replace the signal the +/// dashboard exists for with a restatement of something the user already knows. +/// +/// `exited` and `orphan` because they are facts about the process and the +/// worktree rather than about what the agent is doing, and `unknown` because it +/// means nothing on disk can be believed — including this. +/// +/// `starting` because it means the child has not produced a byte yet, which +/// plan mode does not contradict; it lasts under a second, and its own decay +/// runs off the lifecycle field either way. +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, + }; +} + const testing = std.testing; test "a reported event replaces the status, whatever it was" { @@ -123,5 +156,44 @@ test "every hook event maps to a status a session can actually be in" { const status = apply(.starting, event); try testing.expect(status != .unknown); try testing.expect(status != .orphan); + // Nor `plan`, which is derived rather than reported. Routing an event + // to it here would compile and would put a mode into the lifecycle + // field, where the decay and the `exited` guard would then have to + // reason about it — the thing `present` exists to avoid. + try testing.expect(status != .plan); + } +} + +test "plan mode replaces working, and never replaces being blocked" { + // The two that give way. `lcc start` launches every session in plan mode, + // so "a turn is in flight" is nearly always true and nearly never the thing + // worth a column; "has this been approved to touch files" is. + try testing.expectEqual(Status.plan, present(.active, true)); + try testing.expectEqual(Status.plan, present(.idle, true)); + + // The one that must not. `waiting` is the only status meaning "go here + // now", and the approval prompt at the end of a plan is a permission + // prompt — precisely where showing `plan` would trade the signal the + // dashboard exists for against a restatement of what the user just did. + try testing.expectEqual(Status.waiting, present(.waiting, true)); + + // Facts about the process and the worktree, not about what the agent is + // doing. A dead session in plan mode is dead, and one whose worktree was + // deleted is still the row most worth seeing. + try testing.expectEqual(Status.exited, present(.exited, true)); + try testing.expectEqual(Status.orphan, present(.orphan, true)); + // `unknown` means nothing on disk can be believed, and that includes the + // mode the same file reported. + try testing.expectEqual(Status.unknown, present(.unknown, true)); + // And the child has not spoken yet, which plan mode does not contradict. + try testing.expectEqual(Status.starting, present(.starting, true)); +} + +test "without plan mode, present changes nothing at all" { + // The property that lets every existing caller keep its behaviour: for a + // session that is not planning this is the identity, so nothing about the + // other six statuses moved when `plan` was added. + for ([_]Status{ .starting, .active, .waiting, .idle, .plan, .exited, .orphan, .unknown }) |status| { + try testing.expectEqual(status, present(status, false)); } } diff --git a/src/watch_table.zig b/src/watch_table.zig index a2ee9dd..0880168 100644 --- a/src/watch_table.zig +++ b/src/watch_table.zig @@ -48,7 +48,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, .waiting, .idle, .orphan => true, + .starting, .active, .plan, .waiting, .idle, .orphan => true, .exited, .unknown => false, }; } @@ -62,6 +62,9 @@ fn glyph(status: ?sessions.Status) []const u8 { return switch (status orelse return "·") { .waiting => "●", .active => "◐", + // The only glyph here that is not a circle, because plan mode is the + // only one of these that is not a point in the lifecycle. + .plan => "◈", .idle => "○", .starting => "◌", .exited => "✗", @@ -74,6 +77,9 @@ fn paint(status: ?sessions.Status, palette: ui.Palette) []const u8 { return switch (status orelse return palette.dim) { .waiting => palette.yellow, .active => palette.green, + // Its own colour, not `active`'s: the whole reason the row says `plan` + // is that those two are different answers to "can it touch my files". + .plan => palette.cyan, .orphan => palette.yellow, .exited => palette.red, .idle, .starting, .unknown => palette.dim, @@ -445,11 +451,40 @@ test "a row is attachable only when something is actually behind it" { row.status = .orphan; try testing.expect(row.attachable()); + // A session still writing its plan is very much running, and Enter on it + // should attach rather than try to start a second one in the same worktree. + row.status = .plan; + try testing.expect(row.attachable()); + row.status = null; row.session_id = null; try testing.expect(!row.attachable()); } +test "a planning row says plan, not active" { + var buf: [4096]u8 = undefined; + var w: Io.Writer = .fixed(&buf); + ui.setColor(false); + + const rows = [_]Row{.{ + .key = "/w", + .session_id = "s-1", + .status = .plan, + .issue = "PE-256", + .branch = "feature/pe-256", + .worktree = "/w", + .last_activity_at = 900, + .exit_code = null, + .stale = false, + }}; + _ = render(&w, &rows, fit(measure(&rows), 120), 120, "/w", 1000); + + // The distinction the status is for: this agent has not been approved to + // touch files, and a row reading `active` would say the opposite. + try testing.expect(std.mem.indexOf(u8, w.buffered(), "◈ plan") != null); + try testing.expect(std.mem.indexOf(u8, w.buffered(), "active") == null); +} + test "a worktree with nothing running shows no age, not one measured from the epoch" { var buf: [4096]u8 = undefined; var w: Io.Writer = .fixed(&buf); diff --git a/src/wire.zig b/src/wire.zig index ffa15c7..7499f4e 100644 --- a/src/wire.zig +++ b/src/wire.zig @@ -343,6 +343,12 @@ pub const Hook = struct { cwd: []const u8, session_id: []const u8, event: []const u8, + /// Claude Code's permission mode, when the event that fired carried one. + /// + /// Defaulted rather than required, so this stayed additive: an empty value + /// means "nothing reported", which the daemon already has to handle for the + /// events that never carry a mode at all. + permission_mode: []const u8 = "", }; pub const Kill = struct { session_id: []const u8, signal: []const u8 }; pub const Stop = struct { force: bool };