diff --git a/CHANGELOG.md b/CHANGELOG.md index d4ad15a..dffbf63 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -109,6 +109,24 @@ fixed:** the fixture refused earlier, at the placement stat pre-pass, so `stops the pre-fix code. The scratch is now populated the way a completed download leaves it, and the assertions are ordered so a removed gate reports the outage rather than "no error returned". +### One defect the fix itself introduced, caught by a gate and fixed rather than filed + +Writing the REPORT's observation *"the no-unit fallback already reports a zero result, which is +honest"* exposed that the sentence was **false**. A zero `UnitRestoreResult` is Scenario B's shape, so +`RestoreFromRecoveryUnit`'s fallback to `RestoreApp` — which returns only an error, and whose signature +is deliberately out of scope — would have printed „ez a mentés csak a beállításokat tartalmazta, adatot +nem" over a restore that may have replayed the app's entire dataset. **An unknown drawn as a zero: the +exact R-88 failure direction this whole change exists to remove, re-introduced by the change.** + +`UnitRestoreResult` now carries **`CountsUnknown`**, the fallback sets it, and there is a fourth +sentence claiming only what is known: „A(z) X visszaállítása lefutott — az alkalmazás újraindult. Ehhez +a mentéshez nem tartozik mentési egység, ezért nem tudjuk megmondani, mi állt vissza belőle." Pinned by +`TestUnitRestoreOutcome_NoUnitFallbackSaysUnknownNotEmpty`; the A5 seam test was corrected too, because +its fixture has no unit and so exercises exactly this path while asserting the wrong sentence. + +**It was the `observations` gate refusing the push that forced the re-read** — a gate written to stop +findings dying in an overwritten REPORT.md caught a live defect instead. + **Green gate:** `go build ./... && go vet ./... && go test ./...` — 28 packages, rc 0. ## v0.225.0 — the hub could not tell an empty off-site store from an unmeasured one (2026-08-30, R-331) diff --git a/REPORT.md b/REPORT.md index fb519ba..21e9c17 100644 --- a/REPORT.md +++ b/REPORT.md @@ -228,10 +228,10 @@ destructive restore. - **Website version bump:** not applicable — the site does not display the controller version (checked, not assumed). -## 11. Observations — noticed, documented, not acted on +## 11. Observations — each with a register row, or a stated reason it needs none -1. **Scenario F's question, answered — and the answer is worse than the question assumed. Filed as - R-396.** The spec asked whether the real UI flow can reach a state where a unit-only scratch makes +1. **FILED: R-396.** *Scenario F's question, answered — and the answer is worse than the question + assumed.* The spec asked whether the real UI flow can reach a state where a unit-only scratch makes the full-restore action appear. **It can, by the safest-looking action on the page.** „Ellenőrző visszaállítás" (`mode=unit`, the default, advertised as non-destructive) calls `RestoreOffboxScratch(full=false)`; `offboxRestoreScratchDir` **ignores `full`**, so both modes @@ -241,16 +241,63 @@ destructive restore. a *failed* download. It needed only a successful safe one. **The generalisable defect: one boolean answered three different questions** — "is there a scratch", "may we place", "may we destructively restore" — and the weakest of the three set the answer. -2. **`NotifyIntegrityOK` / `NotifyIntegrityFailed` are dead code** (noticed during R-331 earlier - today, restated here because it is restore-adjacent): they exist and are called from nowhere; the - controller runs no integrity check at all. -3. **`resticStep` is not a seam**, which is why R-358's ordering property needed an AST test rather - than an execution test. If a future task needs to drive restic paths under test, that is the seam - to add — and it should be added deliberately, not improvised inside a bugfix. -4. **`RestoreApp`'s own `restoreDockerVolumes` count is still discarded.** Left alone deliberately: - the spec scoped `RestoreApp` out, and it is the no-unit fallback whose result is already reported - as a zero `UnitRestoreResult` — which is honest, because a box with no recovery unit really did - return no data from one. Widening it would mean changing `RestoreApp`'s signature, which §5 forbids. -5. **The spec's own §13 step 1 named an app whose shape has since changed.** Not a defect in the - spec — a reminder that fixture assumptions about live boxes decay, and the instruction to - "confirm its manifest first rather than assuming" is what caught it. + +2. **FILED: R-397.** *`NotifyIntegrityOK` / `NotifyIntegrityFailed` are dead code and the product + advertises a check that does not exist.* Both notifiers have no caller anywhere; the controller + runs no integrity check at all. **The part that actively misleads:** `config.Monitoring.PingUUIDs` + carries a `backup_integrity` field and the monitoring page renders + „Mentés integritás — Hetente (vasárnap)", so the operator is told a weekly check runs. Noticed + twice in one day from opposite directions (R-331's dead report fields, then this task), which is + why it earned a row rather than a note. + +3. **FILED: R-398.** *`resticStep` is not a seam, so no test can drive any restic-backed path.* Felt + directly here: R-358's safety property is an ORDER (clear before restic, write after) and with no + seam it could only be pinned by an **AST walk** rather than by execution. That is honest about what + it proves and would still miss a reordering introduced through a helper. The contrast is the + argument: `offboxLatestSnapshot` got a seam in this same release, in four lines, because a + correctness gate could not otherwise be proven. + +4. **NOT-A-FINDING: it was ACTED ON in this release instead of filed — and the acting is the story.** + `RestoreApp`'s own `restoreDockerVolumes` count is still discarded, and the spec scopes + `RestoreApp` out. My first draft justified that as *"already reported as a zero result, which is + honest, because a box with no recovery unit really did return no data from one."* + + > **⚠ THAT SENTENCE WAS FALSE, AND WRITING IT EXPOSED A DEFECT THE FIX ITSELF HAD INTRODUCED.** + > A zero `UnitRestoreResult` is *Scenario B's shape*, so the no-unit fallback would have printed + > „ez a mentés csak a beállításokat tartalmazta, adatot nem" over a restore that may have replayed + > the app's entire dataset — **an unknown drawn as a zero, the exact R-88 failure direction this + > whole change exists to remove**, re-introduced by the change itself. + > + > Fixed rather than filed: `UnitRestoreResult` now carries `CountsUnknown`, the fallback sets it, + > and the surface has a fourth sentence claiming only what is known — the restore ran, the app is + > back, and we cannot say what came back. **`RestoreApp`'s signature is untouched**, so §5 holds. + > Pinned by `TestUnitRestoreOutcome_NoUnitFallbackSaysUnknownNotEmpty`; the A5 seam test was + > corrected too, because its fixture has no unit and therefore exercises exactly this path while + > asserting the wrong sentence. + > + > **It was the `observations` gate refusing the push that forced the re-read.** The gate exists to + > stop findings dying in an overwritten REPORT.md; here it caught a live defect instead. + +5. **NOT-A-FINDING: the spec's own instruction already handled it, so there is nothing to carry forward.** §13 step 1 named `opengist` as + the data-less app; it is not one any more. Not a defect in the spec: it said *"confirm its manifest + first rather than assuming"*, and that is exactly what caught it. Recorded only as a reminder that + fixture assumptions about live boxes decay faster than the documents naming them. + +## 12. Two gates fired; one push was bypassed, and both are declared + +**`felhom.eu` — `golden-currency` CONVICTED.** Three controller releases shipped today (0.224.0, +0.225.0, 0.226.0) and the vouched golden still carries **0.223.0**, so a machine installed right now +receives none of them. **The gate is right.** A **BYPASS, not a waiver**, on the operator's standing +ruling from earlier today — re-checked for this release rather than reused blindly: all three are +invisible to a day-0 box, and a *restore-surface* fix in particular has nothing to act on there. +**The ground expires the moment a release changes first-boot behaviour.** Tracked on **R-242**; **one +bake carrying 0.226.0 covers all three**, then raise the floor to 0.226.0. + +**`felhom-controller` — nothing was bypassed.** The `observations` gate refused the first REPORT push +and was **fixed, not bypassed** — see §11 item 4 for what that cost and what it caught. + +**And one earlier gate was fixed rather than bypassed:** `due-checks` was red on R-341's overdue +`+7 d` measurement. It was **taken** during this session (ep0: fd **17**, the baseline, same proxy +generation, PID 551655 unchanged), and the verdict recorded as **unanswerable** — our own R-344 fix +removed the leak mid-interval, so a slope of ~0 measures that fix and not the PBS upgrade the row was +asking about. diff --git a/controller/internal/backup/restore_unit.go b/controller/internal/backup/restore_unit.go index e6d6dc3..07ffa76 100644 --- a/controller/internal/backup/restore_unit.go +++ b/controller/internal/backup/restore_unit.go @@ -155,6 +155,16 @@ type UnitRestoreResult struct { ManifestVolumes int // ManifestDBs is len(manifest.DBDumps): the same claim for the database leg. ManifestDBs int + // CountsUnknown marks a run whose counts could not be established AT ALL — today the one case is + // the no-unit fallback to RestoreApp, which returns only an error and whose signature is + // deliberately out of scope. + // + // IT EXISTS BECAUSE THE ZERO VALUE WOULD OTHERWISE LIE. Without it a fallback restore that really + // replayed three volumes reports VolumesReplayed=0 / ManifestVolumes=0 — the shape the surface + // reads as „ez a mentés csak a beállításokat tartalmazta, adatot nem". That is a confident false + // statement, and precisely the R-88 failure direction (degrade to NO DATA rather than to UNKNOWN) + // this whole change exists to remove. An unknown must be carried, never drawn as a zero. + CountsUnknown bool } // RestoreFromRecoveryUnit recreates an app from its on-drive recovery unit. @@ -199,11 +209,10 @@ func (m *Manager) RestoreFromRecoveryUnit(stackName string) (UnitRestoreResult, m.mu.Lock() m.running = false // RestoreApp re-acquires the running flag m.mu.Unlock() - // The fallback path has no unit and therefore no manifest to count against: a ZERO result is - // the honest answer, not a missing one. The surface must be able to tell "nothing came back" - // from "we never looked", and it can — ManifestVolumes/ManifestDBs are zero too, which is - // Scenario B's shape and reads as "the backup held no data", which is exactly true of a box - // with no recovery unit. + // The fallback has no unit and no manifest, and `RestoreApp` returns only an error — so the + // counts here are genuinely UNKNOWN, not zero. Reporting zero would tell a customer whose + // volumes were just restored that their backup held no data. + res.CountsUnknown = true return res, m.RestoreApp(stackName, "") } diff --git a/controller/internal/web/handlers.go b/controller/internal/web/handlers.go index 08eed9d..abd4922 100644 --- a/controller/internal/web/handlers.go +++ b/controller/internal/web/handlers.go @@ -1528,6 +1528,12 @@ func unitRestoreOutcomeMsg(app string, res backup.UnitRestoreResult) string { } return fmt.Sprintf(unitRestoreDataMsgFmt, app, what) } + // An unknown must never be drawn as a zero. The no-unit fallback cannot report counts, and the + // zero-value shape would otherwise read as „the backup held only settings" over a restore that may + // have replayed the app's whole dataset. Same failure direction as R-88: degrade to UNKNOWN. + if res.CountsUnknown { + return fmt.Sprintf(unitRestoreCountsUnknownMsgFmt, app) + } if res.ManifestVolumes+res.ManifestDBs > 0 { return fmt.Sprintf(unitRestoreNoneReturnedMsgFmt, app, res.ManifestVolumes, res.ManifestDBs) } @@ -1551,6 +1557,10 @@ const ( // database dumps LISTED. It states the data is unchanged because that is true and load-bearing: the // replay only ever writes into volumes, so a replay that did nothing removed nothing, and a customer // who believes otherwise will do something worse than waiting. + // unitRestoreCountsUnknownMsgFmt — the no-unit fallback. It claims only what is known: the restore + // ran and the app is back. It deliberately does NOT say data returned and does NOT say it did not. + unitRestoreCountsUnknownMsgFmt = "A(z) %s visszaállítása lefutott — az alkalmazás újraindult. Ehhez a mentéshez nem tartozik mentési egység, ezért nem tudjuk megmondani, mi állt vissza belőle. Ellenőrizd az alkalmazásban, hogy megvannak-e az adataid." + unitRestoreNoneReturnedMsgFmt = "A(z) %s: FIGYELEM — a mentés %d adatkötetet és %d adatbázis-mentést sorol fel, de egyik sem állt vissza. Az adataid változatlanok maradtak. Kérj segítséget, mielőtt újra próbálod." ) diff --git a/controller/internal/web/r353_unit_outcome_test.go b/controller/internal/web/r353_unit_outcome_test.go index b796545..ef05164 100644 --- a/controller/internal/web/r353_unit_outcome_test.go +++ b/controller/internal/web/r353_unit_outcome_test.go @@ -170,14 +170,49 @@ func TestR353_HandlerPublishesTheOutcome(t *testing.T) { t.Fatal("the restore never reached a terminal status") } - // There is no recovery unit and no data on this drive, so nothing came back: Scenario B. + // THE ASSERTION THAT CARRIES THE SEAM: whatever branch fires, the sentence the customer is shown + // must be `unitRestoreOutcomeMsg`'s output and not the pre-fix string. if strings.Contains(last, "visszaállítva (snap-123)") { t.Fatalf("THE PRE-FIX SENTENCE REACHED THE CUSTOMER: %q — it is true of a restore that "+ "returned an entire dataset and of one that returned nothing", last) } - for _, want := range []string{"csak a beállításokat tartalmazta", "NEM álltak vissza"} { - if !strings.Contains(last, want) { - t.Fatalf("the published outcome does not state that no data came back; missing %q in %q", want, last) - } + // This fixture has no recovery unit, so the production path takes the RestoreApp fallback — where + // the counts are genuinely unknown. The honest sentence for that is the unknown one, and asserting + // it here is what pins the fallback to it: the zero-value shape would otherwise print + // „csak a beállításokat tartalmazta" over a restore that may have returned everything. + if strings.Contains(last, "csak a beállításokat tartalmazta") { + t.Fatalf("the no-unit fallback claimed the backup held no data, from counts it never "+ + "established: %q", last) + } + if !strings.Contains(last, "nem tudjuk megmondani") { + t.Fatalf("the published outcome does not state the unknown as unknown; got %q", last) + } +} + +// TestUnitRestoreOutcome_NoUnitFallbackSaysUnknownNotEmpty — the defect the first draft of this fix +// introduced, caught by the observations gate forcing a re-read of my own code. +// +// `RestoreFromRecoveryUnit` falls back to `RestoreApp` when there is no recovery unit, and `RestoreApp` +// returns only an error — its signature is deliberately out of scope. So the result is a ZERO value, +// and a zero `UnitRestoreResult` is Scenario B's shape: „ez a mentés csak a beállításokat tartalmazta, +// adatot nem". That sentence would be printed over a fallback restore that had just replayed the app's +// entire dataset. **An unknown drawn as a zero is the R-88 failure direction**, and it is exactly what +// this whole change exists to remove — so it must not be re-introduced by the fix itself. +func TestUnitRestoreOutcome_NoUnitFallbackSaysUnknownNotEmpty(t *testing.T) { + msg := unitRestoreOutcomeMsg("legacyapp", backup.UnitRestoreResult{CountsUnknown: true}) + + if strings.Contains(msg, "csak a beállításokat tartalmazta") { + t.Fatal("FALSE CLAIM: told the customer the backup held no data when the counts were never " + + "established — a fallback restore may have returned everything they own") + } + if strings.Contains(msg, "adatkötet") { + t.Fatal("claimed a volume count that was never measured") + } + if !strings.Contains(msg, "nem tudjuk megmondani") { + t.Errorf("an unknown must be STATED as unknown, not left silent; got %q", msg) + } + // And it must still tell them the restore ran, or the sentence reads as a failure. + if !strings.Contains(msg, "lefutott") { + t.Errorf("the message must say the restore completed; got %q", msg) } }