Skip to content

Re-pin: follow a /clear that happens inside a worktree - #57

Open
thomwolf wants to merge 1 commit into
mainfrom
repin/follow-clear-into-subdir
Open

Re-pin: follow a /clear that happens inside a worktree#57
thomwolf wants to merge 1 commit into
mainfrom
repin/follow-clear-into-subdir

Conversation

@thomwolf

Copy link
Copy Markdown
Member

The symptom

A session's reader and its terminal showed two different conversations. Same
pane, same agent: the terminal was mid-conversation while the reader kept
rendering a thread that had ended hours earlier.

Why

The reader reads /api/trace/:id, which resolves to session.sessionUuid — the
pin. runner.js keeps that pin honest with two mechanisms (a SessionStart
breadcrumb from the pane, and a transcript scan as its backstop), and both
asked cwd === workdir
of a candidate conversation.

That equality quietly broke every agent that works in a git worktree. The PTY
starts in the session's folder; the agent enters .claude/worktrees/<name>; and
the conversation a later /clear starts records that deeper cwd. So the
breadcrumb was rejected as cwd mismatch and the scan skipped the file, and the
pin stayed on the abandoned conversation for the rest of the pane's life.

Observed live on this Space: a session pinned edbfc11f… while Claude was
writing b1e23587… under .claude/worktrees/session-sharing. /tmp/am-repin
showed a crumb written and consumed with the pin unmoved.

The wrong reader is the visible half. The other half is worse: the next launch
runs --resume <pinned uuid>, so a restart would restore the pre-/clear
thread and discard everything since — precisely the failure this watcher was
written to prevent (see the comment above scheduleClaudeCapture).

The change

One helper, containment instead of equality:

export function cwdUnderWorkdir(cwd, workdir) {
  const rel = path.relative(path.resolve(workdir), path.resolve(cwd));
  return rel === '' || (rel !== '..' && !rel.startsWith(`..${path.sep}`) && !path.isAbsolute(rel));
}

Containment rather than a prefix test, so /w/proj-a2 is not inside /w/proj-a.

Applied at all four places that asked the same question, because the other three
harnesses carried the identical one-liner and Codex agents here work in worktrees
too:

Path Was
Claude transcript scan (claudeCandidate) head.cwd !== workdir
Claude/Codex/OpenCode breadcrumb (breadcrumbVerdict) crumb.payload.cwd !== facts.workdir
Codex rollout head (tryCaptureCodexId) mp.cwd !== workdir
OpenCode database row (applyBreadcrumb) row.directory !== workdir

folderIsShared widens with it. Now that the scan reaches below the folder, a
live sibling of the same harness in a subdirectory — or in a parent of it,
which is what a session on the workspaces root is — is exactly as unattributable
as one in the same directory, and gets the same refusal to guess. That costs
nothing where it fires: breadcrumbs are the mechanism, and they carry their own
pane/run/pid attribution; the scan is only their backstop.

Attribution is not loosened anywhere else. A crumb still has to match AM_ID,
AM_RUN_ID, the harness, an unclaimed conversation id, and a pid that is the
pane root or its direct child — so a nested claude -p in a subdirectory still
cannot speak for the pane.

What I verified

A control first, because a test that passes both ways proves nothing. With
the fix reverted, against the same fixtures the new tests use:

scan:       worktree conversation claimed -> null
breadcrumb: worktree crumb -> {"repin":null,"why":"cwd mismatch"}

With it applied, the scan claims the worktree conversation and the crumb
re-pins. Both cases, plus eight containment unit cases (the folder itself, a
worktree below it, a plain subdirectory, a prefix neighbour proj-a2, the
parent, elsewhere, and null inputs), are now in server/test/repin.test.mjs.

Full server suite green, exit 0: spawn-group 28, repin 67,
opencode-resume 32, plus terminal-modes, trace-tail, migration (which
boots a real server) and resize.

That run needs @coder/libghostty-vt-node, which a fresh worktree does not have
— from the worktree alone you get 65/67 (the two live-pane checks skip) and a
spurious migration.test.mjs failure that is only the missing dep. I ran the
suite from a clone with a real node_modules symlinked in to confirm it passes
with deps present.

What I did NOT verify: I did not drive a browser, and I did not watch this
fix repair a live pin end to end. The re-pin needs a conversation born inside a
launch window, so proving it live means restarting a pane on this build and
/clear-ing inside a worktree — the deployed server is still main. The
evidence here is the control-vs-fix pair on real transcript fixtures plus the
live diagnosis that produced the same cwd mismatch verdict.

Not in scope: share.js:217 has the same equality in its cwd fallback.
It only runs when the pin matches nothing on disk, and a correct pin makes it
moot, so widening the share path's guesswork belongs in its own change.

🤖 Generated with Claude Code

Every pin path asked `cwd === workdir` of a candidate conversation, and that
quietly broke every agent that works in a git worktree. The PTY starts in the
session's folder, the agent enters `.claude/worktrees/<name>`, and the
conversation a later `/clear` starts records that deeper cwd. Equality rejected
it from both directions — the SessionStart breadcrumb as 'cwd mismatch', the
transcript scan by skipping the file — so the pin stayed on the abandoned
conversation for the rest of the pane's life.

Two things went wrong with that. The reader reads the pin, so it kept showing
the pre-/clear conversation while the terminal showed the new one: one session,
two different conversations side by side. And the next launch runs
`--resume <pinned uuid>`, which would restore the old thread and discard
everything since — the failure this watcher was written to prevent.

Observed live: a session pinned edbfc11f… while claude was writing b1e23587…
under `.claude/worktrees/session-sharing`.

So ask for containment instead of equality, in one helper the three harnesses
share (claude's scan and crumb, codex's rollout head, opencode's database row).
Containment rather than a prefix test, so `/w/proj-a2` is not inside `/w/proj-a`.

`folderIsShared` widens with it: now that the scan reaches below the folder, a
live sibling of the same harness in a subdirectory — or in a parent, which is
what a session on the workspaces root is — is exactly as ambiguous as one in the
same directory, and gets the same refusal to guess. Nothing is lost where it
fires; breadcrumbs are the mechanism and the scan is their backstop.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@thomwolf

Copy link
Copy Markdown
Member Author

The core case works: I built the shapes this PR is about against the real code and a /clear inside .claude/worktrees/<name> is found and claimed — claudeTranscriptsSince already walks every project dir, so the deeper slug (-proj-a--claude-worktrees-wt-1) was never the problem, only the equality test was. The existing suite passes (65/0). Two things below are, I think, genuinely wrong, and one is an asymmetry the PR's own framing claims not to have.

1. folderIsShared now treats the workspaces root as overlapping every folder, so one root-level pane disables scan-based pinning for every same-CLI pane (server/src/runner.js:999-1005)

A session created without explicitly picking a folder gets path: ''sessions.js:66 (path: path ?? (cli === 'files' ? null : '')), and createSession defaults reqPath to '.', which cleanRelPath normalises to '' = the workspaces root (server/src/index.js:1713-1721, and the comment at :1797-1799 says so). This is not an exotic configuration: the UI's quick-start hardcodes it — onQuickStart(quickCli, p, …, '.') at web/src/components/Sidebar.tsx:156, and NewSession.tsx:24,40 default loc to '.'.

For such a session workdir === WORKSPACES_DIR, and cwdUnderWorkdir(<any session folder>, WORKSPACES_DIR) is true. So the new cwdUnderWorkdir(other, workdir) || cwdUnderWorkdir(workdir, other) reports "shared" between a root pane and every other pane of that CLI on the box, in both directions. I confirmed this by calling the exported function against a real WORKSPACES_DIR: overlap(root, proj-a) => true.

Concretely, with one quick-started claude pane running at the root:

  • every other claude pane takes the folderIsShared branch at runner.js:1411 on every beat, logs "folder shared with another live session" once, and never scans again for the life of the pane — i.e. the exact /clear backstop this PR exists to make work is off, fleet-wide, whenever a root pane is live. claudeScanDue (runner.js:1373) is written so that a pane with no working hook keeps the old cadence and "nothing regresses", but folderIsShared is checked first and overrides it, so a pane whose hook never fired now has no mechanism at all.
  • the root pane never scans for itself either (the predicate is symmetric).
  • for codex this is worse than a lost backstop: tryCaptureCodexId is also the initial pin capture on installs without the SessionStart hook (runner.js:841-848), so a live root-level codex pane means every other codex pane can end up with no codexSessionId/codexRollout at all — no Overview digest, no resume path in commandFor.
  • same for opencode at runner.js:1499.

Before this PR the test was exact equality, so only genuine folder-mates suppressed each other. The header comment justifies the widening with "refusing here costs nothing where it fires: the breadcrumb path is unaffected, and it is the mechanism now — the scan is its backstop", but that reasoning holds only for panes that overlap in a way a human would call sharing. The root folder overlaps everything by construction, and it's the default location, so the blast radius is the whole instance rather than a pair of panes.

Worth considering: keep equality as the "rival" test and add overlap only when neither side is WORKSPACES_DIR itself; or treat the root specially, since a pane at the root has no meaningful containment claim on other panes' folders.

A second, smaller consequence of the same fact: for a root-level pane the new cwd check in breadcrumbVerdict (runner.js:1128) accepts any cwd anywhere under /data/workspaces, so the "a crumb written before a pane was moved to another folder must not follow it there" guard no longer discriminates for those panes. amId/runId/pidTrusted still carry the real weight, so this is a loss of defence in depth rather than a hole.

2. The scan has no ownership guard, and "newest anywhere in the tree" now includes conversations this pane did not start (server/src/runner.js:964-987, applied at 1416-1447)

claudeCandidate accepts any transcript whose head cwd is under workdir, isn't pinned by another session, and started after the launch window — and then takes the newest (runner.js:984). There is no upper time bound (start < sinceMs - 15_000 only rejects older threads) and no process-ownership test.

I ran the dangerous shape against the real function: pane's own post-/clear conversation with cwd = <workdir>/.claude/worktrees/wt-1, plus a later transcript with cwd = <workdir>/.claude/worktrees/wt-1/server. claudeCandidate returns the second one. Before this change that second transcript was invisible unless it was born in exactly the workdir; now anything under the tree qualifies, and deeper directories are precisely where a nested agent runs.

The realistic producer is a claude process started under the session tree that is not itself a managed session — a claude -p from a Bash tool call, a hook, a script, or a second interactive claude started by hand in a shell pane inside the folder. claimed only covers other sessions' sessionUuid, so none of those are excluded. The breadcrumb path guards exactly this hazard deliberately (runner.js:1133, "pid not top-level pane agent", with the rationale at :1053-1057), and the codex scan filters thread_source === 'subagent' (runner.js:801); the claude scan has neither, and there is no isSidechain/nested-run filter anywhere in the repo (I grepped server/src, scripts, docs).

The part that makes this more than theoretical: the backstop scan keeps running every 60s even after the hook has proven itself (claudeScanDue, runner.js:1373-1376), and the re-pin at runner.js:1438 compares only hit.uuid !== pin — it does not care that the current pin came from a trusted breadcrumb. So a nested run started after the pane's own conversation will overwrite a correct, hook-supplied pin, and the next launch --resumes the nested agent's thread. That's the same user-visible failure the PR is fixing, arrived at from the other direction.

I'd treat the mechanism as confirmed (I reproduced the selection) and the frequency as reasoned: it needs a nested claude under the tree, which this codebase's own worktree workflow makes ordinary. A cheap mitigation is to keep the newest candidate only when the pane has no breadcrumb-proven pin, or to exclude candidates whose project dir corresponds to a directory below a deeper live session/agent — but the simplest is to not let the scan override a pin that host.exactRepinProven vouched for.

3. The opencode paths disagree with each other after this change (server/src/runner.js:1284 vs server/src/traces.js:422-433)

applyBreadcrumb now accepts a database row whose directory is below workdir, but the fallback discovery it backs up, captureOpencodeSession, still queries select … from session where directory = ? — exact match. So for opencode a /clear in a worktree is followed only when the plugin breadcrumb arrives; on installs without the global plugin (the case the fallback exists for, per the comment at runner.js:859-863) it still isn't. share.js has the same untouched equality at :217, :264, :310 and :317, which is less consequential because those are only reached when the pin is missing, but it does mean the header's "every pin path asks the same question of a candidate conversation" isn't yet true.

Checked and found fine

  • Event-loop cost. cwdUnderWorkdir is pure path.resolve/path.relative arithmetic — no stat, no realpath, no new walk. The scan enumerates the same project dirs it always did (claudeTranscriptsSince, runner.js:901-918); containment only admits more of what was already listed, so there is no added readdir/stat and nothing new lands on the loop. No concern here.
  • Stale pins. The pin is one field replaced through update(); nothing accumulates. headMemo is keyed by path with the existing 500-entry clear and is unaffected.
  • Ordering/races. The post-await re-checks at runner.js:1435-1445 (stillOurs, claimedByOthers, folderIsShared, fresh currentPin) are unchanged and still correct for the widened predicate; a mid-flight scan whose result is invalidated is discarded rather than applied. A late-appearing transcript is picked up on a later beat rather than latched onto stale, since since is fixed per launch and the head cache only memoises definite answers (runner.js:946-957).
  • Non-worktree regression. rel === '' makes containment a strict superset of the old equality, and the pre-existing tests still pass.

What I verified vs. reasoned about

Verified by running code: cwdUnderWorkdir's root-vs-folder overlap, and claudeCandidate preferring the deeper/later transcript, both against the branch's own modules with a stubbed node-pty; and the full server/test/repin.test.mjs suite (65 passed, 0 failed). Verified by reading: the path: '' default and its two call sites, the quick-start '.', the codex/opencode tick structure, the absence of any subagent/sidechain filter on the claude scan, captureOpencodeSession's exact-match query, and the post-await re-check logic.

Reasoned about but not executed: that claude -p and hand-started claude processes write their own transcript files with a cwd under the tree (I did not run a nested claude to confirm the on-disk shape); the practical prevalence of root-level panes on the live Space; and that cwdUnderWorkdir being purely lexical will mismatch if a workspace path is reached through a symlink while the harness records the physical path — note codexRolloutForBreadcrumb in this same file deliberately compares both spellings for exactly that reason, though equality had the same weakness, so it is not a regression.

Not checked: I did not boot a server or exercise a real /clear end to end, did not test codex or opencode against real rollouts/databases, and did not review the client.

@thomwolf

Copy link
Copy Markdown
Member Author

Ran it live, PR code vs main, on the Space this bug was found on

This supersedes the "I did not watch this fix repair a live pin end to end" line
in the description — it has now been watched, on two real servers.

Setup. Two isolated Agent Manager servers on the same box: this branch on
:7991, byte-for-byte upstream/main on :7992 (md5 checked against
git show upstream/main:server/src/runner.js). Each with its own DATA_DIR,
CLAUDE_CONFIG_DIR, CODEX_HOME, OPENCODE_CONFIG_DIR and AM_REPIN_DIR, and
SPACE_ID unset. The deployed server, /data/sessions.json and the live Claude
config were never touched — verified after the run (no test session in the live
store, no sandbox project dirs, live /tmp/am-repin still empty, live server
healthy).

Real in this test: both servers, real PTY panes with the real claude binary
launched by commandFor, the real scripts/am-repin-hook.sh run inside the pane
with that pane's own AM_ID/AM_RUN_ID and the real pid of the process the
server spawned, real sessions.json writes, real /api/trace reads. Stood in
for: the two conversations' .jsonl files, written in Claude's on-disk shape
(metadata lines with no cwd, then conversation lines carrying it) — the pane's
Claude sat on its welcome screen in a fresh config dir, so it wrote none of its
own, and the /clear itself is expressed as the hook event a /clear produces.

1. The breadcrumb path — /clear reported from a subdirectory

main this branch
pin before c1b290cf ae774093
new conversation in <folder>/deep 1e6cecae 390e55b5
pin after c1b290cf — unmoved 390e55b5 — followed
what the reader renders ANSWER FROM c1b290cf in proj-a ANSWER FROM 390e55b5 in deep

Server logs, same hook event, same pane shape:

main   [claude] crumb-a34035: exact breadcrumb rejected (cwd mismatch)
fix    [claude] re-pinning crumb-dad84a: ae774093… -> 390e55b5… (exact clear)

That second row is the reported symptom, reproduced and then fixed: on main the
reader keeps rendering the pre-/clear conversation while the pin is stale; on
this branch it moves to the conversation that actually exists.

2. The transcript-scan backstop — no crumb at all

main this branch
pin after a new conversation appears in <folder>/deep c8dbce3c — unmoved after 30s 0963081e→ followed in 14s
log (silent — the file was skipped) re-pinning scan-4968ee: db0f2017… -> aaa572ed… (conversation was replaced (/clear))

14s is consistent with the cadence: first tick at 5s, then every REPIN_MS.

3. Guard: the widened folderIsShared still refuses to guess

The riskiest part of this change is that the scan now reaches below the folder,
so I checked the new ambiguity case on a live server: session on ov-parent, a
second live session on ov-parent/child, then an unattributable conversation
appearing in ov-parent/deep.

refused_to_guess: true
[claude] ov-parent-45ad3b: folder shared with another live session — following /clear only via breadcrumbs here
[claude] ov-child-5c4296: folder shared with another live session — following /clear only via breadcrumbs here

Pin unmoved, refusal logged for both, and breadcrumbs remain available there —
which is the intended trade. Non-overlapping folders still scan normally, which
is what §2 above demonstrates.

Still not verified

No browser was driven: "what the reader renders" above is the /api/trace/:id
payload the reader renders from, not a screenshot. And this ran against isolated
servers, not against the deployed one — the live Space still runs main, so the
pins currently stranded there (including the one that started this) stay stranded
until this lands and the panes are restarted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant