CB-586: nothing ever prunes refs/wip/*, so CB-578 stage C's snapshots pin objects forever #67

Closed
opened 2026-08-15 13:11:07 +02:00 by ltms · 1 comment
Owner

Follow-up to issue #50 stage C, merged at a3842c8. Not a defect in that change — it is the cost that change deliberately accepted, and it needs an owner before the fleet has run for months.

The growth

Every release with a dirty worktree now commits the worktree's full state to refs/wip/<branch>. Nothing ever deletes those refs.

A ref is a GC root. So every snapshot pins its whole tree, and git gc can never reclaim any of it. Two properties make this add up faster than it looks:

  • Branch names are unique per session (worker/cb578c-92885c-1), so refs accumulate rather than overwrite. Snapshots do not replace each other.
  • A snapshot is a full tree of the repo, not a diff against the last one. Objects are shared with main where files are unchanged, so the marginal cost is small — but it is never zero, and it is never released.

Today this repo has run for weeks and the fleet does maybe a dozen releases a day. It is not a problem now. It is a problem that only ever gets bigger, and it will be discovered as a slow git gc or a large .git rather than as an error.

What is genuinely hard here

A snapshot exists precisely because the work was not committed anywhere else. Deleting one on a timer risks throwing away the only copy of work a lead has not yet looked at — which is the failure CB-576 and stage C were built to stop. So a naive TTL sweep would undo the feature.

The useful distinction is whether the work was recovered. A snapshot whose content is already reachable from main, or from a merged branch, is pure duplication and safe to drop. One that is not reachable from anywhere is the last copy and must stay.

Scope

Decide the retention rule first — the implementation is small once the rule is right. Candidates, roughly in order of how much they respect the above:

  1. Reachability-based. Delete refs/wip/<branch> when its tree is already reachable from main. Safest, and it self-cleans as work merges. Costs a reachability check per ref.
  2. Age plus reachability. As above, plus an age floor so a very recent snapshot is never swept even if it looks reachable.
  3. Age only. Simplest, and the one that can destroy the last copy. Only acceptable with a long TTL and a loud log line naming what it deleted.
  4. Report, do not delete. A bridge_* surface or a startup log line that names the count and total size, leaving the decision to the operator. Weakest, but it makes the growth visible and cannot lose work — a reasonable first step if the rule is contested.

Whatever lands, a deletion must log what it dropped, including the ref name and its commit sha, so an operator who deleted the wrong thing can still recover it from the reflog.

Acceptance criteria

  1. A retention rule is chosen, written down with its reasoning, and implemented.
  2. A snapshot whose content is not reachable from any other ref is never deleted automatically.
  3. Every automatic deletion logs the ref name and commit sha.
  4. An operator can see how many refs/wip/* refs exist and roughly what they cost, without shelling into the repo.
  5. A fleet that has never snapshotted anything behaves exactly as today.

Not in scope

Pushing refs/wip/* to the forge. They are deliberately local: refs/wip/ is outside refs/heads/, so it is not pushed by default and does not appear in git branch. Changing that is a separate decision about where a worker's unfinished work should live.

Follow-up to issue #50 stage C, merged at `a3842c8`. Not a defect in that change — it is the cost that change deliberately accepted, and it needs an owner before the fleet has run for months. ## The growth Every release with a dirty worktree now commits the worktree's full state to `refs/wip/<branch>`. Nothing ever deletes those refs. A ref is a GC root. So every snapshot pins its whole tree, and `git gc` can never reclaim any of it. Two properties make this add up faster than it looks: - Branch names are unique per session (`worker/cb578c-92885c-1`), so refs **accumulate** rather than overwrite. Snapshots do not replace each other. - A snapshot is a full tree of the repo, not a diff against the last one. Objects are shared with `main` where files are unchanged, so the marginal cost is small — but it is never zero, and it is never released. Today this repo has run for weeks and the fleet does maybe a dozen releases a day. It is not a problem now. It is a problem that only ever gets bigger, and it will be discovered as a slow `git gc` or a large `.git` rather than as an error. ## What is genuinely hard here A snapshot exists precisely because the work was **not** committed anywhere else. Deleting one on a timer risks throwing away the only copy of work a lead has not yet looked at — which is the failure CB-576 and stage C were built to stop. So a naive TTL sweep would undo the feature. The useful distinction is whether the work was **recovered**. A snapshot whose content is already reachable from `main`, or from a merged branch, is pure duplication and safe to drop. One that is not reachable from anywhere is the last copy and must stay. ## Scope Decide the retention rule first — the implementation is small once the rule is right. Candidates, roughly in order of how much they respect the above: 1. **Reachability-based.** Delete `refs/wip/<branch>` when its tree is already reachable from `main`. Safest, and it self-cleans as work merges. Costs a reachability check per ref. 2. **Age plus reachability.** As above, plus an age floor so a very recent snapshot is never swept even if it looks reachable. 3. **Age only.** Simplest, and the one that can destroy the last copy. Only acceptable with a long TTL and a loud log line naming what it deleted. 4. **Report, do not delete.** A `bridge_*` surface or a startup log line that names the count and total size, leaving the decision to the operator. Weakest, but it makes the growth visible and cannot lose work — a reasonable first step if the rule is contested. Whatever lands, a deletion must **log what it dropped**, including the ref name and its commit sha, so an operator who deleted the wrong thing can still recover it from the reflog. ## Acceptance criteria 1. A retention rule is chosen, written down with its reasoning, and implemented. 2. A snapshot whose content is not reachable from any other ref is never deleted automatically. 3. Every automatic deletion logs the ref name and commit sha. 4. An operator can see how many `refs/wip/*` refs exist and roughly what they cost, without shelling into the repo. 5. A fleet that has never snapshotted anything behaves exactly as today. ## Not in scope Pushing `refs/wip/*` to the forge. They are deliberately local: `refs/wip/` is outside `refs/heads/`, so it is not pushed by default and does not appear in `git branch`. Changing that is a separate decision about where a worker's unfinished work should live.
ltms added the ready-to-delegate label 2026-08-15 13:11:14 +02:00
ltms added this to the 1.1 — single-host close-out milestone 2026-08-16 16:49:37 +02:00
ltms closed this issue 2026-08-16 20:13:44 +02:00
Author
Owner

Merged to main at 15ff6bc, with a follow-up fix at b14b66a. 860 tests, mvn clean install, BUILD SUCCESS.

I checked the safety rule against real git, not the fakes

The rule is: delete a refs/wip/* snapshot only when its tree is already reachable from main and it is older than 24 hours. I built a throwaway repo with three snapshots and called GitWorktrees.pruneWipRefs(repo, 24h) on it.

census before : count=3 costBytes=52
deleted       : 1
census after  : count=2 costBytes=38
--- refs SURVIVING:
refs/wip/reachable-young
refs/wip/unreachable-old
snapshot reachable from main age result
reachable-old yes > 24h deleted
reachable-young yes < 24h kept
unreachable-old no > 24h kept

Two more cases: a repo with no main deleted nothing, and a repo with no snapshots was a clean no-op. reachableObjectsFromMain returning Set.of() when main cannot be resolved is the safe direction — nothing is reachable, so nothing is deleted.

What I found: the automatic sweep never ran at all

The seam was correct. The caller was not. SessionReaper.lastWipSweepNanos started at Long.MIN_VALUE as a "never swept yet" sentinel, and that sentinel cannot be compared by subtraction:

System.nanoTime()      = 31305820625625   (positive)
now - Long.MIN_VALUE   = -9223340731034150183
interval (6h in nanos) = 21600000000000
gate 'delta < interval' -> true   => returns early

The subtraction overflows, the gate reads it as "swept moments ago", and the method returns before the assignment that would have fixed the field. So the field stays Long.MIN_VALUE for the life of the process and the sweep never runs once — with nothing in the log to say so.

Every existing test still passed, because they all call SessionManager.sweepWipRefs(...) directly and walk around the gate.

Fixed in b14b66a: a separate sweptOnce boolean carries "never yet", so the subtraction only happens once both operands come from nanoTime. The first pass always sweeps — a restart is a fine moment for it, the 24h floor keeps it safe, and it makes the feature observable right after a redeploy instead of six hours later.

The new test theLoopActuallyRunsTheWipRetentionSweep asserts through the reaper loop rather than the seam. I removed my own fix and re-ran it:

AssertionFailedError: the reaper loop must run the refs/wip retention sweep;
it never reached the seam ==> expected: <false> but was: <true>

That is what makes it a test and not a comment.

Still mine to do

The wiki entry for the new /members wipRefs{count,costBytes} field — the worker could not commit wiki/ because it is a submodule, and said so. I am adding it, with the overflow bug recorded as the gotcha.

Merged to `main` at `15ff6bc`, with a follow-up fix at `b14b66a`. 860 tests, `mvn clean install`, BUILD SUCCESS. ## I checked the safety rule against real git, not the fakes The rule is: delete a `refs/wip/*` snapshot only when its tree is already reachable from `main` **and** it is older than 24 hours. I built a throwaway repo with three snapshots and called `GitWorktrees.pruneWipRefs(repo, 24h)` on it. ``` census before : count=3 costBytes=52 deleted : 1 census after : count=2 costBytes=38 --- refs SURVIVING: refs/wip/reachable-young refs/wip/unreachable-old ``` | snapshot | reachable from main | age | result | |---|---|---|---| | `reachable-old` | yes | > 24h | **deleted** | | `reachable-young` | yes | < 24h | kept | | `unreachable-old` | no | > 24h | kept | Two more cases: a repo with no `main` deleted nothing, and a repo with no snapshots was a clean no-op. `reachableObjectsFromMain` returning `Set.of()` when `main` cannot be resolved is the safe direction — nothing is reachable, so nothing is deleted. ## What I found: the automatic sweep never ran at all The seam was correct. The caller was not. `SessionReaper.lastWipSweepNanos` started at `Long.MIN_VALUE` as a "never swept yet" sentinel, and that sentinel cannot be compared by subtraction: ``` System.nanoTime() = 31305820625625 (positive) now - Long.MIN_VALUE = -9223340731034150183 interval (6h in nanos) = 21600000000000 gate 'delta < interval' -> true => returns early ``` The subtraction overflows, the gate reads it as "swept moments ago", and the method returns **before** the assignment that would have fixed the field. So the field stays `Long.MIN_VALUE` for the life of the process and the sweep never runs once — with nothing in the log to say so. Every existing test still passed, because they all call `SessionManager.sweepWipRefs(...)` directly and walk around the gate. Fixed in `b14b66a`: a separate `sweptOnce` boolean carries "never yet", so the subtraction only happens once both operands come from `nanoTime`. The first pass always sweeps — a restart is a fine moment for it, the 24h floor keeps it safe, and it makes the feature observable right after a redeploy instead of six hours later. The new test `theLoopActuallyRunsTheWipRetentionSweep` asserts through the reaper loop rather than the seam. I removed my own fix and re-ran it: ``` AssertionFailedError: the reaper loop must run the refs/wip retention sweep; it never reached the seam ==> expected: <false> but was: <true> ``` That is what makes it a test and not a comment. ## Still mine to do The wiki entry for the new `/members` `wipRefs{count,costBytes}` field — the worker could not commit `wiki/` because it is a submodule, and said so. I am adding it, with the overflow bug recorded as the gotcha.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#67