Repin: fix the pane root, take the transcript scan off the event loop - #35
Merged
Conversation
Two changes to the claude re-pin watcher, the second only effective
because of the first.
1. paneRootPid() still asked tmux for #{pane_pid}. Replacing tmux with a
server-held libghostty grid removed both identifiers that call used
(tmuxName, execFileSync) but left the call itself, so it threw a
ReferenceError into its own bare catch on every tick and returned
null. A null pane root makes pidTrusted false for every breadcrumb,
so the SessionStart hook path added in #23 was silently dead — the
only breadcrumb outcome in the Space logs was "breadcrumb rejected
(pid not in pane)", and /clear in a shared folder was still never
followed. The server owns the PTY now, so the pane root is a property
read with no subprocess per tick, and it carries the same semantic as
tmux's pane_pid.
2. With breadcrumbs working, the transcript scan is a backstop rather
than the mechanism, so it steps down from REPIN_MS to once every 10
minutes for any pane whose hook has proven itself. The scan is a
readdirSync per project dir plus a statSync per transcript, and
CLAUDE_CONFIG_DIR is on the FUSE bucket: measured on the Space, ~3ms
warm and 1.1-1.3s cold, synchronously blocking the one event loop
that also carries every session's PTY. At the 20s beat, once per live
pane, that was a terminal freeze every ~20 seconds.
A pane with no working hook keeps the old cadence, so nothing regresses
where the scan is still the only mechanism.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The scan's cost was never the I/O, it was doing the I/O on the event loop. claudeTranscriptsSince, transcriptHead, transcriptExists and claudeCandidate now await instead of using the *Sync calls, and the watcher tick is async. Measured against the real bucket directory, same code path either way: sync wall 1111-1260ms, ALL of it blocking the event loop async wall 466ms cold / 5-8ms warm, 1ms of block at worst So the cadence no longer has to trade staleness against freezing, and SCAN_BACKSTOP_MS drops from 10 minutes to one. It now only limits how often we walk the bucket for an answer the breadcrumb already gave, and it doubles as the longest a pin stays stale if a crumb is ever lost. Rearming moves out of tick into one place. An awaited tick that throws surfaces as a rejected promise rather than an exception in a setTimeout callback, so `run` catches, logs, and rearms: dropping the watcher would silently stop following /clear for the rest of the pane's life. Exactly one rearm per tick, so a failure can neither kill the watcher nor arm two timers. The scan also re-reads the pin after awaiting rather than trusting the value read at the top of the tick. Left sync: firstLine, on the codex rollout path, which reads CODEX_HOME on local disk rather than the bucket. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The awaited tick opens a window the synchronous one didn't have. `rearm` sets
`claudeCapturing` unconditionally, and `scheduleClaudeCapture` only clears a
PENDING beat — so if the pane exits and is relaunched while a scan is in flight,
the old tick resolves afterwards and overwrites the new watch's timer:
old tick awaits claudeCandidate
-> term.onExit: hosts.delete
-> ensureRunning: hosts.set, scheduleClaudeCapture, rearm -> T_new
-> old tick resolves, returns true (it never re-checks isRunning past the
await), old rearm -> T_old overwrites T_new
Both chains then beat forever and only T_old is reachable by clearTimeout. The
pane walks the bucket twice a beat — the cost this branch exists to remove — and
the orphan carries its own hookProven/lastScanAt, so it never steps down to the
backstop, plus the pre-relaunch `since` the comment at the top of the function
keeps fresh on purpose.
Guard the rearm on host identity, the same way the grid and trace-history timers
already do. Not unit-tested for the reason the `run` wrapper isn't: it needs a
driven watcher loop, not a pure function. Reproduced against a model of this
exact tick/run/rearm structure (two generations survive; with the guard, one),
and the ordinary paths checked against a real PTY driving the real watcher: the
chain keeps beating at REPIN_MS for the life of the pane, and a stop + restart
leaves exactly one chain, not zero and not two.
Also stop the new live-pane checks from aborting the whole suite where
libghostty's native addon isn't built: ensureRunning throws there, and an
unhandled throw took the cadence and installer checks with it and printed no
summary, which reads as a broken run rather than a missing optional dep. Skip on
the already-exported ghosttyReady(). 30 checks with the addon, 28 and one SKIP
without it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four issues from a review pass by codex-1, all in the seam the awaited scan opened. Verified before fixing: `claimed` really is snapshotted at the top of claudeCandidate before any await, and node really does not release a fired timer's `_onTimeout`, so a retained Timeout retains its closure. 1. The scan decided on inputs read before it awaited, then mutated the pin as if nothing had moved. The host-identity guard in the previous commit stopped the superseded chain from REARMING but not from writing. Three ways that bites, all now re-checked immediately before the write: the pane was relaunched and the new watch owns the pin (its `since` is newer, so the old `hit` can be a pre-relaunch thread); a second claude pane went live in the folder, which is exactly what folderIsShared refuses to guess at and which was false when the tick began; or `hit` was claimed by the session it belongs to while the scan walked the disk — taking it anyway would bounce that session's own breadcrumb off 'claimed by another session' permanently, which is cross-attribution that never heals. `transcriptExists` awaits too, so ownership is confirmed once more after it. 2. A stop with no relaunch leaked the fired timer. tick() returns true when the host disappears mid-scan (it only checks isRunning on entry), so rearm ran, found the host gone, and returned without arming — leaving its own fired Timeout in claudeCapturing, holding this closure and the disposed host until the session next started. Nothing else cleans that map: the only deletes are in the tick chain. A departing chain now removes its entry, but only if the entry is still its own, so a newer watch's timer survives. 3. `lastScanAt` was stamped before the await, so a scan that threw was not retried on the next beat the way `run` logs — it was skipped until the backstop expired. Stamped after the scan resolves instead. Nothing can overlap it: the next beat is armed only once the tick returns, which is what guaranteed that all along, not the early stamp. 4. SCAN_BACKSTOP_MS's comment claimed a minute as the maximum staleness. Only a tick can scan, ticks come every REPIN_MS, and each is armed after the previous finishes, so the real bound is SCAN_BACKSTOP_MS + REPIN_MS plus the scan's own duration. Comment corrected rather than the constant. Pulled the three repeated reads (current pin, other sessions' claims, is this watch still the pane's) into named helpers, since the point of all of the above is that they must be read fresh rather than captured. No new tests, for the reason the `run` wrapper has none: this is reachable only by interleaving a relaunch or a rival pane with an in-flight scan, which needs a driven watcher loop rather than a pure function. 30/30 repin checks still pass, and the real watcher driven against a real PTY still beats once per REPIN_MS with exactly one chain across a stop+restart, re-pinning correctly for both reasons. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The reported symptom
The terminal works for 2-3s, freezes for ~5s, then repeats. Input typed during the freeze arrives all at once afterwards, but the cursor keeps blinking and you can switch windows — because the freeze is server-side, not in the browser. While the one event loop is blocked the server can neither drain PTY output nor forward keystrokes; the blinking cursor is a CSS animation and the window switch is your OS.
What it actually was
claudeTranscriptsSince()was areaddirSyncper project dir plus astatSyncper transcript — fully synchronous — running everyREPIN_MS(20s) for the life of every live session.CLAUDE_CONFIG_DIRis on the FUSE bucket (/data/state/claude). Replaying that exact scan against the real directory, 8 times:Bimodal: ~3ms when the mount's attribute cache is warm, 1.1-1.3s cold (~37ms per
statSyncround trip). Each cold scan was a full main-thread block, once per live pane, every 20 seconds.Confirmed from the outside too. Probing
/api/health— which does no I/O, so its latency is block time — gave blocks of 0.9-1.8s at periods of 20.2, 21.3, 21.2, 21.4s. Not 20.0s: the drift equals the block, which is the signature of asetTimeout(tick, N)chain that reschedules after its work. Under load the watchdog logged the same thing asmain loop STALLED 5-6s … maxLag=12671ms.Ruled out along the way:
buildTraces(11.6s of wall time but only 26ms of blocking — properly async, and theactivity=buildTracesbreadcrumb is misleading becausetracked()spans awaits); cgroup CPU throttling (nr_throttledflat, load 4.1/8); GC or CPU-bound JS (zero CPU ticks consumed across each block); sync fs on/datagenerally (p50 0.2ms); stdout pipe backpressure (async on POSIX).How this got here
Both halves of this PR trace back to specific earlier changes, and the order matters — this is not a regression introduced by something recent.
5b5da0e06-29tmuxNamehelper (tmux session name per pane)12a7e8c07-30runner.js9fb59f5#17 07-317112a46#23 08-04paneRootPid(), callingtmuxNameandexecFileSync#17 is where the cost comes from. It changed the pin from captured-once-at-launch to re-checked forever, which was the right fix for a real bug (
/clearorphaning the pin, so--resumerestored the pre-/clearthread). Its own description anticipated exactly this hazard — "this scan now runs for the life of every session against a FUSE mount rather than once per launch" — and memoized transcript heads to bound it. That memoization holds; what it did not cover is thestatSyncsweep that finds the candidates in the first place, which is the 1.2s measured above.#23 added the breadcrumb path to fix what the scan cannot do: attribute a new conversation when several live claude panes share a folder. But it introduced
paneRootPid()as atmux list-panescall five days after12a7e8cdeletedtmuxName, andexecFileSyncis not imported inrunner.jseither.git log -S tmuxName --allshows the definition added in5b5da0e, removed in12a7e8c, and referenced again only by7112a46;paneRootPiditself first appears in7112a46. So it has never returned anything butnull— it throws aReferenceErrorstraight into its own barecatchon every tick.A null pane root makes
pidTrustedfalse for every breadcrumb, which is what the live logs show, invariably:So #23's mechanism has never run, and the bug it was written for —
/clearin a shared folder never being followed — is still live today. This PR is the first point at which that mechanism can work at all, which is also why the cheap fix for the freeze (lean on the breadcrumb, scan less) had to be preceded by repairing the thing it leans on.Three changes
1. The pane root
paneRootPidnow readshosts.get(id).pty.pid: the same semantic tmux'spane_pidhad — the process this server spawned for the session, whichexec claudereplaces in place — as a property read, with no subprocess per tick.2. The scan is awaited
The scan's cost was never the I/O, it was doing the I/O on the event loop.
claudeTranscriptsSince,transcriptHead,transcriptExistsandclaudeCandidatenow await, and the watcher tick is async. Measured against the real bucket directory, same code path either way:Sequential rather than
Promise.allon purpose: 32 parallel FUSE stats would saturate the 4-thread libuv pool and push every other fs operation in the process behind them, and nothing is waiting on the result.3. The scan steps down to a backstop
For any pane whose hook has proven itself — a breadcrumb that named that pane's own conversation, either a re-pin or the
resumeno-opalready pinned— the scan runs once a minute instead of every beat. A crumb rejected for any other reason proves nothing about the pane and does not count. A pane with no working hook keeps the old every-tick cadence, so nothing regresses where the scan is still the only mechanism.Because the scan no longer blocks, this cadence is not trading staleness against freezing any more; it only limits how often we walk the bucket for an answer the breadcrumb already gave. That is why it is a minute and not the ten first proposed — a minute is also the longest a pin can stay stale if a crumb is ever lost (the hook fired but its crumb never reached us), and the cost of being wrong there is one late Overview digest.
Rearming moved out of
tickinto one place. An awaited tick that throws surfaces as a rejected promise rather than an exception in asetTimeoutcallback, soruncatches, logs and rearms — dropping the watcher would silently stop following/clearfor the rest of the pane's life. Exactly one rearm per tick, so a failure can neither kill the watcher nor arm two timers. The scan also re-reads the pin after awaiting rather than trusting the value read at the top of the tick.Scope
Not touched: the codex and opencode watchers from #17 run on the same 20s beat, but read local disk (
/home/node/local/codex-home, and opencode's SQLite was moved off FUSE in1dfb753), so they are cheap.firstLinestays synchronous for the same reason — it is only on the codex rollout path. Claude's scan was the one on the bucket.Testing
server/test/repin.test.mjs— 30 checks, up from 21. Fullnpm testgreen (all 5 suites).got false want true). Note it is the live-session check that catches it —paneRootPid('unknown')returnsnullunder both the broken and fixed versions, which is part of why Sessions: let the pane say which conversation it is on #23's own 21 checks passed while the function never worked.claudeScanDue(): unchanged every-tick behaviour without a proven hook, no scan on the next beat or at 59s with one, due again at a minute, and a proven pane still scanning once before its first backstop.claudeCandidatechecks now await it; their assertions are unchanged, so they still cover the launch window, folder scoping and other sessions' pins across the async conversion.claudeCandidateand pointing it at the real/data/state/claudewhile sampling event-loop lag, rather than inferred from the shape of the change.Not covered by tests: the
runwrapper's throw-and-rearm behaviour, which needs a driven watcher loop rather than a pure function. And not verified end-to-end: that a/clearin a shared folder now re-pins via a trusted breadcrumb on the live Space — that needs this deployed, since the running container still has the brokenpaneRootPid, which is why the log evidence above shows only rejections.🤖 Generated with Claude Code