Skip to content

feat(engine,daemon): worktree session leases - #525

Open
Zerlight wants to merge 3 commits into
ruocheng/code-639from
ruocheng/code-640
Open

feat(engine,daemon): worktree session leases#525
Zerlight wants to merge 3 commits into
ruocheng/code-639from
ruocheng/code-640

Conversation

@Zerlight

@Zerlight Zerlight commented Sep 8, 2026

Copy link
Copy Markdown
Member

Summary

Phase 5 of CODE-627 — Conversation turn graph & immutable attachment store. Linear: https://linear.app/arcbox/issue/CODE-640/featenginedaemon-worktree-session-leases

Stack: #524this PR (ruocheng/code-640, base ruocheng/code-639) ← top of the stack. Merge bottom-up; this PR's diff is only its own commits.

A managed worktree is shared by every session forked from it. Ownership moves from a session_id column to a worktree_sessions lease table on the shared graph connection (primary key worktree path + session id, unique session id, cascade to worktrees, deliberately no foreign key to sessions); WorktreeRecord drops sessionId and gains a deleting state. Releasing the last lease marks the worktree deleting in the same transaction, before any filesystem work, and a new lease on a deleting or removed worktree is refused — a fork racing the last-lease cleanup fails typed conflict instead of landing on a half-deleted directory. Cleanup runs only after the last release; boot reconcile sweeps leases whose session is gone, finishes deleting rows a previous daemon never cleaned, and re-inspects holder-less orphaned worktrees, removing them once clean and pushed (the old "delete the deleted session again to retry" path has no lease left to find). A fork child leases its source's worktree, captured at admit, before its adapter starts there. At most one leaseholder may have a running turn: SessionOrchestrator.admitTurn runs every turn-start's admit-and-persist under a per-worktree permit and returns typed busy while a co-leaseholder is running or holds an open operation; turn.submit, legacy agent.input, prompt rewrite and the automation driver all go through it. Migration 0015 backfills leases from worktrees.session_id (orphan rows get none) before dropping the column.

Commits

  • feat(engine,daemon): lease managed worktrees to sessions
  • feat(engine): share the source's worktree lease with a fork child
  • feat(engine): allow one running turn per managed worktree

Verification

Every commit passed pnpm check:ci and pnpm test at its own tip; the tip (09f5a40f) is at pnpm check:ci 0 errors, pnpm test 3474 passed / 1 skipped. Adversarial review pair (engine axis and daemon-migration axis, isolated read-only worktrees): the engine axis found a P1 (prompt rewrite and the automation driver bypassed the worktree gate) and a P2 (a fork after the source's lease release started the child unleased) — both fixed, the P1 with a reproduce-first test; the daemon axis found no P1/P2 and verified the migration against the shipped SQLite 3.53.4 (DROP COLUMN after DROP INDEX, one transaction, no diff from drizzle-kit generate). Fixes were folded into the commits they revise; the record is on CODE-640. Integration tests on a real git repo with a bare remote cover fork plus parent-first delete (the child keeps the worktree, the child's delete cleans once), the deleting conflict, the one-turn gate for turn.submit, agent.input and rewrite, and the boot sweep; the daemon store test replays migrations 0000–0014 to prove the backfill and asserts the physical schema. Real development daemon: the running daemon applied 0015 on its real database; one user-approved paid claude turn then forked a live source on a managed worktree (two leases on one path, a distinct claude history under the worktree's project directory), deleting the parent first kept the child's worktree and deleting the child removed it; the webview showed the worktree thread under its project with a branch badge, and the sidebar's Close thread ran the whole lease cleanup. Not exercised live: the co-leaseholder busy message (needs a second paid turn; integration-tested). Deferred, recorded on the issue: no client UI for lease state or orphaned worktrees; a legacy agent.input refused by the gate replies request.failed busy without an in-conversation echo.

Checklist

  • pnpm check:ci and pnpm test both pass (no Rust changes)
  • I ran the affected surface and observed the change working — the development daemon (migration and lease rows in SQLite) and the webview against it
  • Wire: unchanged at 82; daemon migration 0015 (worktree_sessions, backfilled from the dropped column)
  • New code and assets are my own work
  • Docs and comments are updated where behavior changed (module docs and the store contract in this branch)

@linear-code

linear-code Bot commented Sep 8, 2026

Copy link
Copy Markdown

CODE-640

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

One recommended change: forking a session whose managed worktree directory has vanished (orphaned) leases it and starts the child in a nonexistent cwd. Everything else checks out — the lease transaction, the deleting ordering, the migration, and the new turn gate all hold up.

The lease-table design is sound, and the two things I most expected to be wrong aren't. Verified rather than assumed:

  • The new per-worktree turn gate cannot deadlock. Semaphore.makeUnsafe(1) is not reentrant, so I traced all four admitTurn call sites: each body persists intent and returns before launchRun/relaunch/startLive/watchTurn run, so the permit is released before any adapter work, and the turn.submit saga's prepared !== undefined path skips the dispatcher's admitTurn entirely (session-input-dispatcher.ts:148). Nothing awaits another session's turn from inside a permit — the sibling check fails fast with busy.
  • onConflictDoNothing is correctly targeted at the composite PK, so only the idempotent same-worktree replay is swallowed; a session trying to lease a second worktree still trips worktree_sessions_session_unique and throws. A bare ON CONFLICT DO NOTHING would have silently defeated that index in SQLite — the explicit target is load-bearing, and the comment above it is right.

🔍 Migration checks

All four pass, so no action needed — recorded because they're invisible in the diff:

  • 0015_snapshot.json prevId equals 0014_snapshot.json id, and _journal.json's new when is strictly greater than the previous entry (a non-monotonic timestamp re-runs non-idempotent DDL at boot).
  • Every WorktreeRecordSchema field still has a column after the DROP COLUMN.
  • The NOT LIKE 'orphan-worktree-%' backfill filter exactly matches the format the deleted orphanSessionId helper produced (orphan-worktree-${sha256hex}, confirmed against base 27ac29b9), so sentinel rows are deliberately left leaseless.
  • DROP INDEX correctly precedes ALTER TABLE … DROP COLUMN, and the FK cascade delete() now relies on really fires — db/database.ts sets foreign_keys = ON on the shared connection.

No wire version move is needed. WorktreeRecordSchema / WorktreeLeaseSchema / WorktreeStateSchema are referenced only by apps/daemon/src/worktree-store.ts and never by a wire payload, so removing sessionId and adding the deleting variant is store-only. Leaving WIRE_PROTOCOL_VERSION at 82 and MIN_COMPATIBLE_WIRE_VERSION at 76 is correct.

⚠️ Boot cleanup is newly destructive — worth a release note

reconcile()'s cleanup branch went from !hasSession && record.state === 'active' to plain !held, so a holder-less record whose directory still exists is now cleaned up regardless of state. Two consequences the diff doesn't make obvious:

  1. scanUnknown-adopted directories — ones LinkCode never created — were previously kept forever (they're orphaned, so the old state === 'active' condition skipped them). They are now removed. Because scanUnknown runs at the end of reconcile, adoption and removal land one boot apart, which is what the renamed test encodes.
  2. Post-migration, every pre-existing sentinel orphan row becomes holder-less (correctly excluded from the backfill) and enters the same path on the first boot after upgrade.

I confirmed this can't eat uncommitted work: cleanupRecord gates on inspectWorktreeCleanup (branch matches, status --porcelain --untracked-files=all empty, upstream configured, nothing unpushed) and then runs git worktree remove without --force; any failure or throw marks the record orphaned and preserves the directory. identifyManagedWorktree also refuses to adopt a standalone repo. So the guard is real — but given the PR explicitly defers lease/orphan UI, "LinkCode deletes a clean, fully-pushed worktree it didn't create, on boot, with no user opt-in" is behavior worth surfacing in the release notes rather than discovering.

✅ Verification

pnpm vitest run over engine-worktree.test.ts, worktree-service.test.ts, and both new worktree-store.test.ts files: 34 passed / 4 files. I did not run the full typecheck/lint gates (slow; CI covers them).

Technical details — why the record `cwd` matters for the fork finding

The fork gap depends on a fact that isn't visible in this diff: a managed-worktree session's SessionRecord.cwd is the worktree path, not the original repo root.

lifecycle-service.ts:

const resolved = yield* worktrees.provision(resolvedIntent, sessionId);
// …
const record: SessionRecord = {
  sessionId,
  kind: resolved.kind,
  cwd: resolved.cwd,

and provision returns withoutBranch(options, worktreePath), i.e. cwd = worktreePath.

So in launch, resolveForRecord(source, undefined, childId) resolves { cwd: record.cwd } — the worktree path — and that is what the child adapter is started in. That's exactly right in the normal case (it's how the child shares the tree), and it's precisely why an orphaned source is a problem: the path no longer exists on disk.

abandon() does release the child's lease on every failure path here (Effect.onExit fires on failure and interruption while the record is still provisional, and cleanupDeletedSession runs even if stop errors), so this leaks nothing and pins no worktree — it's an error-surface gap, not corruption.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

const lease =
sourceWorktreePath === undefined
? Effect.void
: worktrees.acquire(childId, sourceWorktreePath);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

acquireLease refuses only a missing row or deletingnot orphaned. An orphaned record means the directory is gone but a lease is still held (boot reconcile sets it when !existsSync(worktreePath) and the worktree is held), so this acquire succeeds and the child is then started with cwd = a path that no longer exists, failing with an opaque spawn/git error.

The resume path already has the typed guard for exactly this state:

verifyResume(sessionId) {
  const record = this.get(sessionId);
  if (!record || existsSync(record.worktreePath)) return Effect.void;
  return Effect.fail(new RequestError({ code: 'worktree_missing',
    message: `The managed worktree is missing at ${record.worktreePath}. Restore it or delete this session.` }));
}

Nothing equivalent runs on the fork path, and since this PR deletes the unsupported guard that previously blocked forking a worktree session outright, this is newly reachable: user deletes the worktree dir → daemon restarts → record marked orphaned, session keeps its lease → fork. Resuming that session gives a clean, actionable error; forking it does not.

The comment above says the captured path makes a source deleted meanwhile fail "typed conflict … instead of starting the child in a directory being torn down" — which is true for deleting, but orphaned is the case where the directory is already gone, and it falls through. Simplest fix is to reject orphaned in acquire/acquireLease alongside deleting, or to call verifyResume(source.sessionId) at admit so the failure carries worktree_missing.

Worth noting the blast radius is contained: abandon() does release the child's lease on this path, so nothing is pinned.

);
}

private worktreeGate(worktreePath: string): Semaphore.Semaphore {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: worktreeGates is never pruned, so an entry survives for the daemon's lifetime even after the worktree is removed and its record deleted. Harmless in practice (one small entry per distinct worktree path, and the pre-existing semaphores map in worktree-service.ts has the same shape), so this is consistency-with-existing-practice rather than a leak worth fixing — just flagging it in case cleanupRecord is a natural place to drop the gate too.

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