amadan

carlos / platform Public

The replicator's seventeenth build: the sixteenth review's findings

A link's relative target is looked at by itself against a descriptor
for the link's directory (fstatat), so the spelling handed to the kernel
is the target alone and cannot outgrow its path limit where the link
resolves (Astra); the aliased-root fixture; the rendering carries the
platform bit and the test tells every field and list apart; the
registry witness's wording says what the triggers do; the namespace
qualification. Dispositions "Review sixteen" and the reports (Astra's
fifteenth included).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
claude bffc76b50ab7273917bfb9f973add19d1fe33a91 parent f306222
12 files changed, +646 −30
  • docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review15.md +194 −0
  • docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review16.md +184 −0
  • docs/superpowers/specs/DISPOSITIONS-metered-replicator.md +20 −5
  • docs/superpowers/specs/OPUS-REVIEW-metered-replicator-review16.md +97 −0
  • internal/ops/linktarget_other.go +26 −0
  • internal/ops/linktarget_unix.go +46 −0
  • internal/ops/litestream_metered.go +1 −1
  • internal/ops/litestream_metered_test.go +20 −1
  • internal/ops/litestream_owners.go +15 −17
  • internal/ops/litestream_owners_test.go +36 −0
  • internal/registry/observe.go +4 −3
  • internal/registry/observe_ownership_test.go +3 −3

CI failed — run details

diff --git a/docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review15.md b/docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review15.md
new file mode 100644
index 000000000..ae4f3f16a
--- /dev/null
+++ b/docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review15.md
@@ -0,0 +1,194 @@
+| Review-fourteen finding | Closed at `03b19014`? | Would its test pass without the fix? |
+|---|---|---|
+| **Opus P3-1 — target read separately from classification** | **Partly.** Removing the second `Readlink` closes the reported re-point-and-restore case and removes its silently discarded errors. Relative-target classification now has a deterministic resolution bug; finding 5 below. | **Yes.** No controlled classification test distinguishes the old race or the new resolution error. |
+| **Opus P3-2 — newline collision in `render()`** | **Closed for the reported filename collision.** Paths and string-list entries are quoted. | **No.** `TestRenderQuotesEveryEntry` fails with raw skipped entries. It does not independently test every inventory category. |
+| **Opus P3-3 — Accept fixture insensitive to cleaning** | **Closed.** `sub/live.sqlite` versus `sub/./live.sqlite` distinguishes as-written comparison. | **No.** Cleaning both keys admits the changed pair. The fixture now weakens a different regression check; finding 11. |
+| **Opus claims** | **Mostly corrected.** The display-only description, newline-fixture excuse, EINVAL omission and obsolete encoding description are corrected or qualified. The classification claim remains false for relative targets; some fixture descriptions still name `x/..`. | Prose changes have no behavioral test. |
+| **Astra 1 — Litestream pathname opens** | **Open.** | **Yes.** Actual-open substitution remains untested. |
+| **Astra 2 — replaceable ancestry** | **Open.** | **Yes.** Protected endpoints do not establish protected ancestry. |
+| **Astra 3 — delayed admission escapes holds** | **Open.** | **Yes.** No test delays an admitted request beyond the margin across midnight. |
+| **Astra 4 — process exit loses uncertainty** | **Open.** | **Yes.** There is no durable handover/recovery test or implementation. |
+| **Astra 5 — inventory newline collision** | **Closed for the reported case.** Universal injectivity needs qualification below. | **No** for restoring raw skipped-entry rendering. |
+| **Astra 6 — device-open effects** | **Open.** | **Yes.** Final-component tests do not exercise ancestor substitution. |
+| **Astra 7 — second `Readlink` omission/classification race** | **Partly**, for the same reason as Opus P3-1. | **Yes.** The relevant interleaving and relative-resolution cases are absent. |
+| **Astra 8 — hibernating aliases and skipped-file claims** | **Open.** | **Yes.** Competing claims in those cases remain untested. |
+| **Astra 9 — required coverage disappears** | **Open.** | **Yes.** Missing configured console databases and external-inventory reconciliation remain uncovered. |
+| **Astra 10 — tenant activity refuses the whole fleet** | **Open**, explicitly acknowledged. | Tests expect broad refusal and holds; they do not establish isolation between tenants. |
+| **Astra 11 — route identity not bound to filesystem authority** | **Open.** | **Yes.** The coherent-foreign-tree fixture explicitly expects acceptance. |
+| **Astra 12 — weak tests** | **Partly.** Cleaning-sensitive coverage and newline escaping improve; the remaining gaps persist, and the impostor fixture regresses. | **Yes** for the surviving counterfactuals in finding 11. |
+| **Astra claims** | The raw-path/display error, second-read error handling, newline-fixture explanation and unconditional final-witness descriptions are corrected. Relative-target classification remains overstated. | Author-reported test and mutation runs were not independently executed. |
+
+All locations below are at **`03b19014`**. This was a source-only review against `72b4f59a`, including the incremental changes from `ec55b639`. No files were modified, tests run, or network commands issued. Test counterfactuals are based on inspection.
+
+1. **P1 — Litestream’s actual opens remain outside the ownership checks.**
+
+ **Locations:** `internal/lease/meteredstore.go:127`; `internal/ops/litestream_owners.go:382`; `third_party/litestream/db.go:1004`, `:1014`, `:1032`, `:1866`.
+
+ A tenant passes inventory, substitutes another tenant’s database during initialization, and restores the original pathname before the next look. Litestream can retain the substituted handles while every later inventory—including Close’s final look—sees the original inode. Initialization also writes through SQLite. Subsequent WAL opens remain independently exposed.
+
+ **Smallest acceptable fix:** constrain the actual SQLite, database and sidecar opens through pinned directory authority, including subsequent opens. An identity check after an unrestricted open cannot undo its effects.
+
+2. **P1 — Protected endpoints still permit tenant-replaceable ancestry.**
+
+ **Locations:** `internal/ops/litestream_owners.go:512`, `:583`, `:815`, `:843`; `internal/ops/litestream_metered.go:970`.
+
+ T owns `/srv/T`; root owns `/srv/T/bridge`; A owns `bridge/A` and its database. A platform-made link to `bridge/A` passes the immediate-parent checks, although T can replace `bridge` through `/srv/T`. The same construction works above `dataDir`, shadow state, or the directory holding an approved platform link.
+
+ A platform link whose target’s **immediate parent** is tenant-owned is refused. The remaining gap is higher ancestry.
+
+ **Smallest acceptable fix:** establish and pin trusted ancestry for claim roots, approved platform links and shadow state. Approval H alone does not establish this filesystem invariant.
+
+3. **P1 — A delayed admitted attempt can fall outside every inherited hold.**
+
+ **Locations:** `internal/ops/litestream_metered.go:240`, `:325`, `:772`; `internal/lease/requestgate.go:58`.
+
+ A concrete UTC sequence:
+
+ - A admits an attempt on October 2 at **23:58:00**, then pauses before recording.
+ - Ownership changes at **23:58:10**. A detects it and holds through **23:59:10**.
+ - B is built for the new owner and closes unopened while A remains live.
+ - A’s admitted attempt resumes, records and completes at **00:02 on October 3**.
+ - A closes. Because it is already stale, Close skips the final checks and does not extend its window.
+ - C replaces B. B and C inherit A’s window correctly, but **October 3 is absent from every ownership hold**.
+
+ `goStaleFrom` correctly chooses the earlier lower bound. Unopened-fleet inheritance correctly preserves the supplied window. Neither repairs its missing upper bound.
+
+ **Smallest acceptable fix:** track admitted attempts through recording/completion and include their doubtful dates before finalizing handover. A larger fixed margin is insufficient.
+
+4. **P1 — Uncertainty disappears across process exit.**
+
+ **Locations:** `internal/ops/litestream_metered.go:346`, `:379`, `:817`.
+
+ An attempt passes admission, ownership changes, and the attempt completes before invalidation. The process exits. Its successor reads the new owner without a predecessor or durable uncertainty covering that completed attempt.
+
+ Routine restarts also discard inheritance awaiting delivery to replacement recorders. An unfinished ledger row covers a different case.
+
+ **Smallest acceptable fix:** persist ownership/admission uncertainty before affected requests escape, then recover it before establishing successor coverage.
+
+5. **P2 — New: relative-target cleaning can silently hide a directory link without any race.**
+
+ **Location:** `internal/ops/litestream_owners.go:692`.
+
+ Consider a unit tree `/srv/u` containing:
+
+ ```text
+ current -> ../../var/tmp/u/hop/../payload
+ ```
+
+ Outside that tree, the tenant has:
+
+ ```text
+ /var/tmp/u/hop -> /var/tmp/u/else/child
+ /var/tmp/u/else/payload/ # directory containing a database
+ ```
+
+ `/var/tmp/u/payload` does not exist.
+
+ The kernel follows `hop` before applying `..`, so `current` reaches `/var/tmp/u/else/payload`. But `filepath.Join(dir, dest)` cleans the spelling to `/var/tmp/u/payload`. `Stat` returns ENOENT; `current` appears in **none** of the inventory lists. The outside `hop` is not independently walked.
+
+ Making the cleaned destination a regular file instead produces an accept-able skipped entry for a link that actually reaches a directory. Reversing the shapes can wrongly refuse a stable file link. A spelled tree root that is itself a systemd state link creates another opportunity for leading `..` to resolve differently.
+
+ Absolute targets avoid this particular cleaning operation. Removing the mutable second `Readlink` does close the old reported interleaving, but does not justify the new classification claim.
+
+ **Smallest acceptable fix:** preserve the relative spelling when constructing the pathname passed to `Stat`; do not use a cleaning join. Add a deterministic fixture with different lexical and kernel destinations, including an aliased tree root.
+
+6. **P2 — Header inspection can cause device-open effects before rejecting the descriptor.**
+
+ **Location:** `internal/ops/sqliteopen_unix.go:26`.
+
+ The walker observes regular file `T/d/watchdog`. T replaces `d` with a link to `/dev`; the privileged open reaches `/dev/watchdog` on a suitably equipped box. The later regular-file and identity checks cannot undo open-side effects.
+
+ `O_NOFOLLOW` protects the final component. `O_NONBLOCK` addresses FIFO waiting, and `O_NOCTTY` prevents controlling-terminal acquisition. Neither confines ancestor resolution.
+
+ **Smallest acceptable fix:** constrain traversal through pinned directory descriptors before opening the target. Preserve refusal on unsupported platforms.
+
+7. **P2 — Hibernating aliases and skipped-file claims still hide competing ownership.**
+
+ **Locations:** `internal/ops/litestream.go:381`, `:479`, `:540`, `:709`; `internal/ops/litestream_owners.go:166`, `:241`.
+
+ A live unit claims `/var/lib/carlos-app/u`, reached through its platform state link. Another account’s hibernating exec route names the same file as `/var/lib/private/carlos-app/u/main.db`. The hibernating path contributes no candidate, and textual matching misses the shared inode.
+
+ Claims on tree-only skipped files such as `u/.cache/app.db` or `u/main.sqlite` also do not undergo listed-file claimant reconciliation. Two hibernating claims can bypass it altogether.
+
+ **Smallest acceptable fix:** reconcile claims by file identity against listed and skipped files, retaining claimant information for skipped entries.
+
+8. **P2 — Required coverage can still disappear without an entry.**
+
+ **Locations:** `internal/ops/litestream.go:507`, `:525`; `internal/ops/litestream_owners.go:639`, `:662`, `:712`.
+
+ Set `CONSOLE_DB` to a missing file outside the globs and route claims. It disappears like the optional console path on a box without a console. The implementation does not distinguish “configured but missing” from “not required.”
+
+ Empty tree-only files and entries disappearing before their first inspection remain deliberate omissions. A database known only to the external replicator, outside these trees and explicit live claims, reaches no list.
+
+ **Smallest acceptable fix:** distinguish required platform databases from optional ones, and reconcile against the external replicator’s inventory before cutover.
+
+9. **P2 — Ordinary tenant activity can refuse replacement replication for every account.**
+
+ **Locations:** `internal/ops/litestream_owners.go:699`, `:704`; `internal/ops/litestream_metered.go:660`, `:863`, `:889`.
+
+ Installing packages or a virtual environment can introduce many ordinary links. Regular-file links require acceptance; directory links are unconditional problems. The current fleet reports drift, but its replacement refuses. An unrelated ownership change can then leave every account waiting for that tenant’s inventory to be resolved.
+
+ A legitimate concurrent rename caught by either inode check also removes an owned file’s facts and holds every recorder.
+
+ **Smallest acceptable fix:** implement tenant-scoped failures and holds, or explicitly adopt the box-wide policy with an operational recovery procedure before wiring.
+
+10. **P3 — Route identity remains unbound to independently established filesystem authority.**
+
+ **Locations:** `internal/ops/litestream_owners.go:176`, `:435`, `:495`, `:559`; `internal/ops/litestream_owners_unix_test.go:381`.
+
+ Route A naming B’s coherent directory and file attributes B’s file to A. The fixture explicitly expects this acceptance.
+
+ Root-run units and tenants sharing the platform UID can likewise claim otherwise-unclaimed qualifying platform files. They cannot claim files “from anywhere”: directory permissions, links, filesystem boundaries, regular-file status, link count and sidecars still constrain acceptance.
+
+ **Smallest acceptable fix:** bind route claims to independently established roots and identities, with explicit treatment of trusted platform workloads.
+
+11. **P3 — The revised Accept fixture loses its impersonation check; other test gaps remain.**
+
+ **Locations:** `internal/ops/litestream_metered_test.go:1876`, `:1906`, `:1919`; also `:689`, `:963`, `:1062`, `:1380`, `:1496`.
+
+ The accepted target is now `sub/live.sqlite`, but the impostor filename still spells `current -> "live.sqlite"`. Restoring the old unprefixed encoding **and its corresponding link-entry recognizer** leaves these unequal. The impostor is refused anyway, so that case no longer distinguishes the encoding fix.
+
+ The new render test exercises only `Skipped`. Removing quoting from the owned-path branch survives it; selectively leaving `Unowned` or `Problems` raw also survives. Existing ordinary-link expectations mostly reuse `linkEntry` itself.
+
+ Other surviving counterfactuals remain:
+
+ - Removing `store.Close` can satisfy cleared-pointer and refusing-gate assertions; retained handles and worker exit are not asserted.
+ - Removing the watch’s `lookAgain` call leaves direct-call file tests intact.
+ - Swapping recorders in the fleet’s wiring can preserve positive counts for both databases. The lower-level lease test’s asymmetric counts do establish its own wiring.
+ - Removing admission’s pre-witness under-lock stale check is not independently distinguished by the post-witness fixture.
+ - Removing the latch’s under-lock recheck survives sequential tests; racing successors are outside the caller contract.
+ - Removing `prepareShadowState`’s data-directory check survives the fixture refused earlier by inventory.
+ - Removing the cross-check inode comparison or kept-facts duplicate check has no controlled distinguishing case.
+
+ The 300 ms negative Close check can pass before the second goroutine runs. The clean-start test reconstructs time from Unix nanoseconds, so its comparisons depend on wall-clock stability.
+
+ **Smallest acceptable fix:** separate the cleaning and impersonation fixtures; make the impostor spell the actual accepted key under the old encoding. Cover all render categories with independent expected encodings, and add controlled interleavings and direct shutdown/watch/attribution assertions.
+
+The remaining requested checks hold with these limits:
+
+| Area | Result at `03b19014` |
+|---|---|
+| **Tenant-file chain, starting directory and top check** | `litestream_owners.go:464`, `:495`, `:500`, `:512` reject foreign starting directories, tenant-made links and foreign stopping parents on stable paths. Every accepting `tenantFile` path reaches the top check. A final-component `DBPath` link is refused at `:435`. Approved platform links are resolved before the climb; higher replaceable ancestry remains finding 2. |
+| **Root/platform ownership equivalence** | At `litestream_owners.go:559`, only root and the process UID are equivalent. A distinct tenant UID cannot use this exception to claim a root-owned file directly inside its tenant-owned directory. A tenant-owned file directly inside a protected platform directory is also refused by the final comparison. Root-owned or platform-owned files under tenant-writable ancestry remain subject to finding 2. |
+| **All claimant checks and platform layouts** | `claimChecks` at `litestream_owners.go:278` retains every distinct root/kind: two trees, tree-plus-file, and unit-plus-exec claims. Ordinary DynamicUser links, protected operator-chosen paths and isolated-instance directories can pass. No ranking discards a restriction. Stable-layout acceptance is independent of check order. |
+| **One-inode and duplicate rules** | The first successful check’s facts are retained at `litestream_owners.go:193`; later checks must agree. The duplicate pass at `:217` checks those retained facts. Stable distinct files are not confused. Concurrent renames can cause conservative refusal and broad holds; neither check establishes an atomic snapshot. |
+| **Admission and retries** | `litestream_metered.go:214`, `:226`, `:240` cover stale state before locking and during the witness read. A finding after the last load remains an admitted-request gap. `witnessUnchanged` makes at most three reads, accepts recovered errors, stops on a witnessed change and treats exhausted errors as uncertainty. Five-second contexts do not bound mutex waiting or filesystem `Stat`. |
+| **Registry generation** | `registry.go:569`, `:602`, `:612` cover the six consumed columns, insert/delete and rowid immutability. Older-build raw statements still fire the triggers. Same-value updates, unrelated columns and Litestream bookkeeping do not move it. “Nothing else” is too strong: database-less insertions/deletions and semantically irrelevant changes to those columns also move it. |
+| **Replacement chain and `closeOnce`** | `chainClosed` checks a latched predecessor’s own closure first. Window publication, chain detachment and latch publication share `staleMu`; published chain reads take that lock. Within the single-chain contract, replacements cannot open beside open/draining predecessors, and unopened successors cannot latch through them. Intermediate recorders receive inheritance once. Reading a closed stale predecessor’s window is intentional. |
+| **Close’s final look** | Order is store stopped → watch stopped → file look → witness → inheritance → closed, at `litestream_metered.go:746`, `:750`, `:773`, `:776`, `:782`, `:783`. A persistent changed/missing owned file on a current fleet holds from its last clean look. Already-stale fleets skip the checks. Harmless changes after replication stops can over-hold everyone; filesystem operations can block Close. Restored swaps and delayed admitted requests remain findings 1 and 3. |
+| **Lineage** | `ownedReplicaKey` at `litestream_metered.go:948` combines account and cleaned path. Different valid accounts do not share lineage or shadow keys, including after renaming. A→B→A at one path resumes A’s prior lineage; an app change within one account does not create a new key. External-lineage separation remains unenforced. |
+| **Strict inventory and explicit claims** | Database extensions are recognized case-insensitively, though uppercase `.DB` is skipped rather than streamed by tree discovery. Every nonempty regular file not recognized by name gets a header read, including sidecar-shaped names; `sidecarOf` is gone. Missing/relative unit claims, units without `DBPath`, empty explicit claims and discovered files failing their final look are reported. Explicit exec claims receive ownership checks and do not alter external config generation. Findings 5, 7 and 8 qualify completeness. |
+| **Platform-link traversal** | `walkWithoutLinks` at `litestream_owners.go:757` permits systemd’s leading `../private/...` shape and rejects ascents after named components. It does not clean away an intervening link. This separate walker remains correct on stable checked paths; the new cleaning regression is in `strictTreeDBs`. |
+| **Link entries and Accept** | `linkEntry` at `litestream_owners.go:888` is distinct from every absolute plain path and other path/target pair. Relative targets retain literal `Readlink` spelling. Unchanged accepted pairs remain accepted while their classification remains unchanged. Newlines and separators cannot forge another key. Raw plain names in refusal diagnostics can mislead display, but do not grant acceptance. |
+| **Fixtures and root CI** | Relevant modes are explicitly chmodded, and `platformDir` resolves a linked temporary root. Umask `077` does not narrow those fixture modes. Basename-based fake ownership and fixed fake UIDs remain assumptions. Root skips the mixed-root/platform test, unreadable-directory cases and sealed-target case. `.amadan/ci.d/10-gate:52`, `Makefile:18` and `.amadan/README.md:73` do not establish CI’s UID. |
+| **Open-item scope and reports** | The pathname-open gap includes retained handles, reopens and sidecars—not only first open. The exit gap includes routine restarts. A single same-filesystem bind alias remains undetected; two inventoried aliases are rejected by identity. External-inventory comparison is absent. Both reviewers’ review-eight, review-nine and review-fourteen reports are present. |
+
+**Claims the code contradicts**
+
+- **Captured-target classification:** `litestream_owners.go:685` and dispositions `:268`, `:285`, `:299` say classification uses the captured spelling/kernel resolution. The relative branch cleans that spelling at `:692`; finding 5 is a stable counterexample.
+- **Universal `render()` injectivity:** the reported filename collision is fixed, but `litestream_metered.go:679` leaves account, app and host unquoted and omits `OwnedDB.Platform`. For example, identical paths with `(Account="a", App="b c", Host="d")` and `(Account="a", App="b", Host="c d")` render identically. This is not a demonstrated tenant-filename drift bypass: valid route identities are narrower, routes are frozen for each fleet, and registry changes invalidate admission. Nevertheless, “every inventory” is too broad. Quote each field and include every intended comparison field, or document the restricted domain. Production uses the rendering for stored-baseline equality at `:939` and `:666`, not parsing or Accept.
+- **Fixture descriptions:** dispositions `:268`, `:283`, and `litestream_metered_test.go:1860`, `:1897` still describe the Accept case as `x/..` or `..`; the current case is `sub/./live.sqlite`. The impostor case no longer spells that accepted target.
+- **Ownership-only witness wording:** `internal/registry/observe.go:27` and `observe_ownership_test.go:11` remain narrower than the trigger behavior. `registry.go:581` correctly acknowledges conservative invalidations that move no database owner.
+- **Recorded test success:** dispositions `:305` reports the author’s runs. This source-only review neither confirms those runs nor establishes the CI user.
+
+**Verdict: reject.**
\ No newline at end of file
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review16.md b/docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review16.md
new file mode 100644
index 000000000..b4dcc58f8
--- /dev/null
+++ b/docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review16.md
@@ -0,0 +1,184 @@
+| Review-fifteen finding | Closed at `f306222c`? | Would its test pass without the fix? |
+|---|---|---|
+| **Opus P3-1 — relative-target cleaning** | **Closed for the reported lexical-resolution bug.** Concatenation preserves `..`, including through a linked tree root. Full equivalence still has a pathname-length limit; finding 5 below. | **No.** The new outside-link fixture fails with `filepath.Join`. It does not cover the length limit. |
+| **Opus P3-2 — impostor fixture** | **Closed.** The accepted flat pair and impostor now spell the same key under the old encoding. | **No.** Restoring the unprefixed encoding and its recognizer admits the impostor. |
+| **Opus P3-3 — owned-path test and raw owner triple** | **Closed as reported.** The owned path is tested; all four string fields are quoted. | **No** for removing owned-path quoting; **yes** for removing only owner-field quoting. |
+| **Opus claims** | **Mostly corrected.** The classification and impersonation claims now hold for their reported cases. Some fixture descriptions still say `x/..`; universal kernel equivalence needs qualification. | Prose corrections have no behavioral test. |
+| **Astra 1 — Litestream pathname opens** | **Open.** | No distinguishing actual-open substitution test; existing tests permit the defect. |
+| **Astra 2 — replaceable ancestry** | **Open.** | No distinguishing protected-ancestry test. |
+| **Astra 3 — delayed admission escapes holds** | **Open.** | No test delays an admitted attempt beyond the margin across midnight. |
+| **Astra 4 — process exit loses uncertainty** | **Open.** | No durable handover implementation or recovery test. |
+| **Astra 5 — relative-target cleaning** | **Closed for the reported case**, with finding 5 below qualifying equivalence. | **No.** The new outside-link fixture distinguishes concatenation from cleaning. |
+| **Astra 6 — header-open device effects** | **Open.** | **Yes.** Final-component tests do not exercise ancestor substitution. |
+| **Astra 7 — hibernating aliases and skipped-file claims** | **Open.** | No distinguishing alias/skipped-claim reconciliation test. |
+| **Astra 8 — required coverage disappears** | **Open.** | Missing configured console databases and external-inventory reconciliation remain uncovered. |
+| **Astra 9 — whole-fleet refusal and holds** | **Open**, explicitly acknowledged. | Existing tests accept or expect the broad behavior; they do not establish tenant isolation. |
+| **Astra 10 — route identity versus filesystem authority** | **Open.** | The coherent-foreign-tree fixture explicitly expects acceptance. |
+| **Astra 11 — weak tests** | **Partly closed.** Impersonation and owned-path coverage are repaired. Other gaps remain, but the report overstates the fleet recorder-wiring gap: the direct-client test already distinguishes tenant and platform recorders. | **No** for the repaired fixtures. **Yes** for the remaining specific counterfactuals described below. |
+| **Astra claims** | Owner-field quoting and relative lexical resolution are corrected. `Platform` remains absent from rendering; stale fixture descriptions and ownership-only witness wording remain. | Author-reported test and mutation runs were not independently executed. |
+
+All source locations below refer to **`f306222c`**, not the working tree. I read the dispositions first, reviewed both requested diffs, and read Astra’s fifteenth report from the run directory. No files were changed, tests run, or network commands issued. Test counterfactuals are based on inspection.
+
+1. **P1 — Litestream’s actual opens remain outside the ownership checks.**
+
+ **Locations:** `internal/lease/meteredstore.go:127`; `internal/ops/litestream_owners.go:382`; `third_party/litestream/db.go:1004`, `:1014`, `:1032`, `:1866`.
+
+ A tenant passes inventory, substitutes another tenant’s database during initialization, and restores the original pathname before the next look. Litestream can retain the substituted handles while every subsequent inventory, including Close’s final look, sees the original inode. Initialization also performs SQLite writes. Later WAL opens independently follow mutable paths.
+
+ **Smallest acceptable fix:** constrain the actual SQLite, database and sidecar opens through pinned directory authority, including subsequent opens. Checking identity after an unrestricted open cannot undo its effects.
+
+2. **P1 — Protected endpoints still allow tenant-replaceable ancestors.**
+
+ **Locations:** `internal/ops/litestream_owners.go:512`, `:583`, `:820`, `:848`; `internal/ops/litestream_metered.go:970`.
+
+ T owns `/srv/T`; root owns `/srv/T/bridge`; A owns `bridge/A` and its database. A platform-made link to `bridge/A` passes the immediate-parent checks, although T can rename and replace `bridge` through `/srv/T`. The same construction works above `dataDir`, shadow state, or the directory holding an approved platform link.
+
+ A platform link whose target’s **immediate parent** is tenant-owned is refused. The surviving gap is higher ancestry.
+
+ **Smallest acceptable fix:** establish and pin trusted ancestry for claim roots, approved platform links and shadow state. Protected endpoint modes alone do not establish this property.
+
+3. **P1 — A delayed admitted attempt can fall outside every inherited ownership hold.**
+
+ **Locations:** `internal/ops/litestream_metered.go:240`, `:325`, `:772`; `internal/lease/requestgate.go:58`.
+
+ A concrete UTC sequence:
+
+ - A admits an attempt on October 2 at **23:58:00**, then pauses before recording.
+ - Ownership changes at **23:58:10**. A detects it and holds through **23:59:10**.
+ - B is built for the new owner and closes unopened while A remains live.
+ - A’s admitted attempt resumes, records and completes at **00:02 on October 3**.
+ - A closes. Because it is already stale, Close skips the final checks and does not extend its window.
+ - C replaces B. B and C correctly inherit A’s window, but **October 3 is absent from every ownership hold**.
+
+ `goStaleFrom` correctly chooses the earlier lower bound. Unopened-fleet inheritance correctly preserves the supplied window. Neither repairs its missing upper bound.
+
+ **Smallest acceptable fix:** track admitted attempts through recording/completion and include their doubtful dates before finalizing handover. Increasing a fixed margin is insufficient.
+
+4. **P1 — Ownership uncertainty disappears across process exit.**
+
+ **Locations:** `internal/ops/litestream_metered.go:346`, `:379`, `:817`.
+
+ An attempt passes admission, ownership changes, and the attempt completes before invalidation. The process exits. Its successor reads the new owner without a predecessor or durable uncertainty covering that completed attempt.
+
+ Routine restarts also discard inheritance awaiting delivery to replacement recorders. An unfinished ledger row covers a different case.
+
+ **Smallest acceptable fix:** persist ownership/admission uncertainty before affected requests escape, and recover it before establishing successor coverage.
+
+5. **P2 — New: concatenation can wrongly reject a stable, valid file link.**
+
+ **Locations:** `internal/ops/litestream_owners.go:697`, `:699`, `:706`; refusal at `internal/ops/litestream_metered.go:863`.
+
+ On Linux, take a unit-tree root whose absolute spelling is about 2,400 bytes, composed of individually legal directory names. Inside it:
+
+ ```text
+ current -> "./" repeated 1,000 times, followed by "live.sqlite"
+ live.sqlite
+ ```
+
+ The pathname naming `current` and the stored relative target are each below the pathname limit. Following `current` resolves successfully: the kernel processes the relative target from the containing directory.
+
+ The new concatenation instead supplies a pathname exceeding 4,096 bytes to `Stat`. It fails with `ENAMETOOLONG`; the link becomes an unconditional problem, even if its exact pair was accepted. The whole replacement fleet refuses. The new outside-link fixture does not cover this.
+
+ **Smallest acceptable fix:** classify relative targets with `fstatat` against a descriptor for the walked directory, passing the captured target unchanged. Keep absolute targets absolute. Add a fixture where the original link resolves but the concatenated syscall argument exceeds the limit.
+
+6. **P2 — Header inspection can cause device-open effects before rejecting the descriptor.**
+
+ **Location:** `internal/ops/sqliteopen_unix.go:26`.
+
+ The walker observes regular file `T/d/watchdog`. T replaces `d` with a link to `/dev`; the privileged open reaches `/dev/watchdog` on a suitably equipped box. The later regular-file and inode checks cannot undo open-side effects.
+
+ `O_NOFOLLOW` protects the final component. `O_NONBLOCK` addresses FIFO waiting, and `O_NOCTTY` prevents controlling-terminal acquisition. Neither confines ancestor resolution.
+
+ **Smallest acceptable fix:** constrain traversal through pinned directory descriptors before opening the target. Preserve refusal on unsupported platforms.
+
+7. **P2 — Hibernating aliases and skipped-file claims still hide competing ownership.**
+
+ **Locations:** `internal/ops/litestream.go:381`, `:479`, `:540`, `:709`; `internal/ops/litestream_owners.go:166`, `:241`.
+
+ A live unit claims `/var/lib/carlos-app/u`, reached through its platform state link. Another account’s hibernating exec route names the same database as `/var/lib/private/carlos-app/u/main.db`. The hibernating path supplies no candidate, and textual matching misses the shared inode.
+
+ Claims on tree-only skipped files, such as `u/.cache/app.db` or `u/main.sqlite`, also bypass listed-file claimant reconciliation. Two hibernating claims can bypass it altogether.
+
+ **Smallest acceptable fix:** reconcile claims by file identity against listed and skipped files, retaining claimant information for skipped entries.
+
+8. **P2 — Required coverage can disappear without an inventory entry.**
+
+ **Locations:** `internal/ops/litestream.go:507`, `:525`; `internal/ops/litestream_owners.go:639`, `:662`, `:717`.
+
+ Set `CONSOLE_DB` to a missing file outside the globs and route claims. It disappears like the optional console path on a box without a console. “Configured but missing” is not distinguished from “not required.”
+
+ Empty tree-only files and entries disappearing before their first inspection remain deliberate omissions. A database known only to the external replicator, outside these trees and explicit live claims, reaches no list.
+
+ **Smallest acceptable fix:** distinguish required platform databases from optional ones, and reconcile against the external replicator’s inventory before cutover.
+
+9. **P2 — Ordinary tenant activity can refuse replacement replication for every account.**
+
+ **Locations:** `internal/ops/litestream_owners.go:704`, `:709`; `internal/ops/litestream_metered.go:660`, `:863`, `:889`.
+
+ Installing packages or a virtual environment can introduce ordinary file and directory links. File links require acceptance; directory links are unconditional problems. The current fleet reports drift, but its replacement refuses. An unrelated ownership change can then leave every account waiting for that tenant’s inventory to be resolved.
+
+ A legitimate concurrent rename caught by either inode comparison can also remove an owned file’s facts and hold every recorder.
+
+ **Smallest acceptable fix:** implement tenant-scoped failures and holds, or explicitly adopt the whole-box policy with an operational recovery procedure before wiring.
+
+10. **P3 — Route identity remains unbound to independently established filesystem authority.**
+
+ **Locations:** `internal/ops/litestream_owners.go:176`, `:435`, `:495`, `:559`; `internal/ops/litestream_owners_unix_test.go:381`.
+
+ Route A naming B’s coherent directory and file attributes B’s file to A. The fixture explicitly expects this acceptance.
+
+ Root-run units and tenants sharing the platform UID can likewise claim otherwise-unclaimed qualifying platform files. They cannot claim files “from anywhere”: directory permissions, links, filesystem boundaries, regular-file status, link count and sidecars still constrain acceptance.
+
+ **Smallest acceptable fix:** bind route claims to independently established roots and identities, with explicit treatment of trusted platform workloads.
+
+11. **P3 — Several test claims remain stronger than their assertions.**
+
+ **Locations:** `internal/ops/litestream_metered_test.go:689`, `:963`, `:1062`, `:1380`, `:1496`, `:1933`; `internal/ops/litestream_owners.go:194`, `:217`.
+
+ Specific surviving counterfactuals:
+
+ - The Close tests’ cleared-store and refusing-gate assertions do not establish that retained database handles and workers stopped. Removing `store.Close` can satisfy those particular assertions.
+ - Removing the watch’s `lookAgain` call leaves the direct-call file tests intact.
+ - The opened-store test checks positive counts for both databases, not attribution of each request. **However, swapping the fleet’s recorders is caught by the direct-client assertions at `litestream_metered_test.go:334` and `:337`.** Review fifteen overstated this gap.
+ - Removing admission’s pre-witness under-lock stale check is not independently distinguished by the post-witness fixture.
+ - Removing the latch’s under-lock recheck survives sequential tests; racing successors are outside the caller contract.
+ - Removing `prepareShadowState`’s data-directory check survives the fixture refused earlier by inventory.
+ - Neither the cross-check inode comparison nor the kept-facts duplicate comparison has a controlled distinguishing interleaving.
+ - The render test now distinguishes raw owned paths, but does not independently pin `Unowned`, `Problems`, the owner triple or `Platform`.
+
+ The 300 ms negative Close check can pass before the second goroutine runs. The clean-start test reconstructs time from Unix nanoseconds and therefore depends on wall-clock stability.
+
+ **Smallest acceptable fix:** add controlled shutdown/watch/interleaving assertions and independent render expectations for the remaining fields. Preserve the existing asymmetric direct-client attribution test.
+
+The remaining requested checks hold with these limits:
+
+| Area | Result at `f306222c` |
+|---|---|
+| **Tenant-file chain and starting directory** | `litestream_owners.go:464`, `:495`, `:500`, `:512` reject foreign starting directories, tenant-made links and foreign stopping parents on stable paths. Every accepting `tenantFile` path reaches the top check. A final-component `DBPath` link is refused at `:435`. Approved platform links are resolved before the climb; replaceable higher ancestry remains finding 2. |
+| **Root/platform equivalence** | At `litestream_owners.go:559`, only root and the process UID are equivalent. A distinct tenant UID cannot use this exception to claim a root-owned file directly inside its tenant-owned directory. The reverse mismatch is refused too. Root-run units now stop climbing at platform ownership and can accept qualifying platform files; route authority remains finding 10. |
+| **Every claimant’s checks** | `claimChecks` at `litestream_owners.go:278` retains each distinct root/kind: two trees, tree-plus-file, and unit-plus-exec claims. Ordinary DynamicUser links, protected operator-chosen paths and isolated-instance directories can pass. Stable-layout acceptance does not depend on check order. |
+| **One-inode and duplicate rules** | The first successful check’s facts are retained at `litestream_owners.go:193`; later checks must agree. The duplicate pass at `:217` compares those kept facts. Stable distinct files are not confused. Concurrent renames can conservatively refuse a file and hold every recorder. These checks do not create an atomic snapshot. |
+| **Admission and retries** | `litestream_metered.go:214`, `:226`, `:240` cover stale state before locking and during the witness read. Staleness after the last load remains the admitted-request gap. `witnessUnchanged` makes at most three reads, accepts recovered errors, stops on a witnessed change and treats exhausted errors as uncertainty. Its contexts do not bound mutex waiting or filesystem `Stat`. |
+| **Registry generation** | `registry.go:569`, `:602`, `:612` cover the six consumed columns, insert/delete and rowid immutability. Older-build raw statements still fire the triggers. Same-value updates, unrelated columns and Litestream bookkeeping do not move it. “Nothing else” is too strong: database-less insertions/deletions and semantically irrelevant changes to those columns also move it. |
+| **Replacement chain and `closeOnce`** | `chainClosed` checks a latched predecessor’s own closure first. Window publication, chain detachment and latch publication share `staleMu`; published chain reads take that lock. Within the single-chain contract, a replacement cannot open beside an open/draining predecessor, and an unopened successor cannot latch through one. Intermediate recorders receive inheritance once. Reading a closed stale predecessor’s saved window is intentional. |
+| **Close’s final look** | Order is store stopped → watch stopped → file look → witness → inheritance → closed, at `litestream_metered.go:746`, `:750`, `:773`, `:776`, `:782`, `:783`. A persistent changed/missing owned file on a current fleet holds from its last clean look. Already-stale fleets skip the checks. Harmless changes after replication stops can over-hold everyone; filesystem operations can block Close. |
+| **Lineage** | `ownedReplicaKey` at `litestream_metered.go:948` combines account and cleaned path. Different valid accounts do not share lineage or shadow keys, including after renaming. A→B→A at one path resumes A’s prior lineage; changing app within one account does not create a new key. Separation from an external replicator’s prefix remains unenforced. |
+| **Strict walk and explicit claims** | SQLite extensions are recognized case-insensitively, although uppercase `.DB` is skipped rather than streamed by tree discovery. Every nonempty regular file not recognized by name gets a header read, including sidecar-shaped names; `sidecarOf` is gone. Missing/relative unit claims, units without `DBPath`, empty explicit claims and discovered files failing their final look are reported. Explicit exec claims receive ownership checks and do not change external config generation. |
+| **Platform-link traversal** | `walkWithoutLinks` at `litestream_owners.go:762` permits systemd’s leading `../private/...` shape and rejects ascents after named components. It does not clean away an intervening link. Its stable-path resolution is consistent with the kernel within the checked assumptions; ancestry and subsequent substitutions remain separate gaps. |
+| **Captured link classification** | Concatenation at `litestream_owners.go:697` fixes physical `..` resolution through outside links and linked roots. Leading runs of `..` remain uncleaned; absolute targets remain absolute. A link through a directory link to a regular file is classified as a file. Finding 5 limits syscall equivalence. Non-ENOENT lookup failures produce a problem naming the original path and quoted captured target; ordinary dangling non-database links remain omitted. |
+| **Link entries and Accept** | `linkEntry` at `litestream_owners.go:893` is distinct from every absolute plain path and other path/target pair. Relative targets retain literal `Readlink` spelling. Plain-path cleaning cannot make an absolute filename impersonate the prefixed link entry. Unchanged accepted pairs remain accepted while classification stays unchanged. Newlines cannot forge acceptance; raw names in refusal diagnostics can still mislead display. |
+| **Rendering** | `litestream_metered.go:679`, `:686` quote all serialized strings. Filename/newline and owner-field delimiter collisions are closed. `Platform` is omitted, so this is not injective over every possible `FleetInventory`. Under fixed production routes that bit is derived consistently; no filename-driven bypass follows from that omission. Production uses rendering only for baseline equality at `:666` and `:939`, not parsing or Accept. |
+| **Fixtures, umask and root CI** | `platformDir` resolves linked temporary roots; relevant modes are explicitly chmodded, so umask `077` does not narrow those fixture modes. The new outside-link fixture deliberately uses sibling temporary directories. Basename-based fake ownership and fixed fake UIDs remain assumptions. Root skips unreadable-directory/target cases and the root-versus-platform-user distinction. `.amadan/ci.d/10-gate:52`, `Makefile:18` and `.amadan/README.md:73` do not establish CI’s UID. |
+| **Open-item scope and reports** | The pathname gap covers retained handles, reopens and sidecars—not only first open. The exit gap includes routine restarts. One same-filesystem bind alias remains undetected; two inventoried aliases are rejected by identity. External-inventory comparison remains absent. Both reviewers’ review-eight and review-nine reports, and Opus’s review-fifteen report, are present at this commit. |
+
+**Claims the code contradicts**
+
+- **Unqualified kernel equivalence:** `litestream_owners.go:672`, `:697` and `DISPOSITIONS-metered-replicator.md:313` need the pathname-length qualification in finding 5. The lexical `..` defect itself is fixed.
+- **“Every inventory” rendering:** `litestream_metered.go:679` omits `OwnedDB.Platform`. Two otherwise identical constructed inventories differing only in that bit render identically. The owner triple is now correctly quoted.
+- **“Path and owner alike” test coverage:** `litestream_metered_test.go:1942` precedes assertions covering the owned path only. Removing owner-field quoting survives them.
+- **Fixture descriptions:** `litestream_metered_test.go:1860` and dispositions `:283` still describe the Accept case as `..` or `x/..`; the current distinguishing case is `sub/./live.sqlite`. Dispositions `:268` qualifies this historically, but still repeats the obsolete fixture description.
+- **Ownership-only witness wording:** `internal/registry/observe.go:27` and `observe_ownership_test.go:11` remain narrower than the triggers. `registry.go:581` correctly acknowledges conservative invalidations that move no database owner.
+- **Astra review fifteen’s recorder-swap counterfactual:** the live-replication test alone permits it, but the fleet’s direct-client test at `litestream_metered_test.go:334` and `:337` does not.
+- **Recorded test success:** dispositions `:318` reports the author’s runs. This source-only review neither confirms those runs nor establishes the CI user.
+
+**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 ddd2ad742..f92623bd0 100644
--- a/docs/superpowers/specs/DISPOSITIONS-metered-replicator.md
+++ b/docs/superpowers/specs/DISPOSITIONS-metered-replicator.md
@@ -296,7 +296,7 @@ Reports: `OPUS-REVIEW-metered-replicator-review14.md`, `CODEX-FINDINGS-metered-r
| Finding | Disposition | Where | Reproduction |
| --- | --- | --- | --- |
| Opus P3-2, Astra 5 (P2): `render()` wrote names raw, so a name with a line break read as two entries and two as one, and the running fleet's drift comparison could be defeated. | Fixed: every entry in `render()` is quoted (`strconv.Quote`), owned paths included. | `litestream_metered.go` `render` | `TestRenderQuotesEveryEntry`; fails with entries written raw (mutation run) |
-| Opus P3-1, Astra 7 (P2): the second `Readlink` missed a link re-pointed and put back, and dropped a second-read error. | Fixed: the second read is gone; the target is classified by the spelling the entry names (a `Stat` of the target as read, from the link's directory when relative). [Review fifteen: the fifteenth build joined and so cleaned that spelling, which is not the kernel's resolution of a `..` after a link; the sixteenth build concatenates, uncleaned.] What that spelling resolves to is not pinned, as stated before. | `litestream_owners.go` `strictTreeDBs` | no deterministic test (a tenant-timed re-point around one `Stat`); stated |
+| Opus P3-1, Astra 7 (P2): the second `Readlink` missed a link re-pointed and put back, and dropped a second-read error. | Fixed: the second read is gone; the target is classified by the spelling the entry names (a stat of the target as read, from the link's directory when relative). [Review fifteen: the fifteenth build joined and so cleaned that spelling, which is not the kernel's resolution of a `..` after a link; the sixteenth build concatenated, uncleaned; the seventeenth stats the target by itself against a descriptor for the directory, in this process's mount namespace.] What that spelling resolves to is not pinned, as stated before. | `litestream_owners.go` `strictTreeDBs` | no deterministic test (a tenant-timed re-point around one `Stat`); stated |
| Opus P3-3, Astra 12: the Accept test no longer told as-written from cleaned. | Fixed: the fixture accepts `sub/live.sqlite` and re-points to `sub/./live.sqlite`, which cleaning folds to the accepted entry. | tests | `TestAcceptComparesALinkEntryAsWritten`; fails with link entries cleaned by `Accept` (mutation run) |
| Opus claims: the "passes over on purpose" list; the newline-fixture sentence ("a temp root cannot hold it": false). | Corrected: the list names a link that is something else when read; the fixture makes a target with a line break and asserts it quoted. | `litestream_owners.go`, `litestream_owners_test.go` | `TestStrictTreeNamesALinkToAFile` |
| Astra 1 to 4, 6, 8 to 11: the deep items, the hibernating aliases and skipped-file claims, required coverage that can vanish, the box-wide refusal, the uid binding, the remaining test gaps (shutdown, watch, attribution, interleavings, root). | Open, as before, stated where each applies; the box-wide refusal and the uid binding are decisions before wiring. | | |
@@ -304,15 +304,30 @@ Reports: `OPUS-REVIEW-metered-replicator-review14.md`, `CODEX-FINDINGS-metered-r
Package tests pass under `/tmp` and `~/.cache` (the author's runs, not as root). The full gate on this tip is next; the fifteenth build has not been read by a reviewer.
-## Review fifteen (reviewed `03b19014`; Opus accepted with fixes, Astra's report pending at the sixteenth build's commit)
+## Review fifteen (reviewed `03b19014`; Opus accepted with fixes, Astra rejected)
-Report: `OPUS-REVIEW-metered-replicator-review15.md` (Astra's, when it lands, is copied in by the next commit). Opus found review fourteen's three findings closed and one regression in the fifteenth build's own fix: the relative target was joined with `filepath.Join`, which cleans a `..` lexically, where the kernel resolves it physically through whatever the component before it led to; a link whose target steps up through an outside link to a directory was therefore stat'ed beside the link, found nothing, and passed over. The sixteenth build:
+Reports: `OPUS-REVIEW-metered-replicator-review15.md`, `CODEX-FINDINGS-metered-replicator-review15.md` (Astra's in the tree from the seventeenth build's commit; it found the same join regression as its finding 5, and the deep items as before). Opus found review fourteen's three findings closed and one regression in the fifteenth build's own fix: the relative target was joined with `filepath.Join`, which cleans a `..` lexically, where the kernel resolves it physically through whatever the component before it led to; a link whose target steps up through an outside link to a directory was therefore stat'ed beside the link, found nothing, and passed over. The sixteenth build:
| Finding | Disposition | Where | Reproduction |
| --- | --- | --- | --- |
-| Opus P3-1: a relative target resolved lexically, not as the kernel resolves it; a link to a directory outside the tree passed over in silence; a tree root reached through a link not equivalent in general. | Fixed: the spelling is concatenated to the link's directory, never cleaned, and stat'ed as such, so the kernel resolves the `..` physically; the link itself is still never re-read. | `litestream_owners.go` `strictTreeDBs` | `TestStrictTreeNamesALinkToAFile` (`current -> "../o/lnk/../dbs"` with `lnk` an outside link to a directory: a problem); fails with the join (mutation run) |
+| Opus P3-1: a relative target resolved lexically, not as the kernel resolves it; a link to a directory outside the tree passed over in silence; a tree root reached through a link not equivalent in general. | Fixed: the spelling is concatenated to the link's directory, never cleaned, and stat'ed as such, so the kernel resolves the `..` physically; the link itself is still never re-read. [Review sixteen, Astra 5: the concatenation could outgrow PATH_MAX where the link itself resolves; the seventeenth build stats the target by itself against a descriptor for the link's directory (`fstatat`).] | `litestream_owners.go` `strictTreeDBs` | `TestStrictTreeNamesALinkToAFile` (`current -> "../o/lnk/../dbs"` with `lnk` an outside link to a directory: a problem); fails with the join (mutation run) |
| Opus P3-2: the impostor case no longer told the unprefixed encoding (the accepted pair's target had a separator). | Fixed: the impostor step accepts a flat pair (`current2 -> "live2.sqlite"`) and names the impostor exactly so. | tests | `TestAcceptComparesALinkEntryAsWritten`; fails with the unprefixed encoding (mutation run) |
-| Opus P3-3: the owned line's path quoting was untested and the owner triple raw. | Fixed: the whole owned line is quoted and the test has an owned path with a line break. | `litestream_metered.go` `render` | `TestRenderQuotesEveryEntry`; fails with the owned line raw (mutation run) |
+| Opus P3-3: the owned line's path quoting was untested and the owner triple raw. | Fixed: the whole owned line is quoted and the test has an owned path with a line break. | `litestream_metered.go` `render` | `TestRenderQuotesEveryEntry`; fails with the owned path raw (mutation run; the owner triple's quoting was pinned, and the `Platform` bit rendered, in the seventeenth build) |
| Claims: "classified by where it leads (one Stat)"; the test message "through a .."; this file's "follows the kernel's resolution" and "one string"; the review-thirteen row's mutation claim. | Corrected in place. | | |
Astra 1 to 4, 6, 8 to 11 stay open as before. Package tests pass under `/tmp` and `~/.cache` (the author's runs, not as root). The full gate on this tip is next; the sixteenth build has not been read by a reviewer.
+
+## Review sixteen (reviewed `f306222c`; Opus accepted with fixes, Astra rejected)
+
+Reports: `OPUS-REVIEW-metered-replicator-review16.md`, `CODEX-FINDINGS-metered-replicator-review16.md` (in the tree from the seventeenth build's commit). Both found review fifteen's findings closed with tests that fail without them, and the sixteenth build free of new defects in what it changed, bar one bound Astra named: the concatenated spelling handed to `Stat` could outgrow the kernel's path limit where the link itself resolves (a deep root and a long relative target), refusing the whole box for a valid file link. Opus added that "the kernel's resolution" is the fleet's mount namespace, not an isolated instance's. The seventeenth build:
+
+| Finding | Disposition | Where | Reproduction |
+| --- | --- | --- | --- |
+| Astra 5 (P2): the concatenation could exceed PATH_MAX where the link resolves. | Fixed: a relative target is stat'ed by itself against a descriptor for the link's directory (`fstatat`), an absolute one as it is; the spelling handed to the kernel is the target alone. On a platform without `fstatat` the concatenation remains (the replicator does not run there). | `linktarget_unix.go` `statTarget`, `linktarget_other.go` | `TestStrictTreeNamesALinkToAFile` (a root over a thousand bytes deep and a target of 1,500 `./`: the link is named, no problem); fails with the concatenation (mutation run) |
+| Opus P3-1: "the kernel's resolution" means the fleet's mount namespace; an isolated instance's own namespace (private `/tmp`, read-only bind mounts) may resolve the same spelling elsewhere or nowhere. | Stated in the code and here as open: a link naming something outside the tree is classified as the fleet sees it, which is what litestream would see; what the tenant's software sees inside its unit may differ. Nothing is replicated or attributed through such a link either way. | `litestream_owners.go` | |
+| Opus T-1, Astra 11: no aliased tree root fixture; the render test left the owner triple, `Platform`, Unowned and Problems unpinned. | Fixed: a tree root reached through a link with `cur -> ../d` (the directory beside the real tree, absent beside the link) is a problem; `render` writes the `Platform` bit and the test tells the owner fields and each list apart. | tests, `litestream_metered.go` `render` | `TestStrictTreeNamesALinkToAFile`, `TestRenderQuotesEveryEntry`; the `Platform` case fails with the bit omitted (mutation run) |
+| Astra claims: the registry witness's wording in `observe.go` and its test. | Corrected: the witness moves on every change to the six columns, whether or not the owner moved, and on route rows added or removed. | `internal/registry/observe.go`, `observe_ownership_test.go` | |
+| Claims (both): the fixture comment "through a `..`"; this file's rows ("the kernel's resolution", "fails with the owned line raw", Astra's numbering, "x/.." in a Reproduction column). | Corrected in place. | this file, tests | |
+| Astra 1 to 4, 6 to 10, the rest of 11: open as before. | | | |
+
+Package tests pass under `/tmp` and `~/.cache` (the author's runs, not as root). The full gate on this tip is next; the seventeenth build has not been read by a reviewer.
diff --git a/docs/superpowers/specs/OPUS-REVIEW-metered-replicator-review16.md b/docs/superpowers/specs/OPUS-REVIEW-metered-replicator-review16.md
new file mode 100644
index 000000000..67b7c2aa4
--- /dev/null
+++ b/docs/superpowers/specs/OPUS-REVIEW-metered-replicator-review16.md
@@ -0,0 +1,97 @@
+# Sixteenth review: feat/carloku-metered-replicator-20261001 at `f306222c`
+
+**Verdict: accept with fixes.** All three of review fifteen's findings are closed, and so are Astra's 5 and the impostor part of Astra's 11.
+
+The sixteenth build changed only three things:
+- the relative-target spelling in `strictTreeDBs`;
+- the owned line in `render()`;
+- two fixtures and the dispositions.
+
+It brings in no new defect. What is left is wording that is wider than the code, and test gaps. Astra 1–4 are untouched and still block wiring, together with approval H's uid binding and the decision on box-wide refusal.
+
+I read source only through `git show f306222c:`, plus Astra's report from the run directory. I ran nothing.
+
+## Review fifteen, finding by finding
+
+| Finding | At `f306222c` | Would its test pass without the fix? |
+|---|---|---|
+| **Opus P3-1 / Astra 5:** relative target cleaned by `filepath.Join` | **Closed.** `litestream_owners.go:695-699` stats `dir + "/" + dest`, uncleaned. | **No.** With the join, `root/../<outside>/o/lnk/../dbs` cleans to `<outside>/o/dbs`, which does not exist. The link reaches no list and `:731` fails. The fixture's `EvalSymlinks` check (`:727`) is fatal, so a temp root laid out differently cannot let it pass vacuously. The aliased-tree-root case Astra asked for is not fixtured (see T-1). |
+| **Opus P3-2 / Astra 11 (impostor)** | **Closed.** The impostor file is named `<state>/current2 -> "live2.sqlite"`, byte for byte the old unprefixed key of the accepted pair. | **No.** Under the old encoding, Accept compares a plain path cleaned, so the impostor matches the accepted entry and the build succeeds. The test expects `ErrUnmeterable`. |
+| **Opus P3-3:** owned line | **Closed for the path.** Every field is now quoted (`litestream_metered.go:679`). | **No** for the path: with it raw, the `Contains(Quote(path))` assertion fails. **Yes** for the owner triple: going back to `%s/%s %s` passes the test. That is harmless, as below. |
+| **Opus claims** | Mostly corrected. The failure message is fixed (`:1897`). The header comment at `litestream_owners.go:622-623` ("classified by where it leads (one Stat)") is now accurate in the fleet's namespace. Still stale: the test's doc comment at `litestream_metered_test.go:1860` ("through a `..`"), and the Reproduction column of `DISPOSITIONS:268`, which still leads with the `x/..` case. | n/a |
+| **Astra 1** (Litestream's pathname opens, P1) | **Open.** No code change. | Yes. |
+| **Astra 2** (replaceable ancestry, P1) | **Open.** | Yes. |
+| **Astra 3** (a delayed admitted attempt falls outside every hold, P1) | **Open.** The 23:58 → 00:02 sequence still stands, because nothing tracks an admitted attempt through to its completion. | Yes. |
+| **Astra 4** (uncertainty lost at process exit, P1) | **Open.** | Yes. |
+| **Astra 6** (device open through an ancestor swap, P2) | **Open.** | Yes. |
+| **Astra 7** (hibernating aliases, skipped-file claims, P2) | **Open.** | Yes. |
+| **Astra 8** (required coverage can vanish, P2) | **Open.** | Yes. |
+| **Astra 9** (one tenant refuses the whole box, P2) | **Open.** Still a decision to make before wiring. | n/a |
+| **Astra 10** (route identity, P3) | **Open.** | Yes. |
+| **Astra 11** (weak tests, P3) | **Partly closed.** The impostor and owned-path parts are closed. Still open: Unowned and Problems are each untested on their own, the owner triple is unpinned, and the shutdown, watch, recorder-swap, pre-witness, latch-recheck, `prepareShadowState`, inode-check, 300 ms Close and wall-clock items are all unchanged. | Yes, for those. |
+| **Astra claims** | "Captured-target classification": closed. "Universal render injectivity": Astra's example (`App="b c"` against `Host="c d"`) is now fixed by the quoting. The `Platform` omission is harmless because `Platform` is a function of `Account` (`litestream_owners.go:209`, and `:111` for the platform's own entries). Two claims are unchanged: the fixture descriptions (`:1860`) and the ownership-only witness wording (`observe.go:27`). | n/a |
+
+## Findings
+
+No P1 or P2 findings in the sixteenth build's changes.
+
+### P3-1. "The kernel's resolution" means the fleet's mount namespace, not the tenant's (CC2, claim)
+
+**Where:** `litestream_owners.go:691-699`; `DISPOSITIONS:268` ("which is the kernel's resolution of it").
+
+The `Stat` resolves in the fleet process's namespace. Isolated instances run with:
+- `TemporaryFileSystem=/data:ro`, `/var/lib:ro` and `/run/carlos:ro`;
+- `BindPaths=/data/instances/%I /run/carlos/instances/%I` (`carlos-instance-seed@.service:51-62`);
+- `PrivateTmp=yes` (also on `carlos-app@.service:47`).
+
+**Scenario:**
+1. Instance X's tree is `/data/instances/X`. X makes the link `cur -> ../Y/main.db`.
+2. Inside X's namespace, the tree's directory is `/run/carlos/instances/X`. Its `..` is a read-only tmpfs with no `Y` in it, so the tenant's software reaches nothing.
+3. On the host, the walker reaches tenant Y's real `main.db`. It names `link ".../X/cur" -> "../Y/main.db"` as skipped, and an operator may accept it.
+
+The converse also holds. In a `PrivateTmp` unit, `cache -> /tmp/dbs` is a directory inside the unit but missing on the host, so the link is passed over in silence.
+
+**Bound:** litestream never opens a tree link. Both cases name, or miss, something outside the tree, exactly as an absolute link anywhere did at `ec55b639`. So nothing is replicated or attributed wrongly.
+
+**Smallest fix:** say "in the fleet's mount namespace" at `:691` and in `DISPOSITIONS:268`, and add the isolated-instance and `PrivateTmp` divergence to the open items.
+
+### T-1. Test gaps left by this build (DD2, H)
+
+- **No aliased tree root.** No fixture has the shape `/x/state -> private/state` with `cur -> ../d`, where `/x/private/d` is a directory and `/x/d` is absent. `platformDir` resolves links on purpose (`litestream_owners_test.go:22`), so no `strictTreeDBs` fixture has a linked root. Concatenation handles the shape correctly; nothing pins it.
+- **`TestRenderQuotesEveryEntry` (`litestream_metered_test.go:1933`) leaves three things unpinned:**
+ - Unowned and Problems: they share the loop with Skipped, so only a selective mutation survives. Low value.
+ - The owner triple's quoting: harmless under a fleet's frozen routes.
+ - The section separator: safe, because a quoted entry is never `--`.
+
+**Smallest fix:** add the aliased-root fixture. Pinning the triple is optional.
+
+## CC2 and DD2
+
+**CC2: is concatenation the kernel's resolution?**
+
+In the fleet's namespace, on a stable tree, yes.
+- **Tree root reached through a link:** `dir` begins with that link, and the kernel follows it before any `..`. That is the physical directory the link `p` lives in, and the directory the kernel resolves `p`'s body from.
+- **Leading run of `..`:** resolved physically, the same as following `p`.
+- **Absolute target:** unchanged (`Stat(dest)`).
+- **Trailing `/` on a file target:** ENOTDIR both ways.
+- **Relative root:** roots are already reported as problems.
+
+Can a stable file link now be refused wrongly? Only through a narrow case: when `len(dir)+1+len(dest)` reaches PATH_MAX, `Stat` returns ENAMETOOLONG, and the link becomes "a link whose target cannot be looked at", which refuses the box. The kernel, following `p` itself, never builds that string. It needs about 4 KiB of combined spelling, which no real layout reaches, and a tenant has easier ways to refuse the box (Astra 9). Mention it in the comment; it is not worth code.
+
+Races on in-tree ancestors between `ReadDir` and `Stat` stay in the stated tenant-timed class.
+
+**DD2: the impostor fixture and the render test**
+
+The flat pair now tells the unprefixed encoding apart (see the table). The render test still leaves Unowned, Problems, the owner triple and the separator unpinned (T-1). `Platform` needs no pinning, because it is derived from `Account`.
+
+**A–BB2:** unchanged from reviews thirteen to fifteen. Nothing they read changed in this build: the registry, admission, Close, the latch, lineage, claims, the chain climb and `walkWithoutLinks` were all untouched. Whether CI runs as root is still not established by committed source.
+
+## Claims the code contradicts
+
+- **`DISPOSITIONS:268`:** "the kernel's resolution": true only in the fleet's namespace (P3-1). Its Reproduction column still leads with the `x/..` case.
+- **`litestream_metered_test.go:1860`:** "through a `..`". The case is `sub/./live.sqlite`.
+- **`DISPOSITIONS:316-318`:** "Astra 1 to 4, 6, 8 to 11 stay open" uses review fourteen's numbering. Under Astra's fifteenth report it is 1–4, 6–10 and part of 11.
+- **`DISPOSITIONS:316`:** "Fixed: the whole owned line is quoted … fails with the owned line raw". It fails with the path raw. With the triple raw, the test passes.
+- **"Package tests pass":** the author's runs. Not verified here.
+
+**Verdict: accept with fixes.** The fixes are the P3-1 wording, the aliased-root fixture, and the stale claims. Astra 1–4, approval H's uid binding and the box-wide refusal decision still block wiring.
diff --git a/internal/ops/linktarget_other.go b/internal/ops/linktarget_other.go
new file mode 100644
index 000000000..f6cdced30
--- /dev/null
+++ b/internal/ops/linktarget_other.go
@@ -0,0 +1,26 @@
+//go:build !unix
+
+package ops
+
+import (
+ "os"
+ "path/filepath"
+)
+
+type targetKind struct {
+ dir, regular bool
+}
+
+// statTarget on a platform without fstatat: the replicator does not run
+// here (the header read refuses too); the concatenated spelling is looked
+// at for completeness.
+func statTarget(dir, dest string) (targetKind, error) {
+ if !filepath.IsAbs(dest) {
+ dest = dir + string(filepath.Separator) + dest
+ }
+ fi, err := os.Stat(dest)
+ if err != nil {
+ return targetKind{}, err
+ }
+ return targetKind{dir: fi.IsDir(), regular: fi.Mode().IsRegular()}, nil
+}
diff --git a/internal/ops/linktarget_unix.go b/internal/ops/linktarget_unix.go
new file mode 100644
index 000000000..1b5faa6f3
--- /dev/null
+++ b/internal/ops/linktarget_unix.go
@@ -0,0 +1,46 @@
+//go:build unix
+
+package ops
+
+import (
+ "os"
+ "path/filepath"
+
+ "golang.org/x/sys/unix"
+)
+
+// targetKind is what a link's target is, by one look at it: a directory, a
+// regular file, something else, or nothing (err is os.ErrNotExist then).
+type targetKind struct {
+ dir, regular bool
+}
+
+// statTarget looks at a link's target as the link spells it, in this
+// process's mount namespace: an absolute target as it is; a relative one
+// relative to the directory the link is in, through a descriptor for that
+// directory (fstatat), so the kernel resolves every ".." physically, from
+// wherever the component before it led, and the spelling handed to it is
+// the target alone (never a concatenation that could outgrow PATH_MAX
+// while the link itself resolves). The link is never read again.
+func statTarget(dir, dest string) (targetKind, error) {
+ if filepath.IsAbs(dest) {
+ fi, err := os.Stat(dest)
+ if err != nil {
+ return targetKind{}, err
+ }
+ return targetKind{dir: fi.IsDir(), regular: fi.Mode().IsRegular()}, nil
+ }
+ d, err := os.Open(dir)
+ if err != nil {
+ return targetKind{}, err
+ }
+ defer d.Close()
+ var st unix.Stat_t
+ if err := unix.Fstatat(int(d.Fd()), dest, &st, 0); err != nil {
+ if err == unix.ENOENT {
+ return targetKind{}, os.ErrNotExist
+ }
+ return targetKind{}, &os.PathError{Op: "stat", Path: dest, Err: err}
+ }
+ return targetKind{dir: st.Mode&unix.S_IFMT == unix.S_IFDIR, regular: st.Mode&unix.S_IFMT == unix.S_IFREG}, nil
+}
diff --git a/internal/ops/litestream_metered.go b/internal/ops/litestream_metered.go
index 2afc73cfd..c969313d5 100644
--- a/internal/ops/litestream_metered.go
+++ b/internal/ops/litestream_metered.go
@@ -676,7 +676,7 @@ func (inv FleetInventory) render() string {
// one).
var b strings.Builder
for _, o := range inv.Owned {
- fmt.Fprintf(&b, "owned %s %s %s %s\n", strconv.Quote(o.Path), strconv.Quote(o.Account), strconv.Quote(o.App), strconv.Quote(o.Host))
+ fmt.Fprintf(&b, "owned %s %s %s %s %t\n", strconv.Quote(o.Path), strconv.Quote(o.Account), strconv.Quote(o.App), strconv.Quote(o.Host), o.Platform)
}
for _, list := range [][]string{inv.Unowned, inv.Skipped, inv.Problems} {
sorted := append([]string(nil), list...)
diff --git a/internal/ops/litestream_metered_test.go b/internal/ops/litestream_metered_test.go
index e2c99a059..4760fdca7 100644
--- a/internal/ops/litestream_metered_test.go
+++ b/internal/ops/litestream_metered_test.go
@@ -1885,7 +1885,9 @@ func TestAcceptComparesALinkEntryAsWritten(t *testing.T) {
fleet.Close()
// Re-pointed to the same file through a "./": another pair. (A cleaned
// comparison would fold it to the accepted entry and let the build
- // through; review fourteen showed a ".." case did not tell them apart.)
+ // through; review fourteen showed an "x/.." case did not tell them
+ // apart, since cleaning it popped the component holding the link's
+ // own name rather than folding to the accepted entry.)
if err := os.Remove(filepath.Join(state, "current")); err != nil {
t.Fatal(err)
}
@@ -1945,4 +1947,21 @@ func TestRenderQuotesEveryEntry(t *testing.T) {
if c.render() == d.render() || !strings.Contains(c.render(), strconv.Quote(c.Owned[0].Path)) {
t.Fatalf("an owned path with a line break is not quoted:\n%s", c.render())
}
+ // The owner triple and the platform bit too: a space moved between
+ // fields, or the bit alone, renders differently.
+ e := FleetInventory{Owned: []OwnedDB{{Path: "/p", Account: "a", App: "b c", Host: "d"}}}
+ f := FleetInventory{Owned: []OwnedDB{{Path: "/p", Account: "a", App: "b", Host: "c d"}}}
+ g := FleetInventory{Owned: []OwnedDB{{Path: "/p", Account: "a", App: "b c", Host: "d", Platform: true}}}
+ if e.render() == f.render() || e.render() == g.render() {
+ t.Fatalf("the owner fields are not told apart:\n%s\n%s\n%s", e.render(), f.render(), g.render())
+ }
+ // And each list on its own.
+ for _, pair := range [][2]FleetInventory{
+ {{Unowned: []string{"x\ny"}}, {Unowned: []string{"x", "y"}}},
+ {{Problems: []string{"x\ny"}}, {Problems: []string{"x", "y"}}},
+ } {
+ if pair[0].render() == pair[1].render() {
+ t.Fatalf("a list is not quoted:\n%s", pair[0].render())
+ }
+ }
}
diff --git a/internal/ops/litestream_owners.go b/internal/ops/litestream_owners.go
index ab92a1a14..597dc357c 100644
--- a/internal/ops/litestream_owners.go
+++ b/internal/ops/litestream_owners.go
@@ -668,9 +668,10 @@ func strictTreeDBs(root string) (dbs, skipped, problems []string) {
}
switch {
case fi.Mode()&os.ModeSymlink != 0:
- // Classified by where its target's spelling leads (one Stat
- // of that spelling, as the kernel resolves it; the link is
- // never walked into or opened).
+ // Classified by where its target's spelling leads (one stat
+ // of that spelling, as the kernel resolves it in this
+ // process's namespace; the link is never walked into or
+ // opened).
// A link entry is named with its path and target, each
// quoted behind a word no path begins with, so a link
// re-pointed after it was accepted reads as a change and
@@ -685,27 +686,24 @@ func strictTreeDBs(root string) (dbs, skipped, problems []string) {
}
// The entry names the target as read, and the target is
// classified by that very spelling (from the link's own
- // directory when relative), never by reading the link
- // again: what the entry says and what was looked at are one
- // string, whatever the link is re-pointed to meanwhile.
- // The spelling is concatenated, never cleaned: the kernel
- // resolves a ".." in the target physically (through whatever
- // the component before it led to), and a cleaned join would
- // look somewhere else.
- spelled := dest
- if !filepath.IsAbs(spelled) {
- spelled = dir + string(filepath.Separator) + spelled
- }
- target, terr := os.Stat(spelled)
+ // directory when relative, through a descriptor for it:
+ // statTarget), never by reading the link again: what the
+ // entry says and what was looked at are one string, whatever
+ // the link is re-pointed to meanwhile, and the kernel
+ // resolves a ".." in it physically, in this process's mount
+ // namespace (an isolated instance's own namespace, with its
+ // private /tmp and read-only bind mounts, may resolve the
+ // same spelling elsewhere or nowhere; stated as open).
+ target, terr := statTarget(dir, dest)
// A problem names the path first (nothing compares a
// problem with an Accept list); a skipped link is the
// injective entry.
switch {
- case sqliteName(name) || (terr == nil && target.IsDir()):
+ case sqliteName(name) || (terr == nil && target.dir):
problems = append(problems, p+" -> "+strconv.Quote(dest)+": a link that may lead to a database")
case terr != nil && !errors.Is(terr, fs.ErrNotExist):
problems = append(problems, p+" -> "+strconv.Quote(dest)+": a link whose target cannot be looked at")
- case terr == nil && target.Mode().IsRegular():
+ case terr == nil && target.regular:
skipped = append(skipped, linkEntry(p, dest))
}
case fi.IsDir():
diff --git a/internal/ops/litestream_owners_test.go b/internal/ops/litestream_owners_test.go
index 440063bb5..be5a4495f 100644
--- a/internal/ops/litestream_owners_test.go
+++ b/internal/ops/litestream_owners_test.go
@@ -734,6 +734,42 @@ func TestStrictTreeNamesALinkToAFile(t *testing.T) {
if err := os.Remove(filepath.Join(root, "stepup")); err != nil {
t.Fatal(err)
}
+ // A tree root reached through a link (systemd's state-link shape): a
+ // "../d" from inside it steps up from where the link led (the real
+ // directory's parent), not from beside the link. (Review sixteen.)
+ x := platformDir(t)
+ mkdirs(t, filepath.Join(x, "private", "state"), filepath.Join(x, "private", "d"))
+ if err := os.Symlink("private/state", filepath.Join(x, "state")); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.Symlink("../d", filepath.Join(x, "state", "cur")); err != nil {
+ t.Fatal(err)
+ }
+ _, _, problems = strictTreeDBs(filepath.Join(x, "state"))
+ if !containsPrefix(problems, filepath.Join(x, "state", "cur")) {
+ t.Fatalf("a link stepping up from a tree root reached through a link: problems %v; want the directory it reaches said", problems)
+ }
+ // A target whose spelling, concatenated to the link's directory, would
+ // outgrow the kernel's path limit, though the link itself resolves:
+ // the target is looked at by itself, against the directory, and the
+ // link is named as the file link it is. (Review sixteen, Astra 5.)
+ deep := platformDir(t)
+ for i := 0; i < 5; i++ {
+ deep = filepath.Join(deep, strings.Repeat("d", 200))
+ }
+ mkdirs(t, deep)
+ long := writeFile(t, filepath.Join(deep, "live.sqlite"))
+ dest := strings.Repeat("./", 1500) + "live.sqlite"
+ if err := os.Symlink(dest, filepath.Join(deep, "current")); err != nil {
+ t.Fatal(err)
+ }
+ if len(deep)+1+len(dest) <= 4096 {
+ t.Fatalf("fixture: the concatenation is only %d bytes", len(deep)+1+len(dest))
+ }
+ dbs, skipped, problems = strictTreeDBs(deep)
+ if len(problems) != 0 || !contains(skipped, linkEntry(filepath.Join(deep, "current"), dest)) {
+ t.Fatalf("a long relative target: dbs %v skipped %d entries problems %v; want the link named, no problem (target %s)", dbs, len(skipped), problems, long)
+ }
// A link whose target cannot be looked at is a problem, not nothing.
if os.Geteuid() != 0 {
locked := filepath.Join(platformDir(t), "locked")
diff --git a/internal/registry/observe.go b/internal/registry/observe.go
index 268e8e6ae..751bcda79 100644
--- a/internal/registry/observe.go
+++ b/internal/registry/observe.go
@@ -25,9 +25,10 @@ import (
// is never closed leaks one SQLite handle.
//
// A second, narrower witness rides on the same read: the ownership
-// generation (migrateRouteOwnership), which moves only when a route is
-// added, removed, or changes who owns its database or which replicator
-// streams it. OwnershipUnchanged asks that one.
+// generation (migrateRouteOwnership), which moves when a route is added
+// or removed, or when one of the six columns ownership is read from
+// changes value (whether or not the owner moved), and on nothing else.
+// OwnershipUnchanged asks that one.
type RouteObservation struct {
Routes []Route
db *sql.DB
diff --git a/internal/registry/observe_ownership_test.go b/internal/registry/observe_ownership_test.go
index e5cd65a33..e8f21ddaa 100644
--- a/internal/registry/observe_ownership_test.go
+++ b/internal/registry/observe_ownership_test.go
@@ -8,9 +8,9 @@ import (
"testing"
)
-// The ownership witness moves when a route is added, removed, or changes
-// who owns its database or which replicator streams it, and for nothing
-// else. Every row of the second table is a commit that the whole-database
+// The ownership witness moves when a route is added or removed, or when
+// one of the columns ownership is read from changes value, and for
+// nothing else. Every row of the second table is a commit that the whole-database
// witness (Unchanged) reports and that has nothing to do with ownership:
// those are the writes a live box makes all day, and the bookkeeping a
// replicator writes into the registry it replicates.