Skip to content

fix: bisect a tie window too long for one exact PWL solve (#624) - #629

Merged
johanzander merged 2 commits into
mainfrom
fix/issue-624-pwl-window-bisect
Aug 17, 2026
Merged

fix: bisect a tie window too long for one exact PWL solve (#624)#629
johanzander merged 2 commits into
mainfrom
fix/issue-624-pwl-window-bisect

Conversation

@johanzander

@johanzander johanzander commented Aug 17, 2026

Copy link
Copy Markdown
Owner

Summary

  • A merged tie window longer than the exact PWL solver can certify no longer discards the entire schedule — it is bisected and each half solved under the same certification.
  • Fixes the reported symptom: the add-on stuck on "initializing" forever, with every hourly optimization failing identically.

Root cause

detect_tie_windows merges adjacent flagged periods with no cap, so the merged length is an unbounded function of the price curve. run_pwl_window_backward_induction seeds every discharge preimage of the next row's breakpoints, so its breakpoint set compounds per backward step and it can certify only ~8 periods before PWL_MAX_PREIMAGE_SEED_POINTS is exhausted. Raising the budget does not help — measure_tie_coverage.py already documented that a longer horizon "is not reachable by raising budgets".

Nothing owned the join between the two. A nine-period window over volatile SE3 prices raised PWLWindowUnderRefinedError out of optimize_battery_schedule, discarding the whole schedule including every period that solved fine; battery_system_manager turned that into return None; and refresh_health_check re-ran the same effectively-unchanged inputs into the same wall 14 times across two restarts.

The reporter's bit-identical point count of 1,064,944 factors as 10,544 breakpoints × 101 discharge levels — and 10,544 is well under PWL_MAX_BREAKPOINTS (30,000), confirming the preimage cross-product rather than row size is what blew.

Fix

Step 2b catches PWLWindowUnderRefinedError — and only it — and bisects the window at its midpoint, re-solving each half.

This is a re-sizing, not a fallback. Every spliced half carries the same certification a whole window would have; nothing uncertified is ever spliced, and catching it to keep the grid DP's result remains forbidden. P6 is untouched.

Termination is by construction rather than an iteration cap: a horizon-1 window seeds from the four-breakpoint pinned terminal row, so its preimage cross product is ~4 × |discharge levels| — three orders of magnitude under budget, and independent of prices, battery size and grid resolution. A horizon-1 window that still cannot certify is re-raised, because that is not a sizing problem.

Windows are also spliced as they resolve rather than all at once at the end, so a bisected window's second half plans from the SOE the first half actually reached rather than the grid DP's nominal value there. Measured contribution on this fixture: 0.000000 SEK — it closes a latent inconsistency, not an observed error. It is a no-op for separately-detected windows, which by construction never touch.

Test plan

  • ./scripts/quality-check.sh passes locally (re-run after merging origin/main)
  • .venv/bin/pytest -m slow: 553 passed, 8 skipped
  • Observed end-to-end against the reporter's own data, container clock pinned to 2026-08-17 11:08:00 (the minute of the first failure) via libfaketime, using a mock-HA scenario and settings built from the bundle:
    • Near-tied DP decisions detected (#450): re-solving 1 window(s) [(32, 41)] — nine periods, same length as the reporter's
    • PWL window (32, 41) exceeds what the exact solver can certify in one solve (#624) -- splitting at 36
    • Direct Results: ... Savings: 68.61 SEKSchedule applied successfully
    • systemMode: "normal" (was "initializing"), hasCriticalErrors: false, 96-period schedule, zero Optimization failed lines

Evidence the test discriminates

  • Reverted: the if window_horizon <= 1: guard to if True:, i.e. pre-fix behaviour where the exception propagates unconditionally.
  • Result: 6 tests FAILED — all four in test_pwl_window_bisection.py, plus test_scenarios.py::test_all_scenarios[regression_2026_08_17_624] and test_plan_faithfulness.py::test_realized_matches_planned_across_all_fixtures (56 passed).
  • Restored: tree clean, all 6 green.

The discriminator here is raise-vs-schedule, not a cost delta: pre-fix this input produces no schedule at all.

Outcome-level coverage

  • expected_results pinned on the new fixture regression_2026_08_17_624 (auto-discovered by test_scenarios.py), plus its action-selector golden and VPP baseline.
  • R == P via run_scenario_realized in test_the_bisected_schedule_is_executable_as_planned, at the corpus-wide 0.001 SEK gate. It was written at 0.01 first, which passed while proving nothing — a tolerance ten times the effect it guards cannot distinguish a clean seam from a broken one.
  • A premise guard (test_the_reporters_day_still_merges_a_window_no_single_solve_can_certify) fails if tuning ever shortens this window below the ceiling, so the coverage cannot silently evaporate into the ordinary un-bisected path.

Scope assessment

Structural, and the owner is dp_battery_algorithm.py's Step 2b loop rather than tie_detection.py. The detector is a pure function of tie margins and value slopes with no access to battery_settings, capabilities or dt — the things the affordable horizon actually depends on. A length cap there would be a guess coupling detection to solver internals. Step 2b is the only site holding both the window list and the solver.

Workaround check: the diff adds no parameter, flag, default-fallback, second construction site, or extra trigger. It does not route around an ordering, timing or dependency problem — it makes the work submitted to the solver fit what the solver can certify, which is the problem itself.

Not touched: the generic except Exception: return None in battery_system_manager.py. It is pre-existing and correct as a last line of defence; making it degrade to an uncertified schedule would be a real fallback.

Documentation

  • docs/agents/optimizer-architecture.md P6 — new paragraph stating that window size is the caller's problem, that bisection does not relax P6 and is not the cost-gate, and that catching this exception to re-size is permitted while catching it to fall back is not.
  • core/bess/exceptions.py — the PWLWindowUnderRefinedError docstring said it "fires on none of the fixture suite's scenarios" and named the budgets as the knobs to revisit. Both pointed the wrong way; corrected.
  • docs/agents/bess-knowledge.md and docs/SOFTWARE_DESIGN.md: grepped, neither mentions tie windows, the PWL splice, or any budget this change touches. No update needed.

Why this reached production

The fixture corpus was two periods short of catching it. Measured across all 39 pre-existing fixtures, the longest merged tie window any of them produces is 6 periods, against a solver ceiling of ~8 and the reporter's 9. Only 7 of 39 flag a window at all. No amount of running the existing suite could have found this, and nothing reported how close the corpus was to the cliff.

The sharpest detail: synthetic_extreme_volatility flags zero windows. The fixture whose purpose is volatile prices does not exercise the mechanism volatile prices break.

Contributing causes, in order:

  1. An unbounded input met a bounded solver and nobody owned the join. Neither module is wrong alone; the mismatch lives at the call site, which had no opinion about size.
  2. The limit was known, in the wrong half of the codebase. measure_tie_coverage.py documents the ceiling and has test_segment_reference_refuses_a_segment_longer_than_the_solver_can_certify. The measurement tooling refused to try; the production path knew nothing and tried anyway.
  3. "Never fired" was read as "won't fire." The exception docstring's two true statements both pointed the wrong way.
  4. Two changes shipped together and neither re-measured the other. Grid DP discretization leaves 0.01-0.36 SEK/day unrealized on 20/22 benchmark fixtures #512's 0.1 kW grid gives a 10 kW battery 101 discharge levels multiplying every backward step, directly lowering the horizon bug: SOE-grid discretization noise can pick sub-optimal charging window on near-tied periods #450 could afford.

The availability consequence was never sized: a per-window accuracy limit was allowed to propagate through a caller with no partial-success path into return None, then into an hourly retry against unchanged inputs — a guaranteed permanent outage, while the health check reported all seven components healthy.

Note for the reviewer: a separate pre-existing defect this fixture exposes

Adding this fixture breached the corpus-wide zero-tolerance R == P gate by +0.0016 SEK, and it is not caused by this change. Measured, not assumed: with detect_tie_windows patched to return no windows — no PWL solve, no splice, no bisection — the gap is bit-identical at +0.001555.

Cause: at period 32 a 0.0034 kWh solar surplus is an order of magnitude below SOE_STEP_KWH (0.025), so the DP's snapped SOE trajectory cannot represent absorbing it and plans it as export. The command it derives (load_first, charge_rate_pct=100) makes the inverter absorb it anyway. The drift is costless until period 64, where the battery reaches max_soe and the extra 0.0033 kWh leaves that much less room for solar, which is exported instead.

Recorded in KNOWN_PLAN_EXECUTION_GAP_SEK with that evidence. Filed as #630, sized there as negligible (~0.003% relative error, order 0.6 SEK/year): it is a canary for the #497 class, not lost savings.

Closes #624

johanzander and others added 2 commits August 17, 2026 19:21
`detect_tie_windows` merges adjacent near-tied periods into one window with
no cap on the merged length, so window length is an unbounded function of the
price curve. The exact solver that re-certifies a window compounds its
breakpoint set per backward step, so it can only certify roughly eight
periods before `PWL_MAX_PREIMAGE_SEED_POINTS` is exhausted -- and raising the
budget does not extend that, as `measure_tie_coverage.py` already documented.

Nothing owned the join. In the field a nine-period window over volatile SE3
prices raised `PWLWindowUnderRefinedError` out of `optimize_battery_schedule`,
which discarded the entire schedule including every period that had solved
fine. `battery_system_manager` turned that into `return None`, and the health
check's retry fed the same effectively-unchanged inputs into the same wall 14
times across two restarts. The add-on never left "initializing".

Step 2b now catches that one exception -- and only it -- and bisects the
window, re-solving each half under the same certification. This is a re-sizing,
not a fallback: nothing uncertified is ever spliced, so P6 is untouched.
Termination is by construction rather than an iteration cap, since a
horizon-1 window seeds from the four-breakpoint pinned terminal row, three
orders of magnitude under budget and independent of prices, battery size and
grid resolution. A horizon-1 window that still cannot certify is re-raised.

Windows are also spliced as they resolve rather than all at once at the end,
so a bisected window's second half plans from the SOE the first half actually
reached instead of the grid DP's nominal value there. Measured contribution on
this fixture is 0.000000 SEK -- it closes a latent inconsistency, not an
observed error -- and it is a no-op for separately-detected windows, which
never touch.

Verified end-to-end against the reporter's own data with the container clock
at 2026-08-17 11:08, the minute of the first failure: the nine-period window
splits, the schedule is applied, and systemMode reaches "normal".

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019PqZ9uGBqUzU4Q9xTzE6ED
@bess-agent

Copy link
Copy Markdown
Collaborator

@claude-bot review

@bess-agent

Copy link
Copy Markdown
Collaborator

@claude-bot review

@bess-manager-claude-bot bess-manager-claude-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Verified by static review (diff, root-cause evidence, exception raise sites in pwl_window_dp.py, Window dataclass, splice_schedule) — no worktree test run performed for this review, so the PR's stated pytest -m slow / quality-check.sh results are trusted rather than independently reproduced.

Root-cause match: Yes. Stage 2's bess-analyst diagnosis pinned the failure to detect_tie_windows's uncapped merge meeting run_pwl_window_backward_induction's compounding preimage cross-product (~8-period ceiling, confirmed by measure_tie_coverage.py), with battery_system_manager's except Exception: return None turning the resulting raise into a permanent "stuck initializing" outage. This PR's fix targets exactly that join. Note the PR diverges from Stage 2's proposed remedy (a length cap in detect_tie_windows, or a degrade-to-grid-DP fallback) in favor of bisect-and-recertify in dp_battery_algorithm.py Step 2b — correctly, since Stage 2's fallback option would have spliced an uncertified table, which P6 forbids. The PR states this scope assessment explicitly (## Scope assessment), as required by rules.md step 9 / claude-bot.md item 5.

Correctness, verified by reading:

  • dp_battery_algorithm.py:2193-2338 (approx): the pending worklist correctly preserves ordering under nested bisection (pending.insert(0, second_half); pending.insert(0, first_half)), and the window_horizon <= 1: raise guard makes termination unconditional rather than iteration-capped.
  • Incremental per-window splicing is a genuine no-op for separately-detected (non-bisected) windows: splice_schedule only writes spliced_soe[period + 1] for period in range(w.start, w.end), and since non-touching windows satisfy B.start > A.end, an earlier window's splice never touches a later window's start_soe lookup — confirmed this by tracing the index math, not just trusting the comment.
  • The re-derivation-skip fix (resolved_periods / iterating resolved_windows instead of windows) correctly excludes a bisection midpoint from the "period AT end" re-derivation, since that period is now owned by the second half's own flow record (P4).
  • cost_basis_trajectory is deliberately left stale across a bisection seam — reward-only impact, not physics; reasonable per the stated rationale, doesn't affect R == P.

Rule compliance: no Optional[x], no hasattr/silent fallback, no new class, no exception-message matching, PWLWindowUnderRefinedError reused (not duplicated) per exceptions.py's "add here, nowhere else." The except PWLWindowUnderRefinedError catch is scoped to exactly one exception type with three raise sites in pwl_window_dp.py (preimage budget, breakpoint ceiling, refine-iteration budget) — all three are horizon-compounding, so bisection is a valid resize strategy for all of them, not just the one in the fixture.

Test coverage: test_pwl_window_bisection.py's four tests do catch a regression — the PR's revert-based falsification (guard → if True) reportedly fails exactly these four plus two slow tests, which is the right shape of evidence per rules.md's "must be seen to fail without its fix." One nit, not a blocker:

  • core/bess/tests/unit/test_pwl_window_bisection.py:1916-1930 (test_bisection_splits_at_the_midpoint_and_both_halves_certify) asserts f"splitting at {EXPECTED_HALVES[0][1]}" — i.e. hardcodes that bisection lands at the arithmetic midpoint (80). docs/agents/testing.md's "What NOT to Test" explicitly calls out algorithm-specific boundaries (assert slot_start_times == [...]) as bad tests, for the same reason: a future split heuristic (e.g. bisecting at a cheaper-to-recompute point) that still fixes #624 correctly would break this test for no behavioral reason. The companion test test_an_over_long_window_is_bisected_instead_of_killing_the_schedule already covers the actual user-visible behavior (schedule exists, SOE stays physical) without pinning the split point, so this one test could drop the midpoint assertion and just check that exactly one split occurred, without losing real coverage. Not requesting a change — flagging for the author's judgment given it's a narrow regression-fixture test guarding one historical incident, not general algorithm behavior.

Scope: no scope creep. Touches only the Step 2b loop, the one exception's docstring, the P6 doc paragraph, and test/fixture data. The #630 note (pre-existing sub-grid-step passive-solar drift, measured as bit-identical with tie resolution disabled) is correctly called out as filed separately rather than folded into this fix.

No blockers found.

@johanzander
johanzander marked this pull request as ready for review August 17, 2026 19:40
@johanzander
johanzander merged commit 1cd0779 into main Aug 17, 2026
8 checks passed
@johanzander
johanzander deleted the fix/issue-624-pwl-window-bisect branch August 17, 2026 19:42
johanzander added a commit that referenced this pull request Aug 17, 2026
PR #629 merged while this branch was in review, bringing with it the
regression_2026_08_17_624 fixture and a KNOWN_PLAN_EXECUTION_GAP_SEK
entry recording the +0.0016 SEK gap as a known defect awaiting a fix.
This is that fix, so the entry goes -- the fixture's gap is now
+0.000000.

Four consequences, each measured rather than assumed:

- KNOWN_PLAN_EXECUTION_GAP_SEK loses the entry. Its comment described
  the cause as SOE-grid snapping; that was wrong (the forward replay
  carries continuous SoE, and period 32's SoE was held bit-exactly),
  so the replacement note records what it actually was.

- test_the_bisected_schedule_is_executable_as_planned asserted the gap
  equalled the recorded value. With the gap gone it asserts plain
  R == P, which is strictly stronger: a bisection seam error no longer
  has a nonzero expected value to hide inside.

- The fixture's expected_results move by +0.001550
  (battery_solar_cost -47.81617 -> -47.81462). Realized cost is
  unchanged at -47.814616 and planned now equals it exactly -- the plan
  became honest, no saving was lost.

- The selector golden moves. actions, intents and
  intra_period_discharge_allowed are all UNCHANGED; only soe_trajectory
  differs, in 32 of 97 entries (indices 33-64), every one by exactly
  +0.0033077 kWh -- the surplus period 32 now absorbs, carried forward
  until the battery reaches max_soe at index 64 and re-converges. That
  is the issue's own described mechanism, made visible.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015DSyXRJCGQHrsTZ3TC7d8A
johanzander added a commit that referenced this pull request Aug 17, 2026
…630) (#633)

* fix: stop planning a sub-floor solar export the inverter will absorb (#630)

The DP offers a "hold SoE, export this period's own solar surplus"
candidate -- the SOLAR_EXPORT-below-max bypass (#313). Nothing commands
that hold directly: it is delivered by classify_strategic_intent
labelling the period SOLAR_EXPORT, which is the intent that writes charge
rate 0 and so stops the inverter absorbing the surplus.

That label needs grid_exported > FLOW_NOISE_FLOOR_KWH. At or below it the
period falls through to IDLE, whose command is load_first at charge rate
100 -- which absorbs. So the plan booked export revenue on energy the
hardware put in the battery, and the battery ran fuller than planned
until it reached a bound and spilled the difference. On the #629 fixture
that is +0.0016 SEK over a quarterly day, from one 0.0034 kWh surplus.

This is the charge-side twin of the #282/#497 failure that
_residual_cover_p already gates against on the discharge side, so the new
predicate sits beside it and reuses the same constant -- one threshold,
so the candidate space and the classifier cannot drift apart.

Withheld at all three sites that define the action set (P1): the shared
selector and both backward passes. The grid backward pass's existing
"deliberately NOT masked by _discharge_is_unexecutable" exception does
not transfer -- that rests on the coarse lattice having no exact-cover
point nearby, whereas the bypass is an exact action whose executable
neighbour (plain IDLE) is in the action set at every state.

Withholding is always safe: plain IDLE is offered unconditionally, and
where the bypass diverts nothing the two candidates coincide exactly.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015DSyXRJCGQHrsTZ3TC7d8A

* test: retire #629's recorded gap now that its cause is fixed (#630)

PR #629 merged while this branch was in review, bringing with it the
regression_2026_08_17_624 fixture and a KNOWN_PLAN_EXECUTION_GAP_SEK
entry recording the +0.0016 SEK gap as a known defect awaiting a fix.
This is that fix, so the entry goes -- the fixture's gap is now
+0.000000.

Four consequences, each measured rather than assumed:

- KNOWN_PLAN_EXECUTION_GAP_SEK loses the entry. Its comment described
  the cause as SOE-grid snapping; that was wrong (the forward replay
  carries continuous SoE, and period 32's SoE was held bit-exactly),
  so the replacement note records what it actually was.

- test_the_bisected_schedule_is_executable_as_planned asserted the gap
  equalled the recorded value. With the gap gone it asserts plain
  R == P, which is strictly stronger: a bisection seam error no longer
  has a nonzero expected value to hide inside.

- The fixture's expected_results move by +0.001550
  (battery_solar_cost -47.81617 -> -47.81462). Realized cost is
  unchanged at -47.814616 and planned now equals it exactly -- the plan
  became honest, no saving was lost.

- The selector golden moves. actions, intents and
  intra_period_discharge_allowed are all UNCHANGED; only soe_trajectory
  differs, in 32 of 97 entries (indices 33-64), every one by exactly
  +0.0033077 kWh -- the surplus period 32 now absorbs, carried forward
  until the battery reaches max_soe at index 64 and re-converges. That
  is the issue's own described mechanism, made visible.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015DSyXRJCGQHrsTZ3TC7d8A

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
johanzander added a commit that referenced this pull request Aug 17, 2026
…e-argued (#636)

* feat: put PRs on the board so a decision about one is recorded, not re-argued

The board held issues only, so every judgement about a PR had nowhere to live.
"#167 and #354 are blocked", "#437 and #490 are lower priority, later" were
real decisions the maintainer had already made, and the rhythm pass re-reported
all four as due on every tick because nothing recorded them. The same
conversation happened every 30 minutes.

Projects v2 takes PRs as items with the identical field set, so the fix is
membership rather than a parallel mechanism. A PR card carries the same
`Priority` and `Awaiting` an issue card does; the digest emits them as
`pr_board`, and `backlog-rhythm.sh` joins by number and suppresses on them:
an `Awaiting` means parked on someone, `P4` means later-not-never.

`content.type` is what separates PR cards from issue ones, confirmed against a
real card rather than assumed — an added PR reports "PullRequest" with
number/title/url/repository alongside it. Numbers are unique across issues and
PRs in one repository, so this cannot collide with the existing issue lookup.

Suppressed PRs are COUNTED AND LISTED, never dropped: the pass ends with
`deferred: 4 (#490 priority P4; #167 awaiting discussion; ...)`. Silently
vanishing would trade one failure for another — the goal is to stop re-asking
about a settled decision, not to lose the item.

CONTRACT CHANGE: an APPROVED, green, still-draft PR is now its own action,
`mark_ready`, and it is the one thing no board decision can defer. It used to
hand back to `implement-issue` like any other unfinished draft, on the
principle that this pass must not grow a second review loop. That principle
still holds, but it is what left #629 sitting approved, green and draft: the
remedy on offer was a whole `implement-issue` session, and nobody spends one of
those to run a single command. `gh pr ready` is a terminal action, not a loop,
so naming it here duplicates nothing.

`awaiting_maintainer` is deliberately NOT carved out the same way. An approved
PR waiting on a merge is not broken; it is the maintainers call when to take
it, and P4 is exactly how they say later. #490 sat approved for a day and was
reported every tick as though that were news.

Live effect: 31 actions -> 27, with one `deferred: 4` line in place of four
recurring items, and #631 correctly still reported as merge-ready.

No CHANGELOG entry: agent tooling, no user-visible effect.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012LExo6fcbup75vtc9NfoAR

* fix: mark_ready needs green checks, not just a clean merge

Found by running the rule against the live fleet on its first pass, which is
the only reason it was caught before the PR merged.

`mergeable` reports whether the branch merges cleanly and nothing else, so it
reads MERGEABLE while CI is still running or has failed outright. #633 was
APPROVED, MERGEABLE and had Algorithm tests and E2E still IN_PROGRESS, and the
rule duly reported "gh pr ready 633 — then it is the maintainers to merge".
GitHub itself disagreed: `mergeStateStatus` was BLOCKED.

Flipping a red or pending PR out of draft is worse than leaving it there.
`ready` is supposed to mean the maintainer can merge without checking anything
else, and that claim is the only thing making the flag worth setting.

So `mark_ready` now also requires every check to have concluded SUCCESS,
SKIPPED or NEUTRAL. SKIPPED is green on purpose: this repo path-filters
Algorithm tests and Docker build, so every backend-only PR skips them and
treating that as not-green would withhold the action from almost everything.
An empty rollup is green too — a PR with no checks configured has nothing
failing.

The deferred list mirrors the same condition, so an approved-but-pending PR
with a P4 card appears in exactly one place rather than both.

Live effect: #633 falls back to resume_implementation until its CI settles,
which is the correct answer and the one the first version got wrong.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012LExo6fcbup75vtc9NfoAR

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
johanzander added a commit that referenced this pull request Aug 22, 2026
…ove (#653)

* fix: stop the fleet prune from destroying the worktrees it cannot remove

`git worktree remove` is sandbox-denied, and unlike `git worktree add` it
fails DESTRUCTIVELY. Removal deletes the working tree first and only then
unlinks `.git/worktrees/<name>` -- and that unlink is the denied one:

    error: failed to delete '.../worktrees/backlogger': Operation not permitted
    error: failed to delete '.git/worktrees/backlogger': Operation not permitted

By then ~393 tracked files are gone. It does not roll back. What is left is a
carcass: a registered worktree whose `git status` is a few hundred ` D` lines
and nothing else. Both prune loops read that as "uncommitted tracked changes"
and correctly refuse to auto-delete it -- so the failure makes the worktree
permanently unprunable BY ITSELF. Re-running hits the no-`--force` refusal;
`--force` re-hits the denial. `git worktree prune` performs the same unlink,
so it cannot clear the wreckage either.

13 carcasses accumulated across three sweeps (#568, #596, #597, #600, #601,
#603, #609, #612, #617, #629, #633, #634, #641) before anyone read the diff.
The last sweep reported them back as "a real backlog of stranded edits worth
reviewing" -- they were its own wreckage from the previous runs, and not one
byte of real work was in them.

Because the filename set is identical in every worktree, so is APFS's readdir
order, so every carcass loses the SAME ~393 paths (`core/`, `frontend/`,
`bess_manager/`, `pyproject.toml`, ...). Identical damage across many
worktrees is the signature, not a coincidence.

- Both prune loops now report `PRUNE` and emit one `!`-prefixed command for
  the maintainer to run unsandboxed, instead of removing anything themselves.
- Both classify a dirty set that is entirely ` D` as `CARCASS`, distinct from
  real edits. The predicate anchors on `^ D ` (unstaged deletions only), so a
  STAGED deletion still reads as intentional work.
- `local-agent-environment.md` gains the `remove`/`prune` half of the
  `.git/worktrees` denial, next to the `add` half it already documented.

`verify-sandbox.sh:88` predicted this in a parenthetical ("a stray directory
when `git worktree remove` then fails"); nothing acted on it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014FQA38o22RkHZJ4zEHDZS3

* fix: measure the prune half, which exits 0 while failing

The previous commit inferred that `git worktree prune` was denied because it
performs the same `.git/worktrees/<name>` unlink as `remove`. Measured now,
using a carcass as the probe -- `rm -rf` its directory, then prune:

    $ git worktree prune -v; echo "exit=$?"
    Removing worktrees/backlogger: gitdir file points to non-existent location
    error: failed to delete '.../.git/worktrees/backlogger': Operation not permitted
    exit=0

Denied as expected, but it EXITS 0. `remove` at least exits 255; prune reports
success having done nothing, so `git worktree prune && echo done` prints done.
That is the nastier of the two and was not in the inferred version.

Two further measured facts change the guidance:

- The denial is on the `.git/worktrees/<name>` unlink ONLY. `rm -rf` of the
  working tree from Bash SUCCEEDS. An agent can therefore always destroy the
  files and never the registration, and half-doing it converts a carcass into
  a `prunable` phantom -- no better, still needing the same unsandboxed fix.
- A phantom breaks both classification loops. Its directory is gone, so
  `git -C "$wt" branch --show-current` fails, `$b` comes back empty, and the
  detached-HEAD guard swallows it as "SKIP (detached)". Both loops now test
  `[ -d "$wt" ]` first and report PHANTOM. Verified over four cases: missing
  dir with and without a branch name, detached, and on a branch.

The emitted cleanup command gains a trailing `git worktree prune` (a phantom
has no directory, so `remove` has nothing to work with), uses `[ -n "$b" ]`
instead of `&&` since a phantom yields no branch name, and now says it must
run from a NON-worktree-isolated session -- an isolated one refuses the `cd`
to the shared checkout, which is why the first cleanup attempt silently did
nothing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014FQA38o22RkHZJ4zEHDZS3

* docs: record that `git branch -D` warns on .git/config but strands nothing

Third instance of the `.git/config` denial, alongside `checkout -b --track`
and `push -u`, and the same exit-0-while-partially-failing shape as the prune
bullet:

    $ git branch -D worktree-backlogger; echo "exit=$?"
    error: could not lock config file .../.git/config
    warning: update of config-file failed
    Deleted branch worktree-backlogger (was 6c70a77).
    exit=0

The first draft of this bullet asserted the denial leaves a stale
`[branch "<name>"]` stanza behind. Checked before committing, and it does not:
grepping .git/config afterwards found no stanza for the deleted branch, and
the only stale one in the file is an unrelated `undefined`. The branch had no
stanza to drop in the first place -- writing one needs `push -u` or
`checkout -b --track`, both denied by the two bullets above, so branches
created under this sandbox never have one. Recorded as the general case, with
the pre-sandbox branch explicitly marked untested rather than assumed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014FQA38o22RkHZJ4zEHDZS3

* docs: reconcile worktree-remove bullets with the rest of the permissions doc

The new destructive-failure bullets for `git worktree remove`/`prune`
contradicted three claims in the same file: the unattended list (which still
named `git worktree remove`), the "sandbox makes the unattended list safe"
thesis, and the "git already refuses the dangerous case" reasoning for the ask
list. A reader of only the Permissions section could conclude the command was
safe to call from sandboxed Bash — the exact bug the skills no longer call.
Carve the two verbs out as explicit exceptions and scope the git-refuses
reasoning to the cases git actually covers.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* docs: report worktree paths, not branch names, in the prune output

The sweep's emitted maintainer command reconstructed `.claude/worktrees/<name>`
from branch names, but `git worktree list` yields paths and includes sibling
worktrees outside `.claude/worktrees/` — the branch name alone cannot locate
the worktree, so removal would silently miss its target. Report `$wt` in the
PRUNE/CARCASS lines and drive the command from those paths. Also count PHANTOM
in implement-issue's emit-one-command line, matching sweep-prs.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* docs: fold the branch delete into Step 11's deferred worktree removal

The After-Merge cleanup splits in two: `ExitWorktree action=remove` clears the
worktree in-session, or removal is handed to the maintainer when the session
has already left. The old item 3 ran `git branch -D` right after either path,
but git refuses to force-delete a branch while its worktree registration
persists — the exact state the deferred path leaves behind, since only the
maintainer's not-yet-run command clears the registration. Emit the branch
delete as part of that same deferred command (remove first, then delete, as
sweep-prs does), and scope item 3 to the in-session path.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Beta 10 never goes through initialisation

2 participants