feat(daemon): opt-in per-task worktree isolation for local_directory tasks#5496
Open
fsafey wants to merge 3 commits into
Open
feat(daemon): opt-in per-task worktree isolation for local_directory tasks#5496fsafey wants to merge 3 commits into
fsafey wants to merge 3 commits into
Conversation
|
@fsafey is attempting to deploy a commit to the IndexLabs Team on Vercel. A member of the Team first needs to authorize it. |
…(VWO-367) Replace whole-task serialization of local_directory tasks with per-task git worktree isolation, gated behind a per-resource opt-in so the in-place contract general operators rely on (ADR-0019) is unchanged by default. Today a local_directory project_resource binds a task's WorkDir to a user's live checkout and the daemon holds a whole-task path mutex (LocalPathLocker, keyed on the checkout path) for its entire lifetime, because sidecar writes + the git index + cleanup all mutate that one shared tree. For a fleet whose agents all target one checkout, that mutex collapses their designed concurrency to strictly one task at a time. With resource_ref.isolate=true the daemon instead cuts a per-task worktree off the same repository (`git worktree add -b multica/worktree/<shortTaskID> HEAD`): own working tree, own index, sidecars contained in the disposable worktree. Two tasks on one checkout no longer share an index or a sidecar dir, so the whole-task mutex is skipped; only the brief `git worktree add`/`remove` critical sections are serialised (per-source-repo, in-process). Cleanup is idempotent — deferred `git worktree remove --force` + branch delete + prune on the graceful path; GC plus the next task's opportunistic `git worktree prune` reclaim crash orphans. - execenv/local_worktree.go: the worktree lifecycle (create/remove/prune-orphan), reusing the existing execenv git helpers; + full real-git/real-process tests covering concurrent distinct index, provider-failure/cancel cleanup, daemon crash + orphan recovery, repeated cleanup, rebase-conflict surfacing, and no-writes-to-source. - execenv.Prepare: PrepareParams.Isolate; cut the worktree before context files land, so sidecars go inside the isolated tree. env.IsolatedWorktree drives teardown; env.LocalDirectory stays false for isolated tasks. - daemon: skip the whole-task path mutex when the resource opts in; thread Isolate into PrepareParams; defer worktree teardown; Environment.Cleanup reclaims the worktree via git before any RemoveAll. - handler + daemon localDirectoryRef: `isolate` bool (JSONB, no migration) on BOTH structs so the server re-marshal doesn't strip it. Rollout: composes with VWO-365 (single-daemon owner) for the two-daemon case; single-daemon safe on its own. Same-passage serialisation + the shared book/glossary flock are the pub-workstream reducer's job, not the worktree's. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
PruneOrphanLocalWorktrees was written+tested but unreachable from production, so the crash path reclaimed dangling worktree registry entries (via the lightweight prune at next-task creation) but never deleted orphan multica/worktree/* branches - they would accumulate. Extract reapOrphansLocked (prune registry + delete non-live worktree branches) and call it opportunistically when creating the next task's worktree. The fleet runs many tasks per repo, so a crashed task's orphan is fully reaped within one task cycle with no GC wiring. Safe against live worktrees: prune only drops missing-dir entries, reap skips checked-out branches. New test TestIsolated_NextTaskReapsCrashOrphanBranchAndEntry proves both halves. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Close the three review gaps between the isolation module and its integration: - execenv: Prepare-level tests prove the wiring, not just the module — Isolate=true yields WorkDir=envRoot/workdir as a real worktree of the source (shared common gitdir, own index), sidecars land inside the isolated tree and never the source, LocalDirectory=false, and Environment.Cleanup reclaims the registry entry; Isolate on a non-git dir fails closed; the in-place default is byte-for-byte unchanged (WorkDir=user path, LocalDirectory=true). - daemon: an isolate:true resource skips the whole-task path mutex — proceeds immediately while another task HOLDS the lock, holds nothing itself, and the in-place control still locks. - handler: validateLocalDirectoryRef round-trips isolate=true through its re-marshal and keeps isolate=false omitted, so existing rows' payloads are byte-identical and a struct edit can't silently strip the opt-in. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
3131b69 to
f2159cb
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Opt-in per-task worktree isolation for
local_directory(VWO-367)Rebased on current
main(v0.4.3). Mergeable on its own merits: the default path is byte-for-byte unchanged, and the new flag is inert until a resource sets it. It does not depend on #5494 — see the deployment note at the end for how the two compose.Problem
A
local_directoryproject_resource binds a task'sWorkDirto a user's live checkout, and the daemon holds a whole-task path mutex (LocalPathLocker, keyed on the checkout realpath) for the task's entire life — sidecar writes, the git index, and cleanup all mutate that one shared tree. For a fleet whose agents all target one checkout, that collapses their designed concurrency to one task at a time. (Pre-lock versions had the opposite failure: unsynchronized tasks in one tree, producing.claude/skills/<name>-multica-Ncollision litter.)Change
Per-resource opt-in
local_directory.resource_ref.isolate(polymorphic JSONB — no migration; the field lives on bothhandler.localDirectoryRefand the daemon mirror sovalidateLocalDirectoryRef's re-marshal can't strip it).isolate:false(default): unchanged — in-place edit under the whole-task mutex. Preserves the contract of agents that must edit the user's live tree in place. Stored payloads for existing rows are byte-identical (omitempty, pinned by test).isolate:true: the daemon cuts a per-task worktree off the checkout's repository —git worktree add -b multica/worktree/<shortTaskID> <envRoot>/workdir HEAD— own working tree, own git index, sidecars contained in the disposable worktree. The whole-task mutex is skipped; only the briefgit worktree add/removecritical sections are serialized (per-source-repo, in-process; cross-process races are covered by git's own lockfiles).Enabling it: set
"isolate": truein the resource'sresource_refvia the project-resources API (or CLI). A UI toggle can follow independently.New
execenv/local_worktree.go(create / remove / prune-orphan), composed from the existing execenv git helpers. Wired intoexecenv.Prepare(worktree cut before context files land, so sidecars go inside the isolated tree), the daemon acquire path (mutex skip on opt-in),PrepareParams.Isolate, deferred teardown, andEnvironment.Cleanup.Lifecycle ownership
git worktree remove --force+ branch delete + prune on the graceful path. Crash recovery is self-healing with no GC wiring: creating the next task's worktree opportunistically reaps dangling registry entries and orphanmultica/worktree/*branches (reapOrphansLocked); a live sibling can never be touched (prune only drops missing-dir entries; checked-out branches are skipped). Idempotent everywhere.Tests (Go 1.26.1; DB-backed suites against
pgvector/pg17+cmd/migrate up, mirroring CI)go build ./...pass;go vet(daemon/execenv/handler) clean;gofmt -lclean — all on the v0.4.3 rebase.internal/handler(DB-backed),internal/daemon,internal/daemon/execenv.-raceon the isolation + acquire tests passes.TestIsolated_ConcurrentDistinctIndex; provider-failure/cancel cleanupTestIsolated_ProviderFailureLeavesNoOrphan/…RepeatedCleanupIdempotent; crash + orphan recoveryTestIsolated_CrashOrphanRecovery+TestIsolated_NextTaskReapsCrashOrphanBranchAndEntry; rebase conflictTestIsolated_RebaseConflictSurfaces; no writes to the source checkoutTestIsolated_NoWritesToSourceWorkingTree; prune preserves live worktreesTestPruneOrphan_PreservesLiveWorktree.TestPrepare_IsolateCutsWorktree/…IsolateRequiresGitRepo/…InPlaceDefaultUnchanged(execenv),TestAcquireLocalDirectoryLock_IsolateSkipsPathMutex(proceeds immediately while another task holds the lock; in-place control still locks),TestValidateLocalDirectoryRefIsolateRoundTrip(flag survives validation; default payload unchanged).TestValidateLocalPathneeds a non-root$HOMEandTestRunTask_InjectsPrivateTaskTempDirneedsIS_SANDBOX=1(both pre-existing environment guards, untouched here).main's recent MUL-4869 (reuse workdir on manual retry) is server-side only and does not interact — the daemon still skips reuse whenever alocal_directoryassignment exists.Deployment note (for operators enabling
isolateon shared checkouts)Worktree isolation removes working-tree/index/sidecar sharing between tasks of one daemon. If a machine can accidentally run two daemons against the same repository (the failure #5494 addresses), land and deploy the single-owner lock first, then opt in — the two changes compose but are independent to merge. Same-path serialization of tracked shared files across isolated worktrees (when two tasks edit the same file) reconciles through ordinary git conflicts; fleets that need stricter ordering should serialize at the workflow level.
Rollback
Set the resource back to
isolate:false(or drop the field) → the next task reverts to in-place + whole-task mutex, no redeploy. Blast radius of the change is only opted-in resources.🤖 Generated with Claude Code