amadan
Flush-only class, second review answered: a unit-backed route's rung can be cleared; the test waits for the start-up snapshot; the note says what the branch does not deliver
Both reviewers accepted with fixes. The unit-backed refusal blocked clearing a rung such a route still carried; the rail is now target-aware (named rungs refused, the clear let through), with a test. The lease test waits for the level 9 start-up snapshot before it writes, and its control requires the log level to advance with no new snapshot; a snapshot landing mid-case skips. The note and comments are corrected: listings print the raw word; the refusal is the CLI's alone; the snapshot is once per UTC day; the lease comment's unconditional durability claim points at the open items. The inherited Close defect (discussion 200), the unbounded backlog, the unverified wake boundary and the class-blind lag checks stay stated open: this branch does not implement the Free plan's durability.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
9 files changed,
+350
−38
cli.go+6 −3docs/superpowers/specs/2026-10-02-flush-only-replication-class.md+10 −8docs/superpowers/specs/CODEX-FINDINGS-flush-only-class-review2.md+157 −0docs/superpowers/specs/OPUS-REVIEW-flush-only-class-review2.md+118 −0internal/activator/activator.go+7 −6internal/lease/flushonly_test.go+25 −9internal/lease/lease.go+6 −1internal/registry/registry.go+6 −3route_replication_test.go+15 −8
CI passed — run details
diff --git a/cli.go b/cli.go| index 6f9e630fa..f51e95205 100644 |
| --- a/cli.go |
| +++ b/cli.go |
| @@ -1657,7 +1657,7 @@ func unitWord(unit string) string { |
| // |
| // The value's shape is checked at the flag, before the registry is opened. |
| func runRouteReplication(r *registry.Registry, rt registry.Route, class string, dryRun bool) { |
| - if err := replicationRail(rt); err != nil { |
| + if err := replicationRail(rt, class); err != nil { |
| log.Fatalf("route: %s: %v", rt.Host, err) |
| } |
| prefix := "" |
| @@ -1684,12 +1684,15 @@ func runRouteReplication(r *registry.Registry, rt registry.Route, class string, |
| // replicationRail is runRouteReplication's pair of refusals, split out as a |
| // pure function for the reason unitRail and backingRail are: a rail nobody |
| // can exercise without running the process is a rail nobody tests. |
| -func replicationRail(rt registry.Route) error { |
| +func replicationRail(rt registry.Route, class string) error { |
| // A unit-backed route's database is not replicated by the activator at |
| // all (its lease is inert: the unit's own replication owns the file), |
| // so a rung written here would be listed and honoured by nothing. |
| // (Review of the flush-only class, 2026-10-02: the rail accepted it.) |
| - if rt.Backing == registry.BackingUnit { |
| + // Clearing is not a rung: a unit-backed route that still carries one |
| + // (set before this refusal, or before its backing changed) must be |
| + // able to lose it, so the clear is let through (second review). |
| + if rt.Backing == registry.BackingUnit && class != "" { |
| return fmt.Errorf("route is unit-backed — the activator does not replicate a unit-backed route's database (the unit's own replication does), so a rung set here would be written, listed and honoured by nothing") |
| } |
| if !rt.Hibernate { |
diff --git a/docs/superpowers/specs/2026-10-02-flush-only-replication-class.md b/docs/superpowers/specs/2026-10-02-flush-only-replication-class.md| index 9948182d8..8b63a850d 100644 |
| --- a/docs/superpowers/specs/2026-10-02-flush-only-replication-class.md |
| +++ b/docs/superpowers/specs/2026-10-02-flush-only-replication-class.md |
| @@ -1,32 +1,34 @@ |
| # The flush-only replication class |
| -Status: on `feat/carloku-flush-only-class-20261002`, 2026-10-02. **Not mergeable as a feature: its first review (Opus accept with fixes, Astra reject; both reports in this directory) showed that what the class promises is not what the vendored litestream does, and what it should promise is a decision (`DECISIONS-FOR-PAUL.md` 10a on the launch branch).** Until that is decided the class is a word the registry's column may hold and the activator maps; it is not offered to operators and nothing sets it. Nothing is deployed. |
| +Status: on `feat/carloku-flush-only-class-20261002`, 2026-10-02. **This branch does not implement the Free plan's durability; it withdraws an attempt at it. Not mergeable as a feature: its first review (Opus accept with fixes, Astra reject; both reports in this directory) showed that what the class promises is not what the vendored litestream does, and what it should promise is a decision (`DECISIONS-FOR-PAUL.md` 10a on the launch branch).** Until that is decided the class is a word the registry's column may hold and the activator maps; it is not offered to operators and nothing sets it. Nothing is deployed. |
| The approved pricing design (`2026-09-07-carloku-pricing-plans-design.md`, "The free tier and the pool") says: "Durability on free: hibernate-flush only", and "This needs a new replication class... Flush-only must be added as a class and propagated from the account's plan." |
| ## What is on the branch |
| - The word `hibernate-flush` in the registry's `routes.replication` vocabulary (`ValidReplication`), the same word the entitlement record carries, with a test in each direction that the two vocabularies agree. |
| -- It is **not** in `ReplicationClasses()` (the rungs on offer); `carlos route --replication hibernate-flush` is refused as not offered (`replicationOffered`); a listing that meets the word says so. |
| +- It is **not** in `ReplicationClasses()` (the rungs on offer); `carlos route --replication hibernate-flush` is refused as not offered (`replicationOffered`). The refusal is the CLI's alone: `SetReplication` and `Upsert` accept the word, and a route that carried it would be woken on the ten-year interval and listed by `carlos routes`, `carlos instances` and the console API with the raw word (only the setter's own before-and-after line says "not offered"). No code sets it today; the first writer that propagates a plan's class must not reach the setter before decision 10a. |
| - The activator maps it to a sync interval no awake period reaches (ten years). |
| -- The CLI rail refuses any rung on a unit-backed route (the activator does not replicate a unit-backed route's database; the rail accepted the write before, for every class). |
| +- The CLI rail refuses any named rung on a unit-backed route (the activator does not replicate a unit-backed route's database; the rail accepted the write before, for every class). Clearing is let through, so a unit-backed route that still carries a rung can lose it. |
| ## What the ten-year interval does, against the vendored litestream |
| - The replica monitor syncs the local log to the bucket once after the store opens, at the database monitor's first ten-second tick, not at the wake itself, and then not again until Close. |
| - `Replicator.Close` forces a sync and then closes the store, which flushes what was written while awake. |
| -- **The snapshot monitor still uploads the whole database** at its first pass and at each 00:00 UTC while an instance is awake, whatever the interval, and compactions still write. So "nothing between wake and sleep" is false: for an awake period under a day the class behaves as `daily` does. |
| -- **Everything written while awake waits on the box for Close**, and the stop path gives Close thirty seconds. An instance awake for hours with steady writes holds thousands of log files; if they do not ship in time the flush fails, the lease is still released, and the instance is still reported hibernated with its outcome recorded as uncertain. Under `daily` the backlog is at most a day's; here it has no bound. So "hibernation is durable" does not hold unconditionally for this class. |
| +- **The snapshot monitor still uploads the whole database** once per UTC day (at the first wake of that day, or at 00:00 while awake; a recent enough snapshot, an error or a database not yet ready skips the pass), whatever the interval, and compactions still write. So "nothing between wake and sleep" is false: for an awake period under a day the class behaves as `daily` does. |
| +- **The log written while awake and not yet uploaded waits on the box for Close** (a snapshot may already have carried those writes), and the stop path gives Close thirty seconds. An instance awake for hours with steady writes holds thousands of log files; if they do not ship in time the flush fails, the lease is still released, and the instance is still reported hibernated with its outcome recorded as uncertain. Under a healthy `daily` the backlog is about a day's (a failed first sync waits about 48 hours, and repeated failures have no bound); here it has no bound at all. So "hibernation is durable" does not hold unconditionally for this class. |
| - The local log is not trimmed until uploaded (retention works from the remote listing), so a busy instance that does not sleep fills the box. |
| - After a failed first sync the monitor waits its interval and then a backoff of the same length: about 48 hours for `daily`, never (before Close) here. |
| -- The restore drill (`internal/ops`) allows fifteen minutes of lag and knows nothing of classes: an awake instance on this class, or on `daily`, reads as stale. |
| +- The restore drill (`internal/ops`) allows fifteen minutes of lag and knows nothing of classes: an awake instance on this class, or on `daily`, whose replica trails a source that has been quiet for fifteen minutes reads as stale. The lag alert (`internal/metrics/alert.go`, 900 seconds) and the status snapshot's fifteen-minute threshold are class-blind too; nothing feeds them lag yet. |
| ## Open, each needing the decision or its own change |
| 1. What the class promises (decision 10a): a backstop interval, an unbounded flush at sleep, or no such class. Not chosen here; the ten-year interval is what was built first and is not a proposal. |
| 2. Inherited, every class (Astra 1, P1): `Replicator.Close` drops the error of its forced sync; if the database never initialised (a drain right after a wake whose remote lookup failed), litestream's `DB.Close` skips its final sync and Close returns nil having uploaded nothing. And a flush that does fail still ends in hibernation. Filed for its own fix; not changed on this branch. |
| 3. Inherited (Astra 4): the first sync after a wake is asynchronous and can ship an older log position than the wake's. |
| -4. The restore drill's class-blind freshness check (Astra 6). |
| +4. The restore drill's class-blind freshness check (Astra 6), and the class-blind lag alert and status threshold. |
| +4a. The first pass after a wake is not a verified wake boundary, and the approved design's "hibernate = durable" and "no restore to an earlier moment" are requirements this branch does not deliver: existing snapshot and log retention still permits earlier restores. |
| +4b. Tests not built: initialisation failure at Close, a warm re-wake, an immediate Close, deterministic snapshot boundaries. |
| 5. Plan propagation, the Free entitlement, the per-plan idle window, the app cap, storage refusal (`OUTSTANDING.md` 15). |
| ## Mixed versions |
| @@ -35,7 +37,7 @@ An older activator reads an unknown class as litestream's default (stream): neve |
| ## Tests |
| -- `internal/lease` `TestALongSyncIntervalSyncsTheLogAfterOpenAndAtCloseOnly`: litestream against a local file replica. The control waits until the awake rows restore under the default interval; the long interval adds no log (level 0) file and restores the pre-write rows for longer than the control needed; Close restores all rows. It does not show that nothing is uploaded while awake (the snapshot is), and says so. |
| +- `internal/lease` `TestALongSyncIntervalSyncsTheLogAfterOpenAndAtCloseOnly`: litestream against a local file replica. Each case waits for the start-up snapshot (a level 9 file) before it writes. The control waits until the awake rows restore under the default interval and the log level has advanced with no new snapshot; the long interval adds no log (level 0) file and restores the pre-write rows for longer than the control needed; Close restores all rows. A snapshot landing mid-case (a run across 00:00 UTC) skips rather than fails. It characterises the lease and litestream as vendored, passes without this branch's production code, and does not show that nothing is uploaded while awake (the snapshot is). |
| - `internal/activator` `TestActivatorWakeThreadsReplicationClass`: the class threads the long interval, not 0. |
| - `internal/registry` `TestValidReplicationVocabulary`: valid, not offered; three rungs on offer. |
| - `internal/console` and `internal/entitlement`: the two vocabularies agree in both directions. |
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-flush-only-class-review2.md b/docs/superpowers/specs/CODEX-FINDINGS-flush-only-class-review2.md| new file mode 100644 |
| index 000000000..e66cc0158 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-FINDINGS-flush-only-class-review2.md |
| @@ -0,0 +1,157 @@ |
| +All references below are at **`5c327226`**. Both requested diffs were reviewed. No files were modified; no tests, network commands or AWS commands were run. |
| + |
| +## First-review dispositions |
| + |
| +“Closed” below means closed for this branch’s narrowed scope: a reserved, unoffered class—not delivery of the approved Free-tier durability feature. |
| + |
| +| First-review finding | Disposition | Evidence | |
| +|---|---|---| |
| +| **Astra 1:** Close can succeed without flushing; failed flushing still ends in hibernation | **Stated open** | Design note `:27` acknowledges both. `internal/lease/lease.go:396` and `internal/activator/activator.go:2188` remain unchanged. | |
| +| **Opus P2-1:** Unbounded backlog versus thirty-second Close | **Stated open** | Design note `:19`, `:26`. No backlog bound or shutdown change was added. | |
| +| **Opus P2-2 / Astra 2:** Uploads still occur while awake | **Stated open** as a feature requirement; the principal false claim was withdrawn | Design note `:18` acknowledges snapshots and compactions. Their behavior is unchanged. | |
| +| **Astra 3:** Unuploaded local L0 files accumulate | **Stated open** | Design note `:20`; retention remains unchanged. | |
| +| **Astra 4:** Wake does not establish a synchronous, current durable boundary | **Stated open** | Design note `:28`; `Replicate` still starts monitors asynchronously. | |
| +| **Astra 5 / Opus P3-2, unit assignment:** A unit-backed route accepts an ignored class | **Closed for named CLI assignments** | `cli.go:1692` rejects unit backing. It introduces a separate clearing regression below. | |
| +| **Astra 6 / Opus P3-2, freshness and alerts:** Class-blind lag interpretation | **Stated open** | Design note `:29`. Restore verification and the dormant lag consumers are unchanged. | |
| +| **Opus P2-3 / Astra 7:** Test control and startup races | **Wrongly claimed closed in full** | Restored-row control, L0 counting and cleanup are real fixes. The assertion that the startup snapshot precedes the writes remains unsupported; see finding 4. | |
| +| **Opus P3-1 / Astra 8:** Operator guidance knows only three classes | **Closed for the offered vocabulary** | Three offered classes are now correct. Change output describes the reserved word, but ordinary listings do not carry the claimed warning. | |
| +| **Opus P3-2, plan propagation** | **Stated open** | Design note `:30`; no enforcement or propagation production code changed. | |
| +| **Opus P3-3:** Entitlement vocabulary checked only one way | **Closed for all current values** | `internal/entitlement/replication_vocab_test.go:13` checks both directions, including the reserved word and unset value. | |
| + |
| +## Findings |
| + |
| +### P1 — Close can still report success without establishing durability — explicitly open, inherited |
| + |
| +**Locations:** `internal/lease/lease.go:396`; `third_party/litestream/db.go:788`, `:796`, `:1025`; `internal/activator/activator.go:2190`. |
| + |
| +**Scenario:** Drain immediately after wake, before database initialization. The forced `SyncAndWait` fails during the remote-position lookup at `db.go:1427`. Initialization resets `db.db` to nil. `Replicator.Close` discards the error, and `DB.Close` skips both final synchronization stages. Successful lease release can leave Close returning nil despite uploading nothing. |
| + |
| +When an initialized flush returns an error, the activator records uncertainty but still removes the entry and logs “hibernated” at `activator.go:2201`. Subsequent host loss can therefore lose writes **while asleep**. |
| + |
| +**Smallest acceptable fix before offering the feature:** Preserve the forced-sync error unless a subsequent synchronization demonstrably succeeds, and retain a recoverable failed-flush state instead of treating incomplete durability as completed hibernation. |
| + |
| +This is accurately stated open. It is not newly introduced by the second-review delta. |
| + |
| +### P2 — The retained mapping still has unbounded backlog and no guaranteed wake boundary — explicitly open |
| + |
| +**Locations:** `internal/activator/activator.go:328`, `:338`, `:2184`; `third_party/litestream/db.go:1090`, `:2696`, `:2718`; `third_party/litestream/replica.go:170`. |
| + |
| +Two scenarios remain: |
| + |
| +- A continuously busy instance generates local L0 files for days. Retention considers remotely listed files covered by L1 compaction; it cannot reclaim the withheld local backlog. Snapshots do not advance the L0 replica position. Sleep then attempts serial uploads under a thirty-second context. |
| +- After an agent crash, local WAL is newer than existing local L0. Initialization starts the replica monitor before finishing WAL-to-L0 synchronization. Its initial pass can upload the older position, then wait ten years. Host loss can lose changes **from before this wake**, not merely changes made afterward. |
| + |
| +**Smallest acceptable fix before offering the feature:** Decide and implement a bounded replication/backlog policy and an explicitly verified wake boundary. Keeping the class unoffered is acceptable containment for this narrow branch; it does not solve either property. |
| + |
| +### P2 — The new unit-backed refusal prevents clearing an existing misleading class |
| + |
| +**Locations:** `cli.go:638`, `:1660`, `:1692`; `internal/registry/registry.go:1163`. |
| + |
| +**Scenario:** A hibernating unit-backed route carries `daily`, accepted by the previous CLI. Running: |
| + |
| +```text |
| +carlos route --host H --replication - |
| +``` |
| + |
| +resolves the target to `""`, but the unconditional rail rejects it before the setter runs. |
| + |
| +This state remains reachable with this build: assign `daily` to an exec-backed route, then switch its backing to `unit`. `SetBacking` preserves replication. The operator can no longer clear the misleading value without also changing backing or hibernation. |
| + |
| +**Smallest acceptable fix:** Make the refusal target-aware: allow explicit clearing while rejecting every named class on unit backing. Add coverage for clearing an existing unit-backed rung without changing its lifecycle. |
| + |
| +The refusal is otherwise correct for **all named classes**. Production `SplitBackend.pick` dispatches on `BackingUnit` at `internal/activator/backend_unit.go:340`; that backend returns an inert replicator at `:285`. Nested exec instances that happen to run under systemd retain empty backing and are not caught by this new check. |
| + |
| +### P2 — The revised lease test still races the startup snapshot |
| + |
| +**Locations:** `internal/lease/flushonly_test.go:65`, `:73`, `:99`, `:121`, `:127`. |
| + |
| +**Scenario:** The first L0 upload completes. The independent snapshot goroutine is delayed for more than three seconds. The initial restore succeeds from L0 alone, so the test proceeds and inserts its ten awake rows. The delayed snapshot then reads current WAL and uploads those rows at level 9. The long case restores sixty rows and fails despite no additional L0 upload. |
| + |
| +The same snapshot can satisfy the control’s sixty-row condition without demonstrating an awake **L0** upload. The one-second readiness retry in `third_party/litestream/store.go:584` does not guarantee snapshot completion within the test’s three-second sleep. |
| + |
| +The midnight skip also has a smaller race: midnight can occur after the day check and before the restore. |
| + |
| +**Smallest acceptable fix:** Observe completion of the startup snapshot before writing; verify an advancing L0 transaction position in the control; isolate or deterministically control snapshot scheduling during the long-case assertions. |
| + |
| +The revised test is materially stronger. It establishes particular restored states and unchanged L0 file count during its observation window. It can still pass with snapshot/compaction uploads, redundant writes to existing keys, or a synchronization outside that window. Timing the control does not prove the long case’s monitor actually ran. |
| + |
| +### P2 — Restore verification still interprets permitted lag as replication failure — explicitly open, inherited |
| + |
| +**Locations:** `internal/ops/hibernate_verify.go:59`, `:96`; `internal/ops/freshness.go:45`, `:105`. |
| + |
| +**Scenario:** A route writes, checkpoints locally, then stays awake through read traffic. Once its source database has been quiet for fifteen minutes, a behind replica is reported as `STALE`, regardless of the route’s class. |
| + |
| +**Smallest acceptable fix before offering the feature:** Pass class and reliable lifecycle/checkpoint information into freshness evaluation. Distinguish an awake copy permitted to lag from a failed completed hibernation boundary. |
| + |
| +This also affects `daily`. The note correctly leaves the defect open, although its description of when the verifier reports stale is too broad. |
| + |
| +### P3 — Ordinary listings do not identify the class as unoffered |
| + |
| +**Locations:** `cli.go:1735`; `routes_cli.go:119`; `cli.go:347`; design note `:10`. |
| + |
| +**Scenario:** A registry already contains `hibernate-flush`. Both the API-backed route listing and local instance listing print the raw word. Neither calls the new `replicationWord` branch containing “not offered.” |
| + |
| +That explanation appears in change/clear output, not ordinary listings. The helper-only test at `route_replication_test.go:347` does not check either listing. |
| + |
| +**Smallest acceptable fix:** Annotate the reserved word in both listing paths, or narrow the design note and helper comment to describe only change output. |
| + |
| +## Claims the code contradicts |
| + |
| +The broad “wake and sleep only” claim remains false, but the branch now acknowledges that rather than offering it as a feature. |
| + |
| +| Claim at this commit | What the code establishes | |
| +|---|---| |
| +| **“A listing that meets the word says so”** — design note `:10` | Listings print the raw word; finding 6. | |
| +| **The startup snapshot precedes the test’s awake writes** — `internal/lease/flushonly_test.go:28` | Three seconds of sleep does not establish that ordering; finding 4. | |
| +| **Snapshots upload at the first pass after every open** — design note `:18`, test comment `:26` | The monitor attempts a pass. `third_party/litestream/store.go:760` skips it when a sufficiently recent snapshot already exists. Errors and database readiness can also prevent an upload. | |
| +| **“Everything written while awake waits on the box for Close”** — design note `:19`, activator comment `:322` | The **unuploaded L0 backlog** waits. Awake writes can already be remote in snapshots. | |
| +| **Under `daily`, backlog is “at most a day’s”** — design note `:19` | Only under healthy periodic synchronization. An early upload failure waits approximately forty-eight hours before its first ordinary retry; repeated failures have no one-day bound. | |
| +| **An awake instance on either class “reads as stale”** — design note `:22` | Staleness requires the restored counter to trail the source and the source mtime to be older than fifteen minutes. An up-to-date or recently modified source does not fail that check. | |
| +| **The interval never weakens the parked copy; both boundary passes flush in full** — unchanged `internal/lease/lease.go:268` | The acknowledged initialization, wake-ordering and failed-Close cases contradict this unconditional comment. | |
| +| **“Nothing sets it”** — design note `:3` | Correct for current application assignment paths. It is not a restriction in the storage API: `SetReplication` and explicit `Upsert` still accept the word. | |
| + |
| +The approved pricing design’s “hibernate = durable” and “no restore to an earlier moment” requirements at `2026-09-07-carloku-pricing-plans-design.md:318` remain **requirements**, not properties delivered by this branch. Existing snapshot/LTX retention still permits earlier restores. |
| + |
| +**A — Actual synchronization behavior.** `Replicate` returns after `Store.Open`; initialization normally starts on the first ten-second database-monitor tick. `Replica.monitor` performs an initial synchronization attempt, then enforces its ticker before subsequent attempts (`third_party/litestream/replica.go:338`). It does not verify a durable wake boundary. |
| + |
| +Snapshots upload live database/WAL state independently of this interval (`db.go:2609`). L1–L3 compactions also upload, using already-replicated inputs (`compactor.go:113`, `:169`). L0 retention lists and deletes eligible replicated files; it does not upload the pending backlog. |
| + |
| +A healthy Close explicitly invokes database synchronization and replica synchronization through `DB.SyncAndWait` (`db.go:678`), then `Store.Close` cancels and joins maintenance before `DB.Close` performs its final synchronization. Neither flush consults the ten-year interval. Initialization failure can skip the final stages, as above. |
| + |
| +SQLite checkpointing preserves WAL changes in local L0 around the checkpoint (`db.go:2236`); it does not itself force remote upload. Explicit `Held.Checkpoint`, however, calls `SyncAndWait` independently of class and preserves errors (`internal/lease/checkpoint.go:40`, `:47`). The embedded store exposes no configured heartbeat or periodic validation. Lease renewal continues separately; it writes the lock, not database contents. |
| + |
| +Ten years is `315360000000000000` nanoseconds and fits `time.Duration`; doubling it once also fits. It arms a real, finite ticker. After an ordinary first replica-sync error, the code waits a ticker interval **plus** an initial backoff of the same length—approximately twenty years here or forty-eight hours for `daily`. Later doubling is capped. Both waits remain cancellation-aware. Initialization failures use the separate database-monitor retry schedule. Embedded setup imposes no upper bound; vendored CLI validation rejects nonpositive intervals (`cmd/litestream/main.go:532`), and duration logging does not narrow the value. |
| + |
| +**B — Failure outcomes compared with `daily`.** |
| + |
| +| Failure | Outcome | |
| +|---|---| |
| +| Host lost while awake | Loses changes absent from the latest successful remote copy. Snapshots can reduce the window; healthy `daily` additionally drains L0 periodically. | |
| +| Failed wake-time pass | Neither class guarantees current wake state. Pre-wake changes can remain vulnerable. Replica-upload failures and initialization failures have different retry schedules. | |
| +| Failed Close | Neither class guarantees durable hibernation. The retained mapping can accumulate a much larger backlog. | |
| +| Crash loop | Ordinary instance exits enter Close. Worker crashes can restart in place with the same replicator (`activator.go:2323`), so each restart does **not** establish another wake boundary. | |
| +| Instance never sleeps | Snapshots continue when successful; pending L0 accumulates. Healthy `daily` periodically drains that backlog. | |
| +| SQLite checkpoint under the lease | Preserves local replication input, without establishing remote durability. Explicit `Held.Checkpoint` does force remote synchronization. | |
| +| Drain immediately after wake | Healthy forced synchronization bypasses the ticker. Failed initialization exposes the false-success Close case. | |
| + |
| +Consequently, pre-wake loss and loss after failed hibernation are both possible. Neither “everything since wake” nor unconditional durable hibernation is a valid guarantee. |
| + |
| +**C, F and H — Readers, vocabulary and withdrawal.** Withdrawal covers the current assignment paths: the CLI checks `replicationOffered` at `cli.go:650`; no API or reconciler assigns `hibernate-flush`. `ReplicationClasses()` lists three offered names, while `ValidReplication` deliberately accepts four. |
| + |
| +The low-level setter remains capable of writing it (`registry.go:1235`). Upsert preserves it on hibernating rows (`:976`), and disabling hibernation clears it (`:1631`). Provisioning uses narrow updates and clears through the same hibernation coupling (`internal/provision/reconcile.go:888`). Domain minting does not copy the source’s replication class (`internal/domains/reconcile.go:842`). |
| + |
| +The API passes the raw string through (`internal/console/apiroutes.go:155`); instance pages do not display it. No new class-count assumption or class-based billing rejection was found. Durability billing counts observed operations, purpose and outcome (`internal/usage/durability.go:241`), so snapshot and compaction writes remain counted through the replication meter. |
| + |
| +A route already carrying the word is **active**, not quarantined: an exec wake selects ten years at `activator.go:338`, passed into replication at `:1295`. An eligible exec route can change to an offered class or clear it, effective on its next wake. Unit-backed clearing has the regression above. |
| + |
| +Entitlement already contains the identical four names plus unset (`internal/entitlement/entitlement.go:105`, `:147`). The definitions remain separate, but both directions are now checked for current values. No entitlement production/schema change is required for this reserved-vocabulary scope. |
| + |
| +**D — Mixed versions.** The pre-change activator falls through to stream for the unknown word: more replication traffic, not reduced durability. Older class-aware consoles and listings preserve the raw string; the older setter CLI refuses it. The registry column remains unrestricted text (`registry.go:472`); this build needs no new schema and migrates a missing legacy column to `""`. |
| + |
| +**E — Test sensitivity.** Removing the mapping breaks the activator assertion; removing vocabulary acceptance breaks registry/entitlement/console assertions; removing the unit refusal breaks its rail test. The offered-class tests exercise the helper, not its wiring into the command. The lease test supplies its own duration and characterizes unchanged lease/litestream code—it does not require this branch’s production changes to pass. |
| + |
| +**I — Fleet dependency.** The new named-class refusal matches production backend selection and does not reject nested exec routes merely because they use systemd. No committed evidence establishes a fleet dependency on setting a unit-backed rung; any such assignment was already ignored by that backend. Current live fleet state cannot be established from this offline review. |
| + |
| +**J — Open work.** Every numbered open item in the design note remains unchanged in production code: the contract decision, Close error handling, wake-boundary ordering, class-aware verification, plan propagation, Free entitlement policy and runtime caps. The withdrawal contains that unfinished feature; it does not implement the pricing promise. |
| + |
| +**Verdict: accept with fixes.** |
| \ No newline at end of file |
diff --git a/docs/superpowers/specs/OPUS-REVIEW-flush-only-class-review2.md b/docs/superpowers/specs/OPUS-REVIEW-flush-only-class-review2.md| new file mode 100644 |
| index 000000000..47995bff2 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/OPUS-REVIEW-flush-only-class-review2.md |
| @@ -0,0 +1,118 @@ |
| +# Review 2: feat/carloku-flush-only-class-20261002 @ 5c327226 (base 65363554) |
| + |
| +**Verdict: accept with fixes.** This answers the narrow question it set itself. The class is withdrawn, every first-review finding is either closed or stated open, and none is wrongly claimed closed. What remains is one new operator trap, three inaccurate statements and a timing gap in the test. All are P3. |
| + |
| +## Status of the first-round findings |
| + |
| +| Finding | Status at 5c327226 | |
| +|---|---| |
| +| Opus P2-1: unbounded backlog against Close's 30s | **Stated open.** The note's second section and the comment at `activator.go:316-324` say it; it is decision 10a. Withdrawing the class means nothing is exposed to it. | |
| +| Opus P2-2: snapshot and compaction uploads while awake | **Stated open**, and the claims are corrected. The note slightly overstates it (P3-3). | |
| +| Opus P2-3: test counted files, not rows | **Closed.** The control waits for 60 restored rows (`flushonly_test.go:99`), the long case counts L0 only (`:124`) and checks 50 rows (`:127`), there is a midnight skip and a `testing.Short` guard. A residual settle race remains (P3-4). | |
| +| Opus P3-1: CLI word and usage line | **Closed.** `replicationWord` has the case (`cli.go:1735`). The usage line and `docs/site/cli.md` listing three classes are now correct, since only three are offered. | |
| +| Opus P3-2: unit-backed rail | **Closed** (`cli.go:1692`), but it introduced P3-1. | |
| +| Opus P3-2: plan propagation to rows the rail refuses | **Stated open** (note item 5, generically). | |
| +| Opus P3-2: 900s lag alert and 15-minute status threshold are class-blind | **Open and not stated.** The note names only the restore drill (P3-6). | |
| +| Opus P3-3: entitlement pin checked one direction only | **Closed.** `internal/entitlement/replication_vocab_test.go` checks both directions. | |
| +| Astra 1 (P1): Close drops its sync error; uninitialised DB skips the flush; a failed flush still hibernates | **Stated open** (item 2). It is unchanged: `lease.go` and `finishStop` (`activator.go:2181-2202`) are untouched on this branch. | |
| +| Astra 2: uploads while awake | **Stated open** (decision 10a). | |
| +| Astra 3: local L0 accumulates | **Stated** (note, the "not trimmed" bullet under decision 10a). | |
| +| Astra 4: async wake pass can ship an older position | **Stated open** (item 3). | |
| +| Astra 5: unit-backed routes | **Closed.** | |
| +| Astra 6: restore drill is class-blind | **Stated open** (item 4). `internal/ops` is untouched. | |
| +| Astra 7: test | **Partly closed.** Row checks, L0-only counting, waiting for L0 rather than any `.ltx`, and immediate cleanup are done. Deterministic snapshot boundaries, initialisation failure, warm re-wake and immediate-Close cases are not built, and the note does not list them. | |
| +| Astra 8: operator guidance | **Closed** by the withdrawal. | |
| +| Astra's contradicted claims ("at no time between", "up to a day", "never replicates") | **Corrected.** The note now says about 48h for `daily`. | |
| + |
| +## Findings |
| + |
| +### P3-1. The unit-backed refusal also blocks clearing a rung that a unit-backed route already holds |
| +- **Where:** `cli.go:1692`. `replicationRail` runs for `--replication -` too (`runRouteReplication`, `cli.go:1660`, before the `class` check). `SetBacking` (`registry.go:1162`) does not clear `replication`, and Upsert's CASE (`registry.go:976`) keeps an existing rung on a hibernating row. |
| +- **Scenario:** a hibernating exec route is on `daily`. The operator runs `carlos route --host h --backing unit`; the rung stays. `carlos instances` and `carlos routes` print `daily`, and nothing honours it: box-wide litestream streams the file (`ops/litestream.go:339-347`). `carlos route --host h --replication -` is then refused with "unit-backed". The only way out is `--hibernate=false`, which changes the route's behaviour. The same applies to any unit-backed row that got a rung before this commit. |
| +- **Fix:** let `class == ""` through the unit check (clearing is always safe), or have `SetBacking(host, unit)` clear `replication`. Add a test case for the clear. |
| + |
| +### P3-2. The note says "a listing that meets the word says so". Neither listing does. |
| +- **Where:** `cli.go:347` (`carlos instances`) prints `t.Replication` raw. `routes_cli.go:119` (`replicationCell`) and the console API (`apiroutes.go:155`) also pass the word through raw. `replicationWord` is used only in the setter's before→after line and in the hibernate-off message. |
| +- **Scenario:** a row holds `hibernate-flush`. `carlos routes` shows `hibernate-flush` with no "not offered, contract undecided" qualifier. |
| +- **Fix:** correct the note, or route both listings through a word that marks it as not offered. |
| + |
| +### P3-3. Inaccurate statements about litestream in the note and the comments |
| +- **Note: the snapshot monitor uploads the whole database "at its first pass".** `CompactDB` returns `ErrCompactionTooEarly` when a level-9 file exists newer than `PrevCompactionAt` (`store.go:756-760`). So the first pass after a wake uploads only on the first wake of each UTC day, plus once at each 00:00 UTC while awake. Fix: say "once per UTC day: at the first wake that day, or at 00:00 while awake". |
| +- **`activator.go:306` ("no sync of the write-ahead log while awake") and `registry.go:370-372` ("no change-driven or periodic sync … while awake").** The after-open pass happens while awake, and what is synced is L0 LTX, not the WAL. Fix: "no sync after the first pass that follows the wake, until Close". |
| +- **`lease.go:264-271` (unchanged): "the interval never weakens hibernation's parked copy".** That is false for long intervals, as the note itself now says. Fix: add a pointer to the note. |
| + |
| +### P3-4. The test's 3-second settle is a timing guess |
| +- **Where:** `flushonly_test.go:73`. |
| +- **How it can fail:** the startup snapshot runs on the snapshot monitor's 1-second retry after DB initialisation (`store.go:589-593`). |
| + - On a loaded CI runner it can land after `write()`. |
| + - The snapshot then contains the awake rows, `restoreCount` returns 60 at `:127`, and the test fails falsely. |
| + - Rows still only in the WAL are picked up at the next 10-second tick, so this needs a slow box. It is a flake risk, not a hole: it cannot pass with a sync while awake. The L0 count at `:124` is immune. |
| +- **Fix:** instead of sleeping, poll until a file exists under `replica/ltx/9`. |
| + |
| +### P3-5. The withdrawal is enforced only in the CLI |
| +- **Where:** |
| + - `registry.SetReplication` (`registry.go:1236`) and Upsert accept `hibernate-flush`. |
| + - `entitlement.Runtime.validate` (`entitlement.go:314`) lets a record pin it. |
| + - `SetReplication`'s error text lists three classes while it accepts four. |
| +- **Today:** no non-test code sets it. I grepped every writer: only `cli.go:1672` calls `SetReplication`, it is guarded, and no `Route{Replication: …}` literal exists. So "set by nothing" is true. |
| +- **Scenario:** the first plan-propagation writer, or a console door, calls `SetReplication(host, "hibernate-flush")` and gets the undecided contract with no refusal. |
| +- **Fix (optional now, required before propagation):** move the "offered" gate into the registry with a named bypass for the plan path, or record in the note that the gate is CLI-only. |
| + |
| +### P3-6. The class-blind lag thresholds are not in the note's open list |
| +- **Where:** `metrics/alert.go:29` and `status/snapshot.go` (15-minute lag threshold). |
| +- **Scenario:** once something feeds lag (`host/metrics_loop.go:574`), every awake `daily` or `hibernate-flush` route reads as unhealthy. |
| +- **Fix:** add it to item 4 next to the restore drill. |
| + |
| +## A–J in brief |
| + |
| +**A. The claim against vendored litestream.** The ten-year interval is used only at `replica.go:329` (the ticker) and `:387` (backoff seed). Neither overflows, and the backoff doubling is capped at 5 minutes. |
| +- **After the first pass:** the monitor blocks on the ticker (`:341-345`) and never consults notify. |
| +- **First pass:** if `dpos` is zero at `Replica.Start`, called inside `DB.init` (`db.go:1090`), it waits on notify and ships at the first L0 write. |
| +- **Failed first sync:** waits about 20 years. |
| +- **Close:** `Store.Close` joins the monitors first (`store.go:240`), then `DB.Close` final-syncs if initialised (`db.go:788-800`). Shutdown retry uses `ShutdownSyncInterval`, not `SyncInterval`. The uninitialised case is open item 2. |
| +- **Uploads while awake:** heartbeat and validation are off. L0 retention ticks hourly and acts only on remote L0 files covered by L1. The snapshot is the only thing that uploads awake writes. |
| + |
| +**B. Losses.** As in round 1. Nothing is worse than the note now states, and the note no longer claims hibernation is unconditionally durable. |
| + |
| +**C. Readers of the column and class list.** No reader rejects or mis-bills the word. |
| +- Listings show it raw (P3-2). |
| +- `SetHibernate` clears it. |
| +- The provision reconciler clears it when hibernation goes off. |
| +- `ReplicationClasses()` is still three, which is now correct. |
| + |
| +**D. Mixed versions.** |
| +- An older activator maps the word to stream. |
| +- An older CLI cannot set it, prints it raw, and lacks the unit refusal (harmless). |
| +- Nothing changes the schema. |
| + |
| +**E. Tests.** |
| +- The CLI, registry, activator and entitlement tests fail without the code: missing symbols, the 0 interval, or the vocabulary. |
| +- The lease test characterises litestream and does not depend on the branch's production code. It cannot pass with an L0 sync while awake. It has one flake vector (P3-4). |
| + |
| +**F. Entitlement.** One vocabulary, pinned in both directions by a test, with two definitions. Nothing else in `internal/entitlement` needs to change for this branch beyond P3-5's note. |
| + |
| +**G / J. Open items and stated facts.** All five open items are unchanged on this branch: `lease.go`, `finishStop`, `internal/ops` and the plan path are untouched. The statements that are wrong are P3-2 and P3-3. |
| + |
| +**H. Is the withdrawal complete?** Yes, through every door that exists today. A row carrying the word: |
| +- wakes on the ten-year interval; |
| +- is listed raw; |
| +- can be moved to an offered rung or cleared, except on a unit-backed row (P3-1); |
| +- is cleared by turning hibernation off. |
| + |
| +**I. Is the unit-backed refusal right?** It is right for every class. |
| +- `SplitBackend.pick` always sends `BackingUnit` to the inert lease (`backend_unit.go:340-344`, wired at `host/agent.go:336`), so it cannot refuse a route the activator replicates. |
| +- Nothing in the repo depends on setting a rung on a unit-backed route. |
| +- The live fleet is unchecked: I ran no network commands. The check is the REPL and BACKING columns of `carlos instances` on each box. Any hit is a P3-1 case. |
| + |
| +## Claims the code contradicts |
| + |
| +| Claim | Where | What the code does | |
| +|---|---|---| |
| +| "a listing that meets the word says so" | Note, "What is on the branch" | Listings print it raw (P3-2) | |
| +| The snapshot uploads "at its first pass" | Note | Only on the first wake of each UTC day (P3-3) | |
| +| "no sync of the write-ahead log while awake" | `activator.go:306`, `registry.go:370-372` | The after-open pass happens while awake (P3-3) | |
| +| "the interval never weakens hibernation's parked copy" | `lease.go:268-270` (unchanged) | Contradicted by the note's own 30-second backlog finding | |
| + |
| +**Verdict: accept with fixes.** Fix P3-1 and correct the note and comments (P3-2, P3-3). The class stays undecided and is not on offer. |
| + |
| +Read-only throughout: I used `git show` at 5c327226 and ran no tests or network commands. The scratch copies I made under `$TMPDIR` are deleted. |
diff --git a/internal/activator/activator.go b/internal/activator/activator.go| index fbadd8c4f..d92e7076f 100644 |
| --- a/internal/activator/activator.go |
| +++ b/internal/activator/activator.go |
| @@ -303,7 +303,7 @@ const ( |
| replicationStream = registry.ReplicationStream // change-driven ~1s sync — loss window seconds |
| replicationBatched = registry.ReplicationBatched // sync at most every 5m — loss window minutes |
| replicationDaily = registry.ReplicationDaily // sync at wake + every 24h — loss window up to a day |
| - // no sync of the write-ahead log while awake; not offered, not set by anything |
| + // no log sync after the first pass that follows the wake, until Close; not offered, not set by anything |
| replicationHibernateFlush = registry.ReplicationHibernateFlush |
| ) |
| @@ -316,11 +316,12 @@ const ( |
| // |
| // What this does NOT give, as the first review of the class found against |
| // the vendored litestream (2026-10-02-flush-only-replication-class.md): |
| -// the snapshot monitor still uploads the whole database once a day while |
| -// an instance is awake, and compactions still write; everything written |
| -// while awake waits on the box for Close, whose flush has the stop path's |
| -// thirty seconds, so a long-awake instance's sleep may not be durable; and |
| -// the local log is not trimmed until it has been uploaded. The class is |
| +// the snapshot monitor still uploads the whole database once per UTC day |
| +// (at the first wake that day, or at 00:00 while awake), and compactions |
| +// still write; the log written while awake and not yet uploaded waits on |
| +// the box for Close, whose flush has the stop path's thirty seconds, so a |
| +// long-awake instance's sleep may not be durable; and the local log is not |
| +// trimmed until it has been uploaded. The class is |
| // therefore not offered and not set by anything until its contract is |
| // decided. |
| const flushOnlySyncInterval = 10 * 365 * 24 * time.Hour |
diff --git a/internal/lease/flushonly_test.go b/internal/lease/flushonly_test.go| index 9d57a1c77..14adf7d77 100644 |
| --- a/internal/lease/flushonly_test.go |
| +++ b/internal/lease/flushonly_test.go |
| @@ -36,9 +36,9 @@ func TestALongSyncIntervalSyncsTheLogAfterOpenAndAtCloseOnly(t *testing.T) { |
| t.Skip("about 45 seconds of real litestream ticks") |
| } |
| const longInterval = 10 * 365 * 24 * time.Hour // the activator's flushOnlySyncInterval |
| - level0 := func(dir string) int { |
| + level := func(dir string, l string) int { |
| n := 0 |
| - _ = filepath.WalkDir(filepath.Join(dir, "ltx", "0"), func(p string, d fs.DirEntry, err error) error { |
| + _ = filepath.WalkDir(filepath.Join(dir, "ltx", l), func(p string, d fs.DirEntry, err error) error { |
| if err == nil && !d.IsDir() && strings.HasSuffix(p, ".ltx") { |
| n++ |
| } |
| @@ -46,6 +46,7 @@ func TestALongSyncIntervalSyncsTheLogAfterOpenAndAtCloseOnly(t *testing.T) { |
| }) |
| return n |
| } |
| + level0 := func(dir string) int { return level(dir, "0") } |
| start := func(t *testing.T, interval time.Duration) (*Replicator, string, string) { |
| t.Helper() |
| h := localHeld(t, &lockStore{}, time.Minute) |
| @@ -68,9 +69,18 @@ func TestALongSyncIntervalSyncsTheLogAfterOpenAndAtCloseOnly(t *testing.T) { |
| } |
| time.Sleep(250 * time.Millisecond) |
| } |
| - // The start-up snapshot and compaction passes land within about a |
| - // second of it; let them, so nothing counted below is theirs. |
| - time.Sleep(3 * time.Second) |
| + // The start-up snapshot (level 9: the whole database, uploaded by |
| + // the snapshot monitor whatever the interval) is waited for, not |
| + // slept for: it must land before the awake writes, or it would |
| + // carry them to the replica and the long case's restore would |
| + // count them with no log sync having happened. A fresh replica |
| + // has none, so the monitor's first pass always takes one. |
| + for level(replica, "9") == 0 { |
| + if time.Now().After(deadline) { |
| + t.Fatal("the start-up snapshot never landed") |
| + } |
| + time.Sleep(250 * time.Millisecond) |
| + } |
| if n := restoreCount(t, replica, filepath.Join(t.TempDir(), "at-open.db")); n != 50 { |
| t.Fatalf("the first pass restored %d rows; want the 50 seeded", n) |
| } |
| @@ -94,22 +104,28 @@ func TestALongSyncIntervalSyncsTheLogAfterOpenAndAtCloseOnly(t *testing.T) { |
| var streamTook time.Duration |
| t.Run("the default interval replicates the rows while awake (the control)", func(t *testing.T) { |
| _, path, replica := start(t, 0) |
| + logBefore, snapshots := level0(replica), level(replica, "9") |
| write(t, path) |
| at := time.Now() |
| - for restoreCount(t, replica, filepath.Join(t.TempDir(), "awake.db")) != 60 { |
| + // The awake rows restore AND the log level advanced with no new |
| + // snapshot: it was a log sync that carried them, not a snapshot. |
| + for restoreCount(t, replica, filepath.Join(t.TempDir(), "awake.db")) != 60 || level0(replica) == logBefore { |
| if time.Since(at) > 40*time.Second { |
| t.Fatal("the default interval did not replicate the awake rows while the store was open: the control does not hold, so the long-interval wait proves nothing") |
| } |
| time.Sleep(500 * time.Millisecond) |
| } |
| streamTook = time.Since(at) |
| + if level(replica, "9") != snapshots { |
| + t.Skip("a snapshot landed during the control (a run across 00:00 UTC): the rows may have travelled in it") |
| + } |
| }) |
| if streamTook == 0 { |
| t.Fatal("no control") |
| } |
| t.Run("the long interval syncs no log while awake, and everything at Close", func(t *testing.T) { |
| rep, path, replica := start(t, longInterval) |
| - before := level0(replica) |
| + before, snapshots := level0(replica), level(replica, "9") |
| write(t, path) |
| // Half as long again as the control needed, and never less than |
| // the database monitor's own interval. |
| @@ -118,8 +134,8 @@ func TestALongSyncIntervalSyncsTheLogAfterOpenAndAtCloseOnly(t *testing.T) { |
| wait = dbMonitorInterval + 2*time.Second |
| } |
| time.Sleep(wait) |
| - if day() != began { |
| - t.Skip("the run spanned 00:00 UTC: the daily snapshot uploads the awake rows, which is what litestream does and not what this case measures") |
| + if day() != began || level(replica, "9") != snapshots { |
| + t.Skip("a snapshot landed during the wait (a run across 00:00 UTC): it uploads the awake rows, which is what litestream does and not what this case measures") |
| } |
| if after := level0(replica); after != before { |
| t.Fatalf("%d log files on the replica after the writes, %d before; want no log sync while awake", after, before) |
diff --git a/internal/lease/lease.go b/internal/lease/lease.go| index c4191f8b1..fc9489c59 100644 |
| --- a/internal/lease/lease.go |
| +++ b/internal/lease/lease.go |
| @@ -266,7 +266,12 @@ func (h *Held) Release(ctx context.Context) error { |
| // change-notify only short-circuits the idle wait, never this spacing), so 5m |
| // bounds an awake host's hard-crash loss to ~5m and 24h to ~a day, while the |
| // wake-time initial pass and the sleep-time SyncAndWait/Close flush in full |
| -// regardless — the interval never weakens hibernation's parked copy. 0 keeps |
| +// regardless — the interval never weakens hibernation's parked copy. (That |
| +// last claim holds for a healthy wake and a flush that finishes: the first |
| +// pass is asynchronous, a flush has the stop path's budget, and Close can |
| +// return nil with an uninitialised database having flushed nothing. See |
| +// docs/superpowers/specs/2026-10-02-flush-only-replication-class.md, "Open", |
| +// and forge discussion 200.) 0 keeps |
| // litestream's 1s default (the stream class): idle ticks make zero S3 calls, |
| // so stream's request bill is change-driven, not clock-driven. |
| func (h *Held) Replicate(ctx context.Context, dbPath string, syncInterval time.Duration) (*Replicator, error) { |
diff --git a/internal/registry/registry.go b/internal/registry/registry.go| index 209dece07..2f9b221aa 100644 |
| --- a/internal/registry/registry.go |
| +++ b/internal/registry/registry.go |
| @@ -368,11 +368,14 @@ const ( |
| // ReplicationHibernateFlush is the Free plan's class in the approved |
| // pricing design ("hibernate-flush only": no continuous replication), |
| // and the word the entitlement record carries. The column may hold it |
| - // and the activator maps it (no change-driven or periodic sync of the |
| - // write-ahead log while awake; see the activator's interval for what |
| + // and the activator maps it (no log sync after the first pass that |
| + // follows the wake, until Close; see the activator's interval for what |
| // the vendored replicator still does), but it is NOT offered as a rung |
| - // (ReplicationClasses) and nothing sets it: what it promises is not |
| + // (ReplicationClasses) and no code sets it: what it promises is not |
| // decided (docs/superpowers/specs/2026-10-02-flush-only-replication-class.md). |
| + // The refusal is the CLI's alone: SetReplication and Upsert accept the |
| + // word, so the first writer that propagates a plan's class must not |
| + // reach them before that decision. |
| ReplicationHibernateFlush = "hibernate-flush" |
| ) |
diff --git a/route_replication_test.go b/route_replication_test.go| index 937dde13e..a600b48c6 100644 |
| --- a/route_replication_test.go |
| +++ b/route_replication_test.go |
| @@ -26,9 +26,9 @@ func TestReplicationRailRefusesNonHibernatingRoutes(t *testing.T) { |
| Host: "console.carloku.com", Kind: "instance", App: "console", |
| Addr: "127.0.0.1:8090", DBPath: "/data/console.carloku.com.db", |
| } |
| - err := replicationRail(awake) |
| + err := replicationRail(awake, registry.ReplicationDaily) |
| if err == nil { |
| - t.Fatal("replicationRail(non-hibernating route) accepted; want a refusal") |
| + t.Fatal("replicationRail(non-hibernating route, registry.ReplicationDaily) accepted; want a refusal") |
| } |
| for _, want := range []string{"hibernate", "litestream"} { |
| if !strings.Contains(err.Error(), want) { |
| @@ -38,8 +38,8 @@ func TestReplicationRailRefusesNonHibernatingRoutes(t *testing.T) { |
| hibernating := awake |
| hibernating.Hibernate = true |
| - if err := replicationRail(hibernating); err != nil { |
| - t.Fatalf("replicationRail(hibernating route) = %v, want accepted", err) |
| + if err := replicationRail(hibernating, registry.ReplicationDaily); err != nil { |
| + t.Fatalf("replicationRail(hibernating route, registry.ReplicationDaily) = %v, want accepted", err) |
| } |
| } |
| @@ -62,9 +62,9 @@ func TestReplicationRailRefusesAHibernatingRouteWithNoDatabase(t *testing.T) { |
| Host: "static.example.com", Kind: "static", App: "site", |
| Hibernate: true, |
| } |
| - err := replicationRail(pathless) |
| + err := replicationRail(pathless, registry.ReplicationDaily) |
| if err == nil { |
| - t.Fatal("replicationRail(hibernating route with no db_path) accepted; want a refusal") |
| + t.Fatal("replicationRail(hibernating route with no db_path, registry.ReplicationDaily) accepted; want a refusal") |
| } |
| for _, want := range []string{"db_path", "--db"} { |
| if !strings.Contains(err.Error(), want) { |
| @@ -353,11 +353,18 @@ func TestReplicationOfferedRefusesTheFlushOnlyClass(t *testing.T) { |
| // the rail refuses a rung for it, whatever the rung. |
| func TestReplicationRailRefusesAUnitBackedRoute(t *testing.T) { |
| rt := registry.Route{Host: "u.example.com", Hibernate: true, DBPath: "/var/lib/carlos-app/u.example.com/main.db", Backing: registry.BackingUnit} |
| - if err := replicationRail(rt); err == nil || !strings.Contains(err.Error(), "unit-backed") { |
| + if err := replicationRail(rt, registry.ReplicationDaily); err == nil || !strings.Contains(err.Error(), "unit-backed") { |
| t.Fatalf("a unit-backed route: %v; want it refused", err) |
| } |
| + // Clearing a rung the unit-backed route still carries is let through: |
| + // it is the only way to make the listing stop naming a rung nothing |
| + // honours without changing the route's backing or hibernation. |
| + rt.Replication = registry.ReplicationDaily |
| + if err := replicationRail(rt, ""); err != nil { |
| + t.Fatalf("clearing a unit-backed route's rung: %v; want it allowed", err) |
| + } |
| rt.Backing = "" |
| - if err := replicationRail(rt); err != nil { |
| + if err := replicationRail(rt, registry.ReplicationDaily); err != nil { |
| t.Fatalf("the same route, not unit-backed: %v", err) |
| } |
| } |