diff --git a/docs/WORKTREES.md b/docs/WORKTREES.md index c1c5bae1..d8038225 100644 --- a/docs/WORKTREES.md +++ b/docs/WORKTREES.md @@ -105,6 +105,19 @@ It is **read-only**: a roster, not a channel. It never writes a registry file an session. Note the corollary of having no heartbeat anywhere on this host: a `DEAD`/`STALE` verdict is a hint for a human, and must never by itself authorise a destructive action such as reclaiming a claim. +The fence itself lives in [../scripts/coord/session-registry.ps1](../scripts/coord/session-registry.ps1) +and is shared with `sessions.ps1` — one copy on purpose, because two copies of a safety check drift and +the one that drifts is the one nobody is testing. + +**`sessions.ps1 -Rehome` no longer trusts transcript mtime alone.** Moving a transcript out from under a +running session corrupts it *and* relocates a live session — the exact injury `sessions.ps1` exists to +repair. Its old guard used file mtime, which is **not** a liveness signal here: subagent and workflow +output is filed under `/subagents/`, so a session running a long workflow barely touches its +own transcript. Measured on this host: a verifiably-live session sat **32 minutes** idle by mtime — three +times the 10-minute `-MinIdleMinutes` default — while its process was alive and fenced. The guard now +consults the registry *and* mtime, and **refuses if either says live**, because nothing here can prove a +session is gone; only the positive answer is trustworthy. + **Creating a worktree is serialised.** `git worktree add -b ` writes `.git/config`, so two sessions creating worktrees at once race `.git/config.lock` — on Windows that surfaces as `could not lock config file .git/config: File exists`, leaving orphaned branches behind. `new.ps1` wraps that call diff --git a/scripts/coord/presence.ps1 b/scripts/coord/presence.ps1 index c6cb20c3..45795b9f 100644 --- a/scripts/coord/presence.ps1 +++ b/scripts/coord/presence.ps1 @@ -91,17 +91,10 @@ function ConvertTo-Norm([string]$p) { return ($p -replace '\\', '/').TrimEnd('/').ToLowerInvariant() } -# --- Config roots ------------------------------------------------------------------------------- -# Discovered, not hardcoded: several logins can coexist (~\.claude for Desktop, ~\.claude-account-N for -# CLI/VS Code subscriptions) and a session is only ever visible to the login that owns it. -function Get-ConfigRoots { - if ($ConfigRoot) { return @($ConfigRoot | Where-Object { Test-Path $_ }) } - return @( - Get-ChildItem -Path $env:USERPROFILE -Directory -Filter ".claude*" -Force -EA SilentlyContinue | - Where-Object { Test-Path (Join-Path $_.FullName "sessions") } | - ForEach-Object { $_.FullName } - ) -} +# --- Registry access + the liveness fence ------------------------------------------------------- +# Shared with sessions.ps1, which needs the SAME answer to "is this session alive" before it moves a +# transcript. Two copies of a safety check drift, and the copy that drifts is the one nobody tests. +. "$PSScriptRoot\session-registry.ps1" function Get-LoginLabel([string]$RootPath) { $leaf = Split-Path $RootPath -Leaf @@ -120,38 +113,6 @@ function Get-SurfaceLabel([string]$Entrypoint) { } } -# --- The fence ---------------------------------------------------------------------------------- -# LIVE requires the pid to resolve AND the process start time to be consistent with the recorded -# session start. Without the second half this is just a pid check, and a reused pid reads as alive. -function Get-Liveness([int]$ProcId, [object]$StartedAtMs, [int]$SkewMinutes) { - if (-not $ProcId) { return @{ State = "DEAD"; Detail = "no pid in record" } } - $proc = Get-Process -Id $ProcId -EA SilentlyContinue - if (-not $proc) { return @{ State = "DEAD"; Detail = "pid $ProcId not running" } } - - $procStart = $null - try { $procStart = $proc.StartTime } catch { } - if (-not $procStart) { - # Access can be denied for a process owned by another context. Report the uncertainty rather - # than upgrading it to LIVE -- an unverifiable fence is not a passed fence. - return @{ State = "UNVERIFIED"; Detail = "pid $ProcId alive; start time unreadable" } - } - if ($null -eq $StartedAtMs) { - return @{ State = "UNVERIFIED"; Detail = "pid $ProcId alive; record has no startedAt" } - } - - $registered = [DateTimeOffset]::FromUnixTimeMilliseconds([int64]$StartedAtMs).LocalDateTime - # A process cannot have started after the session it hosts registered (allow a small forward slop - # for clock jitter). Started much later => this pid was recycled onto a different process. - $delta = ($procStart - $registered).TotalMinutes - if ($delta -gt 1) { - return @{ State = "STALE"; Detail = "pid $ProcId reused (process started $([int]$delta)m after the session)" } - } - if ($delta -lt (-1 * $SkewMinutes)) { - return @{ State = "STALE"; Detail = "pid $ProcId start precedes the session by $([int](-$delta))m" } - } - return @{ State = "LIVE"; Detail = "" } -} - # --- Which of these sessions is the caller ------------------------------------------------------ # NOT $PID: this script runs as a pwsh child (often a grandchild, via a hook), so its own pid never # appears in the registry. The session that invoked us is an ANCESTOR, so walk the parent chain and @@ -194,10 +155,9 @@ $primaryNorm = ConvertTo-Norm $worktrees[0].Path $selfPids = Get-SelfPids $SelfPid $rows = @() -foreach ($root in (Get-ConfigRoots)) { - $sessDir = Join-Path $root "sessions" - foreach ($f in @(Get-ChildItem $sessDir -Filter *.json -EA SilentlyContinue)) { - try { $rec = Get-Content $f.FullName -Raw -EA Stop | ConvertFrom-Json -EA Stop } catch { continue } +foreach ($entry in (Get-SessionRecords -ConfigRoot $ConfigRoot)) { + $rec = $entry.Record + $root = $entry.Root if (-not $rec.cwd) { continue } # Scope: cwd inside one of this repo's worktrees. Exact match on the worktree root, or a @@ -211,7 +171,7 @@ foreach ($root in (Get-ConfigRoots)) { } if (-not $match) { continue } - $live = Get-Liveness ([int]$rec.pid) $rec.startedAt $StartSkewMinutes + $live = Test-RecordLiveness -Record $rec -StartSkewMinutes $StartSkewMinutes $matchNorm = ConvertTo-Norm $match.Path $rows += [pscustomobject]@{ State = $live.State @@ -229,7 +189,6 @@ foreach ($root in (Get-ConfigRoots)) { IsSelf = ($selfPids -contains [int]$rec.pid) StartedAt = if ($null -ne $rec.startedAt) { [DateTimeOffset]::FromUnixTimeMilliseconds([int64]$rec.startedAt).LocalDateTime.ToString("o") } else { "" } } - } } $order = @{ "LIVE" = 0; "UNVERIFIED" = 1; "STALE" = 2; "DEAD" = 3 } diff --git a/scripts/coord/session-registry.ps1 b/scripts/coord/session-registry.ps1 new file mode 100644 index 00000000..031baf1f --- /dev/null +++ b/scripts/coord/session-registry.ps1 @@ -0,0 +1,136 @@ +<# +.SYNOPSIS + Read the Claude Code session registry, and decide whether a session is actually alive. + +.DESCRIPTION + Dot-source this; it defines functions and does nothing on its own. + + . "$PSScriptRoot\session-registry.ps1" + $l = Get-SessionLiveness -SessionId "1234abcd-..." + if ($l.State -eq "LIVE") { ... } + + ONE COPY OF THE FENCE, ON PURPOSE. Both presence.ps1 (a roster) and sessions.ps1 (which MOVES a + transcript, and must not do that under a running writer) need the same answer to "is this session + alive". Two copies of a safety check drift, and the copy that drifts is the one nobody is testing. + + `/sessions/.json` is the only registry containing EVERY surface -- the Desktop + app's own session tooling enumerates just the sessions it spawned, so a VS Code session is absent + from it entirely. Config roots are discovered dynamically because several logins can coexist + (`~\.claude` plus any `~\.claude-account-N`) and a session is only visible to the login that owns it. + + WHY THIS IS NOT A PID CHECK. Pids get reused, and these records outlive their process. Claude Code + ships a `procStart` field for exactly this fence, but it serialises as absent in practice and its + guard returns true when it cannot tell -- i.e. it fails OPEN toward "still alive". So we read the + process start time ourselves and require it to be consistent with the recorded session start: a + process that started AFTER the session registered is a recycled pid, not that session. + + WHAT THE ANSWERS MEAN, AND WHAT THEY LICENSE: + LIVE pid resolves and its start time is consistent. Trustworthy. + UNVERIFIED pid resolves; the fence could not be evaluated. Treat as possibly-live. + STALE pid resolves but belongs to a different process. The session is gone. + DEAD no such pid. + (Found=$false) no record at all -- it exited cleanly, or was never registered. + + ONLY THE POSITIVE ANSWER IS SAFE TO ACT ON. There is no heartbeat anywhere on this host and + registry writes are event-driven, so nothing here can PROVE a session is gone -- only that it is + present. A DEAD/STALE/not-found verdict must never by itself authorise a destructive action; + combine it with an independent signal and let either one veto. +#> + +# Every config root that actually holds a session registry. +function Get-ClaudeConfigRoots { + [CmdletBinding()] + param([string[]]$ConfigRoot) + if ($ConfigRoot) { return @($ConfigRoot | Where-Object { Test-Path $_ }) } + return @( + Get-ChildItem -Path $env:USERPROFILE -Directory -Filter ".claude*" -Force -EA SilentlyContinue | + Where-Object { Test-Path (Join-Path $_.FullName "sessions") } | + ForEach-Object { $_.FullName } + ) +} + +# Every registry record, with the root it came from attached. +function Get-SessionRecords { + [CmdletBinding()] + param([string[]]$ConfigRoot) + $out = @() + foreach ($root in (Get-ClaudeConfigRoots -ConfigRoot $ConfigRoot)) { + foreach ($f in @(Get-ChildItem (Join-Path $root "sessions") -Filter *.json -EA SilentlyContinue)) { + # A single malformed record must never take down a caller -- one of these runs in a + # SessionStart hook, where a throw replaces the chat's whole starting context. + try { $rec = Get-Content $f.FullName -Raw -EA Stop | ConvertFrom-Json -EA Stop } catch { continue } + if (-not $rec) { continue } + $out += [pscustomobject]@{ Record = $rec; Root = $root; File = $f.FullName } + } + } + return $out +} + +# The fence. See the header for why this is not just "is the pid alive". +function Test-RecordLiveness { + [CmdletBinding()] + param( + [Parameter(Mandatory)][AllowNull()][object]$Record, + # How far a process may have started BEFORE its session registered and still be the same run. + # Generous: registration follows process start, but a cold start on a loaded box can lag. + [int]$StartSkewMinutes = 15 + ) + if (-not $Record) { return @{ State = "DEAD"; Detail = "no record" } } + $procId = [int]$Record.pid + if (-not $procId) { return @{ State = "DEAD"; Detail = "no pid in record" } } + + $proc = Get-Process -Id $procId -EA SilentlyContinue + if (-not $proc) { return @{ State = "DEAD"; Detail = "pid $procId not running" } } + + $procStart = $null + try { $procStart = $proc.StartTime } catch { } + if (-not $procStart) { + # Access can be denied for a process in another context. Report the uncertainty rather than + # upgrading it to LIVE: an unverifiable fence is not a passed fence. + return @{ State = "UNVERIFIED"; Detail = "pid $procId alive; start time unreadable" } + } + if ($null -eq $Record.startedAt) { + return @{ State = "UNVERIFIED"; Detail = "pid $procId alive; record has no startedAt" } + } + + $registered = [DateTimeOffset]::FromUnixTimeMilliseconds([int64]$Record.startedAt).LocalDateTime + # A process cannot have started after the session it hosts registered (small forward slop for + # clock jitter). Started much later => this pid was recycled onto a different process. + $delta = ($procStart - $registered).TotalMinutes + if ($delta -gt 1) { + return @{ State = "STALE"; Detail = "pid $procId reused (process started $([int]$delta)m after the session)" } + } + if ($delta -lt (-1 * $StartSkewMinutes)) { + return @{ State = "STALE"; Detail = "pid $procId start precedes the session by $([int](-$delta))m" } + } + return @{ State = "LIVE"; Detail = "" } +} + +# Look one session up by id (full or unique prefix) and fence it. +function Get-SessionLiveness { + [CmdletBinding()] + param( + [Parameter(Mandatory)][string]$SessionId, + [string[]]$ConfigRoot, + [int]$StartSkewMinutes = 15 + ) + $hit = @(Get-SessionRecords -ConfigRoot $ConfigRoot | + Where-Object { $_.Record.sessionId -and ([string]$_.Record.sessionId).StartsWith($SessionId, 'OrdinalIgnoreCase') }) + + if ($hit.Count -eq 0) { + # Not registered. NOT proof it is gone -- a session that never registered looks identical to + # one that exited cleanly, so callers must fall back to an independent signal. + return @{ Found = $false; State = "UNKNOWN"; Detail = "no registry record"; Record = $null } + } + # More than one match on a prefix: fence them all and report the most-alive, because the caller is + # about to decide whether it is safe to disturb something. + $rank = @{ "LIVE" = 0; "UNVERIFIED" = 1; "STALE" = 2; "DEAD" = 3 } + $best = $null + foreach ($h in $hit) { + $l = Test-RecordLiveness -Record $h.Record -StartSkewMinutes $StartSkewMinutes + if (-not $best -or $rank[$l.State] -lt $rank[$best.State]) { + $best = @{ Found = $true; State = $l.State; Detail = $l.Detail; Record = $h.Record } + } + } + return $best +} diff --git a/scripts/worktree/sessions.ps1 b/scripts/worktree/sessions.ps1 index b633c95f..1db5adeb 100644 --- a/scripts/worktree/sessions.ps1 +++ b/scripts/worktree/sessions.ps1 @@ -229,9 +229,34 @@ if ($Rehome) { if ($hit.Count -gt 1) { throw "'$Rehome' matches $($hit.Count) sessions. Use a longer id prefix." } $s = $hit[0] - # Moving a transcript out from under a RUNNING session's writer corrupts it. Recent writes are the only - # signal we have that the session is live, so treat them as one. Under -WhatIf we only PREVIEW, so the - # liveness guard is relaxed there -- nothing is moved, and you still see what the move would do. + # Moving a transcript out from under a RUNNING session's writer corrupts it -- and relocating a live + # session is precisely the injury this script exists to REPAIR, so a false "it's idle" here is the + # worst failure it can have. Two INDEPENDENT signals, and EITHER may veto. Under -WhatIf we only + # PREVIEW, so both are relaxed there -- nothing is moved, and you still see what the move would do. + # + # (1) The session registry, fenced on pid + process start time (scripts/coord/session-registry.ps1). + # This is the authoritative POSITIVE signal, and adding it is the point of this guard: transcript + # mtime is NOT liveness. Subagent and workflow output is written under /subagents/, so + # a session running a long workflow barely touches its own transcript. Measured on this host: a + # verifiably-live session sat 32 minutes idle by mtime while its process was alive and fenced -- + # three times -MinIdleMinutes, i.e. the old guard would have waved the move straight through. + # + # (2) Transcript mtime, the original signal, KEPT rather than replaced. A session that exits cleanly + # unlinks its registry file, so "no record" is indistinguishable from "never registered" and + # cannot stand on its own either. + # + # Refusing when EITHER says live is deliberate: nothing on this host can PROVE a session is gone + # (no heartbeat, and registry writes are event-driven), so only the positive answer is trustworthy. + # A negative from one signal must never by itself authorise a destructive move. + . "$PSScriptRoot\..\coord\session-registry.ps1" + $reg = Get-SessionLiveness -SessionId $s.Id + if ($reg.State -in @("LIVE", "UNVERIFIED") -and -not $Force -and -not $WhatIfPreference) { + throw ("Session $($s.Id) is $($reg.State) in the session registry (pid $($reg.Record.pid)" + + "$(if ($reg.Detail) { "; $($reg.Detail)" })) -- it is still RUNNING and moving its " + + "transcript would corrupt it. Its transcript may look idle: subagent output is filed " + + "elsewhere, so mtime is not liveness. Close that window, then retry (or -Force).") + } + $idle = (Get-Date) - $s.Last if ($idle.TotalMinutes -lt $MinIdleMinutes -and -not $Force -and -not $WhatIfPreference) { throw ("Session $($s.Id) was written $([int]$idle.TotalMinutes) min ago and may still be RUNNING; " + diff --git a/tests/test_session_registry.py b/tests/test_session_registry.py new file mode 100644 index 00000000..e21587e9 --- /dev/null +++ b/tests/test_session_registry.py @@ -0,0 +1,168 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""Tests for the shared session-liveness fence (``scripts/coord/session-registry.ps1``). + +This module exists so ``presence.ps1`` (a read-only roster) and ``sessions.ps1`` (which MOVES a +transcript) give the SAME answer to "is this session alive". Two copies of a safety check drift, and +the copy that drifts is the one nobody is testing. + +The contract these tests pin, in order of how much damage getting it wrong causes: + +1. **A live session is reported LIVE even when nothing has written its transcript for hours.** + Transcript mtime is not liveness -- subagent and workflow output is filed under + ``/subagents/`` -- and ``sessions.ps1`` used mtime alone to decide whether it was safe + to move a transcript out from under a running writer. +2. **A recycled pid is not LIVE.** The fence compares process start time against the recorded session + start; without it this is a bare pid check and a reused pid reads as alive. +3. **"Not registered" is UNKNOWN, never "dead".** A cleanly-exited session and a never-registered one + are indistinguishable here, so the answer must not license a destructive action by itself. +""" + +from __future__ import annotations + +import json +import os +import shutil +import subprocess +import time +from pathlib import Path +from typing import Any + +import pytest + +REGISTRY = Path(__file__).resolve().parents[1] / "scripts" / "coord" / "session-registry.ps1" + +pytestmark = pytest.mark.skipif( + shutil.which("pwsh") is None or os.name != "nt", + reason="session-registry.ps1 needs pwsh on Windows (Process.StartTime)", +) + +NOW_MS = int(time.time() * 1000) + + +@pytest.fixture +def config_root(tmp_path: Path) -> Path: + root = tmp_path / ".claude" + (root / "sessions").mkdir(parents=True) + return root + + +def write_session( + config_root: Path, *, pid: int, session_id: str, started_at: int | None = None, **extra: Any +) -> None: + rec = { + "pid": pid, + "sessionId": session_id, + "cwd": str(config_root), + "startedAt": NOW_MS if started_at is None else started_at, + "kind": "interactive", + "entrypoint": "claude-desktop", + **extra, + } + (config_root / "sessions" / f"{pid}.json").write_text(json.dumps(rec), encoding="utf-8") + + +def liveness(config_root: Path, session_id: str) -> dict[str, Any]: + """Call the real Get-SessionLiveness and return its result as a dict.""" + script = ( + f". '{REGISTRY}'; " + f"$r = Get-SessionLiveness -SessionId '{session_id}' -ConfigRoot '{config_root}'; " + f"[pscustomobject]@{{Found=$r.Found;State=$r.State;Detail=$r.Detail}} | ConvertTo-Json -Compress" + ) + proc = subprocess.run( + ["pwsh", "-NoProfile", "-NonInteractive", "-Command", script], + capture_output=True, + text=True, + timeout=120, + check=False, + ) + assert proc.returncode == 0, proc.stderr + parsed: dict[str, Any] = json.loads(proc.stdout.strip()) + return parsed + + +def test_live_session_is_live_regardless_of_any_transcript_activity(config_root: Path) -> None: + """THE case that motivated this module. + + On this host a verifiably-live session was measured 32 minutes idle by transcript mtime, because + its workflow output went to a subagent directory. ``sessions.ps1`` would have moved its transcript + out from under it. The registry fence must say LIVE with no reference to file activity at all. + """ + write_session(config_root, pid=os.getpid(), session_id="aaaaaaaa-1111") + got = liveness(config_root, "aaaaaaaa") + assert got["Found"] is True + assert got["State"] == "LIVE" + + +def test_recycled_pid_is_not_live(config_root: Path) -> None: + """A real running pid recorded against a much older session start = a reused pid.""" + write_session( + config_root, + pid=os.getpid(), + session_id="bbbbbbbb-2222", + started_at=NOW_MS - (48 * 60 * 60 * 1000), + ) + got = liveness(config_root, "bbbbbbbb") + assert got["State"] == "STALE" + assert "reused" in got["Detail"] + + +def test_dead_pid_is_dead(config_root: Path) -> None: + write_session(config_root, pid=_find_free_pid(), session_id="cccccccc-3333") + assert liveness(config_root, "cccccccc")["State"] == "DEAD" + + +def test_unregistered_session_is_unknown_not_dead(config_root: Path) -> None: + """A cleanly-exited session leaves no record -- and neither does one that never registered. + + Reporting that as "dead" would let a caller destroy a live session's state on an absence. + """ + got = liveness(config_root, "dddddddd") + assert got["Found"] is False + assert got["State"] == "UNKNOWN" + assert got["State"] != "DEAD" + + +def test_record_without_startedat_is_unverified_not_live(config_root: Path) -> None: + """The fence cannot be evaluated, so it has not been passed. Must not upgrade to LIVE.""" + rec = { + "pid": os.getpid(), + "sessionId": "eeeeeeee-5555", + "cwd": str(config_root), + "kind": "interactive", + "entrypoint": "claude-vscode", + } + (config_root / "sessions" / "99001.json").write_text(json.dumps(rec), encoding="utf-8") + got = liveness(config_root, "eeeeeeee") + assert got["State"] == "UNVERIFIED" + + +def test_vscode_sessions_are_visible_to_the_fence(config_root: Path) -> None: + """The registry is the only source that has them; the Desktop app's tooling does not.""" + write_session( + config_root, pid=os.getpid(), session_id="ffffffff-6666", entrypoint="claude-vscode" + ) + assert liveness(config_root, "ffffffff")["State"] == "LIVE" + + +def test_malformed_record_does_not_break_the_lookup(config_root: Path) -> None: + (config_root / "sessions" / "99002.json").write_text("{not json", encoding="utf-8") + write_session(config_root, pid=os.getpid(), session_id="99999999-7777") + assert liveness(config_root, "99999999")["State"] == "LIVE" + + +def test_prefix_match_reports_the_most_alive_candidate(config_root: Path) -> None: + """Deciding whether it is safe to disturb something: an ambiguous prefix must not resolve to the + dead one and green-light the move.""" + write_session(config_root, pid=_find_free_pid(), session_id="7777abcd-8888") + write_session(config_root, pid=os.getpid(), session_id="7777efgh-9999") + assert liveness(config_root, "7777")["State"] == "LIVE" + + +def _find_free_pid() -> int: + proc = subprocess.Popen( + ["cmd", "/c", "exit"], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL + ) + proc.wait(timeout=30) + time.sleep(0.3) + return proc.pid