Skip to content

source-control: worktree cleanup Step 4a stops processes before the 4b guards decide the worktree is kept #7312

Closed

Description

@kyle-sexton

Problem

context/cleanup.md orders Step 4 as 4a (release OS file locks: stop build servers, worktree-rooted daemons and MCP servers) → 4b (guards: stranded work, claim/stale-lock proof) → reap → remove. So a candidate's processes are stopped before the guards decide whether it is removed at all. When a guard then keeps the worktree, its live processes are already dead.

Observed

Cross-repo cleanup of 82 candidates (WSL2, source-control 1.0.3). Candidate melodic-software-claude-code-plugins-feat-interview-web-view-review was locked by a live lane; worktree-claim.sh stale exited 1 (transcript active within 120 min), so the row was correctly kept. But Step 4a had already sent SIGTERM to that worktree's planning surface server (plugins/planning/surface/server). The owning session lost its running server with no notice.

Expected

Run every keep/remove guard first (Work verdict, dirty check, lock and worktree-claim.sh stale proof, .worktreeinclude carried-file check), and only on a row that will be removed run 4a, then the Workspace down, reap, and removal. Reorder to: guards → 4a → down → reap → remove. A row any guard keeps must have no side effects.

Related: #6007 / #6363 (reap rewrites tracked .claude/settings.json, blocking plain removal; also hit 11 times in this run) and #7308 (cross-repo cleanup).

https://claude.ai/code/session_011er7DGtQLNer16uosSwSwA

Activity

  1. added
    needs-triageNot yet classified. Floor until a type and one priority tier are set.
    on Oct 12, 2026
  2. github-actions commented on Oct 12, 2026

    @github-actions
    Contributor

    Thanks for the detailed report. Understood: in plugins/source-control/skills/worktree/context/cleanup.md, Step 4a (release OS file locks on build servers, worktree-rooted daemons, MCP/surface servers) runs before the Step 4b keep/remove guards (Work verdict, dirty check, lock/stale-lock proof, carried-file check) evaluate the candidate. That means a worktree a guard ultimately keeps can still have its live processes killed first, as you observed with the planning surface server on a lane that was correctly kept. The expected reorder (guards → 4a → down → reap → remove) is clear and matches the "no side effects on a kept row" goal.

    No exact duplicate found; it's distinct from but related to #6007/#6363 (reap dirtying tracked settings) and #7308 (cross-repo cleanup) as you noted. No missing information from my side.

    Automated triage. A maintainer reviews it.

  3. added
    priority: highSignificant impact, or blocks an imminent release; staff this cycle.
    on Oct 12, 2026
  4. added theissue type on Oct 12, 2026
  5. removed
    needs-triageNot yet classified. Floor until a type and one priority tier are set.
    on Oct 12, 2026
  6. kyle-sexton commented on Oct 13, 2026

    @kyle-sexton
    MemberAuthor

    Decision recorded. The owner approved going with independently verified recommendations on 2026-10-13. Two researchers worked this issue separately (code and history; upstream docs and owner intent), and a judge reconciled them.

    • Is the order still 4a before the guards on origin/main, with no fix in flight? → Yes. Step 4 runs each candidate through 4a → 4b → 4c. 4a stops build servers and daemons rooted in the candidate, and 4b holds guard 1 → guard 2 → down → reap → removal. 4b already says 'down comes after the guards so a worktree they keep keeps its services'. 4a is the only side effect still ahead of the guards. Recent commits (fix(source-control): restore the tracked settings file after the worktree reap #7063, fix(source-control): run worktree status and audit outside a repository #7310) did not change the order, and no open PR addresses it.
      Basis: git show origin/main:plugins/source-control/skills/worktree/context/cleanup.md lines 61, 74-81, 96; git log origin/main on cleanup.md; gh pr list --state open
    • Is the reorder safe? → Yes, as written. The guards only read state: git status, unpushed logs, the claim proof and .worktreeinclude comparisons. 4a exists so that directory deletion is not blocked by open handles ('Before removing a candidate'). The husk path still reaches 4a before rm, and --dry-run still exits before 4a. Neither claim has been probed on Windows.
      Basis: cleanup.md:29 (husk removed only after 4a), :74-76, :96, :191 (--dry-run exits before 4a); judgment on Windows read handles
    • What to edit? → Use the order the owner stated: guard 1 → guard 2 → 4a → down → reap → removal. Update the Step 4 phase line '4a → 4b → 4c' and the guard-order sentence at :96, and keep the husk and --dry-run references consistent.
      Basis: issue source-control: worktree cleanup Step 4a stops processes before the 4b guards decide the worktree is kept #7312 body (owner kyle-sexton, Expected order); cleanup.md:61, :96

    Recommendation: Approve the C3 dispatch. Move 4a after the two guards and before down, in the order the owner stated. Update the phase line at :61 and the order sentence at :96, and keep the husk (:29) and --dry-run (:191) wording consistent.

    Where sources disagreed, decided by: Rule 2 (the owner's stated expected order in the #7312 body) and the file's own rule at cleanup.md:96 settle the answer. The C3 work class still requires the one dispatch approval, which is a gate, not an open decision.

    Admission to an agent lane stays with the work-class rules and owner ratification.

    This was generated by AI.

  7. kyle-sexton commented on Oct 13, 2026

    @kyle-sexton
    MemberAuthor

    Owner batch-ratified this admission in an attended session on 2026-10-12: work-class: scoped. Rule: class C1-C3, no trust surface, the independent check confirmed the recorded decision, and no owner step remains.

  8. added
    work-class: scopedA briefed fix or small feature; blast radius bounded by the brief, tests exist.
    agent-readyFully specified and briefed; eligible for autonomous pickup from the frontier.
    and removed
    agent-readyFully specified and briefed; eligible for autonomous pickup from the frontier.
    on Oct 13, 2026
  9. kyle-sexton commented on Oct 18, 2026

    @kyle-sexton
    MemberAuthor

    Agent Brief

    Type: Bug
    Summary: Worktree cleanup runs every keep-or-remove guard before it stops any process rooted in the candidate.

    Current behavior:
    Cleanup runs each candidate through 4a (release file locks: stop build servers, worktree-rooted daemons and MCP or surface servers), then 4b (guards, Workspace down, reap, removal). A worktree a guard then keeps has already lost its live processes.

    Desired behavior:

    • Per candidate the order is: guard 1 (stranded work) → guard 2 (lock and claim/stale proof, .worktreeinclude carried-file check) → file-lock release → Workspace down → reap → removal.
    • A row any guard keeps has no side effects: no process is stopped and nothing is reaped.
    • The husk path still releases locks before deleting, and --dry-run still exits before any process is stopped.
    • Every cross-reference to the old 4a → 4b order in the skill reads the new order.

    Key interfaces:

    • Cleanup Step 4 phase order, as documented in the skill's cleanup procedure.

    Acceptance criteria:

    • The cleanup procedure lists the guards before the file-lock release for every candidate class.
    • It states that a kept row has no side effects.
    • The husk and --dry-run statements still hold in the new order.
    • No text in the worktree skill still says 4a runs before 4b.
    • scripts/check-changelog-fragments.sh passes.

    Out of scope:


    This was generated by AI during backlog shaping. Held, not admitted: this brief is drafted and checked, but the issue lane would stop on it today. It is admitted, by adding agent-ready, once the lane can run the skill-quality check scripts (Automation Lanes' PR #7912). Held under the owner's 2026-10-18 lane-narrowing course correction.

  10. added
    needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.
    agent-readyFully specified and briefed; eligible for autonomous pickup from the frontier.
    and removed
    needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.
    on Oct 18, 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

    agent-readyFully specified and briefed; eligible for autonomous pickup from the frontier.priority: highSignificant impact, or blocks an imminent release; staff this cycle.work-class: scopedA briefed fix or small feature; blast radius bounded by the brief, tests exist.

    Type

    Fields

    No fields configured for Bug.

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions