allow resolve-and-merge follow-ups to adopt a live /claim worktree

server/lib/worktreeOwnership.js

@@ -9,7 +9,10 @@
  * see `worktreeOwnershipReason` for why the claim comes last. Callers can
  * explicitly opt into the differences that are intentional: a reaper may
  * include `.claude/worktrees/`, stale claims may be reclaimed only by branch
- * reconciliation, and only the dispatch side reads a live claim as unowned.
+ * reconciliation, and a live claim reads as unowned only for branch-reconcile's
+ * dispatch side and a non-committing coordinator follow-up (review-loop,
+ * PR-remediation) adopting the exact branch it exists to land — all three name
+ * the branch, not merely the directory.
  */
 
 import { win32 } from 'path';
@@ -86,7 +89,13 @@ export function worktreeOwnershipReason({
 
   const agentId = worktreeAgentId(path);
   const mustBeAgentWorktree = root?.requireAgentId ?? requireAgentId;
-  if (mustBeAgentWorktree && !isAgentWorktreeId(agentId)) return 'worktree-missing-agent-id';
+  // A claim-shaped id inside an agent-only root would otherwise fail here before
+  // ever reaching the human-claim check below — which is exactly the id shape
+  // `allowLiveClaim` exists to admit. Let it through to that check instead of
+  // being turned away one gate early.
+  if (mustBeAgentWorktree && !isAgentWorktreeId(agentId) && !(allowLiveClaim && isHumanClaimWorktree(agentId))) {
+    return 'worktree-missing-agent-id';
+  }
   if (locked) return 'worktree-locked';
   if (activeAgentIds instanceof Set && activeAgentIds.has(agentId)) return 'worktree-active-agent';
   if (requireKnownLiveness && isAgentWorktreeId(agentId) && !(activeAgentIds instanceof Set)) {

server/lib/worktreeOwnership.test.js

@@ -43,6 +43,16 @@ describe('worktree ownership', () => {
     // A root that demands an agent id refuses the claim basename on that ground.
     expect(worktreeOwnershipReason({ path: claim, roots: [{ path: COS_ROOT, requireAgentId: true }] }))
       .toBe('worktree-missing-agent-id');
+    // …unless the caller opted into live-claim adoption, which lets a claim
+    // basename through the agent-id root gate so the claim test below it (not
+    // this one) decides the verdict.
+    expect(worktreeOwnershipReason({
+      path: claim, roots: [{ path: COS_ROOT, requireAgentId: true }], allowLiveClaim: true
+    })).toBeNull();
+    // A non-claim, non-agent basename still can't ride that carve-out through.
+    expect(worktreeOwnershipReason({
+      path: `${COS_ROOT}/next-issue-42`, roots: [{ path: COS_ROOT, requireAgentId: true }], allowLiveClaim: true
+    })).toBe('worktree-missing-agent-id');
   });
 
   it('fails closed when agent liveness is unknown and permits an explicitly non-agent root', () => {

server/services/agentWorkspacePrep.js

@@ -41,6 +41,7 @@ import { resolveTaskTargetBranch } from '../lib/taskTargetBranch.js';
 import { resolveTaskForkHead } from '../lib/forkHead.js';
 import { getAppWorkspace, getAppDataForTask, createJiraTicketForTask } from './agentPromptBuilder.js';
 import { INVESTIGATION_TASK_DELIVERY, isInvestigationTask } from '../lib/investigationTasks.js';
+import { isNonCommittingCoordinatorTask } from './taskTypeHooks.js';
 
 const ROOT_DIR = PATHS.root;
 
@@ -101,10 +102,15 @@ async function getProtectedAgentIds() {
  *
  * `findAdoptableWorktreeForBranch` refuses every holder PortOS doesn't own
  * outright, so this can never move the user's checkout or a live agent's tree.
+ * The one caller-gated exception is `allowLiveClaim`: a non-committing
+ * coordinator follow-up (`isNonCommittingCoordinatorTask` — a review-loop
+ * resolve-and-merge or a PR-remediation follow-up) may take over a `claim-*`
+ * holder too, because its whole deliverable names that exact branch as the one
+ * to finish and land.
  *
  * @returns {Promise<{ worktreeInfo: object, adoptedFrom: string }|null>}
  */
-async function adoptWorktreeHoldingBranch({ agentId, workspacePath, branchName, preferredPath = null, taskId }) {
+async function adoptWorktreeHoldingBranch({ agentId, workspacePath, branchName, preferredPath = null, taskId, allowLiveClaim = false }) {
   // Fail CLOSED on an unreadable agent list: an empty protected set would read as
   // "nothing is running", which is the one wrong answer here — it would move a
   // live run's directory. The caller's timed pause is the safe outcome instead.
@@ -114,7 +120,7 @@ async function adoptWorktreeHoldingBranch({ agentId, workspacePath, branchName,
   });
   if (!activeAgentIds) return null;
 
-  const holder = await findAdoptableWorktreeForBranch(workspacePath, branchName, { activeAgentIds, preferredPath });
+  const holder = await findAdoptableWorktreeForBranch(workspacePath, branchName, { activeAgentIds, preferredPath, allowLiveClaim });
   if (!holder) return null;
 
   const worktreeInfo = await adoptWorktree(agentId, workspacePath, holder.path, branchName).catch(err => {
@@ -150,13 +156,24 @@ async function prepareRequestedWorktree({
   // same safe adoption path review-loop follow-ups use, rather than cutting a
   // fresh branch merely because a cached path could not be moved.
   const resumeWorktreePath = existingBranch ? task.metadata?.resumeWorktreePath : null;
+  // A review-loop / PR-remediation follow-up's whole purpose is landing THIS
+  // branch, and the user's own "resolve and merge" trigger (or pr-reviewer's own
+  // dispatch) is the signal to finish whatever a `/do:next` claim left on it — so
+  // these are the callers allowed to take over a `claim-*` holder instead of
+  // retrying against it until the task gives up (#6243). `isNonCommittingCoordinatorTask`
+  // is the shared predicate for exactly this "same shape" set (taskTypeHooks.js) —
+  // reused here rather than re-listing the flags, so a future follow-up type of the
+  // same shape inherits the carve-out too. A plain resume never targets a claim tree
+  // (its pointer names a CoS `agent-*` worktree) and doesn't set either follow-up
+  // flag, so the predicate is a no-op there.
   const takeoverPromise = existingBranch
     ? adoptWorktreeHoldingBranch({
       agentId,
       workspacePath,
       branchName: existingBranch,
       preferredPath: resumeWorktreePath,
       taskId: task.id,
+      allowLiveClaim: isNonCommittingCoordinatorTask(task),
     })
     : Promise.resolve(null);
 

server/services/agentWorkspacePrep.test.js

@@ -480,6 +480,59 @@ describe('prepareAgentWorkspace — the branch is checked out in another worktre
     expect(updateTask).not.toHaveBeenCalled();
   });
 
+  // A resolve-and-merge follow-up's whole purpose is landing THIS branch, so the
+  // user's own trigger is the signal to take over whatever a `/do:next` claim
+  // left checked out on it — instead of retrying against it until the task gives
+  // up and strands the PR the follow-up exists to land (#6243).
+  it('opts into adopting a live /claim worktree for a review-loop follow-up', async () => {
+    findAdoptableWorktreeForBranch.mockResolvedValue({ path: '/mock/worktrees/claim-issue-42', agentId: 'claim-issue-42' });
+    adoptWorktree.mockResolvedValue({
+      worktreePath: '/mock/worktrees/agent-new', branchName: 'cos/task-x/agent-y',
+      baseBranch: null, existingBranch: true, adopted: true
+    });
+
+    const r = await prepareAgentWorkspace({ agentId: 'agent-new', task: followUpTask() });
+
+    const [, , opts] = findAdoptableWorktreeForBranch.mock.calls.at(-1);
+    expect(opts.allowLiveClaim).toBe(true);
+    expect(r.outcome).toBe('ready');
+    expect(updateTask).not.toHaveBeenCalled();
+  });
+
+  // A PR-remediation follow-up is the same shape one step further out (its
+  // commits land on the contributor's own branch) and hits the identical #6243
+  // bug when that branch is checked out in a claim tree, so it carries the same
+  // carve-out via the shared isNonCommittingCoordinatorTask predicate.
+  it('opts into adopting a live /claim worktree for a PR-remediation follow-up', async () => {
+    findAdoptableWorktreeForBranch.mockResolvedValue({ path: '/mock/worktrees/claim-issue-99', agentId: 'claim-issue-99' });
+    adoptWorktree.mockResolvedValue({
+      worktreePath: '/mock/worktrees/agent-new', branchName: 'contributor/fix-thing',
+      baseBranch: null, existingBranch: true, adopted: true
+    });
+    const task = {
+      id: 't-remediation', taskType: 'internal',
+      metadata: { useWorktree: true, prRemediationFollowUp: true, existingBranch: 'contributor/fix-thing' }
+    };
+
+    const r = await prepareAgentWorkspace({ agentId: 'agent-new', task });
+
+    const [, , opts] = findAdoptableWorktreeForBranch.mock.calls.at(-1);
+    expect(opts.allowLiveClaim).toBe(true);
+    expect(r.outcome).toBe('ready');
+  });
+
+  // A plain resume never targets a claim tree — its pointer names a CoS
+  // `agent-*` worktree — so it must not carry the same carve-out.
+  it('does not opt into live-claim adoption for a plain resume', async () => {
+    findAdoptableWorktreeForBranch.mockResolvedValue(null);
+    const task = { id: 't-resume', taskType: 'user', metadata: { useWorktree: true, existingBranch: 'cos/t-resume/agent-dead' } };
+
+    await prepareAgentWorkspace({ agentId: 'agent-new', task });
+
+    const [, , opts] = findAdoptableWorktreeForBranch.mock.calls.at(-1);
+    expect(opts.allowLiveClaim).toBe(false);
+  });
+
   // Adoption MOVES the directory, so the protected set has to cover every agent
   // that still needs its tree — including a PAUSED one, whose worktree is
   // deliberately preserved as resume context and which is absent from the

server/services/worktreeManager.js

@@ -584,7 +584,8 @@ async function createWorktreeUnlocked(agentId, sourceWorkspace, taskId, options
  *   - the primary checkout, or any tree outside `data/cos/worktrees/` — moving
  *     the user's own checkout out from under them is exactly the branch-jacking
  *     guarded against everywhere else;
- *   - a human `/claim` tree (`claim-*`), owned by the claim flow's cleanup;
+ *   - a human `/claim` tree (`claim-*`), owned by the claim flow's cleanup —
+ *     unless the caller passes `allowLiveClaim` (see below);
  *   - a tree whose agent is still running — it is mid-edit in that directory;
  *   - a locked worktree, whose lock means "don't touch" regardless of owner.
  *
@@ -593,11 +594,21 @@ async function createWorktreeUnlocked(agentId, sourceWorkspace, taskId, options
  * @param {object} [options]
  * @param {Set<string>} [options.activeAgentIds] - agents currently running
  * @param {string} [options.preferredPath] - cached holder path to validate first
+ * @param {boolean} [options.allowLiveClaim=false] - treat a `claim-*` holder as
+ *   adoptable rather than off-limits. The claim flow keeps no durable agent id
+ *   for its branch, so this can't distinguish an idle claim tree from a live
+ *   `/do:next` session in it — pass it only for a task whose whole purpose IS
+ *   that exact branch (a review-loop resolve-and-merge follow-up, or a PR-
+ *   remediation follow-up — `isNonCommittingCoordinatorTask` in taskTypeHooks.js),
+ *   where the task's own deliverable is the signal that the claim's work should
+ *   be finished and landed. Mirrors `branchReconcile.resolveLiveOwnerReason`'s
+ *   dispatch-side carve-out for the same directory shape (#6243).
  * @returns {Promise<{ path: string, agentId: string }|null>}
  */
 export async function findAdoptableWorktreeForBranch(sourceWorkspace, branchName, {
   activeAgentIds = new Set(),
   preferredPath = null,
+  allowLiveClaim = false,
 } = {}) {
   if (!sourceWorkspace || !branchName) return null;
 
@@ -618,6 +629,7 @@ export async function findAdoptableWorktreeForBranch(sourceWorkspace, branchName
     activeAgentIds,
     roots: [{ path: WORKTREES_DIR, requireAgentId: true }],
     requireKnownLiveness: true,
+    allowLiveClaim,
   });
   if (ownershipReason) return null;
 

server/services/worktreeManager.test.js

@@ -832,6 +832,17 @@ describe('findAdoptableWorktreeForBranch (take over the tree that holds the bran
     expect(await findAdoptableWorktreeForBranch(REPO, BRANCH)).toBeNull();
   });
 
+  // A review-loop resolve-and-merge follow-up is the one caller that names this
+  // exact branch as the thing it exists to finish and land, so it opts in via
+  // `allowLiveClaim` instead of retrying `git worktree add` against a branch a
+  // `/do:next` claim already holds (#6243).
+  it('adopts a human /claim worktree when the caller opts into allowLiveClaim', async () => {
+    scriptWorktrees([{ path: cosTree('claim-issue-42'), branch: `refs/heads/${BRANCH}` }]);
+
+    expect(await findAdoptableWorktreeForBranch(REPO, BRANCH, { allowLiveClaim: true }))
+      .toEqual({ path: cosTree('claim-issue-42'), agentId: 'claim-issue-42' });
+  });
+
   it('refuses a non-agent directory in the managed root', async () => {
     scriptWorktrees([{ path: cosTree('next-issue-42'), branch: `refs/heads/${BRANCH}` }]);