diff --git a/README.md b/README.md index 9237692..d0ef0b7 100644 --- a/README.md +++ b/README.md @@ -197,13 +197,33 @@ dump-layout`: рабочий каталог сессии сопоставляе Если zellij не используется, выключите в настройках: он стоит одного процесса раз в пару минут. +## Аварийное завершение + +`kill`, закрытое окно, перезагрузка — `SessionEnd` не приходит, и файл остаётся. +Разбор завалов проверен на каждом случае отдельно: + +| Что осталось | Что с этим происходит | +|---|---| +| файл убитой сессии | процесса нет — файл и его `.lock` удаляются в пределах 20 с | +| файл из прошлой загрузки | pid сверяется по времени старта процесса, а не только по наличию | +| оборванная запись хука (`.tmp`) | удаляется, когда старше пяти минут | +| незакрытый `flock` | ядро снимает блокировку при смерти процесса — тупика не бывает | + +Сверка по времени старта — не перестраховка. Файлы состояния переживают +перезагрузку, а pid после неё раздаются заново: проверка «есть ли `/proc/`» +отвечает лишь «какой-то процесс с таким номером есть». Без этой сверки сессия, +погибшая в аварии, висела бы в панели вечно, требуя ответа, которого некому дать. +Проверено подстановкой постороннего живого процесса на тот же pid. + +Порог в пять минут для `.tmp` тоже осмысленный: хук может писать такой файл +прямо сейчас, и удаление свежего стоило бы потерянной записи. + +Если хук вообще не смог опознать процесс claude, он пишет pid 0 — «неизвестно», +что никогда не путается с «мёртв»; такие записи истекают по возрасту, через +36 часов. + ## Известные шероховатости -- **Протухшие сессии.** Убитый терминал не присылает `SessionEnd`. Живость - перепроверяется каждые 20 с по `/proc/`, файл удаляется — убитая сессия - исчезает в пределах этого окна, а не висит вечно. Если хук вообще не смог - опознать процесс claude, он записывает pid 0 — «неизвестно», что никогда не - путается с «мёртв», — и такие записи истекают по возрасту, через 36 часов. - **`Stop` не отличает «закончил» от «сдался».** Оба читаются как «ждёт ввода», и решение для вас в обоих случаях одно и то же. diff --git a/hooks/claude-status-hook.py b/hooks/claude-status-hook.py index 1f05261..b0ddd89 100755 --- a/hooks/claude-status-hook.py +++ b/hooks/claude-status-hook.py @@ -169,8 +169,32 @@ def find_claude_pid(): return 0 -def alive(pid): - return pid > 0 and os.path.exists("/proc/%d" % pid) +def pid_start_time(pid): + """Field 22 of /proc//stat: when the process started, in clock ticks. + + Pins a pid to one particular process. Pids are reused, and state files + outlive reboots -- without this, a file left by a crashed session whose pid + is later handed to something unrelated reads as a live session forever. + """ + try: + with open("/proc/%d/stat" % pid) as fh: + data = fh.read() + except OSError: + return 0 + # Field 2 is the command name, parenthesised, and may itself contain spaces + # and a ')'. Everything after the last ')' is field 3 onwards. + tail = data[data.rfind(")") + 2:].split() + try: + return int(tail[19]) + except (IndexError, ValueError): + return 0 + + +def alive(pid, start=0): + 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. + return not start or pid_start_time(pid) == start def sweep_dead(keep): @@ -190,12 +214,13 @@ def sweep_dead(keep): path = os.path.join(STATE_DIR, name) try: with open(path, "r") as fh: - pid = json.load(fh).get("pid", 0) + stale = json.load(fh) + pid = stale.get("pid", 0) except (OSError, ValueError, AttributeError): 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): + if pid and not alive(pid, stale.get("pid_start", 0)): for victim in (path, path + ".lock"): try: os.unlink(victim) @@ -300,6 +325,7 @@ def apply_event(event, state, path, now): "event_ts": now, # 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, diff --git a/lib/sessions.js b/lib/sessions.js index 2157b86..bb78e18 100644 --- a/lib/sessions.js +++ b/lib/sessions.js @@ -27,6 +27,12 @@ const LIVENESS_INTERVAL = 20; // seconds // morning, which is exactly the case this indicator exists for. const UNKNOWN_PID_MAX_AGE = 36 * 3600; // seconds +// A hook killed between writing its temporary file and renaming it leaves the +// temporary behind. Old ones are swept; recent ones are left alone, because a +// hook may be part-way through writing one right now and deleting it would +// lose that update. +const TMP_MAX_AGE = 300; // seconds + export function stateRank(state) { const i = STATES.indexOf(state); return i < 0 ? STATES.length : i; @@ -128,7 +134,7 @@ export const SessionStore = GObject.registerClass({ let enumerator; try { enumerator = await this._dir.enumerate_children_async( - 'standard::name', Gio.FileQueryInfoFlags.NONE, + 'standard::name,time::modified', Gio.FileQueryInfoFlags.NONE, GLib.PRIORITY_DEFAULT, cancellable); } catch (e) { // No directory yet means no sessions have ever run; not an error. @@ -145,10 +151,12 @@ export const SessionStore = GObject.registerClass({ break; for (const info of batch) { const name = info.get_name(); - // ".tmp" files are half-written state; "debug" is the hook's - // opt-in event log and is not a session. + // 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')) names.push(name); + else if (name.endsWith('.tmp')) + this._sweepTemp(info, name); } } @@ -161,6 +169,23 @@ export const SessionStore = GObject.registerClass({ return sessions; } + /** Delete an abandoned temporary file, once it is old enough to be sure. */ + _sweepTemp(info, name) { + const modified = info.get_modification_date_time?.(); + if (!modified) + return; + const age = GLib.DateTime.new_now_local().difference(modified) / 1e6; + if (age < TMP_MAX_AGE) + return; + this._dir.get_child(name).delete_async(GLib.PRIORITY_LOW, null, (obj, res) => { + try { + obj.delete_finish(res); + } catch (e) { + // Gone already, or not ours to remove. + } + }); + } + async _readOne(name, cancellable) { const file = this._dir.get_child(name); let raw; @@ -176,12 +201,13 @@ export const SessionStore = GObject.registerClass({ return null; const pid = Number(raw.pid) || 0; + const pidStart = Number(raw.pid_start) || 0; const eventTs = Number(raw.event_ts) || 0; const age = GLib.get_real_time() / 1e6 - eventTs; // pid 0 is "the hook could not tell", not "dead": treating it as dead // would hide a perfectly live session, so those fall back to an age // cutoff instead. - const gone = pid > 0 ? !isAlive(pid) : age > UNKNOWN_PID_MAX_AGE; + const gone = pid > 0 ? !isAlive(pid, pidStart) : age > UNKNOWN_PID_MAX_AGE; if (gone) { // The terminal was killed without a SessionEnd hook. Removing the // file here (rather than only hiding it) keeps the directory from @@ -234,6 +260,33 @@ export const SessionStore = GObject.registerClass({ } }); -function isAlive(pid) { - return pid > 0 && GLib.file_test(`/proc/${pid}`, GLib.FileTest.EXISTS); +/** Is this pid still the process the hook recorded? + * + * Existence alone is not enough. State files outlive reboots, and a pid from a + * previous boot is very likely to belong to something else now -- a session + * that died in a crash would otherwise sit in the panel forever, waiting for + * an answer nobody can give. The start time pins the pid to one process. + */ +function isAlive(pid, startTime) { + if (pid <= 0 || !GLib.file_test(`/proc/${pid}`, GLib.FileTest.EXISTS)) + return false; + // Files written before start times were recorded have nothing to compare. + if (!startTime) + return true; + return readStartTime(pid) === startTime; +} + +function readStartTime(pid) { + try { + const [ok, bytes] = GLib.file_get_contents(`/proc/${pid}/stat`); + if (!ok) + return 0; + const data = new TextDecoder().decode(bytes); + // The command name is parenthesised and may contain spaces and ')', + // so fields are counted from after the last one. + const tail = data.slice(data.lastIndexOf(')') + 2).split(' '); + return Number(tail[19]) || 0; + } catch (e) { + return 0; + } } diff --git a/tests/test-hook.sh b/tests/test-hook.sh index 9573e13..7af2115 100755 --- a/tests/test-hook.sh +++ b/tests/test-hook.sh @@ -36,6 +36,11 @@ ev() { # event [extra json] emit "$(ev SessionStart '"source":"startup"')" check "SessionStart -> waiting" "waiting" "$(field state)" +# Pins the recorded pid to one process, so a state file that outlives a reboot +# cannot be revived by whatever inherits that pid number next. +start=$(field pid_start) +check "process start time recorded" "yes" "$([ -n "$start" ] && [ "$start" != "0" ] && echo yes || echo no)" + emit "$(ev UserPromptSubmit '"prompt":"hi"')" check "UserPromptSubmit -> busy" "busy" "$(field state)" diff --git a/tests/test-sessions.js b/tests/test-sessions.js index c116a68..1d1f49d 100644 --- a/tests/test-sessions.js +++ b/tests/test-sessions.js @@ -27,11 +27,12 @@ function check(name, condition, detail = '') { } const now = GLib.get_real_time() / 1e6; -function write(id, state, cwd, agoSeconds, pid) { +function write(id, state, cwd, agoSeconds, pid, pidStart = 0) { const payload = { session_id: id, state, cwd, since: now - agoSeconds, - event_ts: now, pid, event: 'test', notification_type: '', - message: '', zellij_session: 'ztest', zellij_pane: '1', transcript: '', + event_ts: now, pid, pid_start: pidStart, event: 'test', + notification_type: '', message: '', zellij_session: 'ztest', + zellij_pane: '1', transcript: '', }; GLib.file_set_contents( GLib.build_filenamev([STATE, `${id}.json`]), JSON.stringify(payload)); @@ -50,6 +51,9 @@ write('s-dead', 'waiting', '/home/u/proj-dead', 5, deadPid); // Written by the older hook, which had a fourth state. Files like this // survive an upgrade in a session that was already open. 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); GLib.file_set_contents(GLib.build_filenamev([STATE, 'notes.txt']), 'ignored'); GLib.file_set_contents(GLib.build_filenamev([STATE, 'half.json']), '{"broken'); @@ -67,6 +71,10 @@ store.connect('changed', () => { stateRank('waiting') < stateRank('busy')); check('dead session dropped', !s.some(x => x.sessionId === 's-dead')); + check('reused pid from a previous boot dropped', + !s.some(x => x.sessionId === 's-ghost')); + check('and its file removed', + !GLib.file_test(GLib.build_filenamev([STATE, 's-ghost.json']), GLib.FileTest.EXISTS)); check('truncated file skipped, others survive', s.length === 5, `got ${s.length}: ${s.map(x => x.sessionId).join(',')}`); check('non-json ignored', !s.some(x => x.sessionId.includes('notes')));