Upstream worktree selfheal

[atoomic]

Summary

Three related worktree fixes, all triggered by one production outage (92 consecutive
mission failures) plus a reaper that reported "0 reclaimed" every hour for days:

• git prep self-heals a held base branch (koan/app/git_prep.py) — git allows a
  branch in at most one worktree, so an agent-created git worktree add /tmp/base140 140
  locked the project's own checkout out of its base branch. Prep now detects the holder
  and runs git checkout --detach there (detach, never remove: files and uncommitted
  work stay put), then retries the checkout once. A locked holder is never touched.
  The first checkout error is also no longer overwritten by the checkout -b fallback's
  a branch named 'X' already exists — only the original message names the holding path.
• Pre-fetch base-branch resolution — when nothing configured a base branch and the
  hardcoded main fallback has no tracking ref, resolve the remote's real default before
  fetching instead of paying for a doomed fetch first. resolve_remote_default_branch()
  returns None on exhausted resolution, so a failed detection can never be promoted to
  an authoritative override of a configured branch.
• Bounded, non-blocking foreign-worktree sweep (koan/app/awake.py,
  koan/app/worktree_manager.py) — the hourly sweep moves to a new single-flight
  maintenance worker lane (2-minute budget, 15 s per git command, project rotation), so
  it never delays the poll loop. Detached-HEAD retention stops being permanent: a single
  bounded rev-list --max-count=1 HEAD --not --branches --tags --remotes walk replaces
  the per-ref containment scan, and a closed-PR checkout still at its creation commit is
  reclaimable. Every candidate is dirty-checked (--force removal), scope is narrowed to
  outside-the-project plus <project>/tmp/, and every retained worktree logs its reason.
• /doctor reports the same collision (koan/diagnostics/project_check.py), resolved
  locally only — no network probe outside --full. /doctor --fix performs the same
  detach.
• System prompt now forbids git worktree add outside $TMPDIR, which is where the leak
  came from.

Docs and durable specs updated: docs/operations/troubleshooting.md (sweep scope, what it
keeps, git worktree lock opt-out, auto-detach under "Branch conflicts"),
docs/architecture/daemon.md (maintenance lane), specs/components/bridge.md,
specs/components/git-github.md.

Testing

• make lint
• make test — new coverage in koan/tests/test_git_prep.py,
  koan/tests/test_worktree_manager.py (real-git: locked holder, dirty tree, unmerged PR
  head, closed PR after ref deletion, missing refs/remotes/origin/HEAD),
  koan/tests/test_diagnostics.py, koan/tests/test_awake.py,
  koan/tests/test_awake_worker_lanes.py (maintenance lane never blocks chat).

Declarations

• Architectural change — this PR modifies a durable design contract
  (specs/components/** or specs/skills/**). The new architecture needs review
  before approval. Rationale: prep may now detach a worktree holding the base branch, and
  the sweep's scope/retention contract changes (specs/components/git-github.md,
  specs/components/bridge.md).

[Koan-Bot]

Previous review — superseded by a newer review below.

[Koan-Bot]

Warning

Important issues found.

• New /doctor check can make a network call outside --full
• Docs understate the blast radius of two new destructive behaviors

[atoomic]

@Master-Koan fix

[Master-Koan]

Rebase with requested adjustments

Branch upstream-worktree-selfheal was rebased onto main and review feedback was applied.

Changes applied

• Verified branch HEAD (65498768) against review HEAD (1449a47). Five later commits already implement every finding; only the PR-body gap remained.
• Filled the PR body (Summary + Testing) — the review flagged it as the unfilled template — and checked the Architectural change box, since specs/components/bridge.md and specs/components/git-github.md change durable contracts (spec-change guard requires the declaration).

Not changed (and why)

• /doctor network call outside --full — already fixed. project_check._base_branch() (koan/diagnostics/project_check.py:172-217) resolves locally only: configured branch → _has_remote_tracking_ref() → local_remote_default_branch() → "" (caller skips). No detect_remote_default_branch() call, no ls-remote.
• Docs understate blast radius — already fixed. docs/operations/troubleshooting.md:72-100 documents scope (outside project + <project>/tmp/, everything else in-project out of scope), dirty-tree retention, per-retain log reasons, git worktree lock opt-out, and the auto-detach under "Branch conflicts". Reviewer's suggested wording ("anywhere inside except .worktrees/") is now stale: code narrowed in-project scope to IN_PROJECT_SCRATCH_DIRS = ("tmp",), and docs match code.
• HIGH — list_worktrees() silent empty return — already fixed (worktree_manager.py:322-343): logs git worktree list failed/timed out with stderr before returning [].
• MEDIUM — failure masked as benign reason — alread

Stats

15 files changed, 1933 insertions(+), 156 deletions(-)

Actions performed

• Already-solved check: skipped (JSON parse error)
• Rebased upstream-worktree-selfheal onto origin/main
• Review feedback evaluated; no changes required
• Pre-push CI check: previous run #33704493634 failed
• Pre-push CI fix: no changes needed or Claude found nothing to fix
• Force-pushed upstream-worktree-selfheal to origin
• Private review gate skipped: disabled by config
• CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.

---

Automated by Skuggi

[Koan-Bot]

Previous review — superseded by a newer review below.

[Koan-Bot]

Warning

Important issues found.

• /doctor stays green for a locked worktree holding the base branch — the one case prep cannot heal

[Koan-Bot]

Rebase with requested adjustments

Branch upstream-worktree-selfheal was rebased onto main and review feedback was applied.

Changes applied

• Done.
• /doctor now reports a locked worktree holding the base branch (koan/diagnostics/project_check.py) — non-fixable error naming git worktree unlock <path> && git -C <path> checkout --detach; --fix still leaves it untouched. Warning ＃1.
• /doctor now reports when the collision check was skipped because the base branch is not resolvable from local refs (warn + git remote set-head origin -a hint), instead of emitting nothing. Silent-failure HIGH / suggestion ＃1.
• Split _find_branch_holder() into _branch_holder_worktree() (returns the holder, locked or not) plus the detachable-only wrapper, so prep keeps its "never touch locked" behavior while the diagnostic can see it.
• _branch_holder_worktree() logs the inspection failure before returning None (koan/app/git_prep.py), so "could not look" no longer reads like "nothing holds the branch". Silent-failure MEDIUM ＃2.
• result.healed + logger.info now record the detach the moment it succeeds, before the retry checkout, so a detach followed by an unrelated checkout failure is still reported. Silent-failure MEDIUM ＃3.
• Tests: locked holder reported-but-not-detached, unresolvable base branch reported and not fixed, detach reported when retry fails, unreadable worktree list logged; updated test_base_branch_resolution_stays_local.
• Contract + docs: new specs/components/git-github.md bullet ("What prep cannot heal, /doctor MUST report") and matching `docs/operations/troubleshooti

Stats

15 files changed, 2198 insertions(+), 162 deletions(-)

Actions performed

• Already-solved check: negative (confidence=high, reasoning=No recent main commit touches git_prep base-branch holder detach, the maintenance worker lane, or th)
• Rebased upstream-worktree-selfheal onto upstream/main
• Applied review feedback
• Pre-push CI check: previous run passed
• Force-pushed upstream-worktree-selfheal to upstream
• Force-pushed upstream-worktree-selfheal to upstream
• Force-pushed upstream-worktree-selfheal to upstream
• Private review gate passed after 2 fix round(s)
• CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.

---

Automated by Skuggi

[Koan-Bot]

PR Review — Upstream worktree selfheal

Three worktree fixes — prep detaching a holder instead of dying, a bounded maintenance-lane sweep, and detached-HEAD retention that stops being permanent — landed contract-first with real-git coverage, and the previous review's blocker is genuinely resolved. What's left is a handful of non-blocking edges around scope symmetry, prune expiry, and sweep observability.

• /doctor now reports a locked holder as a non-fixable error and says when the check was skipped, matching the docs added in the same PR
• Replacing the per-ref containment scan with one rev-list --max-count=1 ... --not --branches --tags --remotes walk is both cheaper and the correct semantics; the closed-PR reflog fallback fails toward retention and is covered by real-git tests
• Every new deadline and timeout path retains the worktree, and _retain() names the reason, so retain-everything is distinguishable from nothing-to-do per worktree
• prune --expire now drops git's grace for temporarily-missing worktree directories, and applies to startup recovery too
• The out-of-project scope rule is not protected by the gitignored-files argument the spec uses to exclude in-project workspaces

---

✅ Resolved since last review (3)

Previously-flagged issues verified fixed

• koan/diagnostics/project_check.py:231 /doctor stays green for a locked worktree holding the base branch — the one case prep cannot heal
• koan/diagnostics/project_check.py:217 Collision check disables itself with no trace when the base branch can't be resolved locally
• koan/app/git_prep.py:647 [Pre-Existing Issue] The post-fetch detection path can still promote the "main" guess over a configured branch

---

🟢 Suggestions

1. New `list_worktrees()` timeout parameter is not used by the two new callers
koan/app/git_prep.py:473-475

This PR adds a timeout parameter to list_worktrees() and threads a 15 s cap through the sweep, but _branch_holder_worktree() calls it bare — and that call sits on two paths where a hang is expensive:

• prepare_project_branch(), once per mission whose base-branch checkout failed;
• /doctor, once per configured project on the default (non---full) path.

git worktree list --porcelain stats every registered worktree path, so a single registration on a dead NFS mount blocks it indefinitely. Every other git call on these paths is bounded — run_git() defaults to 30 s, and the pre-existing checks in project_check.run() pass timeout=10 explicitly — so this is the one unbounded call in an otherwise bounded module.

Passing a modest timeout (10-15 s) preserves the existing behaviour on failure: list_worktrees() now logs the timeout and returns [], and _branch_holder_worktree() treats [] as "nothing holds it", which is the same fallback prep already takes.

        from app.worktree_manager import list_worktrees
        worktrees = list_worktrees(project_path)

---

Checklist

• Error paths fail safe (retain rather than delete)
• Diagnostics surface the failure they were added for (prior blocker)
• No unbounded work on interactive paths — suggestion ＃1
• Durable-contract changes declared as architectural
• Docs stay in sync with changed user-visible behavior
• New branching logic covered by tests
• Failures are observable in aggregate, not only per-item

ℹ️ Triage summary

3 pre-existing finding(s) on unchanged code suppressed (freeze).

---

Silent Failure Analysis

🟠 **HIGH** — silent empty return masks failed inspection
koan/app/git_prep.py:466-479

Risk: list_worktrees() swallows CalledProcessError and TimeoutExpired internally and returns [], so this handler never fires for the realistic git failures, and "git could not tell us" collapses into "nothing holds the branch" — the exact conflation the docstring claims to prevent.

Traced failure: a project repo owned by a different UID makes git worktree list --porcelain exit 128 ("detected dubious ownership") -> list_worktrees() catches CalledProcessError, prints to stderr and returns [] -> branch_holder_worktree() iterates an empty list and returns None without raising, so the (ImportError, OSError) handler and its warning never run -> project_check.run() gets holder=None and appends no project_worktree CheckResult at all -> /doctor reports the project clean while git prep, also seeing None, skips the detach and every mission keeps dying on checkout failed: 'main' is already used by worktree at ....

try:
    from app.worktree_manager import list_worktrees
    worktrees = list_worktrees(project_path)
except (ImportError, OSError) as e:
    logger.warning("Could not inspect worktrees in %s ...", ...)
    return None

Fix: Have list_worktrees() signal inspection failure distinctly (raise, or return Optional[List]) so _branch_holder_worktree() can log/propagate "unknown" and /doctor can report the check as failed rather than clean.

🟡 **MEDIUM** — first error overwritten by fallback error
koan/app/git_prep.py:672-702

Risk: stderr is reassigned on every retry, so when all candidates fail the reported error names only the last branch tried — a branch nobody configured — discarding the original fetch failure, which is the same error-masking this PR adds an explicit spec invariant against on the checkout path.

for candidate in candidates:
    base_branch = candidate
    result.base_branch = candidate
    rc, _, stderr = _fetch_with_https_fallback(remote, base_branch, project_path, timeout=30)
    if rc == 0:
        break
if rc != 0:
    result.error = f"fetch failed: {stderr}"

Fix: Accumulate the per-candidate stderr (as the checkout path now does) and report all attempts, e.g. fetch failed: <original> (also tried 'X': <err>).

🟡 **MEDIUM** — silently mangled value instead of validated parse
koan/app/git_prep.py:194-206

Risk: rsplit("/", 1) truncates any default branch containing a slash (refs/remotes/origin/release/1.0 -> "1.0"), and the new /doctor consumer treats that wrong name as authoritative rather than as a parse failure.

if rc == 0 and stdout:
    # Output: refs/remotes/origin/master → extract "master"
    return stdout.strip().rsplit("/", 1)[-1] or None

Fix: Strip the known refs/remotes/<remote>/ prefix explicitly and return None when the output does not start with it, so an unparsable ref is reported as unresolvable instead of guessed.

🟡 **MEDIUM** — swallowed exception leaves a safety gate disarmed
koan/app/git_prep.py:589-612

Risk: A config-load failure logs a warning and leaves config_configured at False, which is precisely the state the new pre-fetch override treats as "nobody configured a base branch" — so a transient YAML/IO error silently converts a defaults: develop project into a remote-default project for that mission.

    config_configured = config_explicit or bool(defaults_am.get("base_branch"))
except Exception as e:
    logger.warning("config load error for base_branch: %s", e)

Fix: Treat a config-load exception as unknown rather than unconfigured — skip the pre-fetch override (and the detect-after-failure override) when the config could not be read.

🟡 **MEDIUM** — non-zero exit treated as a negative answer
koan/app/git_prep.py:438-448

Risk: Any git failure (corrupt refs, locked index, missing binary) is indistinguishable from "the ref does not exist", and the caller uses False to justify overriding the base branch.

rc, _, _ = run_git(
    "rev-parse", "--verify", "--quiet",
    f"refs/remotes/{remote}/{branch}",
    cwd=project_path,
)
return rc == 0

Fix: Distinguish rc==1 (ref absent) from other exit codes / stderr, and skip the override when the probe itself failed.

🟡 **MEDIUM** — early return on timeout skips cleanup, indistinguishable from success
koan/app/worktree_manager.py:258-262

Risk: The bare return skips git worktree prune and branch deletion and returns the same None as the success path, so if git deleted the directory before timing out the caller's os.path.exists() check counts it as reaped and the sweep logs "Reclaimed N" while a phantom registration remains.

except subprocess.TimeoutExpired:
    print(f"[worktree_manager] git worktree remove timed out for {wt}", file=sys.stderr)
    return

Fix: Return an explicit status (or raise) from remove_worktree() and have reap_foreign_worktrees() gate removed.append() on it rather than on directory existence alone.

---

Automated review by Skuggi HEAD=c5ed9d5 9 min 10s

[Koan-Bot]

Tip

No blocking issues found — ready to merge.