Skip to content

Add worktree locking to prevent concurrent agent access#463

Open
mattpocock wants to merge 5 commits intomainfrom
implement/worktree-locking
Open

Add worktree locking to prevent concurrent agent access#463
mattpocock wants to merge 5 commits intomainfrom
implement/worktree-locking

Conversation

@mattpocock
Copy link
Copy Markdown
Owner

Implements PRD #427: file-based locking to prevent two processes from operating on the same worktree simultaneously.

Introduces a WorktreeLock module with acquire(), release(), and pruneStale(). Lock files live at .sandcastle/locks/<worktree-dir-name>.lock with JSON content ({ pid, branch, acquiredAt }). Atomic creation via O_EXCL prevents races. PID liveness checking via process.kill(pid, 0) detects stale locks from crashed processes.

Wired into createWorktree() — lock acquired after worktree creation, released unconditionally on close()/dispose. WorktreeManager.pruneStale() cleans up stale locks alongside orphaned worktree directories.

Closes #427, #428, #429, #430.

Closes #428
Closes #429
Closes #430

mattpocock and others added 3 commits April 23, 2026 16:39
…closes #428

Task: #428 (WorktreeLock module + createWorktree happy path)
PRD: part of worktree-locking issue series

Key decisions:
- acquire() uses O_EXCL atomic file creation; writes {pid, branch, acquiredAt} JSON
- release() is idempotent (swallows ENOENT)
- lock skipped on WorktreeManager reuse path (lock file already exists = another handle owns it)
- release() called unconditionally in close() before dirty check, wrapped in .catch(() => {})
- lockDir (.sandcastle/locks/) created on first acquire()

Files changed:
- src/WorktreeLock.ts (new): acquire() + release() + LockData type
- src/WorktreeLock.test.ts (new): 5 unit tests (create file, remove file, idempotent, fail if exists, create dir)
- src/createWorktree.ts: import acquire/release/basename, wire acquire after create(), wire release in close()
- src/createWorktree.test.ts: integration test (lock exists after create, gone after close)
- .changeset/worktree-lock-module.md: patch changeset

Notes: WorktreeLockError type deferred to #429 per issue spec; reuse-path skip uses existsSync TOCTOU-safe for single-process concurrency.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…429

Task: #429 (WorktreeLock contention detection)
PRD: part of worktree-locking issue series

Key decisions:
- acquire() reads lock JSON on EEXIST, checks PID liveness via process.kill(pid, 0)
- Live PID → throws WorktreeLockError with owningPid, branch, timestamp diagnostics
- Dead PID → removes stale lock file, retries atomic O_EXCL creation
- Corrupt/unreadable lock file treated as stale (removed + retried)
- Lock acquisition moved before WorktreeManager.create for named branches
  to serialize concurrent access; merge-to-head still locks after create
  (unique names, no contention possible)
- WorktreeLockError added to errors.ts following existing TaggedError pattern
- ErrorHandler.ts updated for exhaustive SandboxError handling

Files changed:
- src/errors.ts: add WorktreeLockError (TaggedError with owningPid, branch, timestamp)
- src/ErrorHandler.ts: add WorktreeLockError to formatErrorMessage switch + catchTags
- src/WorktreeLock.ts: enhance acquire() with PID liveness check + stale recovery
- src/WorktreeLock.test.ts: 3 new tests (live PID contention, dead PID recovery, concurrent race)
- src/createWorktree.ts: lock before create for named branches, release on create failure
- src/createWorktree.test.ts: concurrent lock contention integration test, updated reuse tests
- .changeset/worktree-lock-contention.md: patch changeset

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Extract the duplicated "remove stale + retry O_EXCL + handle race"
pattern into a removeStaleAndRetry helper. This eliminates:
- Duplicated retry logic for corrupt-lock and dead-PID paths
- The `lockData = undefined as unknown as LockData` type escape hatch
- Unnecessary `fd!` non-null assertions (fd is now properly typed)
- An inline `import("node:fs/promises").FileHandle` type import

No behavior change — all existing tests pass.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 24, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
sandcastle Ignored Ignored Apr 24, 2026 6:30pm

mattpocock and others added 2 commits April 24, 2026 18:26
#430

Task: #430 (pruneStale lock cleanup)
PRD: part of worktree-locking issue series (#427)

Key decisions:
- pruneStale(lockDir, activeWorktreeNames) scans .sandcastle/locks/ for .lock files
- Orphaned locks (worktree not in active set) → removed
- Dead-PID locks (active worktree but owning process dead) → removed
- Live locks (active worktree + live PID) → preserved
- Missing lockDir handled gracefully (ENOENT → no-op)
- Wired into WorktreeManager.pruneStale() after orphaned directory cleanup
- Active worktree names derived from git worktree list basenames

Files changed:
- src/WorktreeLock.ts: add pruneStale() export, add readdir import
- src/WorktreeLock.test.ts: 4 unit tests (orphaned, dead PID, live, missing dir)
- src/WorktreeManager.ts: import pruneStaleLocks, call after directory cleanup
- src/WorktreeManager.test.ts: integration test (stale lock from simulated crash)
- .changeset/prune-stale-locks.md: patch changeset

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ir missing

The early return on missing .sandcastle/worktrees/ also skipped lock
file pruning. Move the git worktree list fetch before the directory
check and convert the early return to a conditional block so lock
pruning always runs.

Also mark LockData fields readonly for consistency with the rest of the
codebase.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@mattpocock mattpocock marked this pull request as ready for review April 24, 2026 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant