From 139f563126a868bd0d4bf3d9bb706e2cda30faa8 Mon Sep 17 00:00:00 2001 From: Yehonal Date: Thu, 18 Jun 2026 15:55:16 +0000 Subject: [PATCH] docs(syncwheel): harden housekeeping guidance --- CHANGELOG.md | 3 + VERSION | 2 +- docs/design/housekeeping-after-merge.md | 183 ++++++++++++++++++++++++ openpack.json | 2 +- skills/syncwheel/SKILL.md | 49 ++++++- 5 files changed, 231 insertions(+), 8 deletions(-) create mode 100644 docs/design/housekeeping-after-merge.md diff --git a/CHANGELOG.md b/CHANGELOG.md index b78b1b2..aa07dfe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +- Harden the Syncwheel skill with managed-repo detection, post-merge + housekeeping guidance, squash-merge verification, and a housekeeping design + spec. - Default Syncwheel-managed worktrees to repo-relative `.syncwheel/wt/` while preserving explicit `var/syncwheel` manifest settings. - Clarify that feature PRs deliver to their intended branch, never to diff --git a/VERSION b/VERSION index 5a03fb7..847e9ae 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.20.0 +0.20.1 diff --git a/docs/design/housekeeping-after-merge.md b/docs/design/housekeeping-after-merge.md new file mode 100644 index 0000000..46c0812 --- /dev/null +++ b/docs/design/housekeeping-after-merge.md @@ -0,0 +1,183 @@ +# Syncwheel: post-merge housekeeping + +## Purpose + +Add an idempotent housekeeping path so that when a stack's PR lands — including +squash/rebase merges that rewrite commit SHAs — Syncwheel closes the stack and +reaps its local branch, worktree, and stale backup branches automatically, driven +by a trigger that fires right after the merge. This removes the unbounded +accumulation of worktrees, branches, and backups that the current append-only +lifecycle produces. + +## Problem + +The current lifecycle only *adds*: `stack create` opens a branch (and often a +worktree); reconcile/realign drops timestamped `backup/*` branches; merges happen +on the forge (squash). Nothing reaps a stack's branch / worktree / backups when its +PR lands. Over a couple of days of normal multi-stack work a single managed repo +accumulated 9 worktrees and 7 backup branches, and merged stacks lingered as +"active" in the manifest. + +Three concrete defects, all in `scripts/syncwheel.py`: + +### Defect 1 — close detects merges by SHA only; squash/rebase merges are invisible + +`command_stack_close` checks, for each stack commit, `git merge-base --is-ancestor + `. A squash or rebase merge rewrites the SHA, so the original +commit is never an ancestor of the base → `close` refuses without `--force`. Worse, +`reconcile` sees a manifest stack whose branch is missing and plans +`create_pr_branch` to **recreate** the just-merged branch. Net effect: merged +stacks never auto-close, and reconcile actively fights the cleanup. + +### Defect 2 — close never removes the worktree + +`command_stack_close --delete-branch` runs `git branch -d ` but does not +remove the stack's worktree. If the worktree still exists, `git branch -d` fails +anyway (branch is checked out elsewhere). The `git worktree remove --force` +primitive already exists in the materialize helpers but is not wired into close, so +worktrees leak indefinitely — the dominant source of the "too many worktrees" +problem, especially across an integration-scheme change that orphans old worktrees. + +### Defect 3 — backups have no retention + +`backup_branch_command` creates `backup/-before-syncwheel-` (and a +`-before-final-align-` variant) on every reconcile/realign. Nothing ever prunes +them. + +## Goals / Non-goals + +**Goals:** idempotent close + reap of merged stacks (SHA *or* content); bounded +backups; safe under concurrent multi-agent / multi-machine use; never destroy +unmerged or uncommitted work. + +**Non-goals:** deleting unmerged feature branches; reaping worktrees that hold +uncommitted or conflicted changes (skip + report); performing the forge-side PR +merge itself. + +## Design + +### Shared truth vs local reaping (the multi-machine split) + +The manifest and ledger are git-tracked and pushed, so the *closed* state is +shared; worktrees and local branches are machine-local. Responsibility splits along +that line: + +| State | Lives | Cleaned by | +|-------|-------|------------| +| Manifest + ledger (closed status) | Git, shared | CI / any machine, then pushed | +| Worktree + local branch | One machine | Only that machine's local housekeep | + +This is the rule that keeps multi-agent safe: CI cannot (and must not) reach into a +machine's worktrees, and no machine may remove a worktree another machine is using. +The manifest is the contract; local housekeep enforces it per machine. + +### Merge detection: by content, not only SHA + +A stack counts as merged if **either**: + +- (a) every stack commit is reachable from the target ref (today's per-SHA check), **or** +- (b) `git diff --quiet ` — the branch tip carries no content + the target lacks. This covers squash and rebase merges. + +Detection must `git fetch` first and compare against the stack's own +`target_remote`/`target_branch` (e.g. `origin/main`), not the integration branch. + +### New entrypoint: `syncwheel housekeep` + +Idempotent. Reports a plan by default; mutates only with `--apply` (consistent with +`reconcile`). Steps: + +1. `git fetch --all --prune`. +2. For each active stack, if merged (a or b): close it (reuse `command_stack_close` + logic), then reap — + - if the stack worktree is dirty or has conflicts → **skip reap**, keep the + branch, and report the path (never destroy uncommitted work); + - else `git worktree remove --force ` then `git worktree prune`; + - then delete the branch (`-d` for SHA-merged, `-D` for content-merged, since + `-d` won't recognize a squash) — only after the worktree is gone. +3. Prune backups: keep the most recent `backup_retention_count` (default 2) and any + newer than `backup_retention_hours` (default 48); delete older `backup/*`. Always + keep at least the newest. +4. Remove orphaned worktrees: any worktree under the syncwheel worktree root whose + branch is referenced by no active stack and is clean → remove; report dirty + orphans instead. +5. Append a `housekeep` ledger event summarizing closed stacks, reaped worktrees, + pruned backups, and skipped (dirty) items. + +### Triggers (the "right after merge" part) + +1. **Local `post-merge` git hook** — on `git pull`/merge into the base branch, run + `syncwheel housekeep --apply`. This is the natural "the merge reached this + machine" moment and reaps the local branch + worktree for the stack that just + landed. Ship as `githooks/post-merge` and teach `self install-hooks` to install + it into managed repos. +2. **CI job** on `push: main` (or `pull_request: closed` with `merged == true`): + run `syncwheel reconcile --close-merged --json`, then commit and push the updated + manifest + ledger. Keeps the shared truth current for every machine/agent. CI + intentionally does not reap worktrees — those are per-machine. +3. Optional periodic sweep (cron / scheduled run) calling `housekeep` as a backstop. + +### Manifest schema additions + +New optional `housekeeping` block; absent means the safe defaults below: + +```json +"housekeeping": { + "backup_retention_count": 2, + "backup_retention_hours": 48, + "reap_worktrees": true, + "close_merged_by_content": true +} +``` + +## CLI surface + +- `syncwheel housekeep [-r REPO] [--apply] [--json] [--no-reap-worktrees] [--keep-backups N]` +- `syncwheel stack close`: add `--merged-by-content` (close when diff vs target is + empty, without `--force`); make `--delete-branch` reap the worktree first (after a + dirty check). +- `syncwheel reconcile`: add `--close-merged` so detection folds into the reconcile + plan, and suppress `create_pr_branch` when a manifest branch is missing but its + content is already in the target. + +## Acceptance criteria + +1. **Squash-merged stack** (branch diff vs `origin/main` empty): `housekeep` closes + it without `--force`, removes worktree + branch, manifest validates `OK`, ledger + gains `stack_closed` + `housekeep`. (This is exactly the case reconcile currently + mis-plans as `create_pr_branch`.) +2. **Unmerged stack** with real unique commits: untouched. +3. **Dirty/conflicted worktree**: not reaped; reported as skipped; branch kept. +4. **Backup retention**: backups beyond the policy pruned; the most recent K kept; + nothing pruned when count ≤ K. +5. **Orphaned clean worktree** (branch in no active stack): removed; dirty orphan + reported, not removed. +6. **Idempotent**: a second `housekeep` run is a no-op. +7. **Reconcile**: no longer plans `create_pr_branch` for a missing branch whose + content is already in the target. + +## Implementation pointers (`scripts/syncwheel.py`) + +- `command_stack_close` (~L2550): add the merge-by-content branch to the + reachability gate; reap the worktree before deleting the branch. +- Factor `reap_worktree(repo_root, branch)` from the existing `git worktree remove + --force` calls in the materialize helpers (~L2191, L2243), using + `find_worktree_for_branch` (~L1606) and `ensure_clean_worktree` (~L940) for the + dirty check. +- `backup_branch_name` / `backup_branch_command` (~L1158): add + `prune_backups(repo_root, retention)` enumerating `backup/*` refs by committer + date. +- `reconcile_actions` (~L3039) / `classify_stack_reconcile` (~L3203) / + `command_reconcile` (~L3350): add the `--close-merged` path and suppress + `create_pr_branch` for content-merged missing branches. +- New `command_housekeep` + subparser; ledger event type `housekeep` via + `append_ledger_event`. +- `githooks/post-merge` + `self install-hooks` wiring for managed repos. +- Tests under `tests/` covering the 7 acceptance scenarios. + +## Rollout + +1. Land detection + `housekeep` + tests (no behavior change until invoked). +2. Add manifest defaults (safe, backward-compatible). +3. Wire the post-merge hook and the CI job in managed repos. +4. Optionally make `reconcile` close-merged by default once proven in practice. diff --git a/openpack.json b/openpack.json index 4ba146e..ec832ad 100644 --- a/openpack.json +++ b/openpack.json @@ -1,7 +1,7 @@ { "schemaVersion": 2, "name": "NestDevLab/syncwheel", - "version": "0.20.0", + "version": "0.20.1", "provides": [ { "type": "skills", "path": "skills", "required": true } ] diff --git a/skills/syncwheel/SKILL.md b/skills/syncwheel/SKILL.md index 960ee3a..eec1918 100644 --- a/skills/syncwheel/SKILL.md +++ b/skills/syncwheel/SKILL.md @@ -1,6 +1,6 @@ --- name: syncwheel -description: Use Syncwheel for deterministic, multi-agent-safe Git maintenance — PR branches, dedicated worktrees, stacked PRs, and integration branches. Use whenever you are about to create a PR branch, manage a fork/upstream/integration or PR-stack workflow, prepare or rebuild a stacked PR, or coordinate Git work on a repo that other people or agents may touch concurrently. Also covers the decision of whether to commit the Syncwheel manifest (own repo) or keep it untracked (external contribution). +description: Use Syncwheel for deterministic, multi-agent-safe Git maintenance — PR branches, dedicated worktrees, stacked PRs, and integration branches. Use whenever you are about to create a PR branch, manage a fork/upstream/integration or PR-stack workflow, prepare or rebuild a stacked PR, or coordinate Git work on a repo that other people or agents may touch concurrently or that contains a `.syncwheel/` directory. Also covers the decision of whether to commit the Syncwheel manifest (own repo) or keep it untracked (external contribution). allowed-tools: [Bash] --- @@ -14,6 +14,13 @@ unavailable, blocked, or cannot express the needed recovery. ## When to use (Syncwheel-first) +**First, detect the regime.** Before branch, worktree, integration, PR, recovery, +or handoff work in any Git repo, check whether it is Syncwheel-managed: a +`.syncwheel/` directory or manifest is present, or a workspace/project guide says +so. When unsure, run `syncwheel status`. If it is managed — or the repo is shared, +fork/upstream, or touched by multiple agents — it is Syncwheel-first: do not reach +for manual `git` branch/worktree/integration surgery as the default path. + Reach for Syncwheel **before** any of these, not after: - editing or handing off work in a Git repository that is shared, managed by @@ -128,10 +135,39 @@ release branch, but it must not be `main-integration`. `main-integration` is a coordination branch for assembling and testing stacks before delivery. Do not treat it as a PR target or deployment branch. -After the PR merges, fetch the remote, verify the stack commits are reachable -from the delivery branch, align or rebuild `main-integration` from the updated -base, then close the stack with `syncwheel stack close ` and remove the -dedicated PR worktree or branch when safe. +After the PR merges, fetch the remote and confirm the stack's content landed in +the delivery branch — by SHA, or, for a squash/rebase merge, by an empty +`git diff `. Align or rebuild `main-integration` from the +updated base, then run the housekeeping below to close the stack and reap its +worktree and branch. + +## Housekeeping: when and how to clean up + +Worktrees, PR branches, and `backup/*` branches do not reap themselves — close the +loop or they accumulate. Clean up when any of these holds: + +- a stack's PR has merged +- `syncwheel status` shows a worktree or local branch not backing an **active** + manifest stack (orphans — common after an integration-scheme change) +- `backup/*` branches have piled up +- before handing off a managed repo: leave only the active stacks' worktrees plus + the integration worktree + +Procedure (never destroy unmerged or uncommitted work): + +```bash +git fetch --all --prune +git diff --quiet && echo "content merged" # empty diff also covers squash/rebase +git worktree remove # refuses if dirty/conflicted — resolve or stash first, never blind --force +syncwheel stack close -R merged --force # a squash merge is not ancestor-reachable, so --force is expected +git branch -d # use -D only for a squash-merged branch -d won't recognize, after the check above +git worktree prune # drop stale worktree admin entries +``` + +Prune `backup/*` branches that are no longer a useful safety net, keeping the one +or two most recent. Never delete a branch that still carries unique unmerged +commits, and never force-remove a worktree with uncommitted or conflicted changes +— report it instead. ## Decision: Syncwheel tracking policy @@ -216,4 +252,5 @@ state with `syncwheel resume` instead of improvising branch ownership. See `docs/manifest-tracking.md` for the full tracking policy, `docs/ai-agents.md` and `docs/agent-procedure.md` for the agent contract, and `docs/core-procedure.md` -for the canonical recovery procedure. +for the canonical recovery procedure. An automated post-merge cleanup path is +specified in `docs/design/housekeeping-after-merge.md`.