From 40e19c22623c78db6b3d8fbc2e7753dee5b7be1e Mon Sep 17 00:00:00 2001 From: NagyVikt Date: Tue, 9 Jun 2026 22:30:49 +0200 Subject: [PATCH 1/2] fix(locks): see claims written by a submodule under a linked worktree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `git worktree list` inside a submodule that lives in a LINKED worktree reports the gitdir (/.git/worktrees//modules/) as the worktree path, not the actual working tree. load_all_locks only read lock files from those reported roots, so validate could not see claims the same checkout had just recorded — agent commits in such submodules were always blocked ("staged files must be safely claimed"). Always include the caller's resolved repo_root in the roots list. Regression test: parent repo + submodule + linked worktree; claim then validate from inside the worktree's submodule (fails without the fix, 11/11 suite pass with it). --- templates/scripts/agent-file-locks.py | 11 +++++++++- test/agent-file-locks.test.js | 30 +++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/templates/scripts/agent-file-locks.py b/templates/scripts/agent-file-locks.py index f563747..a2b8a4b 100755 --- a/templates/scripts/agent-file-locks.py +++ b/templates/scripts/agent-file-locks.py @@ -230,7 +230,16 @@ def load_all_locks(repo_root: Path) -> dict[str, list[dict[str, Any]]]: disk, so a file's full ownership is only visible by reading them all. With a single worktree this is exactly that worktree's own locks (unchanged).""" merged: dict[str, list[dict[str, Any]]] = {} - for root in list_worktree_roots(repo_root): + roots = list_worktree_roots(repo_root) + # Submodules checked out inside a LINKED worktree report their gitdir + # (/.git/worktrees//modules/) as the worktree path in + # `git worktree list`, not the actual working tree. The caller's own + # repo_root (resolved via --show-toplevel) is the real working tree where + # claims are written, so always include it — otherwise validate never sees + # the claims it itself just recorded. + if repo_root not in roots: + roots.append(repo_root) + for root in roots: try: state = load_state(root) except LockError: diff --git a/test/agent-file-locks.test.js b/test/agent-file-locks.test.js index a9cab4d..1d61474 100644 --- a/test/agent-file-locks.test.js +++ b/test/agent-file-locks.test.js @@ -168,6 +168,36 @@ defineSpawnSuite('agent-file-locks cross-worktree (G2)', () => { assert.match(v1.stderr, /another owner/); }); + test('claims in a submodule under a LINKED worktree are visible to validate (gitdir-root quirk)', () => { + // A submodule checked out inside a linked worktree reports its gitdir + // (/.git/worktrees//modules/) as the worktree path in + // `git worktree list`, so load_all_locks used to miss the REAL working + // tree's lock file — validate rejected claims it had just recorded. + const subSrc = makeRepo(); + const parent = makeRepo(); + assert.equal( + runHumanCmd('git', ['-c', 'protocol.file.allow=always', 'submodule', 'add', subSrc, 'sub'], parent).status, + 0, + 'submodule add must succeed', + ); + assert.equal(runHumanCmd('git', ['commit', '-m', 'add submodule'], parent).status, 0); + + const wtSub = path.join(parent, '..', 'wt-sub'); + assert.equal(runHumanCmd('git', ['worktree', 'add', '-q', '-b', 'agent/sub/lane', wtSub], parent).status, 0); + assert.equal( + runHumanCmd('git', ['-c', 'protocol.file.allow=always', 'submodule', 'update', '--init'], wtSub).status, + 0, + 'submodule init inside the linked worktree must succeed', + ); + + const subWt = path.join(wtSub, 'sub'); + writeFile(subWt, 'sub-file.txt'); + assert.equal(locksAt(subWt, ['claim', '--branch', 'agent/sub/lane', 'sub-file.txt']).status, 0); + + const v = locksAt(subWt, ['validate', '--branch', 'agent/sub/lane', 'sub-file.txt']); + assert.equal(v.status, 0, `validate must see the claim it just wrote: ${v.stderr}`); + }); + test('a lane can still claim + commit a file no other worktree owns', () => { const repoDir = makeRepo(); writeFile(repoDir, 'mine.txt'); From 8d59785de7815efb9839c72e393f1d627dbfd7e3 Mon Sep 17 00:00:00 2001 From: NagyVikt Date: Tue, 9 Jun 2026 22:39:02 +0200 Subject: [PATCH 2/2] refactor(locks): move the repo_root guard into list_worktree_roots Review follow-up: cmd_status iterates list_worktree_roots directly, so the gitdir-quirk fix in load_all_locks left `gx locks status` blind to the same submodule-under-linked-worktree claims. Hoist the guard into list_worktree_roots so every caller (claim conflict scan, validate, status) sees the caller's own lock file; load_all_locks drops its local copy of the guard. --- templates/scripts/agent-file-locks.py | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/templates/scripts/agent-file-locks.py b/templates/scripts/agent-file-locks.py index a2b8a4b..b078210 100755 --- a/templates/scripts/agent-file-locks.py +++ b/templates/scripts/agent-file-locks.py @@ -210,8 +210,13 @@ def staged_changes(repo_root: Path) -> list[tuple[str, str]]: def list_worktree_roots(repo_root: Path) -> list[Path]: - """Every worktree path for this repo (porcelain). Falls back to [repo_root] - when git cannot enumerate, so single-checkout behavior is unchanged.""" + """Every worktree path for this repo (porcelain), ALWAYS including the + caller's own repo_root. Submodules checked out inside a LINKED worktree + report their gitdir (/.git/worktrees//modules/) as the + worktree path, not the actual working tree — repo_root (resolved via + --show-toplevel) is where claims are written, so it must be in the list + or claim/validate/status never see this checkout's own lock file. Falls + back to [repo_root] when git cannot enumerate.""" try: out = run_git(['worktree', 'list', '--porcelain'], cwd=repo_root) except LockError: @@ -221,7 +226,9 @@ def list_worktree_roots(repo_root: Path) -> list[Path]: for line in out.splitlines(): if line.startswith('worktree '): roots.append(Path(line[len('worktree '):].strip()).resolve()) - return roots or [repo_root] + if repo_root not in roots: + roots.append(repo_root) + return roots def load_all_locks(repo_root: Path) -> dict[str, list[dict[str, Any]]]: @@ -230,16 +237,7 @@ def load_all_locks(repo_root: Path) -> dict[str, list[dict[str, Any]]]: disk, so a file's full ownership is only visible by reading them all. With a single worktree this is exactly that worktree's own locks (unchanged).""" merged: dict[str, list[dict[str, Any]]] = {} - roots = list_worktree_roots(repo_root) - # Submodules checked out inside a LINKED worktree report their gitdir - # (/.git/worktrees//modules/) as the worktree path in - # `git worktree list`, not the actual working tree. The caller's own - # repo_root (resolved via --show-toplevel) is the real working tree where - # claims are written, so always include it — otherwise validate never sees - # the claims it itself just recorded. - if repo_root not in roots: - roots.append(repo_root) - for root in roots: + for root in list_worktree_roots(repo_root): try: state = load_state(root) except LockError: