fix(lanes): removing a lane deletes its record only, never the worktree

A lane's `remove` tore down what the lane pointed at: `git worktree remove
--force`, a branch delete, and a drop of the lane's own database. One
mis-click on a dashboard row cost a whole branch's unpushed code — and the
row is the only thing AgentHub actually owns. `remove` now stops the run and
the stack, clears AgentHub's own runtime bookkeeping under
`LANES_ROOT/.state/lane<slot>/`, and deletes the row. The working tree, its
branch, git's worktree records and the database stay exactly where they are,
for the user to delete by hand when they mean to.

That also fixes #28. Removal used to route through `assertDestroyable`, whose
third check demands the cwd appear in `git worktree list`; a managed lane
whose directory git no longer listed answered `ENOTWORKTREE` ("lane is not
listed as a worktree in …") and could never be dropped from the dashboard at
all. Removal no longer asks git anything, so no preflight blocker stops it,
and the unpushed-commits force-gate now applies to `reset` alone — nothing
`remove` does can discard a commit.

The UI follows: one label for both lane kinds ("Delete lane record"), copy
that says what survives, and no Force checkbox on removal.
`removeWorktree`/`removeLaneData` remain in place as guarded primitives; no
route calls them any more.
This commit is contained in:
2026-09-18 09:44:17 +07:00
parent 75a2de60b2
commit 0004cf7036
13 changed files with 136 additions and 103 deletions
+10 -10
View File
@@ -118,18 +118,18 @@ agenthub lanes remove <id> [--force] --yes
agenthub lanes purge <id> --yes
```
Each command first fetches and prints its preflight counts. `reset` and `remove` show `head`, `dirty`, `untracked`, and `unpushed` (plus `database`, the name a data-isolated lane's reset/remove will drop — see "Data isolation" below); `purge` shows `sessions`, `events`, and `tokenRows`. The action refuses to run without `--yes`, and sends those exact facts back as its confirmation. Use `--force` only when the preflight reports unpushed commits. `--keep-db` (reset only) skips the whole drop-recreate-migrate-reseed sequence, leaving the lane's database exactly as it was. An adopted lane points at a directory you own, so the CLI refuses to `reset` it. `remove` IS allowed for an adopted lane: it drops only the dashboard's record and leaves the directory, its contents, and its database (if any) untouched.
Each command first fetches and prints its preflight counts. `reset` and `remove` show `head`, `dirty`, `untracked`, and `unpushed` (plus `database`, the name a data-isolated lane's **reset** will drop — `remove` drops no database at all; see "Data isolation" below); `purge` shows `sessions`, `events`, and `tokenRows`. The action refuses to run without `--yes`, and sends those exact facts back as its confirmation. Use `--force` only when the preflight reports unpushed commits — it applies to `reset` alone, since `remove` deletes nothing that could hold them. `--keep-db` (reset only) skips the whole drop-recreate-migrate-reseed sequence, leaving the lane's database exactly as it was. An adopted lane points at a directory you own, so the CLI refuses to `reset` it. `remove` is allowed for **every** lane kind, adopted or managed, because it drops only the dashboard's record and leaves the working tree, its branch, and its database (if any) untouched — deleting those is yours to do by hand.
## Lifecycle
### The two lane kinds
- **`managed`** — a git worktree the dashboard itself created (`agenthub lanes add --repo`), rooted under `LANES_ROOT`. The dashboard owns its full lifecycle: it may reset the worktree to its base branch or remove it entirely.
- **`managed`** — a git worktree the dashboard itself created (`agenthub lanes add --repo`), rooted under `LANES_ROOT`. The dashboard may reset that worktree to its base branch. It does NOT delete it: removing the lane forgets the dashboard's record and leaves the directory and its branch on disk.
- **`adopted`** — an existing directory you already had (`agenthub lanes add --cwd`). The dashboard only ever tracks it; it never resets or deletes that worktree. "Removing" an adopted lane drops the dashboard's record of it and leaves the directory untouched.
### The three independent safety checks
Before `reset` or `remove` touches a worktree, `assertDestroyable` (`server/lib/worktree.js`) runs three checks, all of which must pass:
Before `reset` touches a worktree, `assertDestroyable` (`server/lib/worktree.js`) runs three checks, all of which must pass:
1. **`lane.kind === "managed"`.** An adopted lane fails immediately with `ENOTMANAGED` — this check alone is what keeps a directory you own off the destroy path.
2. **The lane's `cwd`, resolved with `fs.realpathSync`, must sit inside the resolved `LANES_ROOT` on a path boundary.** `path.relative(root, cwd)` must not start with `..` or be absolute — `EOUTSIDEROOT` otherwise. This guards against a lane row that was hand-edited or drifted to point somewhere else.
@@ -137,14 +137,14 @@ Before `reset` or `remove` touches a worktree, `assertDestroyable` (`server/lib/
These checks run unconditionally inside `resetWorktree`/`removeWorktree` themselves, not only when a confirmation is present — there is no code path to a git-destructive operation that skips them.
One documented exception, which does not weaken them: when a managed lane's directory has already been deleted by hand, `removeWorktree` takes a **prune path** instead. Checks 1 and 2 still hold (an adopted lane is refused outright; the recorded `cwd` must still be inside `LANES_ROOT`, verified lexically because a vanished path cannot be resolved), and check 3 is what the prune replaces — a worktree git no longer lists needs no removal. That path runs `git worktree prune` plus a safe branch delete against the source repo's bookkeeping and touches no directory at all.
`removeWorktree` (with its prune path for a hand-deleted directory) still lives in `server/lib/worktree.js` and still runs those checks, but **no route calls it any more**: the lane `remove` action no longer destroys anything. It is kept as a guarded primitive, not a reachable lane verb.
### Each verb
- **`add`** — either adopts an existing directory (`--cwd`, kind `adopted`) or provisions a new managed worktree (`--repo`, kind `managed`, created under `LANES_ROOT` on branch `feat/<slug>`).
- **`clear`** (UI/API action only, no CLI verb) — resets the lane's own bookkeeping (stage, status, gate/CI fields, notes) back to idle, **including the detected columns** (`detected_stage`/`detected_signal`/`detected_at`), since detection is forward-only and a surviving detection would both claim progress the reset tree no longer has and block every later detection. Touches only the `lanes` row; no git operation, no session/event data is deleted.
- **`reset`** (managed lanes only) — checks out the base branch, `git reset --hard` to it, then `git clean -fd` (no `-x`, so **gitignored files such as `node_modules` and `.env` survive**; only untracked-but-not-ignored files are deleted), then recreates the feature branch off the base and clears the lane's bookkeeping. Requires `force: true` when the preflight reports unpushed commits.
- **`remove`** — for a `managed` lane whose directory exists: `git worktree remove --force`, `git worktree prune`, then a safe branch delete, then the lane row is deleted. For a `managed` lane whose directory was deleted by hand: the prune path above (git's stale record and the branch are cleaned up, then the row is deleted) — a `missing` worktree does NOT make a lane unremovable. For an `adopted` lane: only the lane row is deleted (`removeWorktree` is never invoked for `adopted`, so the directory is never touched). The unpushed-commits force-gate only applies when the worktree is actually touched, i.e. `action === "remove" && kind === "managed"` — forgetting an adopted lane needs no `force` even with unpushed commits, since nothing on disk is at risk.
- **`remove`** — **deletes the dashboard's record of the lane and nothing else**, for every lane kind. The working tree stays on disk, its branch stays in the source repo, git's own worktree bookkeeping is untouched, and the lane's database (if any) is left in place; deleting any of them is the user's own call (`git worktree remove`, `git branch -d`, dropping the database). Stopping the lane's run and stack still happens first — those are processes, not data — and the runtime bookkeeping under `LANES_ROOT/.state/lane<slot>/` (pid files, hook logs) is cleared, because it is AgentHub's own and would otherwise be inherited by the next lane to claim that slot. Consequently there is **no unpushed-commits force-gate on `remove`**: nothing it does can discard a commit.
- **`purge`** — deletes the lane's own `sessions`/`events`/`token_usage` rows (excluding the lane's currently-bound or any still-active session), for its own `cwd` and subdirectories. No git operation, no worktree change; not gated on unpushed commits.
### The preflight contract
@@ -156,11 +156,11 @@ One documented exception, which does not weaken them: when a managed lane's dire
`blocked[]` can contain: `adopted` (not a managed worktree), `missing` (the directory is gone), `unreadable` (directory exists but git failed against it), and `unpushed-commits` (unpushed count > 0 — the only one a `force: true` can override).
**Which entries actually prevent the action depends on the action, and the server is the authority.** All of them block `reset`. **None of them blocks `remove`** — the server supports removing an adopted lane (forget the row), a hand-deleted worktree (prune path), and an unreadable one: it attempts `git worktree remove --force`, and if git itself refuses (e.g. a corrupt worktree `.git` pointer, which git validates and rejects even with a second `--force`), it deregisters the worktree directly from the source repo's bookkeeping instead — the directory itself is never touched either way. A surface that greys out confirmation on any `blocked` entry regardless of action makes a documented capability unreachable; `client/src/components/lanes/DestructiveLaneModal.tsx` encodes the per-action set in `HARD_BLOCKERS` and renders the rest as context.
**Which entries actually prevent the action depends on the action, and the server is the authority.** All of them block `reset`. **None of them blocks `remove`**, including `unpushed-commits`: removal only deletes the lane's row, so an adopted lane, a hand-deleted directory and an unreadable one are all removable and none of them risks anything on disk. For `remove` the preflight counts are context, not a destruction manifest. A surface that greys out confirmation on any `blocked` entry regardless of action makes a documented capability unreachable; `client/src/components/lanes/DestructiveLaneModal.tsx` encodes the per-action set in `HARD_BLOCKERS` and renders the rest as context.
`unpushed` is the count of commits the action would really discard: with remotes, commits on no remote; without remotes, the commits ahead of the lane's `base_branch`, which is this lane's own work. A freshly provisioned worktree in a local-only repository reports `0`, not that repository's whole history — the `no-remote` warning, not an inflated count, is what tells the user nothing is backed up. A lane with no `base_branch` at all (an adopted one) falls back to the total commit count. `warnings[]` holds purely informational facts that never gate the action: currently just `no-remote` (no git remote configured at all — nothing here is backed up remotely, but the action proceeds normally). Both arrays are data, not an exception.
`database` is the database name a `reset` (unless `--keep-db`) or `remove` will drop — `null` when the lane has no slot yet or its profile declares no `DB_PREFIX`. It is derived, not stored, the same way `slot`/`ports` are.
`database` is the database name a `reset` (unless `--keep-db`) will drop — `remove` never drops one — `null` when the lane has no slot yet or its profile declares no `DB_PREFIX`. It is derived, not stored, the same way `slot`/`ports` are.
`bytesEstimate` is a **rough estimate**, not a measured disk size: it is derived as `(events + tokenRows) * 512`, a flat per-row byte guess, purely to give the confirmation dialog an order-of-magnitude sense of what purging will reclaim.
@@ -362,7 +362,7 @@ Parsed with the same literal `KEY=VALUE` reader as `profile.env` — never sourc
- **Provisioning** a new managed worktree (`agenthub lanes add --repo`) — after the worktree is created: seed `.env`, run `bootstrap`, create the database, `migrate`, `seed`. Runs once.
- **`up`** — repair `.env` (a hand-edited or never-seeded file gets fixed), ensure the database exists (cheap when it already does), `migrate` on every boot (a lane's schema drifts while it sits idle), and `seed` only on the boot that actually created the database.
- **`reset`** — refresh `.env` with `--force` (preserving `ENV_PRESERVE` keys from the lane's own file — swapping in the source's `JWT_SECRET` would 401 a running lane's tokens until reboot), re-run `bootstrap` (a reset can land on a branch with new dependencies), clear the declared `LANE_DIRS`, then drop, recreate, migrate and reseed the database — unless `--keep-db`, which skips that whole block.
- **`remove`** — drops the database and its `_test` sibling before the rest of teardown. Best-effort: a failed drop is logged, never blocks removing the lane's record. **Never runs for an adopted lane** — its data was never AgentHub's to create, so it is never AgentHub's to destroy, the same invariant that protects an adopted lane's worktree.
- **`remove`** — drops **nothing**. `removeLaneData` still exists in `server/lib/lane-runtime.js` (and still refuses an adopted lane), but the `remove` action no longer calls it: a lane's database is the user's data and outlives the dashboard's record of it. Drop it yourself when you actually want it gone.
A missing source `.env` falls back to `.env.example` with a loud warning, never a silent success. And any hook output that echoes a value from `secrets.env` (the password, specifically) is redacted before it reaches `$LOG_DIR/<hook>.log` or the `lane_hook_output` WebSocket message — that stream reaches a browser tab, and a hook debugging its own environment must not publish a database password to everyone watching.
@@ -1321,7 +1321,7 @@ curl -X POST http://localhost:4820/api/lanes/5/clear
### remove
Delete the lane entirely, including all its history. **Requires confirmation** (`confirm: true`):
Delete the dashboard's record of the lane, including its recorded history. The working tree, its branch and its database are left untouched. **Requires confirmation** (`confirm: true`):
```bash
curl -X POST http://localhost:4820/api/lanes/5/remove \
@@ -1329,7 +1329,7 @@ curl -X POST http://localhost:4820/api/lanes/5/remove \
-d '{"confirm": true}'
```
**Actions gated behind confirmation:** `remove` requires the `confirm` flag to prevent accidental deletion.
**Actions gated behind confirmation:** `remove` requires the `confirm` flag to prevent accidentally dropping a lane's record. It deletes that record only — never the worktree, the branch or the database.
## Proof gallery