amadan
The replicator's tenth build: the ninth review's P2 and bounded findings
Close looks at the files one last time before it asks the witness, so a swap still in place at a rebuild is held by the fleet that streamed through it (Opus P2-1). In strict mode an exec route's database outside the globs is a claim the inventory looks at and says when absent (Astra 5, carried since review seven), and a hibernating route over a unit claim or the platform's file is a problem (Opus P3-2). Tests pin the latch inside Close and the read count after a witnessed change; the last two bare MkdirAll are mkdirs. Comments both reviewers listed are corrected; a routine restart is named beside the crash. Dispositions "Review nine" and the ninth review's reports.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
10 files changed,
+523
−48
docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review9.md+127 −0docs/superpowers/specs/DISPOSITIONS-metered-replicator.md+23 −6docs/superpowers/specs/OPUS-REVIEW-metered-replicator-review9.md+124 −0internal/ops/litestream.go+43 −1internal/ops/litestream_metered.go+44 −20internal/ops/litestream_metered_test.go+92 −3internal/ops/litestream_owners.go+4 −2internal/ops/litestream_owners_test.go+59 −10internal/ops/litestream_owners_unix_test.go+6 −5internal/registry/registry.go+1 −1
CI passed — run details
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review9.md b/docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review9.md| new file mode 100644 |
| index 000000000..a77cd04d3 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review9.md |
| @@ -0,0 +1,127 @@ |
| +The review-eight report files are absent from `142ef5d0` and the local locations searched. The comparison below therefore uses their finding labels and summaries in the dispositions; I cannot certify closure of claims omitted from those summaries. All source locations below refer to **142ef5d0**, reconstructed from `b5aecf13` and the requested diff. No files were changed, tests run, or network commands issued. |
| + |
| +| Review-eight finding | Closed? | Would the relevant test pass without the fix? | |
| +|---|---|---| |
| +| **Opus P1-1:** latched predecessor mistaken for closed; latch publication race | **Closed by inspection.** `chainClosed` checks the predecessor’s own `closed` first. `latch` publishes the window, clears `replaces`, and sets `inherited` under `staleMu`. Production chain reads use that lock. | **No** for restoring the old walk order: the new tests at `litestream_metered_test.go:1574` and `:1636` distinguish both reported sequences. They do **not** independently establish atomic publication under concurrency. | |
| +| **Opus P3-1:** root/platform-UID tenants | **Open**, explicitly deferred. | No rejecting regression test exists. The platform-user fixtures deliberately accept this configuration. | |
| +| **Opus P3-2:** `..` after a tenant-controlled component | **Closed for that traversal.** Only a leading run of `..` is accepted. | **No:** the exact refusal assertion at `litestream_owners_unix_test.go:313` distinguishes removal of the rule. | |
| +| **Opus P3-3:** one tenant’s file problem holds everyone | **Acknowledged, not fixed.** The description overstates repeated holding; see claims below. | No test establishes isolation between affected and unaffected owners. Existing file-change tests expect every recorder to be held. | |
| +| **Opus P3-4:** weak tests, chain retention, timing and fixtures | **Partly closed.** Chain release is asserted; the arbitrary 100 ms scheduling allowance is removed; relevant fixture modes are explicit. | Removing chain release fails the new assertion. Several other mutations remain invisible, detailed below. | |
| +| **Astra 1:** replacement overlap and premature inheritance through a live latched fleet | **Closed**, with the same qualifications as Opus P1-1. An unopened successor remains linked until the live predecessor closes. | **No:** both new latch tests distinguish the old ordering. | |
| +| **Astra 2:** Litestream’s pathname opens | **Open.** Every relevant reopen and WAL open remains exposed. | **Yes:** existing tests do not force substitution during those opens. | |
| +| **Astra 3:** protected endpoints below replaceable ancestry | **Open.** Endpoint ownership is not ancestry protection. | **Yes:** the shadow-state tests inspect endpoints, not a replaceable ancestor above them. | |
| +| **Astra 4:** inventory omissions | **Open.** The explicit exec claim outside the globs is still omitted. | **Yes:** the inventory regression test covers unit claims, not that exec case. | |
| +| **Astra 5:** header open redirected through an ancestor | **Open.** `O_NOFOLLOW` protects only the final component. | **Yes:** existing substitution tests do not redirect an ancestor to a device. | |
| +| **Astra 6:** tests do not establish their claims | **Partly closed.** Stronger latch and clean-boundary tests exist; shutdown, attribution and watch coverage remain incomplete. | **Yes** for several individual mutations described below. | |
| +| **Astra 7:** route identity versus filesystem UID; shared platform UID | **Open**, explicitly deferred. | No test requires rejection of a route naming another user’s otherwise valid tree. | |
| +| **Astra 8:** fixture and timing dependence | **Mostly closed for umask 077 and a symlinked temp root; not wholly closed.** | Relevant mode assertions now distinguish the fixes under 077. Timing-based negative checks and fixed fake UIDs remain assumptions. | |
| +| **Both reviewers’ claims lists** | **Partly corrected.** The revised admission, inheritance and first-open qualifications are substantially more accurate. Contradictions remain below. | Comments are not generally pinned by tests. | |
| + |
| +The other requested checks that hold by inspection are: |
| + |
| +- **Admission and retries:** `litestream_metered.go:221` rechecks after acquiring the mutex, and `:235` rechecks after reading the witness. A file finding during that read refuses admission. A finding after the final check belongs to the already-admitted-request gap below. `witnessUnchanged` makes three attempts, with two 200 ms pauses and a five-second context per read; a successful unchanged answer recovers a transient error, a witnessed change stops immediately, and exhausted errors make the fleet stale. This is a bounded retry count, not a hard wall-clock bound on mutex waits or filesystem `Stat`. An already-running retry sequence can continue reading after a file finding, but its admission is subsequently refused. |
| +- **Registry generation:** `registry.go:569` covers the six columns discovery and attribution currently consume. Insert/delete triggers and changed-value updates cover raw statements from older builds. The rowid trigger at `:612` closes the `UPDATE OR REPLACE` deletion escape, including rowid aliases. Status/bookkeeping updates do not move the counter. “Nothing else moves it” is true only at the stated column level: inserting a database-less route still moves it. |
| +- **Inheritance and shutdown:** within the documented single replacement chain, a replacement cannot open beside an open or draining latched predecessor. `closeOnce` makes concurrent callers wait for the same shutdown. The final witness read runs after workers stop and is skipped when already stale. Inheritance is applied once per fleet; own and inherited windows are retained together. There is no remaining split publication of the latch/window/link in the inspected production paths. |
| +- **Lineage:** `litestream_metered.go:886` keys both replica and shadow state by account plus path. Different accounts sharing a pathname over time do not share that key; changing both account and pathname cannot carry the old account’s key. Returning to the same account and path deliberately reuses that account’s lineage. |
| + |
| +1. **P1 — Path checks still permit another tenant’s database to be opened and streamed.** |
| + **Locations:** `internal/lease/meteredstore.go:127`; `internal/ops/litestream_owners.go:268`; `internal/ops/litestream_metered.go:624`. |
| + |
| + Tenant A passes inventory, then substitutes a database, WAL, or ancestor link before Litestream opens it. Litestream opens tenant B’s data; A restores the original pathname before the next inventory. The registry remains unchanged, and `lookAgain` sees A’s original inode. The retained handle can continue supplying B’s contents to A’s replica. |
| + |
| + This is larger than a “first open” window: subsequent database and WAL opens remain exposed. Detecting the opened identity afterwards would detect some breaches but would not prevent the initial read or write. |
| + |
| + **Smallest acceptable fix:** make Litestream’s database and sidecar access use validated, pinned directory/file identities with constrained resolution, including subsequent opens. Add a deterministic substitution test at the actual open boundary. |
| + |
| +2. **P1 — A protected directory can still be replaced through an unchecked ancestor.** |
| + **Locations:** `internal/ops/litestream_metered.go:908`; `internal/ops/litestream_owners.go:462`, `:645`, `:673`. |
| + |
| + Configure `dataDir` as `/srv/tenant/container/data`, where `container` and `data` are platform-owned but `/srv/tenant` is tenant-writable. Every inspected endpoint can pass. The tenant can nevertheless rename `container` and substitute the hierarchy through which shadow state is subsequently accessed. |
| + |
| + The same construction works around a platform-made state link: its immediate parent and target’s immediate parent are protected, but a higher ancestor remains replaceable. The ninth build’s leading-`..` rule does not address that construction. |
| + |
| + For an ordinary distinct-UID `tenantFile` chain, the starting-directory check rejects another tenant’s directory, the climb rejects a foreign stopping parent, and an accepted climb reaches the top-of-chain check. A direct `DBPath` symlink to another tenant’s file is rejected by the file’s `Lstat`. Those checks do not establish the safety of ancestry above the protected endpoint. |
| + |
| + **Smallest acceptable fix:** establish and pin a trusted ancestry boundary for data, shadow state and claim roots; refuse paths whose hierarchy can be replaced by tenants. Checking another endpoint with `platformOwned` is insufficient. |
| + |
| +3. **P1 — A delayed admitted request can fall on a day absent from every inherited window.** |
| + **Locations:** `internal/ops/litestream_metered.go:235`, `:315`, `:730`. |
| + |
| + A concrete sequence covering the unopened-successor case: |
| + |
| + - A admits a request at **23:58:00**, which pauses before entering the meter. |
| + - Ownership changes at **23:58:10**. A detects it immediately and holds through **23:59:10**. |
| + - B is built for the new owner and closed unopened while A is still live. Correctly, B does not latch yet. |
| + - A’s admitted request resumes at **00:02**, is recorded and sent. |
| + - A closes. Because it is already stale, Close does not extend its window. |
| + - C replaces B. B and C correctly inherit A’s existing window—but that window contains only the previous day. |
| + |
| + The new chain logic preserves the window; it cannot supply the missing day. The lower bound correctly uses the earlier registry proof and clean file look. The unbounded delay defeats the upper margin. |
| + |
| + **Smallest acceptable fix:** track admitted attempts through recording/completion and incorporate their actual doubtful dates into a handover that cannot latch prematurely. A larger fixed margin is insufficient. |
| + |
| +4. **P1 — Ownership doubt still disappears across a process crash.** |
| + **Locations:** `internal/ops/litestream_metered.go:341`, `:375`, `:770`. |
| + |
| + A request passes admission, ownership changes, and the request completes before the watch detects the change. The process crashes. On restart, the constructor observes only the new owner; no durable transition requires either owner’s completed day to be held. An unfinished ledger row helps a different case, not this completed request. |
| + |
| + The dispositions now state this accurately. It remains independent of the corrected in-process inheritance chain. |
| + |
| + **Smallest acceptable fix:** persist the ownership/admission uncertainty before affected requests can escape, and recover that uncertainty before allowing the replacement to establish coverage. |
| + |
| +5. **P2 — Explicit exec database claims still disappear from every inventory report.** |
| + **Locations:** `internal/ops/litestream.go:385`, `:420`, `:437`; `internal/ops/litestream_owners.go:522`, `:542`. |
| + |
| + A non-hibernating exec route names `/srv/customer/live.sqlite`. Its path enters `claimed`, but it enters neither discovery glob nor `unitPaths`. The constructor can succeed with that database absent from `Owned`, `Skipped` and `Problems`. |
| + |
| + Remaining silent omissions also include empty files found only inside a tree, entries disappearing before their first `Lstat`, and unrecognised-name links to regular database files. No external inventory comparison closes those omissions. |
| + |
| + The bounded reports do work: relative/missing unit claims, units without `DBPath`, empty explicitly considered files, and discovered files failing their final look are reported. Case-insensitive database suffixes are recognised, although uppercase `.DB` is skipped rather than replicated. Every other non-empty regular file is header-read, including sidecar names; `sidecarOf` no longer exists, so a linked base cannot suppress that read. |
| + |
| + **Smallest acceptable fix:** explicitly enumerate every non-hibernating route claim independently of the globs, and require a disposition for every external-inventory entry before cutover. Preserve explicit reporting for files already discovered. |
| + |
| +6. **P2 — The header probe can cause device-open effects before rejecting the descriptor.** |
| + **Location:** `internal/ops/sqliteopen_unix.go:26`. |
| + |
| + The walker observes an ordinary file named `watchdog`. Before the open, its tenant replaces an ancestor with a link to `/dev`. `O_NOFOLLOW` does not reject that ancestor, and the privileged open can reach `/dev/watchdog`. The later regular-file/identity rejection cannot undo an open-side effect. |
| + |
| + The final-component regular-file swap is detected; FIFO nonblocking protection and `O_NOCTTY` address their narrower cases. Non-Unix builds now refuse header reads, which closes the previous fallback. |
| + |
| + **Smallest acceptable fix:** constrain ancestor resolution through pinned directory descriptors and establish the acceptable file type before an effects-bearing open. Do not treat post-open `fstat` alone as prevention. |
| + |
| +7. **P3 — Filesystem ownership remains inferred from the claimed file, not bound to the route.** |
| + **Locations:** `internal/ops/litestream_owners.go:321`, `:360`, `:380`, `:438`. |
| + |
| + An operator-configured route for A can name B’s regular database beneath B’s otherwise valid directory chain. The code derives the expected UID from that file and chain; it has no independent route-to-UID binding to reject the claim. |
| + |
| + A tenant sharing the platform UID receives platform treatment for directories and links. This does not mean literally every pathname passes—regular-file, permissions, link-count and parent checks still apply—but it removes the intended isolation boundary. Root-owned unit databases likewise stop climbing at the first root parent and normally pass under protected directories; they are not rejected for being root-owned. |
| + |
| + **Smallest acceptable fix:** bind unit routes to an independently established state root and UID; reject untrusted tenants using root or the platform UID, with an explicit trusted-platform exception. |
| + |
| +8. **P3 — Several tests still pass with the property they claim to establish removed.** |
| + **Locations:** `internal/ops/litestream_metered_test.go:979`, `:1035`, `:1095`, `:1371`, `:1437`; `internal/ops/litestream_owners_unix_test.go:251`. |
| + |
| + Concrete examples: |
| + |
| + - The final-link header test still rejects the different target inode without `O_NOFOLLOW`. It does not pin the no-follow flag. |
| + - `TestAFileSwapIsHeldFromBeforeTheLookBegan` still holds the previous day if `lookStart` is ignored: its registry proof is at 00:00:30, and the margin already reaches yesterday. The newer last-clean-look test does distinguish the lower-bound fix. |
| + - Removing `store.Close` can leave workers and handles alive while shutdown assertions pass: the stale gate suppresses requests, and the fleet’s store pointer is cleared separately. |
| + - The opened-store test says every request was counted, but checks positive activity rather than equality or correct attribution per database. |
| + - The retry test does not assert the call count for witnessed-change cases. Retrying a definitive change unnecessarily can still pass. |
| + - The 300 ms second-Close check can pass if a broken second caller simply is not scheduled during that interval. Sequential successor checks likewise do not establish concurrent latch publication. |
| + |
| + Under umask 077, the relevant directory modes are now explicitly restored; resolving `platformDir` removes dependence on a symlinked temp-root spelling. Remaining assumptions include fixed fake UIDs `44001/44002`, basename-based ownership substitution, wall-clock timestamps, and integration-test timing windows. |
| + |
| + **Smallest acceptable fix:** use mutation-distinguishing fixtures and synchronization barriers; retain store/database references to assert shutdown; compare received and attributed attempts; and drive the watch’s file-check invocation explicitly. |
| + |
| +Claims the code still contradicts: |
| + |
| +- **Repeated holding after deletion:** `litestream_metered.go:602` and dispositions line **201** describe holding on “every look until the rebuild.” `lookAgain` returns immediately once stale at **:612**. There is an initial fleet-wide hold, not continual extension by later looks. |
| +- **Inventory changes after disappearance:** `litestream_metered.go:41` still includes “one has gone” in the benign inventory-change description, and the test comment at `litestream_metered_test.go:823` repeats it. A vanished owned file now causes `ErrOwnershipStale`. |
| +- **Resolved-path reporting:** `litestream_owners_unix_test.go:82` and `:126`, and dispositions line **141**, still describe ownership/reporting by the resolved path. `fleetInventory` stores the spelled path at `litestream_owners.go:190`; the checked facts belong to its resolved target. |
| +- **All fixture directories explicitly chmodded:** dispositions line **202** is broader than the code. Direct `MkdirAll` calls remain at `litestream_owners_test.go:211` and `:488`. Their narrower modes are harmless to these assertions; that does not make the literal claim true. |
| +- **Sidecar classification:** the comment at `litestream_owners_test.go:531` still describes classification by a neighbouring database. Current discovery header-reads sidecar-named regular files independently. |
| +- **Same-filesystem bind mounts:** the open-item wording needs qualification. Device checks do not identify such mounts, but two listed aliases of the same file are rejected by the identity comparison. The undetected case requires the original alias to escape that comparison. |
| +- **Test success:** the dispositions now correctly identify package/race results as the author’s runs. This source-only review neither confirms nor contradicts those execution results. |
| + |
| +**Verdict: reject.** |
| \ No newline at end of file |
diff --git a/docs/superpowers/specs/DISPOSITIONS-metered-replicator.md b/docs/superpowers/specs/DISPOSITIONS-metered-replicator.md| index 3f8d195f0..2603f7ca6 100644 |
| --- a/docs/superpowers/specs/DISPOSITIONS-metered-replicator.md |
| +++ b/docs/superpowers/specs/DISPOSITIONS-metered-replicator.md |
| @@ -80,7 +80,7 @@ Reports: `OPUS-REVIEW-metered-replicator-review3.md`, `CODEX-FINDINGS-metered-re |
| | Finding | Disposition | Where | Reproduction | |
| | --- | --- | --- | --- | |
| -| Opus P1-1, Astra P1: a unit route that names one file was checked as a named file: `Dir(p)` reached through `Stat`, nothing above it checked, no owner check. A tenant who renames the directory and puts a link in its place has another tenant's database streamed as its own. | Fixed. A unit route's file is a `tenantFile`: the check climbs from the file's directory while the directory above is the same user's and not a link, requires the directory above the top of that chain to be the platform's, refuses a link anywhere from the top down (the top included), and requires the file to be the chain's user's. An exec route's file stays a `namedFile` (its directory is the activator's). | `litestream_owners.go` `checkOwnedFile` | `TestFleetDBOwnersCheckAUnitRoutesNamedFileLikeATree` | |
| +| Opus P1-1, Astra P1: a unit route that names one file was checked as a named file: `Dir(p)` reached through `Stat`, nothing above it checked, no owner check. A tenant who renames the directory and puts a link in its place has another tenant's database streamed as its own. | Fixed. A unit route's file is a `tenantFile`: the check climbs from the file's directory while the directory above is the same user's and not a link, requires the directory above the top of that chain to be the platform's, refuses a link anywhere from the top down (the top included), and requires the file to be the chain's user's. An exec route's file stays a `namedFile` (its directory is the activator's). [Review nine (Opus P3-1): the climb stops at the first directory owned by root or by the platform's uid (01f2b072, for the CI runner), so a root-run unit, or a tenant on the platform's uid, is accepted beneath any protected directory (`/var/lib/fail2ban/fail2ban.db` on a root-run route); refusing a root or platform-uid owner unless the route is the ops account is the fix, with approval H.] | `litestream_owners.go` `checkOwnedFile` | `TestFleetDBOwnersCheckAUnitRoutesNamedFileLikeATree` | |
| | Opus P1-2: a swap during litestream's first open pins it to the wrong file for good; `lookAgain` compares the path's current file, not what litestream has open. | Open. Needs litestream to expose the identity of the file it opened, or to open beneath a pinned root; a litestream patch, not made. Stated in `checkOwnedFile`'s comment and here: the second look does not narrow this case. [Review six withdrew the identity comparison as a preventive fix: it detects a swap after the open, it does not prevent one; and review eight: every reopen and WAL open litestream makes by path is exposed, not the first open alone.] | | | |
| | Opus P2-1: the hold was passed to the replacement at build, so a fleet built before the old one noticed inherited nothing. | Fixed. `Close` asks the witness one last time before releasing it, and the replacement reads the old fleet's window at `Open`, which already requires the old fleet closed. | `litestream_metered.go` `Close`, `Open` | `TestMeteredFleetReplacementInheritsTheHold` (a case where the old fleet notices only as it closes) | |
| | Opus P2-2: any route added or removed stops the replicator and holds every day; most such routes have no database. Not stated. | Stated. The generation moves on every insert and delete of a route on purpose: whether a new route claims a database is not known until its files are looked at, and a rebuild is how they are. Narrowing it to rows with a `db_path` or unit backing is a later refinement, with the hold narrowed to the recorders affected; neither is built. On a busy box this means frequent rebuilds, which the caller that nothing yet is must do. | this file | | |
| @@ -92,7 +92,7 @@ Reports: `OPUS-REVIEW-metered-replicator-review3.md`, `CODEX-FINDINGS-metered-re |
| | Astra P2-5: an `UPDATE OR REPLACE` that moves a row onto another's rowid deletes that row without its delete trigger and names no watched column. | Fixed: a trigger refuses any change to a route's rowid. | `registry.go` `routes_rowid_immutable` | `TestRoutesRowidIsImmutable` | |
| | Astra P1-2: `platformOwned(dataDir)` checks the data directory and not its ancestors; a tenant-writable parent could swap the whole directory. | Open, stated. The data directory's ancestors on a box are `/data` and `/`, root's; the check does not establish that. | | | |
| | Astra P1-3 (rest), Opus (crash): what a process that dies between the change and the rebuild leaves; `Finish` and `Abandon` of an admitted request go to the old owner's recorder; the transition exists only in memory. | Open, stated more exactly: requests admitted before the change and finished before the crash are complete rows on a day nobody held; writing the transition down before the request is sent is the fix, not built. | | | |
| -| Opus P3-3, Astra P1 (part): a bind mount from the same filesystem has the same device number. | Open, stated: undetected whenever whoever holds mount authority in the replicator's namespace makes one [corrected in review six; the earlier "only an operator or systemd" was too narrow]. | | | |
| +| Opus P3-3, Astra P1 (part): a bind mount from the same filesystem has the same device number. | Open, stated: undetected when only one alias of the file is inventoried and whoever holds mount authority in the replicator's namespace made the mount; two listed aliases of one file are refused by the identity comparison [corrected in reviews six and nine; the earlier "only an operator or systemd" was too narrow]. | | | |
| | Astra P2-7: the shutdown tests pass with `store.Close` removed; the opened test does not compare attempts received with attempts recorded; the second look is not exercised through the watch. | Open; the tests are not strengthened in this build. | | | |
| | Opus: comments claimed more than the code (the link rule, "narrows the window", the margin). | Corrected where the code changed; the rest stated above as open. | | | |
| @@ -110,7 +110,7 @@ Reports: `OPUS-REVIEW-metered-replicator-review4.md`, `CODEX-FINDINGS-metered-re |
| | Finding | Disposition | Where | Reproduction | |
| | --- | --- | --- | --- | |
| | Opus P2-2, Astra 2 (P1): an admission whose witness read overlapped the file check's `goStale` was still admitted. | Fixed in part: the stale flag is checked again after the read, before the proof is advanced and nil returned, so no admission that saw the fleet current before its read and stale after it succeeds. Not fixed: a request admitted before the change and sent after it; that is the hold's job (goStale), as stated, and tracking such requests to completion is not built. | `litestream_metered.go` `admit` | `TestAdmissionIsRefusedWhenTheFleetGoesStaleDuringTheRead` (a witness read held open, the fleet made stale meanwhile, the read released: refused) | |
| -| Opus P2-3, Astra 7 (P2): the header read opened every regular file by path after an `Lstat`, so a FIFO, a device or a link swapped in would hang the walk, the watch and `Close`. | Fixed: the header read opens with `O_NOFOLLOW|O_NONBLOCK` and reads only the file that was looked at (same device and inode, regular), else reports it; unix only, the CLI's other platforms keep an ordinary open and never run the replicator. | `litestream_owners.go` `sqliteHeader`, `sqliteopen_unix.go` | `TestStrictTreeDoesNotBlockOnAFIFO` (a FIFO in a file's place; a link in a file's place) | |
| +| Opus P2-3, Astra 7 (P2): the header read opened every regular file by path after an `Lstat`, so a FIFO, a device or a link swapped in would hang the walk, the watch and `Close`. | Fixed: the header read opens with `O_NOFOLLOW|O_NONBLOCK` and reads only the file that was looked at (same device and inode, regular), else reports it; unix only; on the CLI's other platforms the header read refuses (`sqliteopen_other.go`, from the eighth build; before it they kept an ordinary open), and the replicator does not run there. | `litestream_owners.go` `sqliteHeader`, `sqliteopen_unix.go` | `TestStrictTreeDoesNotBlockOnAFIFO` (a FIFO in a file's place; a link in a file's place) | |
| | Astra 5 (P2): the climb examined each parent's owner but never the file's own directory, so a directory of another user's immediately above the file was accepted. | Fixed: the file's own directory is checked first (the owner's or the platform's, a directory, not a link), then the climb as before; a test with owners the test user cannot create (the `factsOf` seam) covers a tenant of its own, another user's directory in the chain, another user's file, and the tenant's own link. | `litestream_owners.go`; `filefacts_unix.go` (`factsOf` is a variable) | `TestUnitFileChainWithATenantOfItsOwn` | |
| | Opus P2-1: a unit route naming a file in a DynamicUser state directory (`/var/lib/carlos-app/<host>`, a root-made link to `/var/lib/private/...`) was always unowned, so the whole fleet refused. | Fixed: a link at the file's own directory is followed when the link, its parent and the resolved directory's parent are all the platform's; the chain is then the resolved one; the file stays owned under the path the route spells. A tenant-made link is refused on its owner. | `litestream_owners.go` | `TestUnitFileThroughAPlatformMadeStateLink`; `TestFleetDBOwnersCheckAUnitRoutesNamedFileLikeATree` (the platform-user shape) | |
| | Opus P2-4, Astra 6 (P2) in part: a relative unit `DBPath` and a missing unit claim were dropped in silence; a file the strict walk found that failed its final look vanished. | Fixed: a relative unit path is a problem; a missing unit claim is listed as skipped for the platform's replicator; files the walk found count as claimed for the final look's reporting. Not fixed: there is still no comparison against an external replicator's inventory at cutover (open, as before). | `litestream.go` | `TestFleetInventoryNamesWhatItCannotReplicate` | |
| @@ -138,7 +138,7 @@ Reports: `OPUS-REVIEW-metered-replicator-review5.md`, `CODEX-FINDINGS-metered-re |
| | Opus P2-1, Astra 2 (P1): the hold for a file swap ran from the start of the look that found it, not from the last look that found every file as it was; a second finding could not widen the first's window. | Fixed: `lastClean` is the start of the last look that found nothing (seeded by the constructor's inventory); a swap is held from before it; a later finding widens the window and holds only the days it adds. Not fixed: requests tracked to completion across midnight, and the crash case, as stated before. | `litestream_metered.go` `lookAgain`, `goStaleFrom` | `TestAFileSwapIsHeldFromTheLastCleanLook` (driven through `lookAgain`, with the registry proof an hour old and the last clean look two days old: the clean look's day is held; a finding within the window adds nothing; one further back widens) | |
| | Astra 3 (P1), Opus P3-1 in part: a replacement that went stale after inheriting passed on its own window alone; a fleet closed before its predecessor latched an empty inheritance. | Fixed: `staleWindow` is the union of own and inherited; `inheritHold` latches only once the predecessor has closed, and an unlatched fleet answers for its predecessor through `staleWindow`. | `litestream_metered.go` | `TestInheritedWindowIsTheUnion` (B stale after inheriting A: C holds both days; B closed before A: C holds A's later day through B) | |
| | Opus P3-1, Astra 5 (P2): `Open` checked one predecessor; `Open` racing `Close` could open a store already torn down. | Fixed: `Open` requires every fleet in the chain it replaces to be closed; `Open` and the one shutdown hold a lifecycle lock, and `Open` after `Close` is `ErrOwnershipStale`. [Review eight: untrue from the eighth build to the ninth for a predecessor that had latched; see "Review eight".] | `litestream_metered.go` | `TestOpenRefusesWhileAnyPredecessorIsOpen`, `TestCloseBlocksEveryCallerUntilTheStoreHasStopped` (a shutdown held open by a hook: a second caller does not return, a replacement cannot open, the fleet does not read as closed, until it finishes) | |
| -| Opus P3-3, Astra 4 (P1) in part: a link inside the spelled path above the chain's top was followed, so another tenant's tree was reached under a tenant's route; the platform-made start link resolved through whatever its target passed through. | Fixed: the spelled directory path is resolved one component at a time, following only the platform's links (the link and its parent the platform's, an absolute target with no further link, the target's parent the platform's) and refusing any other; the chain top's ancestry must then equal its resolution. A file nested at any depth beneath a platform-made link is owned by its resolved path (Astra 6). Not fixed: `dataDir`'s own ancestry and the shadow directories (Astra 4): `platformOwned` still checks a directory, not its ancestry; a verified immutable hierarchy is approval H's question. | `litestream_owners.go` `resolvePlatformLinks` | `TestUnitFileChainRefusesALinkAboveIt`, `TestUnitFileThroughAPlatformMadeStateLink` (nested file; a platform link whose target passes through the tenant's link, refused) | |
| +| Opus P3-3, Astra 4 (P1) in part: a link inside the spelled path above the chain's top was followed, so another tenant's tree was reached under a tenant's route; the platform-made start link resolved through whatever its target passed through. | Fixed: the spelled directory path is resolved one component at a time, following only the platform's links (the link and its parent the platform's, an absolute target with no further link, the target's parent the platform's) and refusing any other; the chain top's ancestry must then equal its resolution. [Review nine: the inventory keeps the path the route spells; the facts checked are the resolved file's.] A file nested at any depth beneath a platform-made link is owned by its resolved path (Astra 6). Not fixed: `dataDir`'s own ancestry and the shadow directories (Astra 4): `platformOwned` still checks a directory, not its ancestry; a verified immutable hierarchy is approval H's question. | `litestream_owners.go` `resolvePlatformLinks` | `TestUnitFileChainRefusesALinkAboveIt`, `TestUnitFileThroughAPlatformMadeStateLink` (nested file; a platform link whose target passes through the tenant's link, refused) | |
| | Opus P3-4: a platform-owned start directory was not checked for world-writability; a tenant whose uid is the platform's has no isolation. | Fixed for the first: a platform-owned start directory must be writable by nobody else. The second is stated: an untrusted tenant must not share the platform's uid (operator rule, with approval H). | `litestream_owners.go` | `TestUnitFileInAWorldWritablePlatformDirectory` | |
| | Opus P3-6, Astra 8 (P2): a database under a sidecar name beside an unrelated regular file was never read. | Fixed: a sidecar name is a sidecar only beside a database (by name or by header). | `litestream_owners.go` `sidecarOf` | `TestStrictTreeReadsAStandaloneSidecarName` | |
| | Opus P3-6, Astra 7 (P2): a claimed or discovered file that vanished before its final look disappeared. | Fixed: it is listed as skipped. | `litestream.go` | no deterministic test (the race between the walk and the final look is not simulated); stated | |
| @@ -198,11 +198,28 @@ Reports: `OPUS-REVIEW-metered-replicator-review8.md`, `CODEX-FINDINGS-metered-re |
| | --- | --- | --- | --- | |
| | Opus P1-1, Astra 1 (P1): a latched predecessor was taken for a closed one by `Open` and by `inheritHold`. | Fixed: the chain walk (`chainClosed`) reads each fleet's own closed flag first and trusts its latch only for the fleets behind it; the latch itself (`latch`) stores the window, clears the chain and sets the flag under the stale lock in one step, and the walks read the chain under that lock, so no reader sees a latched fleet without its window (the data race Opus names). | `litestream_metered.go` `chainClosed`, `link`, `latch`, `inheritHold`, `Open` | `TestALatchedPredecessorMustStillBeClosed` (C over a latched open B, then a latched draining B, refused; after B closes, opened); `TestAnUnopenedFleetDoesNotLatchThroughALiveLatchedPredecessor` (Astra's sequence: C closed unopened over a live latched B stays unlatched; B's later days reach C's recorder and D's). Both fail with the walk's old order (mutation run). | |
| | Opus P3-2: a `..` after a tenant-owned component of a platform link's target was accepted. | Fixed: a `..` is taken only in the target's leading run (systemd's `../private/...`); one after any named component refuses. | `litestream_owners.go` `walkWithoutLinks` | `TestAPlatformLinksRelativeTargetIsWalkedNotCleaned`, second half flipped to a refusal, third half accepts the leading run; fails with the rule removed (mutation run). | |
| -| Opus P3-3: one tenant's file deletion holds every account's days on the box, every look until the rebuild. | Stated as a known availability cost, in `lookAgain`'s comment and here: a finding on one file holds every recorder the fleet has, because the fleet is one unit of doubt and the inherited window carries no paths. Holding by owner needs the window to carry which paths it is about; not built. The direction is safe (over-holding). | `litestream_metered.go` `lookAgain` | | |
| -| Opus P3-4, Astra 6 and 8 (tests): nothing pinned `replaces = nil`; the "once" check passed without the intermediate CAS; the clean-start test assumed 100 ms of scheduling; fixtures depended on the umask and on the temp root. | Fixed in part: the latch's effect on the chain is asserted (`link()` after the latch); the once check also covers a second successor of the same fleet (the CAS is gone: the latch under the lock is the once); the clean-start test asserts the look's whole delay fits between the recorded instant and the look's end, and that the hold runs from exactly that instant less the margin, with no bound on scheduling; every fixture directory is made 0755 by an explicit chmod (`mkdirs`) and the "root is open to others" shadow fixture too; `platformDir` resolves its path once. The package passes under `umask 077` and under the race detector (author's runs). Open: the 300 ms wait for a second `Close` caller (a negative check: it cannot fail wrongly, only pass wrongly on a very slow box); `withOwners` keys by base name (the seam takes a `FileInfo`, which carries no path; the fixture names are chosen not to occur in a temp root's components). | tests | | |
| +| Opus P3-3: one tenant's file deletion holds every account's days on the box. | Stated as a known availability cost (once: the fleet is stale from the first look that finds it, and later looks return at once; "every look until the rebuild" was wrong, as Astra's ninth review says), in `lookAgain`'s comment and here: a finding on one file holds every recorder the fleet has, because the fleet is one unit of doubt and the inherited window carries no paths. Holding by owner needs the window to carry which paths it is about; not built. The direction is safe (over-holding). | `litestream_metered.go` `lookAgain` | | |
| +| Opus P3-4, Astra 6 and 8 (tests): nothing pinned `replaces = nil`; the "once" check passed without the intermediate CAS; the clean-start test assumed 100 ms of scheduling; fixtures depended on the umask and on the temp root. | Fixed in part: the latch's effect on the chain is asserted (`link()` after the latch); the once check also covers a second successor of the same fleet (the CAS is gone; serially the once is the chain let go, `replaces = nil`, and the re-check under the lock in `latch` covers only two successors racing, which the caller forbids, and is untested); the clean-start test asserts the look's whole delay fits between the recorded instant and the look's end, and that the hold runs from exactly that instant less the margin, with no bound on scheduling; every fixture directory whose mode a test depends on is made 0755 by an explicit chmod (`mkdirs`) and the "root is open to others" shadow fixture too (two bare `MkdirAll` calls remained in fixtures whose mode nothing depends on; the tenth build makes them `mkdirs` as well); `platformDir` resolves its path once. The package passes under `umask 077` and under the race detector (author's runs). Open: the 300 ms wait for a second `Close` caller (a negative check: it cannot fail wrongly, only pass wrongly on a very slow box); `withOwners` keys by base name (the seam takes a `FileInfo`, which carries no path; the fixture names are chosen not to occur in a temp root's components). | tests | | |
| | Opus P3-1, Astra 7 (P3): units running as root, or a tenant sharing the platform's uid. | Open, as before, with approval H. | | | |
| | Astra 2 (P1), Astra 3 (P1), Astra 5 (P2): litestream's own opens by path (every reopen and WAL open, not the first alone); protected endpoints under tenant-replaceable ancestry; the header open steered by a swapped ancestor. | Open, as before: a walk by directory descriptor and a litestream change; required before wiring. Stated more exactly now in `openRegularNoFollow`'s comment and in the row for Opus P1-2 of review three. | | | |
| | Astra 4 (P2): an exec route's claimed file outside the globs is in no list, and the other omissions listed. | Open, as before; the next bounded item after this one. | | | |
| | Claims (both lists): `ErrOwnershipStale` "from then on" and its duplicated comment; `Close` "holds nobody's day" and its duplicate; `goStale` "the last proof"; "at most one look's span"; `lookAgain` "gone is ErrInventoryChanged"; "its target is absolute" (twice); "a tenant's link above the chain is caught here"; the strict-walk doc on which files are header-read; the `..` doc; `openRegularNoFollow` "without harmful effects"; the registry witness "only ownership moves"; `platformDir`'s umask sentence; "reports the file by its resolved path"; the dispositions rows named by line (the identity comparison, the bind-mount sentence, "a replacement can never open beside a draining store", "every regular file", "package tests pass"). | Corrected, each in place; the `Replaces` doc now says a fleet's recorders must outlive its `Close` until the chain has latched. | | | |
| Package tests pass under `/tmp` and under `~/.cache`, under `umask 077`, and under `-race` (the author's runs). The full gate on this tip is next; the ninth build has not been read by a reviewer. |
| + |
| +## Review nine (reviewed `142ef5d0`; Opus accepted with fixes, Astra rejected) |
| + |
| +Reports: `OPUS-REVIEW-metered-replicator-review9.md`, `CODEX-FINDINGS-metered-replicator-review9.md` (the eighth review's reports were not in the tree at `142ef5d0`; `e5e67b25` added them, and the tenth build's commit carries the ninth's). Both found the eighth review's P1 closed, with the two tests failing without the fix, and the `..` rule closed. Opus found one P2 in the handover: `Close` asked the registry alone, so a swap still in place at a rebuild, with no look between the swap and the close, was held by nobody (the successor's own inventory compares nothing with the old fleet's facts). Astra's P1s are the deep items, restated; its P2s are the exec claim outside the globs (built now) and the header open through a swapped ancestor (open). The tenth build: |
| + |
| +| Finding | Disposition | Where | Reproduction | |
| +| --- | --- | --- | --- | |
| +| Opus P2-1: `Close` took no last look at the files. | Fixed: `closeNow` takes the look (`lookNow`, the body of `lookAgain`) after the store and the watch have stopped and before the witness is read, so a changed or vanished owned file holds from the last clean look. | `litestream_metered.go` `closeNow`, `lookNow` | `TestCloseLooksAtTheFilesOneLastTime` (a swap after the last clean look, then `Close` with no look between: held from that look less the margin, on the recorders); fails with the look removed (mutation run). | |
| +| Astra 5 (P2), carried since review seven: an exec route's database outside the globs was in no list. | Fixed, in strict mode (the platform's replicator): every non-hibernating route's absolute `DBPath` is a claim the inventory looks at, owned when the file is there and said (skipped or a problem) when it is not; a relative one is a problem. The external replicator's config keeps to the globs. | `litestream.go` | `TestExplicitClaimsOutsideTheGlobsAreInventoried`; fails with the explicit claims not appended (mutation run). | |
| +| Opus P3-2: a hibernating route silently dropped a listed path equal to its `DBPath`, a unit claim or the platform's file included. | Fixed: in strict mode such a path is a problem (two claims on one path). | `litestream.go` | the same test (a hibernating route over a unit route's path); fails with the report removed (mutation run). | |
| +| Opus P3-3: a routine restart drops the inherited window, not only a crash. | Stated: any process exit; the durability ledger's box-wide incompleteness is what covers it, which the fleet does not rely on. | `litestream_metered.go` `MeteredFleetStore` doc | | |
| +| Opus P3-4, Astra 8: nothing pinned `inheritHold` inside `closeNow`; a witnessed change's read count was not asserted; two bare `MkdirAll`; the latch's re-check under the lock untested. | Fixed in part: `TestCloseOfAnUnopenedFleetLatchesItsChain` (a fleet built over a stale, closed one and closed unopened: latched inside `Close`, its recorders told); the retry table asserts the read count for a witnessed change (the errors before it and the one that saw it, no more); the two `MkdirAll` are `mkdirs`. Still open: the re-check under the lock (two successors racing, which the caller forbids); the 300 ms negative check; `withOwners` by base name; retained handles and worker exit; the watch's own call of the look; per-database attribution; the shadow-state directory check in `prepareShadowState` (refused earlier by the inventory's `platformDirectory` check, so untested on its own). | tests | | |
| +| Opus P3-1, Astra 7: root-run units and platform-uid tenants. | Open, with approval H; the review-three row now says what the climb does. | | | |
| +| Astra 1 to 4, 6 (P1s and the device open): litestream's opens by path; tenant-replaceable ancestry above protected endpoints (`dataDir`, shadow state, a platform link's target); a delayed admitted request past the margin; the crash gap; the header open through a swapped ancestor. | Open, as before, each stated in the code where it applies; the walk by directory descriptor and durable transition state are the fixes and are required before wiring. | | | |
| +| Claims (both): `registry.go` "and on none that did" (now "misses none that did"); `ErrInventoryChanged` "one has gone"; `lookAgain` "every look until the rebuild"; "passes nothing over in silence"; "by its resolved path" (tests); the sidecar test's comment; dispositions rows (the other platforms' open; the climb's stop; "every fixture directory"; "the latch under the lock is the once"; the bind-mount sentence). | Corrected in place. | | | |
| + |
| +Package tests pass under `/tmp` and `~/.cache` (the author's runs). The full gate on this tip is next; the tenth build has not been read by a reviewer. |
diff --git a/docs/superpowers/specs/OPUS-REVIEW-metered-replicator-review9.md b/docs/superpowers/specs/OPUS-REVIEW-metered-replicator-review9.md| new file mode 100644 |
| index 000000000..2ca0645cc |
| --- /dev/null |
| +++ b/docs/superpowers/specs/OPUS-REVIEW-metered-replicator-review9.md |
| @@ -0,0 +1,124 @@ |
| +I read the code at 142ef5d0, the reviewed commit (the brief's `git show b5aecf13:` looks like a slip). The review-eight reports are not in the tree at 142ef5d0; I read them from the run's `logs/` (`opus-replicator-review8.md`, `astra-replicator-review8.md`). In the ninth build, `litestream_owners.go` changes only in comments, in the `..` rule and in `walkWithoutLinks`. The climb's stop at the platform's uid dates from 01f2b072. I ran nothing. |
| + |
| +## Review eight, finding by finding |
| + |
| +| Finding | Now | Would its test pass without the fix? | |
| +|---|---|---| |
| +| Opus P1-1 / Astra 1: a latched predecessor taken for a closed one | **Closed.** `chainClosed` (`litestream_metered.go:415-427`) reads `closed` before it trusts the latch. `Open` (`:543`) and `inheritHold` (`:482`) both use it. `latch` (`:451-468`) stores the window, sets `replaces = nil` and sets `inherited` under `staleMu`. Every read of the chain goes through `link()` or `staleWindow`, both under the lock. | **No.** With the old loop, `TestALatchedPredecessorMustStillBeClosed` opens C beside B. `TestAnUnopenedFleetDoesNotLatchThroughALiveLatchedPredecessor` finds C latched. | |
| +| Opus P3-1 / Astra 7: root-run units, platform-uid tenants | **Open**, unchanged (approval H). See A. | n/a | |
| +| Opus P3-2: a `..` after a named component | **Closed** (`litestream_owners.go:592-604`). | **No.** The flipped second half (`unix_test.go:309-314`) fails without the rule. | |
| +| Opus P3-3: one tenant's deletion holds every recorder | **Stated** (`:602-607`, dispositions). | n/a | |
| +| Opus P3-4 / Astra 6, 8: tests | **Partly closed.** | **No** for `replaces = nil` (the `link()` assertion at `:1551-1555`). **No** for the clean-start test (an end-of-look stamp fails `recorded+delay > after`). **Yes** for the re-check under the lock at `:459-461` (see H). Retained handles, worker exit, the watch's own `lookAgain` call and per-database attribution are still untested. The 300 ms wait and the base-name `withOwners` remain. | |
| +| Opus claims | **Corrected**, except that the registry sentence is now wrong another way (see claims). | n/a | |
| +| Astra 2 (litestream's opens by path), 3 (ancestry), 5 (device via an ancestor) | **Open**, stated correctly. `openRegularNoFollow`'s doc (`sqliteopen_unix.go:11-20`) is now honest. | n/a | |
| +| Astra 4: exec claims outside the globs | **Open**, stated. | n/a | |
| +| Astra claims | **Mostly corrected.** "Passes nothing over in silence" (`litestream_owners.go:488`) remains, and dispositions `:113` is stale (see claims). | n/a | |
| + |
| +## Findings |
| + |
| +### P2-1. `Close` takes no last look at the files, so a swap still in place at a rebuild is held by nobody (D, M) |
| + |
| +- **Where:** `litestream_metered.go:724-738`. `closeNow` asks only the registry. `lookAgain` (`:611`) runs only from the watch, which `closeNow` has already stopped at `:716-717`. The successor's constructor (`:810`) takes fresh facts and never compares them with the old fleet's. |
| +- **Scenario:** |
| + 1. B is current. At 23:59:45 a look finds a new tenant database. `drift` is set, and the look is clean, so `lastClean` = 23:59:45. |
| + 2. At 23:59:50 T renames `main.db` aside and puts a link to V's database in its place. Litestream's next WAL open by path ships V's pages under T's lineage, recorded to T on Oct 2. |
| + 3. At 00:00:10 the caller acts on `ErrInventoryChanged`, or a binary roll restarts the process. It closes B before the 00:00:15 look. |
| + 4. B's last witness read is unchanged, so B holds nothing. |
| + 5. C's constructor sees the link and refuses (`Unowned`). If T has restored the file by then, C accepts it as T's. |
| + 6. Either way, no recorder holds Oct 2. The code's own model says this swap is caught "when the swap is still in place when the look runs"; at Close, no look runs. |
| +- **Fix:** in `closeNow`, after the store and the watch have stopped and before the witness read, run the file comparison (`lookAgain`'s body without its `closed` early return). That way a changed or vanished owned file calls `goStaleFrom(lastClean, now)`. Test: swap after a clean look, then `Close` with no look in between, and assert the hold. |
| + |
| +### P3-1. Units run as root, or a tenant on the platform's uid: accepted from almost anywhere (A, J; carried as Opus P3-1) |
| + |
| +- **Where:** `litestream_owners.go:360-369`, `:380`, `:397`, `:438`. |
| +- **Platform-uid tenant:** if the file's owner is root or `euid`, the climb stops at once (`:380`). The file is then accepted when its directory and that directory's parent are platform-owned and not group- or world-writable. |
| +- **Root-run units:** this covers nearly every root-owned SQLite file on the box. For example, a route with `DBPath=/var/lib/fail2ban/fail2ban.db` is accepted. So is a leftover `/data/instances/<id>/<host>.db` whose route has gone; it is streamed into the claiming tenant's lineage, and that tenant can restore it. |
| +- **Fails closed** when the victim still has a live claim: two accounts on one file are refused at `:167`, and two spellings of one file at `:133`. |
| +- **Tenant with its own uid (A):** it cannot get a link or another user's directory into its chain: |
| + - its own links are refused by `resolvePlatformLinks` (`:648`); |
| + - another user's parent ends the climb and then fails `platformOwned` (`:397`); |
| + - a foreign start directory is refused at `:367`; |
| + - the walk refuses links from the top down (`:419`). |
| +- **Top-of-chain checks:** `:385`, `:394` and `:397` are reached on every accepting path. `:385` and `:394` can only trip on a race. |
| +- **The ninth build** changes nothing here. |
| +- **Fix (with H):** refuse a `tenantFile` or `tenantTree` whose owner uid is 0 or `euid` unless the route is the ops account. Bind the uid to the unit. |
| + |
| +### P3-2. A hibernating route silently drops any listed path equal to its `DBPath` (L) |
| + |
| +- **Where:** `litestream.go:380-382` and `:515-517`. The map is built from non-unit routes and applied to every match: unit claims, the registry and the console. |
| +- **Scenario:** an exec route with `Hibernate=true` and `DBPath` set to an active unit route's file, or to `/data/console/console.db`. That file leaves the inventory with no skip and no problem reported, where two claims would otherwise be a conflict. |
| +- **Fix:** in strict mode, report a hibernation-suppressed path that is also a unit claim or a platform path as a problem. |
| + |
| +### P3-3. A process restart drops the inherited window, not only a crash (G) |
| + |
| +- **Where:** `litestream_metered.go:772-779`. |
| +- **What happens:** the chain lives only in memory. After a roll, the new process builds with `Replaces: nil`. |
| +- **Scenario:** B's last read finds a move to owner `c` and holds B's recorders. The new owner's recorder, which no fleet ever had, is told nothing. |
| +- **Why only P3:** this is covered only because the durability ledger's incompleteness is box-wide (`:94-96`), which the fleet says it does not rely on. |
| +- **Fix:** say "any process exit, including a routine restart" and name the box-wide ledger as what covers it, or persist the transition. |
| + |
| +### P3-4. Tests (H, L) |
| + |
| +- **The latch's "once" under concurrency:** removing the re-check at `litestream_metered.go:459-461` passes every test. |
| + - The `c2` check (`_test.go:1558-1559`) never touches B. B is latched with `replaces == nil`, so the loop breaks at `:493`. |
| + - Serial "once" comes from `replaces = nil`. Only two successors of one fleet could race, and the caller forbids that. |
| +- **The data-directory check in `prepareShadowState`** (`:909-911`) is untested. The "the data directory can be written by anyone" fixture (`_test.go:805-809`) is refused earlier, by the registry's `platformDirectory` check in the inventory (`Unowned`, `:831`). |
| +- **The new chain tests** use hand-built fleets with `closed.Store` (`_test.go:1640-1662`). Nothing pins `closeNow` running `inheritHold` before `closed.Store` (`:737-738`). |
| +- **Umask and temp root (by reading):** |
| + - Under `umask 077` no fixture changes meaning. `platformOwned` refuses only 022 bits, and every fixture that needs an open mode now chmods. |
| + - A temp root reached through a link is resolved once in `platformDir`, and every fixture path derives from it. |
| + - The remaining fragility is base-name keying in `withOwners`. It is harmless for current names, because the climb stops below the temp root. |
| +- **Clean-start test:** it mixes a monotonic `before` with a wall-only `recorded` (`_test.go:1485`), so a wall-clock step backwards fails it. Negligible. |
| + |
| +## B, C, E, F, I, J, K, G (no new finding) |
| + |
| +- **B. Request gate and admission:** |
| + - The stale checks at `:209`, `:221` and `:235` are complete up to the return. A request admitted at `:235` can still reach the network after a later finding; the margin covers that. |
| + - Retries are bounded at 3 × 5 s plus 2 × 200 ms per read. That excludes the `os.Stat` in `OwnershipUnchanged`, which ignores the context, and time spent waiting for `f.mu`. |
| + - Only an error that keeps failing, or a witnessed `false`, counts as a change. |
| +- **C. Ownership generation:** unchanged and correct. |
| + - The triggers cover insert, delete, changed-value updates of the six columns, and an immutable rowid. |
| + - Discovery and ownership read only those columns, so nothing else needs to move it. |
| + - Raw statements from an older build fire the triggers too. |
| +- **E. Lineage and shadow state:** unchanged. |
| + - Keyed by account plus path, so owners of different accounts never share keys. |
| + - A→B→A resumes A's lineage. It is still unstated. |
| +- **F. Strict walk:** |
| + - Names in other cases are skipped and reported; empty files are passed over (stated). |
| + - Every other non-empty regular file is header-read. |
| + - An unreadable directory is a problem, and so is a unit route with no `DBPath`. |
| + - Empty claimed files are skipped and reported. |
| +- **I. Admission and `closeOnce`:** |
| + - A replacement cannot open beside a draining store: `closed` is set at `:738`, after `store.Close` and the watch's exit. |
| + - The only post-finding admission is the instant after `:235`. |
| +- **J. Chain walk:** |
| + - A successor closed unopened over a live latched fleet does not latch. |
| + - The window and the chain change together, under one lock. |
| + - A `DBPath` that is itself a link is refused at `:322`. |
| +- **J. A `DBPath` naming another tenant's file:** |
| + - Through an operator-made link, it is accepted only if that tenant has no live claim (P3-1's class). |
| + - Through a platform link into a tenant-replaceable ancestry, it is Astra 3, carried. |
| +- **K. `walkWithoutLinks`:** |
| + - systemd's shapes (`private/x`, `../private/a/b`, `../../…`) are all leading runs. `path_make_relative` never produces a `..` after a name, so none is refused. |
| + - A leading `..` from a link-free prefix is what the kernel resolves. |
| +- **K. The open:** |
| + - The race between the `Lstat` and the open is caught by the inode check, after the open. |
| + - `sidecarOf` is gone, and a link base is refused on its own. |
| + - Platforms without `O_NOFOLLOW` (`!unix`) refuse the read. |
| +- **G. Open items:** |
| + - The crash gap is larger than stated: any restart (P3-3). |
| + - The litestream-open swap, ancestry, same-filesystem bind mounts and the external-inventory comparison are stated correctly. |
| + - P2-1 is not listed. |
| + |
| +## Claims the code contradicts |
| + |
| +- **`registry.go:583-584`:** "a reader stops on some changes that moved no owner, and on none that did." Read literally, it says the reader never stops on a real ownership change. It should say "and misses none that did". |
| +- **`litestream_owners.go:488`:** "passes nothing over in silence". The same doc then lists silent ignores at `:504-507`: vanishing entries, empty files, and links not named like databases that lead to files. |
| +- **Dispositions `:113`:** "the CLI's other platforms keep an ordinary open". `sqliteopen_other.go:13-15` refuses. |
| +- **Dispositions `:82`:** the `tenantFile` climb is described without its stop at root's or the platform's uid (01f2b072). |
| +- **Dispositions, "Review eight" tests row:** |
| + - "every fixture directory is made 0755 by an explicit chmod (`mkdirs`)". `litestream_owners_test.go:211` and `:488` still use a bare `MkdirAll`. It is harmless, but the claim is not true. |
| + - "the latch under the lock is the once". Serially, the once is `replaces = nil`. The re-check under the lock is untested (P3-4). |
| +- **Dispositions, "Package tests pass … under `umask 077` and under `-race`":** not established by this review, which ran nothing. |
| + |
| +**Verdict: accept with fixes.** Fix P2-1 and the registry sentence. P3-1, Astra 2, Astra 3 and the walk by directory descriptor still block wiring and attestation. |
diff --git a/internal/ops/litestream.go b/internal/ops/litestream.go| index 75ec3b807..749f3552b 100644 |
| --- a/internal/ops/litestream.go |
| +++ b/internal/ops/litestream.go |
| @@ -354,6 +354,7 @@ func FleetDBsWithSkipped(reg *registry.Registry, dataDir string) (dbs []string, |
| // the caller, and a tree walk's problems carry the claiming host. |
| func fleetDBsFromRoutes(routes []registry.Route, registryPath, dataDir string, tree func(root string) (dbs, skipped, problems []string)) (dbs []string, skipped []string, problems []string, err error) { |
| hibernating := map[string]bool{} |
| + explicit := map[string]bool{} |
| unitPaths := map[string]string{} // claimed path -> the first host that claims it |
| for _, r := range routes { |
| if r.Backing == registry.BackingUnit { |
| @@ -379,6 +380,18 @@ func fleetDBsFromRoutes(routes []registry.Route, registryPath, dataDir string, t |
| } |
| if r.Hibernate && r.DBPath != "" { |
| hibernating[filepath.Clean(r.DBPath)] = true |
| + continue |
| + } |
| + // An exec route's database outside the globs (a path of the |
| + // operator's choosing): the platform's own replicator inventories |
| + // it like any other claim, since a route named it; the external |
| + // replicator's config keeps to the globs as it always has. |
| + if tree != nil && r.DBPath != "" { |
| + if !filepath.IsAbs(r.DBPath) { |
| + problems = append(problems, r.Host+": a route with a relative database path ("+r.DBPath+")") |
| + } else { |
| + explicit[filepath.Clean(r.DBPath)] = true |
| + } |
| } |
| } |
| @@ -417,6 +430,10 @@ func fleetDBsFromRoutes(routes []registry.Route, registryPath, dataDir string, t |
| if registryPath != "" { |
| claimed[filepath.Clean(registryPath)] = true |
| } |
| + platformPaths := map[string]bool{} |
| + for p := range claimed { |
| + platformPaths[p] = true |
| + } |
| for _, r := range routes { |
| if r.DBPath != "" { |
| claimed[filepath.Clean(r.DBPath)] = true |
| @@ -433,6 +450,12 @@ func fleetDBsFromRoutes(routes []registry.Route, registryPath, dataDir string, t |
| skipped = append(skipped, filepath.Clean(p)) |
| } |
| matches = kept |
| + // Explicit claims the globs did not propose, in a fixed order. |
| + for _, p := range sortedKeys(explicit) { |
| + if !globbed[p] { |
| + matches = append(matches, p) |
| + } |
| + } |
| // Unit-tenant DBPaths are added on top of the glob: they typically live |
| // entirely outside dataDir, at their StateDirectory path. A DBPath naming |
| @@ -497,7 +520,7 @@ func fleetDBsFromRoutes(routes []registry.Route, registryPath, dataDir string, t |
| switch { |
| case err != nil && !errors.Is(err, fs.ErrNotExist): |
| problems = append(problems, p+": cannot be looked at") |
| - case err != nil && (discovered[p] || unitPaths[p] != "" || globbed[p]): |
| + case err != nil && (discovered[p] || unitPaths[p] != "" || globbed[p] || explicit[p]): |
| // Found a moment ago and gone now: a tenant renaming a |
| // database under the look, or an activator directory |
| // in churn. Said, so a database that comes back after |
| @@ -513,6 +536,14 @@ func fleetDBsFromRoutes(routes []registry.Route, registryPath, dataDir string, t |
| continue |
| } |
| if hibernating[p] { |
| + // A hibernating route's database is the activator's to |
| + // replicate, not this fleet's. One that is also a unit route's |
| + // claim or the platform's own file is two claims on one path, |
| + // and the platform's replicator says so rather than letting |
| + // the hibernating one hide the other. |
| + if tree != nil && (unitPaths[p] != "" || platformPaths[p]) { |
| + problems = append(problems, p+": claimed by a hibernating route and by a unit route or the platform") |
| + } |
| continue |
| } |
| seen[p] = true |
| @@ -655,3 +686,14 @@ func writeIfChanged(path, content string) (bool, error) { |
| } |
| return true, nil |
| } |
| + |
| +// sortedKeys is a map's keys in order, for an inventory that reads the |
| +// same each time it is taken. |
| +func sortedKeys(m map[string]bool) []string { |
| + keys := make([]string, 0, len(m)) |
| + for k := range m { |
| + keys = append(keys, k) |
| + } |
| + sort.Strings(keys) |
| + return keys |
| +} |
diff --git a/internal/ops/litestream_metered.go b/internal/ops/litestream_metered.go| index e74648348..22d1606fe 100644 |
| --- a/internal/ops/litestream_metered.go |
| +++ b/internal/ops/litestream_metered.go |
| @@ -38,9 +38,11 @@ var ErrUnmeterable = errors.New("ops: a replicated database has no owner to mete |
| var ErrOwnershipStale = errors.New("ops: ownership of the replicated databases can no longer be vouched for; the replicator must be rebuilt") |
| // ErrInventoryChanged: the databases on the box are no longer the ones |
| -// this replicator was built over (a tenant made a new one, or one has |
| -// gone). Nothing it streams is in doubt, so it goes on streaming and |
| -// counting; it has to be rebuilt to cover what is new. |
| +// this replicator was built over (a tenant made a new one, or a file it |
| +// does not stream appeared or went). Nothing it streams is in doubt, so |
| +// it goes on streaming and counting; it has to be rebuilt to cover what |
| +// is new. An owned file that has gone is not this: it is doubt |
| +// (ErrOwnershipStale), see lookAgain. |
| var ErrInventoryChanged = errors.New("ops: the box's databases have changed since the replicator was built; rebuild it to cover them") |
| // How often an opened fleet asks the registry's ownership witness without |
| @@ -602,16 +604,26 @@ func (f *MeteredFleet) watch() { |
| // A finding on one file holds every recorder this fleet has, not only |
| // the changed file's owner's: the fleet is one unit of doubt and is |
| // rebuilt whole. That over-holds the other owners' days (an ordinary |
| -// deletion by one tenant, found every look until the rebuild, keeps every |
| -// account's day on the box in doubt); holding by owner needs the window |
| -// to carry which paths it is about, and is not built. |
| +// deletion by one tenant puts every account's day on the box in doubt, |
| +// once: the fleet is stale from then and later looks return at once); |
| +// holding by owner needs the window to carry which paths it is about, |
| +// and is not built. |
| // |
| // This is a second look, not a guard. Between two looks a tenant can swap |
| -// a file and swap it back. See checkOwnedFile. |
| +// a file and swap it back. See checkOwnedFile. Close takes this look one |
| +// last time (lookNow) before it asks the witness, so a swap still in |
| +// place at a rebuild is held by the fleet that was streaming through it. |
| func (f *MeteredFleet) lookAgain() { |
| if f.closed.Load() || f.stale.Load() { |
| return |
| } |
| + f.lookNow() |
| +} |
| + |
| +// lookNow is lookAgain without its guard: the look itself, from the |
| +// routes this fleet was built over, held from the last clean look when it |
| +// finds a change. |
| +func (f *MeteredFleet) lookNow() { |
| began := time.Now() |
| if f.lookDelay > 0 { |
| time.Sleep(f.lookDelay) |
| @@ -721,12 +733,20 @@ func (f *MeteredFleet) closeNow() error { |
| f.closed.Store(true) |
| return first |
| } |
| - // One last reading before the witness is released: a change that |
| - // landed after the last admission, and that no watch tick saw, is |
| - // still this fleet's to hold, and the fleet that replaces it reads the |
| - // hold at Open. The reading has the same patience as an admission's: |
| - // one busy read must not hold every account's days on this fleet and |
| - // its replacement. |
| + // One last look at the files, and one last reading of the witness, |
| + // before the witness is released: a swap still in place, or a change |
| + // in the registry, that landed after the last admission and that no |
| + // watch tick saw (the watch has stopped by now) is still this fleet's |
| + // to hold, and the fleet that replaces it reads the hold at Open. The |
| + // files first: the successor's own inventory compares nothing with |
| + // this fleet's facts, so a swap it finds in place it refuses and a |
| + // swap restored by then it accepts, and only this look holds the days |
| + // litestream may have shipped through it. The reading has the same |
| + // patience as an admission's: one busy read must not hold every |
| + // account's days on this fleet and its replacement. |
| + if !f.stale.Load() { |
| + f.lookNow() |
| + } |
| if !f.stale.Load() { |
| if ok, err := f.witnessUnchanged(); err != nil || !ok { |
| f.goStale(time.Now()) |
| @@ -769,14 +789,18 @@ func (f *MeteredFleet) closeNow() error { |
| // every recorder (goStale), and on the recorders of the fleet that |
| // replaces this one; one held between admission and recording for longer |
| // than the margin (a 23:58 admission recorded after midnight) can fall on |
| -// a day nobody holds. A process that dies |
| -// between the change and the rebuild takes that hold with it: a request |
| -// it had started and not finished is left as a started row, which the |
| -// ledger holds; one it had finished (recorded) before the change was |
| +// a day nobody holds. A process that exits |
| +// between the change and the rebuild, for any reason (a crash, or a |
| +// routine restart: the chain of fleets lives in memory, and a new process |
| +// builds its first fleet with no predecessor), takes that hold with it: a |
| +// request it had started and not finished is left as a started row, which |
| +// the ledger holds; one it had finished (recorded) before the change was |
| // noticed is a settled row, held by nothing; and the new owner's recorder |
| -// is told nothing. Closing that needs the registry's writer and the |
| -// replicator to share a lock, or the hold to be written down before the |
| -// request is sent; neither is built. |
| +// is told nothing. What covers it today is the durability ledger's |
| +// box-wide incompleteness, which this fleet does not rely on. Closing it |
| +// here needs the registry's writer and the replicator to share a lock, or |
| +// the hold to be written down before the request is sent; neither is |
| +// built. |
| // |
| // The store is returned unopened, and nothing here attests anything. A box |
| // may say that every replica write was counted only when this is the only |
diff --git a/internal/ops/litestream_metered_test.go b/internal/ops/litestream_metered_test.go| index 80c0b2bea..a7764c01f 100644 |
| --- a/internal/ops/litestream_metered_test.go |
| +++ b/internal/ops/litestream_metered_test.go |
| @@ -821,9 +821,9 @@ func TestMeteredFleetKeepsShadowStateOutOfTenantsReach(t *testing.T) { |
| } |
| // The fleet looks at the files again on a timer. A database that is no |
| -// longer the file it was when its ownership was checked stops the fleet |
| -// and holds every recorder's day. A database that is new, or gone, leaves |
| -// it streaming and says it has to be rebuilt. |
| +// longer the file it was when its ownership was checked, or has gone, |
| +// stops the fleet and holds every recorder's day. A database that is new |
| +// leaves it streaming and says it has to be rebuilt. |
| func TestMeteredFleetLooksAtTheFilesAgain(t *testing.T) { |
| type fixture struct { |
| reg *registry.Registry |
| @@ -1440,6 +1440,11 @@ func TestWitnessReadsAreRetriedBeforeTheyCount(t *testing.T) { |
| if tc.want != nil && !tc.changed && calls != witnessAttempts { |
| t.Fatalf("%d reads before giving up, want %d", calls, witnessAttempts) |
| } |
| + // A witnessed change is final at once: no read after the one |
| + // that saw it. |
| + if tc.changed && calls != len(tc.answers) { |
| + t.Fatalf("%d reads around a witnessed change, want %d (the errors before it and the one that saw it)", calls, len(tc.answers)) |
| + } |
| }) |
| } |
| } |
| @@ -1698,3 +1703,87 @@ func TestAVanishedFileIsDoubtNotACleanLook(t *testing.T) { |
| } |
| } |
| } |
| + |
| +// Close looks at the files one last time: a swap still in place when the |
| +// fleet is closed (a rebuild, a roll) with no look between the swap and |
| +// the close is held by this fleet, from before its last clean look. |
| +// (Review nine, Opus P2-1: closeNow asked the registry alone, and the |
| +// successor's own inventory compares nothing with this fleet's facts.) |
| +func TestCloseLooksAtTheFilesOneLastTime(t *testing.T) { |
| + newFakeBucket(t) |
| + dir := platformDir(t) |
| + reg := openRegistry(t, filepath.Join(dir, "registry.db")) |
| + db := writeFile(t, filepath.Join(dir, "a.example.com.db")) |
| + upsert(t, reg, registry.Route{Host: "a.example.com", Account: "bac", App: "one", DBPath: db}) |
| + recs := &recorders{} |
| + o := fleetOptions(reg, dir) |
| + o.Meter = recs.meter |
| + fleet, err := MeteredFleetStore(o) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + lastClean := time.Now().UTC().Add(-26 * time.Hour) |
| + fleet.lastClean.Store(lastClean.UnixNano()) |
| + fleet.proven.Store(time.Now().UnixNano()) |
| + swapped := writeFile(t, filepath.Join(platformDir(t), "other.db")) |
| + if err := os.Rename(swapped, db); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := fleet.Close(); err != nil { |
| + t.Fatal(err) |
| + } |
| + from, _, stale := fleet.staleWindow() |
| + if !stale || !from.Equal(lastClean.Add(-ownershipMargin)) { |
| + t.Fatalf("after Close over a swapped file: stale %v from %v; want held from the last clean look %v less the margin", stale, from, lastClean) |
| + } |
| + want := lastClean.Format("2006-01-02") |
| + for path, rec := range recs.all() { |
| + if !rec.days()[want] { |
| + t.Fatalf("%s holds %v; want the last clean look's day %s", path, rec.days(), want) |
| + } |
| + } |
| +} |
| + |
| +// Closing a fleet that was never opened still latches its chain, inside |
| +// Close, before the fleet reads as closed: the fleet's own recorders are |
| +// told its predecessor's days. (Review nine, Opus P3-4: the chain tests |
| +// drove inheritHold by hand and pinned nothing in closeNow.) |
| +func TestCloseOfAnUnopenedFleetLatchesItsChain(t *testing.T) { |
| + newFakeBucket(t) |
| + dir := platformDir(t) |
| + reg := openRegistry(t, filepath.Join(dir, "registry.db")) |
| + db := writeFile(t, filepath.Join(dir, "a.example.com.db")) |
| + upsert(t, reg, registry.Route{Host: "a.example.com", Account: "bac", App: "one", DBPath: db}) |
| + o := fleetOptions(reg, dir) |
| + a, err := MeteredFleetStore(o) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + day := time.Date(2026, 10, 2, 12, 0, 0, 0, time.UTC) |
| + a.proven.Store(day.UnixNano()) |
| + a.lastClean.Store(day.UnixNano()) |
| + a.goStale(day) |
| + if err := a.Close(); err != nil { |
| + t.Fatal(err) |
| + } |
| + recs := &recorders{} |
| + o.Meter, o.Replaces = recs.meter, a |
| + b, err := MeteredFleetStore(o) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if prev, latched := b.link(); latched || prev != a { |
| + t.Fatalf("B before Close: latched %v, predecessor %v; want unlatched over A", latched, prev) |
| + } |
| + if err := b.Close(); err != nil { |
| + t.Fatal(err) |
| + } |
| + if prev, latched := b.link(); !latched || prev != nil { |
| + t.Fatalf("B after Close: latched %v, predecessor %v; want latched and the chain let go", latched, prev) |
| + } |
| + for path, rec := range recs.all() { |
| + if !rec.days()["2026-10-02"] { |
| + t.Fatalf("%s holds %v; want A's day, told at B's Close", path, rec.days()) |
| + } |
| + } |
| +} |
diff --git a/internal/ops/litestream_owners.go b/internal/ops/litestream_owners.go| index be42576d0..d797b7eea 100644 |
| --- a/internal/ops/litestream_owners.go |
| +++ b/internal/ops/litestream_owners.go |
| @@ -485,8 +485,10 @@ const strictTreeMaxDepth = 32 |
| // strictTreeDBs walks a unit tenant's state directory for the platform's |
| // own replicator. It finds what treeDBs finds (non-empty *.db files, no |
| // deeper than treeDBWalkDepth, outside dot-directories) and, unlike it, |
| -// passes nothing over in silence, because a replicator that has to say |
| -// every database was counted cannot have places it did not look: |
| +// passes nothing over in silence that could hold a database, because a |
| +// replicator that has to say every database was counted cannot have |
| +// places it did not look (what it does pass over on purpose is listed at |
| +// the end): |
| // |
| // - a database it would not replicate is reported in skipped: one in a |
| // dot-directory, one deeper than treeDBWalkDepth, one under another |
diff --git a/internal/ops/litestream_owners_test.go b/internal/ops/litestream_owners_test.go| index 2681433f2..13c4ea84e 100644 |
| --- a/internal/ops/litestream_owners_test.go |
| +++ b/internal/ops/litestream_owners_test.go |
| @@ -208,9 +208,7 @@ func TestFleetDBOwnersFindTheRegistryWhereItIsOpen(t *testing.T) { |
| t.Run(name, func(t *testing.T) { |
| dir := platformDir(t) |
| path := at(dir) |
| - if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { |
| - t.Fatal(err) |
| - } |
| + mkdirs(t, filepath.Dir(path)) |
| reg := openRegistry(t, path) |
| inv, err := FleetDBOwners(reg, dir) |
| if err != nil { |
| @@ -485,9 +483,7 @@ func TestFleetInventoryNamesWhatItCannotReplicate(t *testing.T) { |
| reg := openRegistry(t, filepath.Join(dir, "registry.db")) |
| missing := filepath.Join(platformDir(t), "gone", "app.db") |
| empty := filepath.Join(platformDir(t), "empty", "app.db") |
| - if err := os.MkdirAll(filepath.Dir(empty), 0o755); err != nil { |
| - t.Fatal(err) |
| - } |
| + mkdirs(t, filepath.Dir(empty)) |
| if err := os.WriteFile(empty, nil, 0o644); err != nil { |
| t.Fatal(err) |
| } |
| @@ -528,10 +524,10 @@ func TestFleetInventoryNamesWhatItCannotReplicate(t *testing.T) { |
| } |
| } |
| -// A file named like a database's sidecar (-wal, -shm, -journal) is a |
| -// sidecar only when the database it would belong to is beside it; on its |
| -// own it is any other file, read for its header, and a database under |
| -// such a name is reported. |
| +// A file named like a database's sidecar (-wal, -shm, -journal) is read |
| +// for its header like any other regular file, whether or not a database |
| +// is beside it (a real write-ahead log begins with its own magic, never |
| +// SQLite's), and a database under such a name is reported. |
| func TestStrictTreeReadsAStandaloneSidecarName(t *testing.T) { |
| root := platformDir(t) |
| header := append([]byte("SQLite format 3\x00"), make([]byte, 100)...) |
| @@ -572,3 +568,56 @@ func TestStrictTreeReadsAStandaloneSidecarName(t *testing.T) { |
| t.Fatalf("a database under a sidecar name is not reported: skipped %v problems %v", skipped, problems) |
| } |
| } |
| + |
| +// An exec route's database outside the globs (a path of the operator's |
| +// choosing) is in the platform replicator's inventory because a route |
| +// named it: owned when the file is there, said when it is not. A |
| +// hibernating route whose path is also a unit route's claim is two claims |
| +// on one path, and is reported. (Review nine: Astra 5 and Opus P3-2.) |
| +func TestExplicitClaimsOutsideTheGlobsAreInventoried(t *testing.T) { |
| + dir := platformDir(t) |
| + reg := openRegistry(t, filepath.Join(dir, "registry.db")) |
| + custom := writeFile(t, filepath.Join(platformDir(t), "srv", "custom", "archive.sqlite")) |
| + missing := filepath.Join(platformDir(t), "srv", "custom", "later.sqlite") |
| + upsert(t, reg, registry.Route{Host: "custom.example.com", Account: "bac", App: "one", DBPath: custom}) |
| + upsert(t, reg, registry.Route{Host: "later.example.com", Account: "bad", App: "two", DBPath: missing}) |
| + inv, err := FleetDBOwners(reg, dir) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if o, ok := ownedByPath(inv)[custom]; !ok || o.Account != "bac" { |
| + t.Fatalf("an exec route's database outside the globs: owned %+v, unowned %v, skipped %v, problems %v", inv.Owned, inv.Unowned, inv.Skipped, inv.Problems) |
| + } |
| + if !contains(inv.Skipped, missing) && !containsPrefix(inv.Problems, missing) { |
| + t.Fatalf("an exec route's database that is not there: skipped %v, problems %v; want it said", inv.Skipped, inv.Problems) |
| + } |
| + // Two claims on one path, one of them hibernating. |
| + unit := writeFile(t, filepath.Join(platformDir(t), "apps", "u", "main.db")) |
| + upsert(t, reg, registry.Route{Host: "u.example.com", Account: "bae", App: "three", DBPath: unit, Backing: registry.BackingUnit}) |
| + upsert(t, reg, registry.Route{Host: "sleepy.example.com", Account: "baf", App: "four", DBPath: unit, Hibernate: true}) |
| + inv, err = FleetDBOwners(reg, dir) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if !containsPrefix(inv.Problems, unit) { |
| + t.Fatalf("a hibernating route over a unit route's path: owned %+v, problems %v; want a problem", inv.Owned, inv.Problems) |
| + } |
| +} |
| + |
| +func contains(list []string, s string) bool { |
| + for _, x := range list { |
| + if x == s { |
| + return true |
| + } |
| + } |
| + return false |
| +} |
| + |
| +func containsPrefix(list []string, prefix string) bool { |
| + for _, x := range list { |
| + if strings.HasPrefix(x, prefix) { |
| + return true |
| + } |
| + } |
| + return false |
| +} |
diff --git a/internal/ops/litestream_owners_unix_test.go b/internal/ops/litestream_owners_unix_test.go| index 7444ae4d6..67ea817b6 100644 |
| --- a/internal/ops/litestream_owners_unix_test.go |
| +++ b/internal/ops/litestream_owners_unix_test.go |
| @@ -79,8 +79,9 @@ func TestUnitFileChainWithATenantOfItsOwn(t *testing.T) { |
| // systemd's DynamicUser shape: /var/lib/carlos-app/<host> is a root-made |
| // link to /var/lib/private/carlos-app/<host>, which the dynamic user owns. |
| -// A unit route naming a file through the link is owned, by its resolved |
| -// path; the same shape with the link made by the tenant is not. |
| +// A unit route naming a file through the link is owned, under the path |
| +// the route spells, with the resolved file's facts; the same shape with |
| +// the link made by the tenant is not. |
| func TestUnitFileThroughAPlatformMadeStateLink(t *testing.T) { |
| const tenant = 44001 |
| top := platformDir(t) |
| @@ -123,9 +124,9 @@ func TestUnitFileThroughAPlatformMadeStateLink(t *testing.T) { |
| if _, why := checkOwnedFile(filepath.Dir(spelled), spelled, tenantFile); !strings.Contains(why, "not the platform's") { |
| t.Fatalf("the tenant's own link in the platform's directory: %q", why) |
| } |
| - // A file named beneath the platform's link at any depth is owned by its |
| - // resolved path too (the link is resolved wherever it sits in the |
| - // spelled path, not only as the file's own directory). |
| + // A file named beneath the platform's link at any depth is owned too, |
| + // its facts the resolved file's (the link is resolved wherever it sits |
| + // in the spelled path, not only as the file's own directory). |
| withOwners(t, map[string]uint32{"carlos-app": 0, "private": 0, "keymail.dev": tenant, "data": tenant, "nested.db": tenant, "link:keymail.dev": 0}) |
| nested := writeFile(t, filepath.Join(private, "keymail.dev", "data", "nested.db")) |
| spelledNested := filepath.Join(link, "data", "nested.db") |
diff --git a/internal/registry/registry.go b/internal/registry/registry.go| index 5bbe8e328..726e74efd 100644 |
| --- a/internal/registry/registry.go |
| +++ b/internal/registry/registry.go |
| @@ -581,7 +581,7 @@ var routeOwnershipColumns = []string{"host", "account", "app", "db_path", "backi |
| // that those commits do not move. What moves it is every insert and |
| // delete of a route, whether or not that route has a database, and every |
| // change to the six columns ownership is read from; a reader stops on |
| -// some changes that moved no owner, and on none that did. |
| +// some changes that moved no owner, and misses none that did. |
| // |
| // The counter is moved by triggers, so every writer moves it whatever |
| // build it is and whichever statement it uses, and a change that is made |