diff --git a/README.md b/README.md index cfb6a38..1544fbf 100644 --- a/README.md +++ b/README.md @@ -260,5 +260,16 @@ tests/test-hook.sh # события -> состояния, бло будет дописываться в него: ```sh -touch ~/.local/state/claude-code-status/debug +touch ~/.local/state/claude-code-status/debug # включить +rm ~/.local/state/claude-code-status/debug # выключить ``` + +**Он пишет много лишнего о вас.** В лог попадают тексты ваших запросов целиком, +пути к транскриптам и командные строки шести процессов-предков — включая то, как +запущен claude, и адрес сокета zellij. Это диагностический инструмент, а не +телеметрия: включайте, когда что-то сломалось, и удаляйте файл после. Рост +ограничен восемью мегабайтами, дальше запись прекращается. + +Сборка через `gnome-extensions pack` обязательна: `schemas/gschemas.compiled` не +хранится в репозитории, и zip, собранный вручную из чекаута, оставит расширение +без схемы настроек. diff --git a/hooks/claude-status-hook.py b/hooks/claude-status-hook.py index 8aa0349..18d9a86 100755 --- a/hooks/claude-status-hook.py +++ b/hooks/claude-status-hook.py @@ -21,6 +21,7 @@ Design notes that are easy to get wrong: * No stdlib import beyond what is needed: this runs once per tool call. """ +import errno import fcntl import json import os @@ -52,6 +53,36 @@ NOTIFICATION_STATES = { # inflate the count with TaskCreate, TaskUpdate and the like. AGENT_TOOLS = {"Agent", "Task"} +# Events that mean the question has been dealt with. Anything else leaves a +# "blocked" session blocked: a subagent finishing, or the next tool starting, +# says nothing about the prompt still sitting on your screen, and clearing it +# would hide the one state this indicator exists to surface. +BLOCK_CLEARING = {"PostToolUse", "UserPromptSubmit", "Stop", "SessionStart"} + +# Nothing here is displayed at full length, and both ends of the pipe have to +# survive a hostile or merely absurd value: a multi-megabyte cwd would be copied +# into the state file, read back by the compositor and handed to Pango. +MAX_TEXT = 512 +# A stored timestamp further ahead than this is not a concurrent write, it is +# corruption -- and left alone it would refuse every later event forever, +# freezing the session's displayed state for good. +FUTURE_SLACK = 60 # seconds + + +def number(value, default=0): + """Coerce a value read back from disk. Files are ordinary user-writable + JSON: one corrupt field must not take the hook down for every session.""" + try: + n = float(value) + except (TypeError, ValueError): + return default + return default if n != n or n in (float("inf"), float("-inf")) else n + + +def clip(value): + text = value if isinstance(value, str) else "" + return text[:MAX_TEXT] + EVENT_STATES = { # A session that has just opened is waiting for your first prompt, which is # the same thing as one that has finished a turn: the input line is free @@ -74,10 +105,16 @@ def debug_log(event): environment, which cannot be changed without restarting the session. """ marker = os.path.join(STATE_DIR, "debug") - if not os.path.exists(marker): + try: + # Not followed through a symlink, and not grown without limit: this + # records prompts verbatim and the command lines of ancestor processes. + if os.lstat(marker).st_size > 8 << 20: + return + except OSError: return try: - with open(marker, "a") as fh: + fd = os.open(marker, os.O_WRONLY | os.O_APPEND | os.O_NOFOLLOW) + with os.fdopen(fd, "a") as fh: chain, pid = [], os.getppid() for _ in range(6): if pid <= 1: @@ -215,9 +252,13 @@ def pid_start_time(pid): def alive(pid, start=0): + """Is that pid still the process it was? Inputs come from disk, so both + arguments are coerced rather than trusted.""" + pid = int(number(pid)) if pid <= 0 or not os.path.exists("/proc/%d" % pid): return False # A file written before start times were recorded has nothing to compare. + start = number(start) return not start or pid_start_time(pid) == start @@ -239,9 +280,17 @@ def sweep_dead(keep): try: with open(path, "r") as fh: stale = json.load(fh) + if not isinstance(stale, dict): + continue pid = stale.get("pid", 0) except (OSError, ValueError, AttributeError): continue + except Exception: + # One unreadable file must not abort the sweep, and above all must + # not abort the caller: this runs before the hook writes its own + # state, so an exception here would stop new sessions appearing at + # all, for as long as the bad file sits there. + continue # pid 0 means the hook could not identify the process; there is nothing # to test for liveness, so leave it to the reader's age cutoff. if pid and not alive(pid, stale.get("pid_start", 0)): @@ -259,6 +308,22 @@ def write_atomic(path, payload): os.replace(tmp, path) +def open_lock(path): + """Open the lock file without following a symlink. + + A plain open() on a symlinked lock path truncates whatever it points at. + Nothing is escalated by that on a single-user machine, but a status + indicator has no business truncating files it was pointed at. + """ + flags = os.O_RDWR | os.O_CREAT | os.O_NOFOLLOW + try: + return os.fdopen(os.open(path, flags, 0o600), "r+") + except OSError as exc: + if exc.errno in (errno.ELOOP, errno.EMLINK): + return None + raise + + def apply_event(event, state, path, now): """Read the current state, decide, and write. Must run under the lock.""" if event.get("hook_event_name") == "SessionStart": @@ -268,11 +333,13 @@ def apply_event(event, state, path, now): sweep_dead(keep=os.path.basename(path)) if state == "end": - for victim in (path, path + ".lock"): - try: - os.unlink(victim) - except OSError: - pass + # Only the state file. Unlinking the lock while holding it drops mutual + # exclusion -- a hook already blocked on the old inode and one that + # creates a new file are then both inside the critical section. + try: + os.unlink(path) + except OSError: + pass return previous = None @@ -287,38 +354,67 @@ def apply_event(event, state, path, now): # Normalised here so the comparison below is against what would actually be # stored: comparing a stored "" to a raw notification message rewrites the # file on every idle_prompt for no change at all. - message = event.get("message", "") if state == "blocked" else "" + message = clip(event.get("message", "")) if state == "blocked" else "" name = event.get("hook_event_name") - agents = int(previous.get("agents", 0)) if previous else 0 + agents = int(number(previous.get("agents"))) if previous else 0 + agents = max(0, min(agents, 999)) # Whether the main agent has finished its turn. Tracked separately from the # state because with background subagents both are true at once: the turn is # over and work is still running. stopped = bool(previous.get("stopped")) if previous else False + # An event that lost a race carries an older timestamp than what is already + # stored. Its *state* must not be applied -- that is last-writer-wins, and + # replaying an old one would resurrect a state the session has left. + previous_ts = number(previous.get("event_ts")) if previous else 0 + if previous_ts > now + FUTURE_SLACK: + previous_ts = 0 # corrupt, and trusting it would freeze this session + stale = previous_ts > now + + # The subagent count is different in kind: the mutations are deltas, and + # deltas commute, so every one has to land whatever order it arrives in. + # Dropping a "+1" because its timestamp lost a race leaves the count one + # short, and the batch then frees the session while a subagent is still + # running -- the exact failure counting was added to prevent. The timestamps + # cannot be trusted for this at all: each is taken when its hook process + # starts, tens of milliseconds before it reaches the lock. if name in ("SessionStart", "UserPromptSubmit"): # A new turn from you starts a new batch. This also bounds the damage # when a subagent dies without its SubagentStop ever arriving: the count # cannot leak past the next thing you type. - agents, stopped = 0, False + agents = 0 elif name == "PreToolUse": agents += 1 elif name == "SubagentStop": agents = max(0, agents - 1) - elif name == "Stop": - stopped = True - elif name == "PostToolUse": - stopped = False - if name == "SubagentStop": - # The last subagent finishing is what finally frees a session whose main - # agent stopped long ago. - state = "waiting" if (stopped and agents == 0) else "busy" - elif state == "waiting" and agents > 0: - # The turn ended but the batch is still running, and the session will - # pick the results up itself. Calling it "waiting" would send you to a - # terminal that does not need you. - state = "busy" + if stale: + # Keep the stored state and flag; the delta above still gets persisted. + state = previous.get("state", state) + else: + if name in ("SessionStart", "UserPromptSubmit"): + stopped = False + elif name == "Stop": + stopped = True + elif name == "PostToolUse": + stopped = False + + if name == "SubagentStop": + # The last subagent finishing is what finally frees a session whose + # main agent stopped long ago. + state = "waiting" if (stopped and agents == 0) else "busy" + elif state == "waiting" and agents > 0: + # The turn ended but the batch is still running, and the session + # will pick the results up itself. Calling it "waiting" would send + # you to a terminal that does not need you. + state = "busy" + + # A pending question outlives everything except an answer to it. + if (previous and previous.get("state") == "blocked" + and state != "blocked" and name not in BLOCK_CLEARING): + state = "blocked" + message = previous.get("message", "") # Resolved before the unchanged-check, not after, so that a pid which has # changed forces a write. A session resumed under a new pid, or one whose @@ -330,12 +426,9 @@ def apply_event(event, state, path, now): if previous: # Auto-compaction raises SessionStart again, in the middle of a turn the # session is still working on. Taking it at face value would flip a busy - # session to idle until the next tool call corrected it. + # session to waiting until the next tool call corrected it. if event.get("hook_event_name") == "SessionStart" and event.get("source") == "compact": return - # Refuse events that lost a race with a newer one. - if previous.get("event_ts", 0) > now: - return # Nothing new to publish: stay quiet so the directory monitor stays quiet. if (previous.get("state") == state and previous.get("message", "") == message @@ -349,22 +442,27 @@ def apply_event(event, state, path, now): write_atomic(path, { "session_id": event.get("session_id"), "state": state, - "cwd": event.get("cwd") or "", + "cwd": clip(event.get("cwd") or ""), # Age is measured from the moment the state was entered, not from the # last event, so "waiting 40 min" survives unrelated later writes. "since": previous["since"] if previous and previous.get("state") == state else now, - "event_ts": now, + # When the session itself began, as opposed to when it entered this + # state. Seniority between chips is decided on this: a session that + # changed state a moment ago has not become the younger of the two. + "started": (previous.get("started") if previous else None) or now, + "event_ts": max(now, previous_ts), # 0 means "could not tell"; the reader must not take that for "dead". "pid": claude_pid, "pid_start": pid_start_time(claude_pid) if claude_pid else 0, "event": event.get("hook_event_name", ""), "notification_type": event.get("notification_type", ""), - "message": message, + "message": clip(message), "agents": agents, "stopped": stopped, - "zellij_session": env.get("ZELLIJ_SESSION_NAME", ""), - "zellij_pane": env.get("ZELLIJ_PANE_ID", ""), - "transcript": event.get("transcript_path", ""), + # Kept from the previous write when this event could not identify the + # process: a momentary failure should not blank out where the session is. + "zellij_session": clip(env.get("ZELLIJ_SESSION_NAME") + or (previous.get("zellij_session", "") if previous else "")), }) @@ -395,7 +493,10 @@ def main(): # without a lock both processes read the same "previous" and the loser's # write still lands last, pinning a finished session at "busy". The lock is # a separate file because write_atomic replaces the inode of the real one. - with open(path + ".lock", "w") as lock: + lock = open_lock(path + ".lock") + if lock is None: + return 0 + with lock: fcntl.flock(lock, fcntl.LOCK_EX) apply_event(event, state, path, now) return 0 diff --git a/hooks/install.py b/hooks/install.py index 2aca5f6..227581f 100755 --- a/hooks/install.py +++ b/hooks/install.py @@ -11,7 +11,9 @@ Usage: install.py [--uninstall] [--settings PATH] import json import os +import shlex import shutil +import stat import sys HOOK = os.path.join(os.path.dirname(os.path.abspath(__file__)), "claude-status-hook.py") @@ -42,7 +44,9 @@ EVENTS = { def entry(spec): async_, matcher = spec - hook = {"type": "command", "command": HOOK, "timeout": 5} + # Claude Code runs the command through a shell, so a repository path + # containing a space -- or worse -- has to survive the trip. + hook = {"type": "command", "command": shlex.quote(HOOK), "timeout": 5} if async_: hook["async"] = True return {"matcher": matcher, "hooks": [hook]} @@ -50,7 +54,7 @@ def entry(spec): def is_ours(group): return any( - h.get("command", "").endswith("claude-status-hook.py") + h.get("command", "").rstrip("'\"").endswith("claude-status-hook.py") for h in group.get("hooks", []) if isinstance(h, dict) ) @@ -60,7 +64,15 @@ def main(): uninstall = "--uninstall" in sys.argv path = os.path.expanduser("~/.claude/settings.json") if "--settings" in sys.argv: - path = sys.argv[sys.argv.index("--settings") + 1] + try: + path = sys.argv[sys.argv.index("--settings") + 1] + except IndexError: + sys.exit("--settings needs a path") + # Written through, not over: a settings.json symlinked out of a dotfiles + # repository would otherwise be replaced by a regular file, silently + # detaching it from the repository that is supposed to manage it. + path = os.path.realpath(path) + os.makedirs(os.path.dirname(path), exist_ok=True) try: with open(path) as fh: @@ -70,8 +82,12 @@ def main(): except ValueError as exc: sys.exit("refusing to touch malformed %s: %s" % (path, exc)) - if os.path.exists(path): + # Kept from the first run only. Overwriting it on every run would, on the + # second run, replace the pristine copy with the already-modified one -- + # which is exactly when someone reaches for a backup. + if os.path.exists(path) and not os.path.exists(path + ".bak"): shutil.copyfile(path, path + ".bak") + shutil.copymode(path, path + ".bak") hooks = settings.setdefault("hooks", {}) for event in EVENTS: @@ -92,10 +108,17 @@ def main(): with open(tmp, "w") as fh: json.dump(settings, fh, indent=2) fh.write("\n") + # A replace discards the original's mode. settings.json may hold API keys + # and may have been deliberately narrowed to 0600; silently widening it to + # the umask default would undo that without a word. + try: + os.chmod(tmp, stat.S_IMODE(os.stat(path).st_mode)) + except OSError: + pass os.replace(tmp, path) print("%s %s in %s" % ("removed" if uninstall else "installed", HOOK, path)) - print("restart running claude sessions for the change to take effect") + print("running sessions pick this up on their own; no restart needed") if __name__ == "__main__": diff --git a/lib/abbrev.js b/lib/abbrev.js index b0fec47..5e68077 100644 --- a/lib/abbrev.js +++ b/lib/abbrev.js @@ -13,9 +13,12 @@ const MAX = 3; /** Split on separators and camelCase humps: "pet-project-server", "outlineMcp". */ function segments(name) { + // Unicode-aware: splitting on [^a-zA-Z0-9] makes every Cyrillic letter a + // separator, so "проект" reduces to nothing and every non-Latin project + // ends up sharing the label "?". return name - .replace(/([a-z0-9])([A-Z])/g, '$1 $2') - .split(/[^a-zA-Z0-9]+/) + .replace(/(\p{Ll}|\p{N})(\p{Lu})/gu, '$1 $2') + .split(/[^\p{L}\p{N}]+/u) .filter(Boolean); } diff --git a/lib/format.js b/lib/format.js index 47e7591..5e13277 100644 --- a/lib/format.js +++ b/lib/format.js @@ -12,7 +12,11 @@ export function formatAge(seconds) { if (m < 60) return `${m}m`; const h = Math.floor(m / 60); - return `${h}h ${m % 60}m`; + if (h < 24) + return `${h}h ${m % 60}m`; + // Days, or a session left over the weekend reads as "120h 0m" and widens + // the very row the chip cap exists to keep narrow. + return `${Math.floor(h / 24)}d ${h % 24}h`; } /** Last path segment, with ~ collapsed. Two worktrees of one repo share a diff --git a/lib/glyph.js b/lib/glyph.js index f31a838..c3f852d 100644 --- a/lib/glyph.js +++ b/lib/glyph.js @@ -1,8 +1,8 @@ // State glyphs, drawn with cairo alone. // -// The panel is monochrome, so shape is the only channel left and these four -// have to stay apart at 14 px. Kept free of St/Clutter/GNOME imports so the -// shapes can be rendered to a file and looked at, rather than guessed about. +// The panel is monochrome, so shape is the only channel left and the three +// states have to stay apart at 14 px. Kept free of St/Clutter/GNOME imports so +// the shapes can be rendered to a file and looked at, rather than guessed at. /** Draw `state` filling the given box, in the colour passed as {r,g,b,a} 0..1. */ export function drawState(cr, state, width, height, color) { @@ -21,7 +21,7 @@ export function drawState(cr, state, width, height, color) { switch (state) { case 'blocked': - // Disc inside a ring: the most ink of the four, for the only state + // Disc inside a ring: the most ink of the three, for the only state // where a session is stuck until you act. The inner disc has to be // big enough to register at 14 px, or this reads as plain "working". cr.arc(cx, cy, radius, 0, 2 * Math.PI); diff --git a/lib/indicator.js b/lib/indicator.js index 6929613..01f7bf0 100644 --- a/lib/indicator.js +++ b/lib/indicator.js @@ -38,8 +38,13 @@ function drawStateDot(area, state) { try { const [w, h] = area.get_surface_size(); const c = area.get_theme_node().get_foreground_color(); + // Normalised by inspection rather than by assumption: the colour struct + // behind this changed between shell versions, and a wrong guess either + // way paints the glyph invisible or fully saturated. + const scale = Math.max(c.red, c.green, c.blue, c.alpha) > 1 ? 255 : 1; drawState(cr, state, w, h, { - r: c.red / 255, g: c.green / 255, b: c.blue / 255, a: c.alpha / 255, + r: c.red / scale, g: c.green / scale, + b: c.blue / scale, a: c.alpha / scale, }); } finally { cr.$dispose(); @@ -63,6 +68,12 @@ class ClaudeStatusIndicator extends PanelMenu.Button { this._changedId = this._store.connect('changed', () => this._update()); this._settingsChangedId = this._settings.connect('changed', () => this._update()); + // Connected, not overridden: clutter_actor_destroy is not a vfunc, so a + // destroy() override only runs when JS calls it. An actor torn down any + // other way -- another extension rebuilding the panel boxes -- would + // leave the timer and the file monitor running against a disposed + // actor, screaming into the log every 20 seconds. + this.connect('destroy', () => this._onDestroy()); this._store.start(); } @@ -95,18 +106,24 @@ class ClaudeStatusIndicator extends PanelMenu.Button { style_class: 'ccs-dot', y_align: Clutter.ActorAlign.CENTER, }); - dot.set_width(14); - dot.set_height(14); + // Size comes from the stylesheet so St scales it: setting it here would + // pin the glyph to physical pixels and halve it on a HiDPI display. dot.connect('repaint', area => drawStateDot(area, session.state)); chip.add_child(dot); let age = null; if (this._settings.get_boolean('show-project-name')) { - chip.add_child(new St.Label({ + const text = new St.Label({ style_class: 'ccs-chip-label', y_align: Clutter.ActorAlign.CENTER, text: label, - })); + }); + // With shortening off the label is a whole project name, which can + // be arbitrarily long: an unbounded label in the panel pushes the + // clock aside and, past a point, hands Pango a width it cannot + // represent. + text.clutter_text.ellipsize = Pango.EllipsizeMode.END; + chip.add_child(text); } if (withAge) { age = new St.Label({ @@ -163,7 +180,10 @@ class ClaudeStatusIndicator extends PanelMenu.Button { this._chipLabels = assignChips( sessions.map(s => ({ sessionId: s.sessionId, - since: s.since, + // Session age, not time in the current state: seniority decides + // who keeps the clean label, and a session that changed state a + // second ago has not thereby become the youngest. + since: s.started || s.since, base: projectName(s.cwd), })), this._chipLabels); @@ -198,7 +218,7 @@ class ClaudeStatusIndicator extends PanelMenu.Button { const { chip, age } = this._buildChip( session, labelFor(session), ageOnFirst && i === 0); if (age) - this._ageLabel = { age, session }; + this._ageLabel = { age, sessionId: session.sessionId }; this._chipBox.add_child(chip); }); if (hidden > 0) { @@ -211,8 +231,14 @@ class ClaudeStatusIndicator extends PanelMenu.Button { this._chipSignature = signature; } - if (this._ageLabel) - this._ageLabel.age.text = formatAge(this._ageOf(this._ageLabel.session)); + if (this._ageLabel) { + // Looked up again rather than captured: a session that returns to + // the same state within one refresh keeps the signature unchanged, + // and a captured object would then show an age that stopped moving. + const current = sessions.find(s => s.sessionId === this._ageLabel.sessionId); + if (current) + this._ageLabel.age.text = formatAge(this._ageOf(current)); + } } _updateMenu(sessions) { @@ -229,8 +255,9 @@ class ClaudeStatusIndicator extends PanelMenu.Button { this._rebuildRows(sessions); this._rowSignature = signature; } + const byId = new Map(sessions.map(s => [s.sessionId, s])); for (const row of this._rows) { - const session = sessions.find(s => s.sessionId === row.sessionId); + const session = byId.get(row.sessionId); if (!session) continue; row.age.text = formatAge(this._ageOf(session)); @@ -288,6 +315,7 @@ class ClaudeStatusIndicator extends PanelMenu.Button { style_class: `ccs-row-title ccs-${session.state}`, x_expand: true, }); + title.clutter_text.ellipsize = Pango.EllipsizeMode.END; const age = new St.Label({ text: formatAge(this._ageOf(session)), style_class: 'ccs-row-age', @@ -368,11 +396,11 @@ class ClaudeStatusIndicator extends PanelMenu.Button { .catch(e => logError(e, 'claude-code-status: zellij refresh failed')); } - // ---- Visuals -------------------------------------------------------- - // ---- Teardown ------------------------------------------------------- - destroy() { + _onDestroy() { + if (this._destroyed) + return; this._destroyed = true; this._zellij.destroy(); if (this._changedId) { @@ -384,6 +412,5 @@ class ClaudeStatusIndicator extends PanelMenu.Button { this._settingsChangedId = 0; } this._store.destroy(); - super.destroy(); } }); diff --git a/lib/sessions.js b/lib/sessions.js index bb78e18..dc008ab 100644 --- a/lib/sessions.js +++ b/lib/sessions.js @@ -33,6 +33,17 @@ const UNKNOWN_PID_MAX_AGE = 36 * 3600; // seconds // lose that update. const TMP_MAX_AGE = 300; // seconds +// A state file is a few hundred bytes. Anything larger is corrupt or hostile, +// and reading it whole would happen inside the compositor: a symlink to +// /dev/zero took a test process past 4 GB in three seconds, which in +// gnome-shell is the session ending. The size is checked before the read. +const MAX_STATE_BYTES = 64 * 1024; + +// Work here is on the compositor's main loop, and every session costs a menu +// row of five actors. Well past any real use, and cheap insurance against a +// directory someone filled up. +const MAX_SESSIONS = 64; + export function stateRank(state) { const i = STATES.indexOf(state); return i < 0 ? STATES.length : i; @@ -134,16 +145,20 @@ export const SessionStore = GObject.registerClass({ let enumerator; try { enumerator = await this._dir.enumerate_children_async( - 'standard::name,time::modified', Gio.FileQueryInfoFlags.NONE, + 'standard::name,standard::size,time::modified', Gio.FileQueryInfoFlags.NONE, GLib.PRIORITY_DEFAULT, cancellable); } catch (e) { // No directory yet means no sessions have ever run; not an error. - if (e.matches?.(Gio.IOErrorEnum, Gio.IOErrorEnum.NOT_FOUND)) + // NOT_DIRECTORY means something took the path -- also not worth a + // stack trace every 20 seconds for as long as it stays that way. + if (e.matches?.(Gio.IOErrorEnum, Gio.IOErrorEnum.NOT_FOUND) || + e.matches?.(Gio.IOErrorEnum, Gio.IOErrorEnum.NOT_DIRECTORY)) return []; throw e; } const names = []; + const locks = []; for (;;) { const batch = await enumerator.next_files_async( 32, GLib.PRIORITY_DEFAULT, cancellable); @@ -153,15 +168,29 @@ export const SessionStore = GObject.registerClass({ const name = info.get_name(); // Only ".json" is state. ".lock" belongs to the hook, "debug" // is its opt-in event log, and ".tmp" is an interrupted write. - if (name.endsWith('.json')) + if (name.endsWith('.json')) { + if (info.get_size() > MAX_STATE_BYTES) { + // Not read at all: the point is to never allocate it. + continue; + } names.push(name); - else if (name.endsWith('.tmp')) - this._sweepTemp(info, name); + } else if (name.endsWith('.tmp')) { + this._sweepStale(info, name); + } else if (name.endsWith('.json.lock')) { + locks.push({ info, name }); + } } } + // A lock whose state file is gone belongs to nothing; the hook only + // removes the pair together, so nobody else would ever clear it. + for (const { info, name } of locks) { + if (!names.includes(name.slice(0, -'.lock'.length))) + this._sweepStale(info, name); + } + const sessions = []; - for (const name of names) { + for (const name of names.slice(0, MAX_SESSIONS)) { const session = await this._readOne(name, cancellable); if (session) sessions.push(session); @@ -169,8 +198,9 @@ export const SessionStore = GObject.registerClass({ return sessions; } - /** Delete an abandoned temporary file, once it is old enough to be sure. */ - _sweepTemp(info, name) { + /** Delete an abandoned file, once it is old enough to be sure nobody is + * part-way through writing it. */ + _sweepStale(info, name) { const modified = info.get_modification_date_time?.(); if (!modified) return; @@ -235,12 +265,10 @@ export const SessionStore = GObject.registerClass({ state, cwd: String(raw.cwd ?? ''), since: Number(raw.since) || 0, + started: Number(raw.started) || 0, pid, - message: String(raw.message ?? ''), agents: Math.max(0, Number(raw.agents) || 0), - notificationType: String(raw.notification_type ?? ''), zellijSession: String(raw.zellij_session ?? ''), - zellijPane: String(raw.zellij_pane ?? ''), }; } diff --git a/lib/zellij.js b/lib/zellij.js index 3bf0e39..2ef7258 100644 --- a/lib/zellij.js +++ b/lib/zellij.js @@ -20,12 +20,18 @@ Gio._promisify(Gio.Subprocess.prototype, 'communicate_utf8_async'); // does not spawn a zellij process every time it runs. const CACHE_TTL = 120; // seconds +// One subprocess per distinct zellij session named in the state directory. +// In real use that is one or two; the bound is there because the names come +// from files, and a directory full of them would fork a process per name. +const MAX_SESSIONS = 8; + export class ZellijTabs { constructor() { this._cache = new Map(); // zellij session -> { at, tabs: [{name, cwds}] } this._inFlight = new Map(); this._available = null; this._cancellable = new Gio.Cancellable(); + this._children = new Set(); } /** Tab name for a working directory, or null when unknown. */ @@ -58,7 +64,7 @@ export class ZellijTabs { return; const now = GLib.get_monotonic_time() / 1e6; const work = []; - for (const name of new Set(zellijSessions)) { + for (const name of [...new Set(zellijSessions)].slice(0, MAX_SESSIONS)) { if (!name) continue; const entry = this._cache.get(name); @@ -97,10 +103,15 @@ export class ZellijTabs { const proc = Gio.Subprocess.new( ['zellij', '--session', session, 'action', 'dump-layout'], Gio.SubprocessFlags.STDOUT_PIPE | Gio.SubprocessFlags.STDERR_SILENCE); - const [stdout] = await proc.communicate_utf8_async(null, this._cancellable); - if (!proc.get_successful()) - return null; - return stdout ?? ''; + this._children.add(proc); + try { + const [stdout] = await proc.communicate_utf8_async(null, this._cancellable); + if (!proc.get_successful()) + return null; + return stdout ?? ''; + } finally { + this._children.delete(proc); + } } catch (e) { // zellij not installed, or not on the shell's PATH: stop trying. if (e.matches?.(GLib.SpawnError, GLib.SpawnError.NOENT)) @@ -112,6 +123,11 @@ export class ZellijTabs { /** Abandon any layout dump still running; the extension is going away. */ destroy() { this._cancellable.cancel(); + // Cancelling only abandons the read; the child keeps running. A wedged + // zellij server would otherwise outlive the extension being disabled. + for (const proc of this._children) + proc.force_exit(); + this._children.clear(); this._cache.clear(); this._inFlight.clear(); } @@ -135,11 +151,23 @@ export function parseLayout(text) { tabs.push(current); continue; } + // A dump ends with new_tab_template and swap_tiled_layout blocks, whose + // own `tab` lines carry no name. Their panes belong to no tab at all; + // left attached to whatever came before, they make the last tab in the + // dump answer for every unmatched directory -- confidently and wrongly. + if (/^\s*(tab\s|tab\s*\{|new_tab_template|swap_tiled_layout|swap_floating_layout)/.test(line)) { + current = null; + continue; + } if (!current) continue; const paneMatch = line.match(/^\s*pane\s.*?\bcwd="([^"]*)"/); if (paneMatch) { const cwd = paneMatch[1]; + // A relative pane cwd is meaningless without the layout-level one; + // joining against "" yields a relative path that matches nothing. + if (!cwd.startsWith('/') && !base) + continue; const absolute = cwd.startsWith('/') ? cwd : GLib.build_filenamev([base, cwd]); diff --git a/prefs.js b/prefs.js index 2926abb..646f21c 100644 --- a/prefs.js +++ b/prefs.js @@ -85,6 +85,11 @@ export default class ClaudeCodeStatusPreferences extends ExtensionPreferences { _hooksStatus() { const path = GLib.build_filenamev([GLib.get_home_dir(), '.claude', 'settings.json']); + // Checked before reading: file_get_contents throws on a missing file, + // so without this the person who has installed nothing -- the one who + // most needs the instructions -- is told the file cannot be parsed. + if (!GLib.file_test(path, GLib.FileTest.EXISTS)) + return _('No ~/.claude/settings.json yet — run the install command below'); try { const [ok, bytes] = GLib.file_get_contents(path); if (!ok) @@ -95,7 +100,7 @@ export default class ClaudeCodeStatusPreferences extends ExtensionPreferences { (g.hooks ?? []).some(h => (h.command ?? '').includes('claude-status-hook.py')))) .map(([event]) => event); if (!events.length) - return _('Not installed — run the install command below, then restart your sessions'); + return _('Not installed — run the install command below'); return `${_('Installed for')}: ${events.join(', ')}`; } catch (e) { return _('~/.claude/settings.json could not be parsed'); diff --git a/tests/test-hook.sh b/tests/test-hook.sh index db11dc6..74a75e2 100755 --- a/tests/test-hook.sh +++ b/tests/test-hook.sh @@ -194,7 +194,10 @@ check "hook proceeds once the lock is released" "waiting" "$(field state)" # --- teardown -------------------------------------------------------------- emit "$(ev SessionEnd '"reason":"other"')" [ -e "$FILE" ]; check "SessionEnd removes the state file" "1" "$?" -[ -e "$FILE.lock" ]; check "SessionEnd removes the lock file" "1" "$?" +# The lock deliberately stays. Unlinking it while holding it would drop mutual +# exclusion for any hook already blocked on the old inode; the reader sweeps it +# once it is orphaned and old. +[ -e "$FILE.lock" ]; check "SessionEnd keeps the lock inode" "0" "$?" rm -rf "$XDG_STATE_HOME" if [ "$failures" -gt 0 ]; then diff --git a/tests/test-sessions.js b/tests/test-sessions.js index 1d1f49d..1d3689f 100644 --- a/tests/test-sessions.js +++ b/tests/test-sessions.js @@ -54,6 +54,14 @@ write('s-legacy', 'idle', '/home/u/proj-legacy', 5, livePid); // Survived a reboot: the pid exists again, but belongs to something else now. // Without an identity check this sits in the panel forever as a live session. write('s-ghost', 'waiting', '/home/u/proj-ghost', 99999, livePid, 1); +// Left by a session that ended: the hook keeps the lock inode deliberately, +// so nobody but the reader would ever clear it. +GLib.file_set_contents(GLib.build_filenamev([STATE, 's-gone.json.lock']), ''); +GLib.spawn_command_line_sync( + `touch -d '1 hour ago' ${GLib.build_filenamev([STATE, 's-gone.json.lock'])}`); +// A state file far larger than any real one must not be read at all. +GLib.file_set_contents(GLib.build_filenamev([STATE, 'huge.json']), + `{"session_id":"huge","state":"waiting","pid":1,"cwd":"${'x'.repeat(70000)}"}`); GLib.file_set_contents(GLib.build_filenamev([STATE, 'notes.txt']), 'ignored'); GLib.file_set_contents(GLib.build_filenamev([STATE, 'half.json']), '{"broken'); @@ -90,6 +98,10 @@ store.connect('changed', () => { s[3]?.sessionId === 's-legacy', s[3]?.sessionId); check('busy after waiting', s[4]?.sessionId === 's-busy', s[4]?.sessionId); + check('oversized state file not loaded', !s.some(x => x.sessionId === 'huge')); + check('orphaned lock swept', + !GLib.file_test(GLib.build_filenamev([STATE, 's-gone.json.lock']), GLib.FileTest.EXISTS)); + check('dead session file removed from disk', !GLib.file_test(GLib.build_filenamev([STATE, 's-dead.json']), GLib.FileTest.EXISTS));