fix(sessions): two data-loss bugs around nested session names #66

Merged
lz merged 1 commit from fix/session-worktree-enumeration into main 2026-07-13 21:16:33 +02:00
Owner

Found while reviewing #47. Both are pre-existing on main, independent of that PR, and both destroy operator work. Nested session names (feat/login) are explicitly supported — validateGitRefName allows them and AGENTS.md advertises them — and both bugs follow from such a worktree living two levels deep.

1. convergeSessions hard-deleted every nested session on restart

listWorktrees enumerated with ls -1 /workspace/.nexus/worktrees — non-recursive. Verified live in a running worker:

$ ls -1 /workspace/.nexus/worktrees
feat                       # not feat/codegraph, not feat/forward-scope

$ git -C /workspace worktree list --porcelain | grep ^worktree
worktree /workspace
worktree /workspace/.nexus/worktrees/feat/codegraph
worktree /workspace/.nexus/worktrees/feat/forward-scope

So for a row named feat/codegraph, in convergeSessions:

  • the tmux window is absent (every container restart starts a fresh tmux server), and
  • worktreesOk === true but worktreeNames.has('feat/codegraph') === false

which is exactly the "worktree positively confirmed gone" branch. The row was hard-deleted — along with its pinned claude_session_id, i.e. the conversation — while the worktree sat untouched on disk. It even logs deleting orphan row (worktree gone) with the worktree right there.

The documented safety property ("uncertainty never triggers deletion", AGENTS.md fact #15) did not help: ls exits 0, so absence looked confirmed rather than unknown.

Fix: ask git, not the filesystem. git worktree list --porcelain reports the full nested path. parseWorktreePorcelain and worktreeNamesFromPaths are pure and unit-tested.

2. createSession's recovery rm -rf could delete a live session's worktree

Git refs form a directory tree, so feat and feat/login cannot coexist. Creating feat while feat/login is running:

  1. fails the git worktree add (ref D/F conflict),
  2. falls into the recovery block, which runs rm -rf /workspace/.nexus/worktrees/feat
  3. the parent directory of the running session's checkout, uncommitted work included.

The retry then fails too, so the operator gets an error and a destroyed session.

Fix: reject the clash up front (conflictingSessionName) with an error naming the conflicting session, so the destructive path is unreachable. Belt and braces: the recovery rm -rf now refuses a directory that still contains a registered worktree (a worktree can outlive its row), and worktree prune runs before the retry so a stale registration can't wedge the add forever.

Why the suite missed this

The fake docker returned the worktree names verbatim, modelling ls -1 as if it were recursive. The mock was more capable than the real command, so the bug was invisible to a green suite. It now emits genuine porcelain — which is what makes the new regression test meaningful.

Tests

  • restores a nested-name session instead of deleting it — the regression.
  • still deletes a nested-name row when the worktree really is gone — the guard didn't get blunted.
  • unit tests for parseWorktreePorcelain / worktreeNamesFromPaths (incl. ignoring /workspace and non-session worktrees).
  • unit tests for conflictingSessionName, including that feature and feat-login do not conflict with feat/login (segment-wise, not string-prefix).

pnpm test 572/572 · svelte-check 0 errors · eslint clean.

Note

deleteSession and removeWorker are unaffected — they remove a specific path they own. The nested-name D/F rule also means no session worktree can ever be an ancestor of another, which is the property the codegraph work in #47 depends on.

Found while reviewing #47. Both are pre-existing on `main`, independent of that PR, and both destroy operator work. Nested session names (`feat/login`) are explicitly supported — `validateGitRefName` allows them and AGENTS.md advertises them — and both bugs follow from such a worktree living two levels deep. ## 1. `convergeSessions` hard-deleted every nested session on restart `listWorktrees` enumerated with `ls -1 /workspace/.nexus/worktrees` — non-recursive. Verified live in a running worker: ``` $ ls -1 /workspace/.nexus/worktrees feat # not feat/codegraph, not feat/forward-scope $ git -C /workspace worktree list --porcelain | grep ^worktree worktree /workspace worktree /workspace/.nexus/worktrees/feat/codegraph worktree /workspace/.nexus/worktrees/feat/forward-scope ``` So for a row named `feat/codegraph`, in `convergeSessions`: - the tmux window is absent (every container restart starts a fresh tmux server), **and** - `worktreesOk === true` but `worktreeNames.has('feat/codegraph') === false` which is exactly the *"worktree positively confirmed gone"* branch. The row was **hard-deleted** — along with its pinned `claude_session_id`, i.e. the conversation — while the worktree sat untouched on disk. It even logs `deleting orphan row (worktree gone)` with the worktree right there. The documented safety property (*"uncertainty never triggers deletion"*, AGENTS.md fact #15) did not help: `ls` exits 0, so absence looked **confirmed** rather than **unknown**. **Fix:** ask git, not the filesystem. `git worktree list --porcelain` reports the full nested path. `parseWorktreePorcelain` and `worktreeNamesFromPaths` are pure and unit-tested. ## 2. `createSession`'s recovery `rm -rf` could delete a live session's worktree Git refs form a directory tree, so `feat` and `feat/login` cannot coexist. Creating `feat` while `feat/login` is running: 1. fails the `git worktree add` (ref D/F conflict), 2. falls into the recovery block, which runs `rm -rf /workspace/.nexus/worktrees/feat` — 3. the **parent directory of the running session's checkout**, uncommitted work included. The retry then fails too, so the operator gets an error *and* a destroyed session. **Fix:** reject the clash up front (`conflictingSessionName`) with an error naming the conflicting session, so the destructive path is unreachable. Belt and braces: the recovery `rm -rf` now refuses a directory that still contains a registered worktree (a worktree can outlive its row), and `worktree prune` runs before the retry so a stale registration can't wedge the add forever. ## Why the suite missed this The fake docker returned the worktree names **verbatim**, modelling `ls -1` as if it were recursive. The mock was more capable than the real command, so the bug was invisible to a green suite. It now emits genuine porcelain — which is what makes the new regression test meaningful. ## Tests - `restores a nested-name session instead of deleting it` — the regression. - `still deletes a nested-name row when the worktree really is gone` — the guard didn't get blunted. - unit tests for `parseWorktreePorcelain` / `worktreeNamesFromPaths` (incl. ignoring `/workspace` and non-session worktrees). - unit tests for `conflictingSessionName`, including that `feature` and `feat-login` do **not** conflict with `feat/login` (segment-wise, not string-prefix). `pnpm test` 572/572 · `svelte-check` 0 errors · `eslint` clean. ## Note `deleteSession` and `removeWorker` are unaffected — they remove a specific path they own. The nested-name D/F rule also means no session worktree can ever be an ancestor of another, which is the property the codegraph work in #47 depends on.
fix(sessions): two data-loss bugs around nested session names
All checks were successful
ci / nexus (pull_request) Successful in 3m8s
ci / images (pull_request) Successful in 4m54s
pr-image-cleanup / delete-pr-images (pull_request) Successful in 6s
b4ff012dbc
Session names may contain slashes — `validateGitRefName` allows nested branches
and AGENTS.md advertises them (`feature/login`). Both bugs below follow from the
worktree for such a session living two levels deep, and both destroy operator work.

1. convergeSessions hard-deleted every nested session on restart.

   `listWorktrees` enumerated with `ls -1 /workspace/.nexus/worktrees`, which is
   non-recursive: a session named `feat/login` showed up as `feat`. So in
   convergeSessions the row's name was not in the set, while the tmux window was
   absent (a container restart starts a fresh tmux server) — which is exactly the
   "worktree positively confirmed gone" branch. The row was HARD-DELETED along
   with its pinned claude_session_id (the conversation), while the worktree sat
   untouched on disk. The "uncertainty never triggers deletion" guard did not
   help: `ls` exited 0, so absence looked confirmed rather than unknown.

   Ask git instead of the filesystem: `git worktree list --porcelain` reports the
   full nested path. `parseWorktreePorcelain` + `worktreeNamesFromPaths` are pure
   and unit-tested.

2. createSession's recovery `rm -rf` could delete a LIVE session's worktree.

   Git refs form a directory tree, so `feat` and `feat/login` cannot coexist.
   Creating `feat` while `feat/login` ran failed the `git worktree add` (ref D/F
   conflict), fell into the recovery path, and ran
   `rm -rf /workspace/.nexus/worktrees/feat` — the parent directory of the running
   session's checkout, uncommitted work included.

   Reject the clash up front (`conflictingSessionName`) so the destructive path is
   unreachable, and guard the `rm -rf` so it refuses a directory that still holds a
   registered worktree (a worktree can outlive its row). `worktree prune` before
   the retry, so a stale registration cannot wedge the add forever.

The suite never caught (1) because the fake docker returned worktree names
verbatim — modelling `ls -1` as if it were recursive. The mock was more capable
than the real command. It now emits genuine porcelain.
lz merged commit 62b8fee2b0 into main 2026-07-13 21:16:33 +02:00
lz deleted branch fix/session-worktree-enumeration 2026-07-13 21:16:33 +02:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
lz/agent-nexus!66
No description provided.