diff --git a/CHANGELOG.md b/CHANGELOG.md index df187cd..4216e42 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,57 @@ +## v0.232.0 — one writer at a time, and a check that can actually run (2026-09-01, R-411/R-408/R-407, R-414, R-412a) +**MinAgent: 0.129.0** (unchanged) + +**THE WALK FOUND THREE MORE ENTRY POINTS THAN THE REPORT DID.** R-411 named one function missing +`acquireRunning`. Fixing it and then pinning the invariant with an AST walk surfaced **four** in total: + +| function | why it mattered | +|---|---| +| `RestoreOffboxScratch` | the reported one | +| `OffboxRestorePrepareFull` | **the second request in the customer's own two-step full restore, and the one that actually shells `restic stats`.** The UI reaches it FIRST, so flagging only the restore would have left the collision reachable by the ordinary path | +| `RestoreSharesScratch` | R-411's exact shape on the **shares** tier: `unlockStale` + `resticStep`, a live web caller, and its sibling `PlaceSharesRestore` has always taken the flag | +| `RestoreOffbox` | no production caller today, but the same pattern — flagged so a future caller inherits the guard, not the defect | + +`OffsiteInventoryList` is **registered exempt with its reason**: it issues only `restic snapshots +--json`, measured not to take a lock, and flagging it would make browsing a page refuse during a +backup for no safety gain. + +- **THE REAL DELIVERABLE IS THE WALK, not the acquire.** `offbox_integrity.go:28` asserted *"Every + off-site operation takes `acquireRunning`"* since v0.227.0, nothing checked it, and it was **false + for months** — the **ninth** instance of this project's most-repeated class. `resticStep`'s licence + to run `unlock --remove-all` rests entirely on that sentence, so a false sentence there is a licence + to delete a live operation's lock. It is an **AST walk, not `strings.Contains`** — a commented-out + call still contains the string. **Red-proofed twice:** removing the acquire fails it naming + `RestoreOffboxScratch`; an unregistered fake entry point fails it naming the fake. +- **R-407 — two sentences corrected in place, not deleted** (R-360's rule). `check` *does* write a lock + file; and **`restic stats` takes one too**, which is the fact nobody had and the one that made R-411 + possible at all. +- **R-414 — the proof could not run at all on a box with no registered drive.** The determination came + out as **neither** "missed" nor "deliberate": R-356's own test comments say the scratch resolver + *"still resolves … only the DESTINATION moves"*, so it was **out of scope**, and it was never ruled + out on state-only grounds — the one comment about a `systemDataPath` fallback belonged to + `PlaceOffsiteRestore`, concerned bulk **userdata**, and R-356 overruled even that. So §6.3's rule + applies and **now has a fourth consumer**. +- **The fallback is SCOPED**, because the two callers ask different questions and one predicate + answering both is the R-356 defect itself: a **unit-only** restore may fall back to the system data + path (§7 records as `[FACT]` that a driveless app's unit already lives there indefinitely, and that + the same-device placement is *"intended, not a defect"*); a **full** restore keeps today's refusal, + because it pulls bulk userdata onto a state-only tier. +- **And the silence ends either way:** a proof that cannot start records `cannot_run` rather than an + `Err`, so `last_proof_result` is **never absent** — absent already means *"controller too old"*, and + a second meaning on one field is the `StatsKnown` trap one level up. Recorded **without** advancing + per-snapshot due-ness, so the app stays retryable once a drive is registered. +- **A defect I introduced and live validation caught:** the fallback resolved a scratch the cleanup + then refused to delete (*"not inside a proof root"* — its accepted-roots list is built from + registered drives, which a driveless box has none of). Every nightly proof would have left a copy + behind on exactly the boxes the fallback exists for. Fixed, and pinned by a pair of tests — one that + the copy IS deleted, one that a path outside every proof root is still **refused**. +- **R-412 leg 1** — a per-app push whose unit carried no dump and no tar now says so, at `WARN`. + Wording only; no guard, and the capture is untouched (08 §8.2). **Leg 2 stays OPEN.** +- 18 new tests, 1689 → 1707. Red-proofs run and reverted byte-identical for the walk (×2), the + driveless verdict, the push wording and the scratch leak. + +--- + ## v0.231.0 — the box proves its own off-site copy still holds something (2026-08-31, R-87) **MinAgent: 0.129.0** (unchanged) diff --git a/CONTEXT.md b/CONTEXT.md index 027825e..0bc122e 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -7,7 +7,63 @@ > > Ask Claude Code: "Please update CONTEXT.md with what we did today" -Last updated: 2026-08-31 (v0.231.0 — R-87: the box proves its own off-site copy still holds something) +Last updated: 2026-09-01 (v0.232.0 — R-411/R-408/R-407 the lock family, R-414 reachability, R-412a wording) + +> **2026-09-01 — v0.232.0. THREE RULINGS.** +> +> **1. `restic stats` TAKES A REPOSITORY LOCK, and that is the fact the whole R-411 chain rested on.** +> Nobody had it. Clean-room measured on demo-hp 2026-08-31: nothing else running, four invocations, +> the sampler reads `locks=1`. A customer FULL restore shells `stats` in its size probe, so it holds a +> lock — and until v0.232.0 it held no single-writer flag, so the integrity check was not blocked, ran, +> met that lock, and `resticStep` removed it with `unlock --remove-all` while logging *"a stale +> exclusive lock left by a previous crash"*. There was no crash. Also measured, and recorded so the +> next reader does not re-derive it: `restic check` takes a lock; `restic snapshots` and `restic list` +> do **not**. +> +> **2. THE INVARIANT IS PINNED BY A WALK, NOT BY A COMMENT — and the walk is the deliverable, not the +> acquire.** `offbox_integrity.go:28` asserted *"Every off-site operation takes `acquireRunning`"* from +> v0.227.0 and it was false for months, which is the ninth instance of this project's most-repeated +> class. `TestR408_EveryOffsiteEntryPointTakesTheFlagOrIsRegistered` is an AST pass over +> `internal/backup` — deliberately not `strings.Contains`, because a commented-out call still contains +> the string. **On its first run it found three entry points nobody had named**: +> `OffboxRestorePrepareFull` (the request the customer's UI reaches FIRST, and the one that shells +> `stats`), `RestoreSharesScratch` (R-411's exact shape on the shares tier, with a live caller) and +> `RestoreOffbox` (no caller today). All four now take the flag. `OffsiteInventoryList` is registered +> EXEMPT with its reason — `snapshots` only, measured not to lock, and flagging it would make a page +> refuse to load during a backup for no safety gain. **Adding a line to `offsiteExempt` is a deliberate +> act and belongs in the commit that adds it.** +> +> **3. R-414's BRANCH, AND THE EVIDENCE FOR IT.** The question was whether `offboxRestoreScratchDir` +> was MISSED by R-356's unification or EXCLUDED on purpose. **Neither label fits: it was consciously +> OUT OF SCOPE.** R-356's own commit (`08eb1a6`) says so in its test comments — *"the prepared scratch +> still resolves to the registered storage path … only the DESTINATION moves, which is precisely what +> this change is about"* — and every one of its fixtures assumed a registered storage path exists. +> `demo-felhom`, with `storage_paths: []`, is the case it never had. It was **never ruled out on +> state-only grounds**: the one comment about a `systemDataPath` fallback belonged to +> `PlaceOffsiteRestore`, concerned bulk **userdata**, and R-356 deleted it deliberately. This +> function's own documented exclusion is `cfg.Paths.DataDir` — the **rootfs** — a different filesystem. +> +> **So §6.3's `[DESIGN]` rule applies and now has a FOURTH consumer.** But the fallback is **SCOPED**, +> because the two callers ask different questions and one predicate answering both is the R-356 defect +> itself: **unit-only** may fall back (§7 records as `[FACT]` that a driveless app's unit already lives +> on `systemDataPath` indefinitely and that the same-device placement is *"intended, not a defect"*); +> **full** keeps the R-252 refusal, because it pulls bulk userdata onto a state-only tier (§2.2). +> +> **AND THE SILENCE ENDS EITHER WAY.** `ProofResultCannotRun` is recorded through +> `RecordProofVerdict`, so `last_proof_result` is never ABSENT — absent already means *"controller +> older than v0.231.0"*, and giving one field two meanings is the `StatsKnown` trap one level up. It +> does **not** advance per-snapshot due-ness: nothing was proved, and marking one proved would stop the +> app being retried once a drive is finally registered. +> +> **A MISTAKE OF MINE, RECORDED BECAUSE LIVE VALIDATION IS WHAT CAUGHT IT.** The fallback resolved a +> scratch that `removeProofScratch` then refused to delete — its accepted-roots list is built from +> REGISTERED drives, and a driveless box has none. Observed on `demo-felhom`: *"refusing to remove … +> it is not inside a proof root"*, with the copy still on disk. Every nightly proof would have left one +> behind, on exactly the boxes the fallback exists for. **The unit tests all registered a drive, so +> none of them could see it.** Fixed, and pinned by a pair — one that the copy IS removed on a +> driveless box, one that a path outside every proof root is still REFUSED, so the fix is not a +> widening into uselessness. + > **2026-08-31 — v0.231.0. FOUR RULINGS, recorded so none is re-litigated.** > diff --git a/controller/README.md b/controller/README.md index b611286..0bfc58a 100644 --- a/controller/README.md +++ b/controller/README.md @@ -1114,8 +1114,9 @@ restores cleanly, and gives the customer nothing back. Measured on `demo-hp` 202 | where the expectation comes from | the unit's **own** `compose/docker-compose.yml`, never the live box — the snapshot may predate the app's current shape. Database: `DBServiceNames` (the same discriminator the restore path uses). Volumes: `ParseComposeNamedVolumes`, as an **existence** check, not a name match | | outcomes | **three:** pass, fail (readable and empty), and **cannot judge**. An app that legitimately has no database and no named volumes **passes** | | repository writes | **none.** `--no-lock`, no `unlockStale`, and the exec seam rather than `resticStep`, so the `unlock --remove-all` escalation is unreachable. Asserted on the argv as a non-effect | -| guard | takes the single-writer flag itself and **SKIPS rather than waits** (`RestoreOffboxScratch` does not take it — R-408) | -| scratch | `backups/offsite-proof/` — a **separate root** from the customer's `backups/offsite-restore/`, so the nightly delete can never reach a copy the customer made, and a proof copy can never be offered for placement | +| guard | takes the single-writer flag itself and **SKIPS rather than waits**. **Since v0.232.0 every off-site entry point takes it** — `RestoreOffboxScratch`, `OffboxRestorePrepareFull`, `RestoreSharesScratch` and `RestoreOffbox` were all missing it (R-411/R-408), and the invariant is now pinned by an AST walk rather than asserted in a comment | +| scratch | `backups/offsite-proof/` — a **separate root** from the customer's `backups/offsite-restore/`, so the nightly delete can never reach a copy the customer made, and a proof copy can never be offered for placement. **On a box with NO registered data drive it falls back to the system data path** (v0.232.0, R-414), because that is where a driveless app's unit already lives; the customer's FULL restore does not fall back and still refuses | +| if it cannot run at all | it records **`cannot_run`**, not silence (v0.232.0, R-414). A proof that never started is a standing property of the machine, not a transient failure, so it is written where the hub can read it — `last_proof_result` is never ABSENT, because absent already means *a controller too old to have the feature*. Per-snapshot due-ness is NOT advanced, so the app is retried once a drive is registered | | timeout | 10 min (`proofRestoreTimeout`) — ~150x the slowest single app measured | | cost, measured on demo-hp | **one app 2.3–4.0 s**, all eight back to back **25 s**, peak scratch = that app's logical size (213 MB largest). The weekly check beside it takes 40.3 s | | result | persisted on `settings.OffboxTarget` (`proved_snapshots`, `last_proof_*`) and published on `OffboxReportStatus`. `last_proof_result` absent = **NOT RECORDED** (a pre-0.231.0 controller), never "failed" |