Skip to content

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

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

feat(engine,daemon): worktree session leases#525
Zerlight wants to merge 4 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) ← #529. 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𝕏

Comment thread packages/host/engine/src/session/fork-service.ts
Comment thread packages/host/engine/src/session/orchestrator.ts
Copilot AI lite review requested due to automatic review settings September 10, 2026 08:14

@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.

✅ No new issues found.

Reviewed changes — the delta since my review at 09f5a40 is one new commit plus a rebase, and it closes the [!IMPORTANT] finding from that round.

  • Guarded the fork path against a vanished worktreeverifyResume(source) is now the first statement of admit's Effect.gen (fork-service.ts:179), so forking a session whose managed worktree is gone from disk fails typed worktree_missing with the same message a resume gives, instead of starting the child in a nonexistent cwd.
  • Added reproduce-first coverageengine-worktree.test.ts:620 asserts the exact reply, that no child lease was written, and that no child adapter started.
  • Rebased onto the new base tip — the three original commits were rewritten with identical content after ruocheng/code-639 gained c604564.

The chosen fix is the better of the two options I offered. Refusing orphaned in acquireLease would have missed a directory removed since the last reconcile — the row is still active until boot re-inspects it — whereas the admit-time existsSync covers both, which is exactly the reasoning in the reply. Placement matters too and is right: verifyResume runs before persistOperation, so a refused fork stores no operation and a retry after restoring the directory proceeds cleanly rather than replaying a stored failure. abandon() still releases the child's lease on every failure path.

I also confirmed the test can actually fail: reverting the verifyResume line locally makes it fail with kind: 'session.forked'. pnpm vitest run over the four worktree files is 35 passed / 4 files (34 last round, +1 new). The rebase is clean — c604564's abandon() log change survives alongside this PR's cleanupDeletedSession insertion.

One item from round 1 is still open and has no thread tracking it: the reconcile() cleanup branch moving from !hasSession && state === 'active' to plain !held means LinkCode now removes a clean, fully-pushed scanUnknown-adopted worktree it never created, and sweeps pre-existing sentinel orphan rows on the first boot after the migration. It provably can't touch dirty or unpushed work, so it isn't a bug — just worth a line in the release notes given the lease/orphan UI is deferred.

Pullfrog  | View workflow run | Using Claude Opus𝕏

@Zerlight

Copy link
Copy Markdown
Member Author

Review round (2026-09-10). The fork-of-a-missing-worktree finding is confirmed by reproduction and fixed in 2bcd53b (fix(engine)): session.fork admit runs the same disk check as resume, so a vanished directory fails typed worktree_missing instead of starting the child in it. The gate-map nit is declined on its thread (bounded, same shape as the existing per-repo semaphores, no removal hook in the orchestrator). The branch is rebased onto the updated ruocheng/code-639 (c604564); the three original commits are unchanged in content. Gates at 2bcd53b: pnpm check:ci 0 errors, pnpm test 3471 passed.

Release-note item recorded on CODE-640, as suggested: boot reconcile now removes any holder-less managed worktree that is clean and fully pushed — including directories adopted from the managed root (one boot after adoption) and pre-existing orphan rows on the first boot after the 0015 migration. cleanupRecord still refuses a dirty or unpushed tree and never passes --force.

Copilot AI 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.

🟡 Changes recommended

Session deletion is not serialized with turn admission, allowing persistence after the lease or session is released.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds durable managed-worktree leases, fork sharing, cleanup reconciliation, and per-worktree turn admission.

Changes:

  • Adds worktree_sessions persistence and migration backfill.
  • Shares leases with fork children and coordinates last-lease cleanup.
  • Gates turns, legacy input, rewrites, and automation by worktree.

Critical review note: deletion must be serialized with turn admission or revalidated before durable persistence.

File summaries
File Summary
packages/host/engine/tests/integration/worktree-service.test.ts Tests cleanup and boot reconciliation.
packages/host/engine/tests/integration/engine-worktree.test.ts Tests fork sharing and turn gating.
packages/host/engine/src/worktree/worktree-store.ts Defines lease storage behavior.
packages/host/engine/src/worktree/worktree-service.ts Manages leases, cleanup, and reconciliation.
packages/host/engine/src/session/session-input-dispatcher.ts Routes legacy turns through admission.
packages/host/engine/src/session/orchestrator.ts Adds per-worktree turn admission.
packages/host/engine/src/session/lifecycle-service.ts Integrates lease cleanup and gated turns.
packages/host/engine/src/session/fork-service.ts Leases the source worktree for fork children.
packages/host/engine/src/index.ts Integrates engine session and worktree behavior.
packages/host/engine/src/engine.ts Wires boot reconciliation.
packages/host/engine/src/__tests__/worktree-store.test.ts Tests engine worktree-store behavior.
packages/foundation/schema/src/model/worktree.ts Adds lease and deleting-state schemas.
apps/daemon/tests/integration/worktree-store.test.ts Tests daemon lease persistence and migration behavior.
apps/daemon/src/worktree-store.ts Implements SQLite lease persistence.
apps/daemon/src/index.ts Integrates daemon worktree-store behavior.
apps/daemon/src/db/schema.ts Adds the lease table and constraints.
apps/daemon/src/__tests__/worktree-store.test.ts Tests SQLite lease behavior.
apps/daemon/drizzle/meta/0015_snapshot.json Records migration 0015 metadata.
apps/daemon/drizzle/meta/_journal.json Records the migration journal update.
apps/daemon/drizzle/0015_fast_martin_li.sql Migrates and backfills worktree leases.
Review details
  • Files reviewed: 20/20 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +94 to +98
return this.worktreeGate(worktree.worktreePath).withPermit(
Effect.gen(function* () {
const siblings = worktrees.coLeaseholders(sessionId);
for (let i = 0, len = siblings.length; i < len; i++) {
const sibling = siblings[i];
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.

2 participants