workdir: PR-dispatch derives branch from pr.number, conflicts with actual PR head branch #119

New issue

Closed

opened 2026-04-20 08:54:07 +00:00 by claude-desktop · 0 comments

claude-desktop commented

2026-04-20 08:54:07 +00:00

Collaborator

User story

As the operator, I want agent dispatches on a PR to use the PR's actual head branch for worktree acquisition, so that repeated address-review / merge dispatches on the same PR don't trip the "worktree on wrong branch — release first" error and get stuck.

What happens today

buildAgentRequest(agent, repo, pr.number, task) stuffs issue_number: pr.number into the TaskRequest. Downstream:

// agent-runner.ts
const branch = deriveIssueBranch(config.branch_prefix, task.issue_number);
// → "boss/115" when dispatched on PR #115

But PR #115's actual head branch is boss/26 (the issue it closes). So:

First dispatch: acquireWorktree creates a new local branch boss/115 at HEAD, in directory .../boss-default/charles__claude-hooks__boss%2F115.
Boss's skill fetches origin, checks out origin/boss/26 inside the worktree, pushes to boss/26 — work lands correctly.
The worktree now has directory name boss%2F115 but branch boss/26.
Second dispatch: acquireWorktree computes path=boss%2F115, expected branch=boss/115, finds the existing worktree — current branch is boss/26, raises [workdir] worktree at X is on branch boss/26, expected boss/115 — release first.

Hit live on PR #115 today (2026-04-20). Multiple address-review and the final handleApproved → dispatchMerge dispatch failed with this error:

[boss-default] 9d0f1a1d: acquiring worktree boss/115
[workdir] worktree at /state/worktrees/boss-default/charles__claude-hooks__boss%2F115
          is on branch boss/26, expected boss/115 — release first

I unstuck by deleting the stale worktree and manually merging via merge_pull_request. But the underlying bug will trip every subsequent multi-round PR.

Root cause

Two data models fighting:

Our branch convention — the implement skill pushes to <prefix>/<issue_number>, so boss/26 is the branch for issue #26's work. The PR is a separate numeric space (#115).
Our dispatcher — handleReviewRequested / handleChangesRequested / handleApproved all pass pr.number as issue_number through buildAgentRequest, which then feeds deriveIssueBranch and worktreePath.

The mismatch is harmless on the first dispatch (creates a throwaway branch at HEAD), becomes load-bearing after the first successful run (worktree's actual branch is now boss/26, not boss/<pr.number>).

Fix options

A — Fetch PR head.ref at dispatch time

Each PR-scoped handler already calls getPullRequest(repo, pr.number, token) in some paths. Extend buildAgentRequest to accept an optional branch override, and pass prDetail.head.ref when known. Fall back to deriveIssueBranch for issue-scoped dispatches.

Cleanest, but touches every PR-scoped handler.

B — Make `acquireWorktree` recover from branch-mismatch

If the existing worktree is on a different branch, treat that as the authoritative state (it's what git reflects) and use that branch instead of erroring. Log the discrepancy.

Simplest code change; trades strictness for resilience. Risk: masks legitimate bugs (e.g. two agents fighting over the same path).

C — Key the worktree path on issue number, not branch name

If worktreePath encoded <agent>__<repo>__issue-<number> instead of <agent>__<repo>__<branch>, PR dispatches on #115 would land in issue-115/ regardless of what branch got checked out inside. Single-branch-per-worktree would still be git's problem, but we'd stop making it worse with our own mismatched-path-derivation.

Broader refactor; changes the on-disk layout.

Acceptance criteria

Pick one of A/B/C (or a variant) based on the maintainer's judgment.
Repeat-dispatching handleChangesRequested / handleApproved on the same PR twice in a row must not produce the "is on branch X, expected Y — release first" error.
Regression test: simulate a worktree that's been checked-out to a different branch than its dir name implies; acquireWorktree either reconciles or errors with a clearer actionable message.
Log the path-derivation decision (issue_number=115 → branch=boss/26 via PR head.ref or similar) so operators can diagnose from journalctl next time.

Out of scope

Cleanup of existing stale worktrees (manual git worktree remove works; one-shot script is nice-to-have).
Rekeying historical worktree directories on disk.

References

src/workdir.ts:388 — the worktree at X is on branch Y, expected Z — release first error.
src/webhook-handlers.ts:159 — buildAgentRequest that stuffs pr.number as issue_number.
src/agent-runner.ts:481 — deriveIssueBranch(config.branch_prefix, task.issue_number).
PR #115 on 2026-04-20 — live incident (tasks 9d0f1a1d, 536f0df0, a258fe62 all failed with this error).

Dependencies

Blocked by: nothing.
Blocks: long-running multi-round PRs (any time boss authors a PR and reviewer goes multi-round, this can trip).
Branch off: main.

## User story As the **operator**, I want agent dispatches on a PR to use the PR's actual head branch for worktree acquisition, so that repeated address-review / merge dispatches on the same PR don't trip the "worktree on wrong branch — release first" error and get stuck. ## What happens today `buildAgentRequest(agent, repo, pr.number, task)` stuffs `issue_number: pr.number` into the `TaskRequest`. Downstream: ```ts // agent-runner.ts const branch = deriveIssueBranch(config.branch_prefix, task.issue_number); // → "boss/115" when dispatched on PR #115 ``` But PR #115's actual head branch is `boss/26` (the issue it closes). So: 1. First dispatch: `acquireWorktree` creates a new local branch `boss/115` at HEAD, in directory `.../boss-default/charles__claude-hooks__boss%2F115`. 2. Boss's skill fetches origin, checks out `origin/boss/26` inside the worktree, pushes to `boss/26` — work lands correctly. 3. The worktree now has directory name `boss%2F115` but branch `boss/26`. 4. Second dispatch: `acquireWorktree` computes path=`boss%2F115`, expected branch=`boss/115`, finds the existing worktree — current branch is `boss/26`, raises `[workdir] worktree at X is on branch boss/26, expected boss/115 — release first`. **Hit live on PR #115** today (2026-04-20). Multiple address-review and the final `handleApproved` → `dispatchMerge` dispatch failed with this error: ``` [boss-default] 9d0f1a1d: acquiring worktree boss/115 [workdir] worktree at /state/worktrees/boss-default/charles__claude-hooks__boss%2F115 is on branch boss/26, expected boss/115 — release first ``` I unstuck by deleting the stale worktree and manually merging via `merge_pull_request`. But the underlying bug will trip every subsequent multi-round PR. ## Root cause Two data models fighting: - **Our branch convention** — the `implement` skill pushes to `<prefix>/<issue_number>`, so `boss/26` is the branch for issue #26's work. The PR is a separate numeric space (#115). - **Our dispatcher** — `handleReviewRequested` / `handleChangesRequested` / `handleApproved` all pass `pr.number` as `issue_number` through `buildAgentRequest`, which then feeds `deriveIssueBranch` and `worktreePath`. The mismatch is harmless on the first dispatch (creates a throwaway branch at HEAD), becomes load-bearing after the first successful run (worktree's actual branch is now `boss/26`, not `boss/<pr.number>`). ## Fix options ### A — Fetch PR head.ref at dispatch time Each PR-scoped handler already calls `getPullRequest(repo, pr.number, token)` in some paths. Extend `buildAgentRequest` to accept an optional `branch` override, and pass `prDetail.head.ref` when known. Fall back to `deriveIssueBranch` for issue-scoped dispatches. Cleanest, but touches every PR-scoped handler. ### B — Make `acquireWorktree` recover from branch-mismatch If the existing worktree is on a different branch, treat that as the authoritative state (it's what git reflects) and use that branch instead of erroring. Log the discrepancy. Simplest code change; trades strictness for resilience. Risk: masks legitimate bugs (e.g. two agents fighting over the same path). ### C — Key the worktree path on issue number, not branch name If `worktreePath` encoded `<agent>__<repo>__issue-<number>` instead of `<agent>__<repo>__<branch>`, PR dispatches on #115 would land in `issue-115/` regardless of what branch got checked out inside. Single-branch-per-worktree would still be git's problem, but we'd stop making it worse with our own mismatched-path-derivation. Broader refactor; changes the on-disk layout. ## Acceptance criteria - [ ] Pick one of A/B/C (or a variant) based on the maintainer's judgment. - [ ] Repeat-dispatching `handleChangesRequested` / `handleApproved` on the same PR twice in a row must not produce the "is on branch X, expected Y — release first" error. - [ ] Regression test: simulate a worktree that's been checked-out to a different branch than its dir name implies; `acquireWorktree` either reconciles or errors with a clearer actionable message. - [ ] Log the path-derivation decision (`issue_number=115 → branch=boss/26 via PR head.ref` or similar) so operators can diagnose from `journalctl` next time. ## Out of scope - Cleanup of existing stale worktrees (manual `git worktree remove` works; one-shot script is nice-to-have). - Rekeying historical worktree directories on disk. ## References - `src/workdir.ts:388` — the `worktree at X is on branch Y, expected Z — release first` error. - `src/webhook-handlers.ts:159` — `buildAgentRequest` that stuffs `pr.number` as `issue_number`. - `src/agent-runner.ts:481` — `deriveIssueBranch(config.branch_prefix, task.issue_number)`. - PR #115 on 2026-04-20 — live incident (tasks `9d0f1a1d`, `536f0df0`, `a258fe62` all failed with this error). ## Dependencies - **Blocked by:** nothing. - **Blocks:** long-running multi-round PRs (any time boss authors a PR and reviewer goes multi-round, this can trip). - **Branch off:** `main`.

claude-desktop added the

area:workdir

type:bug

labels

2026-04-20 08:54:12 +00:00