Sibling of the two carries shipped in #406 (22e79e0 for the harvest, 5c401f3 for the sweep bundle close), same mechanism, third producer — and the only one of the three still uncovered.
Engine._record_review_budget_followup writes self.workspace.paths.deferred_work (engine.py:3739). Under scm.isolation = "worktree" that is the unit worktree's ledger, and finalize_commit stages with git add -A (verify.py), which skips a gitignored path in silence. So the entry never rides the unit branch, _merge_local brings nothing over, and close_unit_workspace(success=True) removes the worktree unconditionally — the DONE leg takes no capture_diff, so there is not even a changes.patch copy.
The DONE-leg carry added in 22e79e0 does not cover this: _carry_harvested_deferrals re-files task.harvested_deferrals alone, and this entry is written by a different producer that records nothing.
Measured
Sandbox project fixture, .gitignore = .bmad-loop/runs/ + deferred-work.md, isolation = "worktree", dev then two non-converging review passes so the default limits.max_followup_reviews = 1 damping cap fires.
Gitignored ledger:
DAMPED {'kind': 'review-followup-damped', 'story_key': '1-1-a', 'cycle': 2,
'cap': 1, 'refiled': 'DW-1', 're_review_capped': False}
MAIN LEDGER [] # project.deferred_work does not exist at all
The run reaches DONE, merges, and journals refiled: DW-1 — it believes it filed. The main checkout has no ledger.
Same run, ledger tracked (the current fixture shape):
MAIN LEDGER [('DW-1', 'Follow-up review still recommended for 1-1-a after the damping cap was spent')]
So the fixtures pass today for the same reason test_a_done_isolated_unit_files_its_harvest_exactly_once did before #406: nothing in the suite gitignores the ledger on this path.
Correction to the framing this was first filed under
The original note said the damped caller (engine.py:2545) "lacks the not self._isolated guard its exhaustion counterpart has" (:2603). That reading is wrong and would produce the wrong fix.
The not self._isolated on the exhaustion caller guards the rescue, not the ledger write — it sits inside if refileable_followup and not self._isolated and self._verify_review(task).ok:, and its comment says so: "in worktree isolation a defer already keeps the unit's worktree + patch (no work is lost), so there is nothing to rescue and committing into the main repo would be wrong." The damped path is not a rescue; it force-converges a story that is finalized and verify-green, and it is correct for it to run under isolation.
The defect is only that its ledger write can be silently dropped. The fix direction is therefore to extend the carry, not to add a guard — adding one would suppress a legitimate entry on every isolated run, including the tracked-ledger runs where it works today.
Fix shape
Record the intent, then carry it on the DONE leg, exactly as the two shipped carries do:
- a persisted
StoryTask field for the refiled entry's append_entry kwargs (mirroring harvested_deferrals), assigned in _record_review_budget_followup;
- re-file it from
Engine._carry_isolated_ledger_writes (the hook 22e79e0 introduced), after _merge_local returns;
append_entry dedups on origin: + source_spec: against open entries, so a tracked ledger whose entry merged normally is a no-op — the same property that makes both existing carries safe unconditionally.
The exhaustion caller needs nothing: under isolation it is unreachable.
Guard rows should pin (a) the gitignored isolated case reaching the main ledger, (b) no duplicate on the tracked isolated case, and (c) — the one the existing coverage is missing — that a fixture with a gitignored ledger is used at all, or the row is vacuous.
Cross-refs: #405 (the hotfix that surfaced this family), #423 (the pathspec exposure the carry's commit rides on — git add -- <ignored path> returns rc 1, which is what makes the miss journalled rather than silent).
Sibling of the two carries shipped in #406 (
22e79e0for the harvest,5c401f3for the sweep bundle close), same mechanism, third producer — and the only one of the three still uncovered.Engine._record_review_budget_followupwritesself.workspace.paths.deferred_work(engine.py:3739). Underscm.isolation = "worktree"that is the unit worktree's ledger, andfinalize_commitstages withgit add -A(verify.py), which skips a gitignored path in silence. So the entry never rides the unit branch,_merge_localbrings nothing over, andclose_unit_workspace(success=True)removes the worktree unconditionally — the DONE leg takes nocapture_diff, so there is not even achanges.patchcopy.The DONE-leg carry added in
22e79e0does not cover this:_carry_harvested_deferralsre-filestask.harvested_deferralsalone, and this entry is written by a different producer that records nothing.Measured
Sandbox
projectfixture,.gitignore=.bmad-loop/runs/+deferred-work.md,isolation = "worktree", dev then two non-converging review passes so the defaultlimits.max_followup_reviews = 1damping cap fires.Gitignored ledger:
The run reaches DONE, merges, and journals
refiled: DW-1— it believes it filed. The main checkout has no ledger.Same run, ledger tracked (the current fixture shape):
So the fixtures pass today for the same reason
test_a_done_isolated_unit_files_its_harvest_exactly_oncedid before #406: nothing in the suite gitignores the ledger on this path.Correction to the framing this was first filed under
The original note said the damped caller (
engine.py:2545) "lacks thenot self._isolatedguard its exhaustion counterpart has" (:2603). That reading is wrong and would produce the wrong fix.The
not self._isolatedon the exhaustion caller guards the rescue, not the ledger write — it sits insideif refileable_followup and not self._isolated and self._verify_review(task).ok:, and its comment says so: "in worktree isolation a defer already keeps the unit's worktree + patch (no work is lost), so there is nothing to rescue and committing into the main repo would be wrong." The damped path is not a rescue; it force-converges a story that is finalized and verify-green, and it is correct for it to run under isolation.The defect is only that its ledger write can be silently dropped. The fix direction is therefore to extend the carry, not to add a guard — adding one would suppress a legitimate entry on every isolated run, including the tracked-ledger runs where it works today.
Fix shape
Record the intent, then carry it on the DONE leg, exactly as the two shipped carries do:
StoryTaskfield for the refiled entry'sappend_entrykwargs (mirroringharvested_deferrals), assigned in_record_review_budget_followup;Engine._carry_isolated_ledger_writes(the hook22e79e0introduced), after_merge_localreturns;append_entrydedups onorigin:+source_spec:against open entries, so a tracked ledger whose entry merged normally is a no-op — the same property that makes both existing carries safe unconditionally.The exhaustion caller needs nothing: under isolation it is unreachable.
Guard rows should pin (a) the gitignored isolated case reaching the main ledger, (b) no duplicate on the tracked isolated case, and (c) — the one the existing coverage is missing — that a fixture with a gitignored ledger is used at all, or the row is vacuous.
Cross-refs: #405 (the hotfix that surfaced this family), #423 (the pathspec exposure the carry's commit rides on —
git add -- <ignored path>returns rc 1, which is what makes the miss journalled rather than silent).