Skip to content

Deepen Worktree ownership of safe Issue branch acquisition #485

Closed

Description

@JacobStephens2

What to build

Give the Worktree module ownership of acquiring an Issue branch without replacing local-only work. A Run or Spec run must be able to ask for a fresh checkout or a Continuation through the existing Worktree interface without first arranging a safety check elsewhere.

Today Issue branch selection checks the local head, then Run startup settles the Base branch and Claim before Worktree takes its lock, fetches again and resets the local branch. A local commit or origin change in that gap invalidates the earlier check. Keep the current early refusal and ordinary Claim behavior, while making acquisition independently safe.

Acceptance criteria

  • Fresh and Continuation acquisition independently enforce local Issue branch safety through the Worktree interface, including when called without Issue branch selection.
  • Acquisition holds the existing worktree lock through fetching, snapshotting the starting commit, reading and validating the local branch, and adding the worktree. Interruption retains the existing ordinary-execution behavior.
  • A fresh acquisition refuses any existing local copy of its chosen Issue branch. A Continuation allows an absent local branch or an existing branch exactly equal to the newly fetched origin head; any different head is refused, including a local ancestor when origin advanced after preflight.
  • A Continuation with an equal existing local branch attaches to that branch without resetting it. With no local branch, it creates one without force at the pinned origin commit. Fresh acquisition similarly creates without force at a pinned Base branch commit.
  • No acquisition command forcibly resets or overwrites an existing local branch. Refusal or add failure preserves every pre-existing local branch head and every pre-existing worktree or occupied path.
  • The local-head comparison and refusal diagnostics have one implementation owned by Worktree. Issue branch selection delegates its existing advisory preflight to that rule, preserving early refusal before a Claim for already-visible mismatches.
  • A mismatch discovered only during acquisition fails before any session or Delivery starts and follows the existing acquisition-failure Claim path; no new Claim policy is introduced.
  • Preserve Issue branch numbering, Continuation PR/Base branch precedence, sibling worktree naming, normal cleanup and Failed run preservation. Keep existing progress wording identifying the source as origin and preserve the two local-branch refusal messages.
  • Real-Git tests through the Worktree interface cover direct divergence, a local commit added after successful preflight, origin movement after preflight, equal and absent local branches, and occupied-path refusal. Assert exact retained refs and filesystem outcomes.
  • Retain CLI coverage for ordinary mismatch refusal and successful fresh, numbered and continued Runs, including Spec run Ticket checkout and Claim behavior. Replace superseded internal safety tests rather than duplicating them.
  • Acquisition comments describe Worktree's own guarantee and no longer require callers to run selection first. No new domain term or ADR is required.
  • Run focused Worktree, Issue branch selection and Continuation tests, then the repository's formatting, clippy and full test checks.

Blocked by

None (can start immediately).

Decisions

The design tree was settled in rounds: existing semantics → ownership → acquisition and failure behavior → tests and scope. Every branch is settled.

  1. Keep existing Issue branch semantics. ADR 0002 still decides fresh versus Continuation from remote branches and PR history, and a Continuation's open PR still chooses the Base branch. The friction concerns safe local acquisition, not that decision.
  2. Deepen the existing Worktree module and keep its caller interface. Both acquisition entry points own safety, so standalone Runs, Spec runs and direct tests get the same guarantee without a new ordering prerequisite.
  3. Own one refusal rule in Worktree. Given the sampled local head and expected origin head, the rule accepts absence, accepts exact equality for a Continuation, and refuses every other existing local branch with the existing diagnostics. Sharing it with selection preserves locality without two definitions of safety.
  4. Retain selection's early preflight as advisory. Already-visible mismatches currently fail before a Claim is made. Removing that check would leave a Continuation claimed after its refusal because its Issue branch already exists on origin. Delegation preserves this behavior; callers do not rely on preflight for acquisition safety.
  5. Validate again under the existing acquisition lock after fetching. The selection sample can become stale during Base branch and Claim work or while waiting for the lock. The acquisition decision must use the state it is about to acquire.
  6. Pin the commit once. Resolve the fetched starting ref to one SHA and use that value for comparison and any new local branch creation. Other Runs can move shared remote-tracking refs during fetches; a second interpretation of the ref must not change the selected starting commit.
  7. Attach an equal existing branch without any reset. Use Git's existing-branch checkout form; create an absent branch with the non-forcing new-branch form at the pinned commit. An advisory file lock does not serialize unrelated manual Git writes, so merely moving a check before a forced reset would leave the same failure possible.
  8. Keep conservative equality rather than accepting ancestors. An origin move after preflight can make the local branch differ even without new local work. Refuse that case consistently with the current rule rather than introducing a new policy for replacing local refs.
  9. Leave occupied paths and checked-out branches to Git's non-forcing refusal. Never reset a local ref in preparation for an add that may fail. Preserve acquisition errors and avoid invoking normal Worktree Drop cleanup when no Worktree was acquired.
  10. Keep current partial new-branch failure behavior. Native Git may create a previously absent branch before an add fails. Leave that new ref in place, as acquisition currently does; this Ticket does not add rollback or sweep paths and refs. The guarantee is that acquisition never replaces or deletes pre-existing local work.
  11. Preserve surrounding lifecycle behavior. Fresh/Continuation preflight ordering, late-refusal Claim ending, normal cleanup, Failed run preservation, instruction linking and Delivery remain on their existing paths. Keep source labels in progress lines even when the Git command uses a pinned SHA.
  12. Use the existing local-substitutable test seam. Real Git, a temporary launch clone and a bare local origin are already available as test adapters. Tests cross the same Worktree interface as callers; no new external port or mock adapter is needed.
  13. Test observable preservation and interleavings. Assert branch SHA, worktree existence, contents of occupied paths and refusal results; use controlled barriers or callbacks for late changes rather than timing guesses. Keep branch-selection tests for selection and advisory preflight, and migrate any safety-only assertions superseded by acquisition tests.
  14. Publish one standalone Ticket. The complete path changes one ownership decision across acquisition, advisory preflight and their tests; it fits a fresh context window and needs no prefactor or blocker. Existing terminology and ADRs suffice, and the implementation choice is reversible, so no CONTEXT or ADR change is part of this work.

Validation from this review

Reviewed main at 39c846eac30ec831e894647032bb6542b7b136a1. A real-Git regression through the Worktree interface failed on that head: acquisition replaced a local-only commit with origin's older head. A disposable prototype passed all 19 Worktree tests (including five acquisition probes), all 20 Issue branch selection tests, and cargo fmt --check. Separate Git 2.43.0 experiments confirmed that forced reset happens even before occupied-path refusal, while non-forcing attachment preserves existing refs. This verifies feasibility; implementation and the full required checks remain this Ticket's work.

Architecture review

Strength: Strong · Dependencies: local-substitutable

Files: src/worktree.rs, src/branch.rs, src/run.rs, tests/continuation.rs

Before

flowchart LR
  R["Run / Spec run"] --> B["Issue branch selection module<br/>earlier local-head check"]
  R --> W["Worktree module<br/>lock → fetch → forced reset → add"]
  B -. "safety depends on caller ordering" .-> W
  W --> G["Git adapter"]
  T["Tests"] --> B
  T --> W
  classDef leak stroke:#dc2626,stroke-width:2px;
  class B,W leak;
Loading

After

flowchart LR
  R["Run / Spec run"] --> B["Issue branch selection module<br/>naming and advisory preflight"]
  R --> W["Worktree module<br/>lock → pin → validate → acquire"]
  B -->|"shared refusal rule"| W
  W --> G["Git adapter"]
  T["Tests"] --> B
  T --> W
  classDef deep stroke:#2563eb,stroke-width:4px;
  class W deep;
Loading

Problem: Worktree's interface relies on a caller's earlier check across a seam, allowing a later forced reset to erase local Issue branch work even when worktree creation fails.

Solution: Increase Worktree's depth by owning acquisition safety behind its existing interface, sharing its advisory refusal rule and using Git without resetting existing branches.

Wins

  • Locality keeps validation beside acquisition
  • Leverage covers Runs and Spec runs
  • Existing branch heads stay untouched
  • Tests cross the acquisition interface

Deletion test: Removing this ownership would put safety knowledge back across callers or restore forced resets.

Coverage: No open issue proposes this deepening; #120 and #53 concern error guidance. ADR 0002 is preserved.

Activity

  1. added
    needs-triageMaintainer needs to evaluate this issue
    ready-for-agentFully specified, ready for an AFK agent
    architect-planAn Architect plan: the Spec or Ticket an Architecture review published
    in-progressA Claim: thirdshift has taken this issue
    and removed
    needs-triageMaintainer needs to evaluate this issue
    ready-for-agentFully specified, ready for an AFK agent
    on Oct 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    architect-planAn Architect plan: the Spec or Ticket an Architecture review published

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions