feat(dev): lab — unified teardown + worktree/branch cleanup #140

Merged

dominik.polakovics merged 1 commit from afk/135 into main

2026-06-08 10:37:52 +02:00

dominik.polakovics commented

2026-06-08 02:10:44 +02:00

Owner

Slice 2/3 of the per-instance-worktrees design (ADR-0017). Collapses every teardown onto one guarded rule and adds the cleanup sweeps so parked worktrees/branches don't leak.

Stacked on #139 — read first

This is blocked by #134 and built on its branch: the PR targets afk/134 (slice 1, PR #139), so the diff shows only slice 2. Merge #139 first, then either merge this into afk/134 or retarget it to main. Basing on main would have mixed slice 1's ~1k-line refactor into this diff and risked double-applying it.

What changed

One guarded teardown everywhere. The AFK reaper now routes both success and failure through the shared teardownInstance → teardownGuarded (dirty → keep worktree+branch; clean → remove worktree, delete branch iff merged), replacing slice 1's remove-on-success / keep-on-failure. Manual Stop, startup reconciliation, and the runtime sweep use the same rule. AFK keeps only its outcome accounting (consecutive-failure counter + budget-clock neutrality); a manual AFK Stop stays neutral (keeps worktree + afk/<N> branch, ADR-0013).
Startup reconciliation (reconcile.go): every orphan worktree (a lab//afk/ worktree with no live session) gets the guarded teardown; every merged dangling lab//afk/ branch is deleted — finally GC'ing the afk/<N> claim branches lab kept forever. Runs synchronously before the schedulers, so no Start races it.
Throttled runtime sweep: piggybacks the reaper loop on afkSweepInterval (10 min, not every 30s tick); best-effort git fetch per project, then merged-only GC of lab//afk/ branches + their clean worktrees. Never touches dirty/unmerged.
git seam: Fetch, Worktrees (porcelain parse), Branches.
Concurrency guard: a mid-Start instance is registered in a starting set before its worktree exists and cleared once its session is live, so the sweep can't remove a not-yet-live worktree out from under an in-flight Start (a fresh branch reads as "merged"). Without it the sweep would introduce a data-loss race (caught in adversarial review).
Docs: ADR-0007 superseding note (reaper now guard-tears-down on both outcomes); ADR-0017 rollout marks slice 2 done.

Acceptance criteria

Stop, the reaper (success+failure), startup reconciliation, and the runtime sweep all route through one guarded-teardown helper.
After a simulated crash/reboot (sessions gone, worktrees on disk), startup removes clean orphans and keeps dirty ones — TestReconcileWorktrees_realGitCrashRecovery (real git).
Merged lab/ and afk/ branches + their clean worktrees are deleted at runtime; dirty/unmerged are never auto-removed — TestSweepProject.
AFK still reaps on PR/death/timeout, counts failures, and treats a manual Stop as neutral.
ADR-0007 superseding note added; go test ./... passes.

Verification

gofmt, go vet, go build, go test -race ./... all pass locally (lab Go is not built by the eval-only pre-commit). The pre-commit fw eval passed on commit.

Note (pre-existing, self-healing)

A failed git worktree add -b can leave a branch at the base commit with no worktree; the Start rollback doesn't delete it. Since such a branch sits at origin/<default> it reads as merged, so the new sweep GCs it within a sweep interval. Pre-existing (slice 1), out of scope here, and now self-heals via this PR's sweep — flagged for transparency.

Closes #135

Slice 2/3 of the per-instance-worktrees design (ADR-0017). Collapses every teardown onto one guarded rule and adds the cleanup sweeps so parked worktrees/branches don't leak. ## Stacked on #139 — read first This is **blocked by #134** and built on its branch: the PR targets `afk/134` (slice 1, PR #139), so the diff shows only slice 2. **Merge #139 first**, then either merge this into `afk/134` or retarget it to `main`. Basing on `main` would have mixed slice 1's ~1k-line refactor into this diff and risked double-applying it. ## What changed - **One guarded teardown everywhere.** The AFK reaper now routes *both* success and failure through the shared `teardownInstance` → `teardownGuarded` (dirty → keep worktree+branch; clean → remove worktree, delete branch iff merged), replacing slice 1's remove-on-success / keep-on-failure. Manual Stop, startup reconciliation, and the runtime sweep use the same rule. AFK keeps only its outcome accounting (consecutive-failure counter + budget-clock neutrality); a manual AFK Stop stays neutral (keeps worktree + `afk/<N>` branch, ADR-0013). - **Startup reconciliation** (`reconcile.go`): every orphan worktree (a `lab//afk/` worktree with no live session) gets the guarded teardown; every merged dangling `lab//afk/` branch is deleted — finally GC'ing the `afk/<N>` claim branches lab kept forever. Runs synchronously before the schedulers, so no Start races it. - **Throttled runtime sweep**: piggybacks the reaper loop on `afkSweepInterval` (10 min, not every 30s tick); best-effort `git fetch` per project, then **merged-only** GC of `lab//afk/` branches + their **clean** worktrees. Never touches dirty/unmerged. - **git seam**: `Fetch`, `Worktrees` (porcelain parse), `Branches`. - **Concurrency guard**: a mid-Start instance is registered in a `starting` set before its worktree exists and cleared once its session is live, so the sweep can't remove a not-yet-live worktree out from under an in-flight Start (a fresh branch reads as "merged"). Without it the sweep would introduce a data-loss race (caught in adversarial review). - **Docs**: ADR-0007 superseding note (reaper now guard-tears-down on both outcomes); ADR-0017 rollout marks slice 2 done. ## Acceptance criteria - [x] Stop, the reaper (success+failure), startup reconciliation, and the runtime sweep all route through one guarded-teardown helper. - [x] After a simulated crash/reboot (sessions gone, worktrees on disk), startup removes clean orphans and keeps dirty ones — `TestReconcileWorktrees_realGitCrashRecovery` (real git). - [x] Merged `lab/` and `afk/` branches + their clean worktrees are deleted at runtime; dirty/unmerged are never auto-removed — `TestSweepProject`. - [x] AFK still reaps on PR/death/timeout, counts failures, and treats a manual Stop as neutral. - [x] ADR-0007 superseding note added; `go test ./...` passes. ## Verification `gofmt`, `go vet`, `go build`, `go test -race ./...` all pass locally (lab Go is not built by the eval-only pre-commit). The pre-commit `fw` eval passed on commit. ## Note (pre-existing, self-healing) A failed `git worktree add -b` can leave a branch at the base commit with no worktree; the Start rollback doesn't delete it. Since such a branch sits at `origin/<default>` it reads as merged, so the new sweep GCs it within a sweep interval. Pre-existing (slice 1), out of scope here, and now self-heals via this PR's sweep — flagged for transparency. Closes #135

dominik.polakovics added 1 commit

2026-06-08 02:10:44 +02:00

feat(dev): lab — unified teardown + worktree/branch cleanup 7c0d0d0075

Slice 2/3 of ADR-0017 (per-instance worktrees): collapse every teardown
site onto the one guarded rule and add the cleanup sweeps so parked
worktrees and branches don't leak.

- Reaper: route both success and failure through the guarded teardown
  (teardownInstance), replacing slice 1's remove-on-success /
  keep-on-failure. A dirty worktree is kept whole; a clean one is removed
  and its branch deleted only once merged. AFK keeps only its outcome
  accounting (consecutive-failure counter, budget-clock neutrality); a
  manual AFK Stop stays neutral.
- Startup reconciliation: orphan worktrees (no live session) get the
  guarded teardown; merged dangling lab//afk/ branches are deleted —
  finally GC'ing the afk/<N> claim branches lab kept forever.
- Runtime sweep: piggyback the reaper on a throttle (afkSweepInterval),
  best-effort fetch per project, then merged-only GC of lab//afk/
  branches and their clean worktrees; never touch dirty/unmerged.
- git seam: add Fetch, Worktrees, Branches.
- Guard a mid-Start instance from the sweep via a "starting" set, so the
  sweep can't remove a not-yet-live worktree (a fresh branch reads
  "merged") out from under an in-flight Start.
- ADR-0007 superseding note; ADR-0017 rollout marks slice 2 done.

dominik.polakovics changed target branch from afk/134 to main

2026-06-08 10:37:17 +02:00

dominik.polakovics commented

2026-06-08 10:37:52 +02:00

Author

Owner

This was generated by AI while landing a PR.

Validation: PASS — landing now (retargeted to main after #139).

Stack handled: base was afk/134; with #139 merged, I retargeted this PR to main, so its diff is exactly slice 2 (10 files) and Closes #135 fires on merge to main.
Verification signal: ran the project's own checks on afk/135 (cumulative slice1+slice2): gofmt/vet/build clean, go test -race ./... green (43.2s), incl. TestReconcileWorktrees_realGitCrashRecovery and TestSweepProject (real git).
Diff review: the new reconcile.go race guard checks out — gatherRefs reads branches → worktrees → starting → live, with owned = starting ∪ live, closing the window where an in-flight Start's fresh (merged-looking) branch could be GC'd. managedBranch only touches lab//afk/ branches; every teardown routes through the one teardownGuarded.
Conventions: Conventional-Commits title ✓; no secrets.yaml / stateVersion; Closes #135 present.

Merging via merge-commit.

> *This was generated by AI while landing a PR.* **Validation: PASS** — landing now (retargeted to `main` after #139). - **Stack handled:** base was `afk/134`; with #139 merged, I retargeted this PR to `main`, so its diff is exactly slice 2 (10 files) and `Closes #135` fires on merge to main. - **Verification signal:** ran the project's own checks on `afk/135` (cumulative slice1+slice2): `gofmt`/`vet`/`build` clean, `go test -race ./...` green (43.2s), incl. `TestReconcileWorktrees_realGitCrashRecovery` and `TestSweepProject` (real git). - **Diff review:** the new `reconcile.go` race guard checks out — `gatherRefs` reads branches → worktrees → `starting` → `live`, with `owned = starting ∪ live`, closing the window where an in-flight Start's fresh (merged-looking) branch could be GC'd. `managedBranch` only touches `lab/`/`afk/` branches; every teardown routes through the one `teardownGuarded`. - **Conventions:** Conventional-Commits title ✓; no `secrets.yaml` / `stateVersion`; `Closes #135` present. Merging via merge-commit.

dominik.polakovics merged commit 7dbc91db61 into main

2026-06-08 10:37:52 +02:00

dominik.polakovics referenced this pull request from a commit

2026-06-08 10:37:53 +02:00

Merge pull request 'feat(dev): lab — unified teardown + worktree/branch cleanup' (#140) from afk/135 into main