amadan
Eighteenth replicator build, Astra's seventeenth report answered: sections told apart in render, the owner-check comment, the witness test's name, the outstanding item
Astra's seventeenth review (reject, on the deep items) names the same two new defects Opus did, fixed in the previous commit, and claims the code contradicts: the Accept fixture's comment, this file's description of the classification as concatenation, the open-item numbering that omitted Astra 7, an owner-check comment wider than the comparison it describes, a test named for a narrower property than it tests, and OUTSTANDING.md 1 describing a third build. Each is corrected in place. Of its finding 12, render is now compared across sections (the same entry in another list, and an entry spelt like the separator, render differently) and the directory-substitution window has its fixture; the rest stay open as named. Ops and registry packages pass.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
6 files changed,
+250
−21
docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review17.md+208 −0docs/superpowers/specs/DISPOSITIONS-metered-replicator.md+8 −6implementation/carloku-paid-launch/OUTSTANDING.md+1 −1internal/ops/litestream_metered_test.go+15 −2internal/ops/litestream_owners.go+10 −6internal/registry/observe_ownership_test.go+8 −6
CI passed — run details
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review17.md b/docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review17.md| new file mode 100644 |
| index 000000000..e5039675f |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review17.md |
| @@ -0,0 +1,208 @@ |
| +| Review-sixteen finding | Closed at `bffc76b5`? | Would its test pass without the fix? | |
| +|---|---|---| |
| +| **Opus P3-1 — fleet versus tenant mount namespace** | **Closed as a wording request.** `litestream_owners.go:693` and dispositions `:327` state the limitation. Namespace divergence remains open. | No behavioral regression test; changing the wording affects no test. | |
| +| **Opus T-1 — aliased root and rendering coverage** | **Mostly.** The aliased-root fixture and owner/list assertions exist. Section separation remains independently untested. The new long-path fixture also introduces a temp-root dependency. | The aliased-root case passes the previous concatenation implementation, correctly. Restoring raw owner triples or omitting `Platform` fails the new render assertions. Removing section separators survives these assertions. | |
| +| **Opus claims** | **Partly corrected.** Several descriptions explicitly reported as corrected remain unchanged; listed below. | Prose corrections have no behavioral test. | |
| +| **Astra 1 — Litestream pathname opens** | **Open.** | Existing tests permit the defect; no controlled substitution at Litestream’s actual open. | |
| +| **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 transition or recovery test. | |
| +| **Astra 5 — concatenation exceeds `PATH_MAX`** | **Closed in the Unix implementation for stable directories.** The replacement directory open introduces a separate defect. | With a sufficiently long temporary root, **no**: restoring concatenation fails. With an ordinary short `/tmp` root, the fixture fails **with either implementation**, before exercising the distinction. | |
| +| **Astra 6 — header-open device effects** | **Open.** | 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 and 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.** Owner-triple, `Platform`, Unowned and Problems rendering assertions were added. Shutdown, watch, interleaving and timing gaps remain. | **No** for restoring the reported raw rendering or omitting `Platform`; **yes** for the surviving counterfactuals below. | |
| +| **Astra claims** | Rendering and the witness’s explanatory wording are corrected. Fixture descriptions and historical disposition claims remain partly stale. | Author-reported test and mutation runs were not independently executed. | |
| + |
| +All locations below refer to **`bffc76b5`**. This was a source-only review of the branch against `72b4f59a` and the incremental change from `f306222c`. No files were changed, tests run, or network commands issued. Test counterfactuals are deductions from source. |
| + |
| +1. **P1 — Litestream’s actual database and sidecar 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, then restores the original pathname before the next look. Litestream can retain the substituted handles while subsequent inventories—including Close’s final look—see the original inode. Initialization performs SQLite writes, and later WAL opens independently follow mutable paths. |
| + |
| + This is larger than a first-open race: it includes retained handles, initialization effects, subsequent opens and sidecars. |
| + |
| + **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 directories still have tenant-replaceable ancestors.** |
| + |
| + **Locations:** `internal/ops/litestream_owners.go:512`, `:583`, `:818`, `:846`; `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`. Equivalent layouts work above `dataDir`, shadow state and 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. Protected endpoint ownership and modes alone do not establish that property. |
| + |
| +3. **P1 — An admitted request delayed past the margin can escape every inherited hold.** |
| + |
| + **Locations:** `internal/ops/litestream_metered.go:240`, `:325`, `:772`; `internal/lease/requestgate.go:58`. |
| + |
| + 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` chooses the correct earlier lower bound. Unopened fleets preserve the supplied window. Neither supplies the missing upper bound. |
| + |
| + **Smallest acceptable fix:** track admitted attempts through recording/completion and include their doubtful dates before finalizing handover. A larger fixed margin does not close this. |
| + |
| +4. **P1 — Ownership uncertainty remains entirely in memory 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 addresses a different case. |
| + |
| + **Smallest acceptable fix:** persist ownership/admission uncertainty before affected requests escape, and recover it before establishing successor coverage. |
| + |
| +5. **P2 — New: `statTarget` opens a replaceable pathname as though it were the walked directory.** |
| + |
| + **Locations:** `internal/ops/linktarget_unix.go:33`, `:39`; `internal/ops/litestream_owners.go:650`, `:679`, `:697`; `internal/ops/litestream_metered.go:750`, `:773`. |
| + |
| + The descriptor is **not necessarily the directory that `ReadDir` listed**. The walk closes its directory read, obtains the link target by pathname, then `statTarget` opens `dir` again. |
| + |
| + Concrete scenarios: |
| + |
| + - After `Readlink`, the tenant renames `T/d` and places a FIFO at `T/d`. `os.Open(dir)` uses an ordinary blocking read open. With no writer, it blocks before `Fstatat`. During the watch this stalls subsequent ownership ticks; Close waits for that watch. During Close’s final look, Close itself blocks. |
| + - Replacing `d` with another directory makes the captured relative target resolve from that replacement. If its target is absent, the link can disappear from the inventory as `ENOENT`, despite the original listed directory containing a directory-valued target. |
| + - Replacing `d` with a link to a device can cause open-side effects before `Fstatat` rejects the descriptor. |
| + |
| + This adds an unrestricted open to what was previously a `Stat` classification. |
| + |
| + **Smallest acceptable fix:** use the same safely opened directory descriptor for listing, reading the link and classifying its captured target. Directory acquisition must reject non-directories before opening them as ordinary files; a directory-only open is the immediate minimum against FIFO/device substitution. |
| + |
| +6. **P2 — New: the long-path fixture fails under an ordinary short temporary root.** |
| + |
| + **Location:** `internal/ops/litestream_owners_test.go:756`, `:766`. |
| + |
| + The fixture adds five 201-byte path components and a 3,011-byte target. Its concatenation length is: |
| + |
| + ```text |
| + len(platformDir) + 1005 + 1 + 3011 |
| + = len(platformDir) + 4017 |
| + ``` |
| + |
| + A valid ordinary root such as: |
| + |
| + ```text |
| + /tmp/TestStrictTreeNamesALinkToAFile1234567890/007 |
| + ``` |
| + |
| + is 50 bytes. The combined spelling is **4,067 bytes**, so line 766 calls `Fatal` even with the fixed implementation. The fixture requires a temporary root longer than 79 bytes; resolving a linked temp root can also shorten it below that threshold. |
| + |
| + **Smallest acceptable fix:** derive the nesting depth from the resolved root’s actual length so the concatenated spelling deliberately exceeds the Linux limit while the directory and target individually remain valid. Keep the fixture’s prerequisite assertion. |
| + |
| +7. **P2 — Header inspection still permits device-open effects through an ancestor swap.** |
| + |
| + **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 subsequent regular-file and inode checks cannot undo open-side effects. |
| + |
| + `O_NOFOLLOW` protects the final component. `O_NONBLOCK` prevents the ordinary FIFO wait; `O_NOCTTY` prevents controlling-terminal acquisition. Neither confines ancestors. |
| + |
| + **Smallest acceptable fix:** constrain traversal through pinned directory descriptors before opening the target. Preserve refusal on unsupported platforms. |
| + |
| +8. **P2 — Hibernating aliases and claims on skipped files 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`, bypass listed-file claimant reconciliation. Two hibernating claims can bypass it altogether. |
| + |
| + **Smallest acceptable fix:** reconcile claims by file identity against both listed and skipped files, retaining claimant information for skipped entries. |
| + |
| +9. **P2 — Required coverage can still disappear without an inventory entry.** |
| + |
| + **Locations:** `internal/ops/litestream.go:507`, `:525`; `internal/ops/litestream_owners.go:639`, `:662`, `:715`. |
| + |
| + 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. |
| + |
| +10. **P2 — Ordinary tenant changes can refuse replacement replication or hold days for every account.** |
| + |
| + **Locations:** `internal/ops/litestream_owners.go:702`, `:707`; `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 awaiting resolution of that tenant’s inventory. |
| + |
| + A legitimate concurrent rename detected 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. |
| + |
| +11. **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 modes, 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. The deployment approval remains outside this review. |
| + |
| +12. **P3 — Several test assertions still permit removal of the behavior they describe.** |
| + |
| + **Locations:** `internal/ops/litestream_metered_test.go:689`, `:963`, `:1062`, `:1380`, `:1496`, `:1935`; `internal/ops/litestream_owners.go:194`, `:217`. |
| + |
| + Surviving counterfactuals and gaps: |
| + |
| + - Clearing the store pointer and refusing requests does not establish that retained database handles and workers stopped. Removing `store.Close` can satisfy those shutdown 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 each request’s attribution. **The asymmetric direct-client assertions at `:334` and `:337` do catch swapped fleet recorders.** |
| + - 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 inode comparison has a controlled distinguishing interleaving. |
| + - Removing render’s section separators survives the current within-section comparisons. |
| + - The 300 ms negative Close check can pass before the second goroutine runs. The clean-start test reconstructs time from Unix nanoseconds and depends on wall-clock stability. |
| + - No fixture exercises the new directory-substitution window in `statTarget`. |
| + |
| + **Smallest acceptable fix:** add controlled shutdown/watch/interleaving assertions, a cross-section render comparison, and a directory-substitution regression. Preserve the existing asymmetric attribution test. |
| + |
| +The remaining requested checks hold with these qualifications: |
| + |
| +| Area | Result at `bffc76b5` | |
| +|---|---| |
| +| **Tenant-file chain, starting directory and top check — A/J/P/U** | `litestream_owners.go:464`, `:495`, `:500`, `:512` reject a distinct foreign tenant’s starting directory, tenant-made links and foreign stopping parents on stable paths. Every accepting `tenantFile` path reaches the top check. Protected platform directories are an explicit exception, not evidence that every component has the tenant UID. Higher replaceable ancestry remains finding 2. | |
| +| **Root and platform UID** | `platformUID` at `:880` equates root and this process’s UID only. A root/platform-owned file directly beneath a distinct tenant-owned directory is refused; the exception does not equate that tenant with root. Root-run units stop climbing at platform ownership rather than walking to `/`; qualifying platform files can pass. This does not bind them to a route. | |
| +| **Platform-made links and `DBPath` links** | A platform link to a target with a tenant-owned immediate parent fails `:846`. Higher ancestry remains unchecked. A file-valued `DBPath` that is itself a link fails `:435` or the downward link check at `:534`; it is not accepted as another tenant’s file. | |
| +| **Admission and witness retries — B/I** | `litestream_metered.go:226` and `:240` cover staleness while waiting for the lock and during the witness read. Staleness after the final load remains the already-admitted-request gap. `witnessUnchanged` performs at most three reads, accepts recovered errors, stops immediately on a witnessed change and treats exhausted errors as uncertainty. Its contexts do not bound mutex waiting or filesystem `Stat`. | |
| +| **Registry generation — C** | `registry.go:569`, `:602`, `:612` cover insert/delete and changes to all six consumed columns, including older-build raw statements. Same-value updates, unrelated columns and Litestream bookkeeping do not move it. The rowid trigger refuses the `UPDATE OR REPLACE` deletion bypass. Database-less inserts/deletes and semantically irrelevant changes to those six columns still move it conservatively. | |
| +| **Replacement chain and `closeOnce` — D/I/J/M** | `chainClosed` at `litestream_metered.go:420` 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 documented single-chain contract, a replacement cannot open beside an open/draining predecessor, and an unopened successor cannot latch through one. Intermediate recorders receive the inherited window once. | |
| +| **Close’s last look — M** | Order at `litestream_metered.go:746`, `:750`, `:773`, `:776`, `:782`, `:783` is store stopped → watch stopped → file look → witness → inheritance → closed. A persistent changed/missing owned file on a current fleet holds from its last clean look. Already-stale fleets skip both checks. Harmless changes after replication stops can over-hold everyone; filesystem operations can block Close, now including finding 5. | |
| +| **Owner-keyed lineage — E** | `ownedReplicaKey` at `litestream_metered.go:948` combines account and cleaned path. Different valid accounts do not share lineage or shadow keys, including after a rename. A→B→A at one path resumes A’s earlier lineage; changing app within one account does not create a new key. Separation from an external replicator’s prefix remains unenforced. | |
| +| **Strict walk and reporting — F/K/L/N** | SQLite extensions are recognized case-insensitively; uppercase `.DB` is skipped rather than streamed by tree discovery. Every nonempty regular file not recognized by name receives 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. Findings 8–9 identify remaining omissions. | |
| +| **Explicit claims and every claimant’s checks — N/P/Q/S** | Explicit live exec claims outside the globs receive ownership checks and do not change external config generation. `claimChecks` at `litestream_owners.go:278` retains each distinct root/kind, including two trees, tree-plus-file and unit-plus-exec claims. Ordinary DynamicUser links, protected operator-selected paths and isolated-instance directories can pass. Stable acceptance is independent of check order. | |
| +| **One inode and kept-facts duplicate check — W/Z** | The first successful check’s facts are retained at `: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 establish an atomic snapshot. | |
| +| **`walkWithoutLinks` — K** | At `litestream_owners.go:760`, systemd’s leading `../private/...` shape is permitted; ascents after named components are refused. It does not clean away an intervening link. Stable checked paths resolve consistently within those assumptions; ancestry and later substitutions remain separate gaps. | |
| +| **Captured-target classification — R/BB2/CC2/EE2** | On a stable tree, `Fstatat(fd, dest, …, 0)` follows links inside the captured relative target and resolves `..` physically from the opened directory, including an aliased tree root. Absolute targets use `Stat` unchanged. A target reached through a directory link can be classified as a regular file. Finding 5 prevents claiming that the descriptor is necessarily the directory originally walked. | |
| +| **Failed targets and non-Unix fallback** | Non-`ENOENT` failures produce a problem naming the path and quoted captured target; ordinary dangling non-database links remain omitted. `linktarget_other.go:17` retains concatenation, but `filefacts_other.go:16` refuses ownership and `sqliteopen_other.go:13` refuses header reads, so that fallback does not enable this fleet there. | |
| +| **Link entries and Accept — T/V/Y** | `linkEntry` at `litestream_owners.go:891` is distinct from absolute plain paths and other path/target pairs. Relative targets retain their literal `Readlink` spelling. Plain-path cleaning cannot forge the prefixed entry. Unchanged accepted pairs remain accepted while classification stays unchanged. Newlines cannot forge acceptance; raw plain names in refusal diagnostics can still mislead display. | |
| +| **Rendering — AA2/FF2** | `litestream_metered.go:679`, `:686` quote all strings, include `Platform`, and preserve section boundaries. The reported inventory content is now unambiguous, with list order normalized. This is not serialization of the entire struct: private inode facts are intentionally omitted and checked separately at `:649`. Production compares rendering at `:666` and `:939`; it does not parse it or use it for Accept. | |
| +| **Fixtures, umask and root CI — H/L/X** | `platformDir` resolves linked temp roots; relevant modes are explicitly chmodded, so umask `077` does not narrow them. The new length fixture nevertheless depends on the resolved root’s length. 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 — G/O** | The pathname gap includes reopens and sidecars; the exit gap includes routine restarts. A single same-filesystem bind alias remains undetected, while two inventoried aliases are rejected by identity. External-inventory comparison remains absent. Both reviewers’ review-eight, review-nine and review-sixteen reports are present at this commit. | |
| + |
| +**Claims the code contradicts** |
| + |
| +- **“Corrected in place,” dispositions `:330`:** `litestream_metered_test.go:1860` still describes the Accept fixture as `..`; its distinguishing target is `sub/./live.sqlite`. Dispositions `:268` and `:283` still describe the reproduction using `x/..`, and `:318` still omits Astra 7 from its open-item numbering. |
| +- **Current classification description, dispositions `:268`:** it still describes concatenation and unqualified kernel resolution. The Unix implementation now uses `fstatat`, in the fleet’s namespace, with the directory-acquisition race in finding 5. |
| +- **“The directory the link is in,” `linktarget_unix.go:20`:** this is only assured for a stable pathname. The descriptor at `:33` can name a replacement directory—or open something that is not a directory. |
| +- **“A tenant’s file in a directory the platform owns is refused,” `litestream_owners.go:557`:** too broad. For `/srv/A/bridge/a.db`, with A owning `A` and `a.db` and root owning protected `bridge`, the climb can reach `A` and accept. The final comparison is against the **chain root**, not necessarily the file’s immediate directory. |
| +- **Rendering claims:** the previous `Platform` omission is fixed. “One to one” is accurate for the reported inventory content, not for private file facts or insignificant list ordering. |
| +- **Ownership-only wording:** the explanatory comments in `observe.go` and its test are corrected, but `TestOwnershipWitnessMovesOnTheColumnsItReads` still overstates the narrower property its name suggests. The implementation deliberately invalidates on some changes that move no database owner. |
| +- **`OUTSTANDING.md:35`:** still describes a third build awaiting review, “only ownership” moving the generation, and “no link or mount.” Those are stale or broader than this source establishes. |
| +- **Package-pass claims, dispositions `:333`:** these are author reports, not independently verified here. They do not establish temp-root independence; the concrete short-root fixture failure above remains in this commit. |
| + |
| +**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 812016dfc..8a6e0ab12 100644 |
| --- a/docs/superpowers/specs/DISPOSITIONS-metered-replicator.md |
| +++ b/docs/superpowers/specs/DISPOSITIONS-metered-replicator.md |
| @@ -52,7 +52,7 @@ Three commits: a request gate in `internal/lease`, an ownership generation in th |
| | Review two | Now | Where | Reproduction | |
| | --- | --- | --- | --- | |
| | Both, P1: a stale-owner refusal does not stop the request | Fixed. Permission to send is its own middleware (`lease.RequestGate`), outside the request meter in the same step whichever option is applied first. A refusal returns `ErrRequestRefused` before dispatch: nothing reaches the network or the meter, the SDK does not retry it, and litestream sees a failed upload. Every metered database must name a gate. The fleet's gate asks the ownership witness on every attempt. | `lease/requestgate.go`, `lease/meteredstore.go`, `ops/litestream_metered.go` `admit` | `TestRequestGateStopsAnAttemptBeforeItIsSentOrCounted` (both option orders), `TestRequestGateIsAskedAgainForARetry`, `TestMeteredStoreClientIsStoppedByItsGate`, `TestMeteredFleetStopsSendingWhenOwnershipChanges` | |
| -| Opus P1, Astra P2: the witness is moved by the replicator's own writes and by every unrelated commit | Fixed. The registry keeps an ownership generation, a counter moved by triggers when a route is added, removed, or changes host, account, app, database path, backing or hibernation to a different value. `RouteObservation.OwnershipUnchanged` reads it on the pinned connection. A change made and undone moves it twice. | `registry/registry.go` `migrateRouteOwnership`, `registry/observe.go` | `TestOwnershipWitnessMovesOnlyWhenOwnershipChanges` (ten writes that move it, eight that do not, litestream's bookkeeping among them), `TestMeteredFleetOpenedReplicatesCountsAndStops` (litestream really writing its tables into the registry it replicates) | |
| +| Opus P1, Astra P2: the witness is moved by the replicator's own writes and by every unrelated commit | Fixed. The registry keeps an ownership generation, a counter moved by triggers when a route is added, removed, or changes host, account, app, database path, backing or hibernation to a different value. `RouteObservation.OwnershipUnchanged` reads it on the pinned connection. A change made and undone moves it twice. | `registry/registry.go` `migrateRouteOwnership`, `registry/observe.go` | `TestOwnershipWitnessMovesOnTheColumnsItReads` (ten writes that move it, eight that do not, litestream's bookkeeping among them), `TestMeteredFleetOpenedReplicatesCountsAndStops` (litestream really writing its tables into the registry it replicates) | |
| | Astra P2: replacing the registry file leaves the witness unchanged | Fixed for a replacement that stands. The observation records which file the path named and answers changed when it names another. Discovery and attribution now work from the witnessed rows, not a second read. A path swapped away and back between two looks is not told apart. | `registry/observe.go` | `TestOwnershipWitnessSeesTheRegistryReplaced` | |
| | Astra P1: requests crossing a change stay with the old owner and nobody else is told; invalidation waits for another request | Changed, not closed. A request sent before the change is the old owner's by rule. The ones in doubt were admitted before the change and sent after it: when the fleet goes stale it tells every recorder it was given that each day from a minute before the last proof to now is incomplete, a watch asks the witness every second whether or not anyone makes a request, and the fleet built to replace it tells its own recorders the same days, which reaches a new owner. Still open: a process that dies between the change and the rebuild takes the hold with it. The started rows it leaves are held by the ledger; the new owner's recorder is told nothing. | `ops/litestream_metered.go` `goStale`, `holdDays`, `watch`, `Replaces` | `TestMeteredFleetWatchHoldsEveryRecorderWithoutARequest`, `TestStaleFleetHoldsEveryDayInDoubt`, `TestMeteredFleetReplacementInheritsTheHold` | |
| | Both, P1: `Close` left the store running | Fixed in the previous commit; now shown on an opened store. Closing a current fleet lets its last sync through the gate and holds nobody's day. Closing a stale one tells litestream to try its last sync once, not for its whole retry window. A replacement does not open while the fleet it replaces is open. | `ops/litestream_metered.go` `Close`, `Open` | `TestMeteredFleetCloseStopsTheStore`, `TestMeteredFleetOpenedReplicatesCountsAndStops` | |
| @@ -265,7 +265,7 @@ Reports: `OPUS-REVIEW-metered-replicator-review12.md`, `CODEX-FINDINGS-metered-r |
| | Finding | Disposition | Where | Reproduction | |
| | --- | --- | --- | --- | |
| -| Opus P3-1, Astra 5 (P2): the link entry was not a safe acceptance key or encoding. | Fixed in the thirteenth build as `path -> "target"` with the target quoted, `Accept` comparing a link entry as written and a plain path as cleaned [the fourteenth build made it `link "path" -> "target"`, both halves quoted; the fifteenth build's Accept fixture (`sub/live.sqlite` against `sub/./live.sqlite`) is the one that tells as-written from cleaned, the fourteenth's `x/..` case did not]. [Review thirteen: a regular file whose own name spelt the pair read as the accepted link entry; the fourteenth build quotes both halves behind the word `link`, which no absolute path begins with, so the encoding is one to one.] Relative targets are rendered as `Readlink` gives them (the entry names the spelling; from the sixteenth build the classification stats that spelling concatenated to the link's directory, uncleaned, which is the kernel's resolution of it). | `litestream_owners.go` `linkEntry`, `isLinkEntry`; `litestream_metered.go` `MeteredFleetStore` | `TestAcceptComparesALinkEntryAsWritten` (the pair accepted builds the store; the link re-pointed to the same file through `x/..` refuses it until the new pair is accepted); `TestStrictTreeNamesALinkToAFile` (a target spelt through `..` is named as written); each fails with the target cleaned, or with link entries cleaned by `Accept` (mutation runs at the time; from the fifteenth build the as-written case is `sub/./live.sqlite`) | |
| +| Opus P3-1, Astra 5 (P2): the link entry was not a safe acceptance key or encoding. | Fixed in the thirteenth build as `path -> "target"` with the target quoted, `Accept` comparing a link entry as written and a plain path as cleaned [the fourteenth build made it `link "path" -> "target"`, both halves quoted; the fifteenth build's Accept fixture (`sub/live.sqlite` against `sub/./live.sqlite`) is the one that tells as-written from cleaned, the fourteenth's `x/..` case did not]. [Review thirteen: a regular file whose own name spelt the pair read as the accepted link entry; the fourteenth build quotes both halves behind the word `link`, which no absolute path begins with, so the encoding is one to one.] Relative targets are rendered as `Readlink` gives them (the entry names the spelling; from the sixteenth build the classification stats that spelling concatenated to the link's directory, uncleaned, which is the kernel's resolution of it in this process's mount namespace [the seventeenth build hands the kernel the spelling alone against a descriptor for the link's directory, with `fstatat`; the eighteenth opens that directory as a directory and nothing else, and the directory is still resolved again from its path: review seventeen]). | `litestream_owners.go` `linkEntry`, `isLinkEntry`; `litestream_metered.go` `MeteredFleetStore` | `TestAcceptComparesALinkEntryAsWritten` (the pair accepted builds the store; the link re-pointed to the same file through `x/..` refuses it until the new pair is accepted); `TestStrictTreeNamesALinkToAFile` (a target spelt through `..` is named as written); each fails with the target cleaned, or with link entries cleaned by `Accept` (mutation runs at the time; from the fifteenth build the as-written case is `sub/./live.sqlite`) | |
| | Opus P3-2: the checks on one file were not required to see one inode. | Fixed: the facts kept are the first check's, and a later check whose `Dev`/`Ino` differ refuses the file ("changed under the look"). No deterministic test: the case is a rename between two checks of one inventory; stated. | `litestream_owners.go` `fleetInventory` | | |
| | Opus P3-3, Astra 11: the every-claim test could not tell "every check" from "the tree check only", and its comment called the needed layout impossible; the root/platform test is vacuous as root; the sealed-link case is skipped as root. | Fixed: the layout exists (the tree's user's file in another user's directory inside the tree: the tree check accepts, the file check refuses the directory) and is the test's second case; the root/platform test skips as root with a message. The sealed case stays skipped as root (root can look at anything), stated. | tests | `TestEveryClaimOnAFileMustPass`; fails with tree checks alone whenever a tree claim exists (mutation run) | |
| | Opus P3-4: a `Readlink` error was dropped. | Fixed: ENOENT and EINVAL (no longer a link) are churn; any other error is a problem. | `litestream_owners.go` `strictTreeDBs` | no deterministic test (a link that cannot be read by this process while its directory can be listed is not arranged); stated | |
| @@ -315,7 +315,7 @@ Reports: `OPUS-REVIEW-metered-replicator-review15.md`, `CODEX-FINDINGS-metered-r |
| | 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. |
| +Astra 1 to 4, 6 to 11 stay open as before (the numbering here omitted 7 until review seventeen named it). 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) |
| @@ -332,15 +332,17 @@ Reports: `OPUS-REVIEW-metered-replicator-review16.md`, `CODEX-FINDINGS-metered-r |
| The seventeenth build's claim here that its package tests passed under `/tmp` was false: the long-target fixture it added passed only under a temp root of about 34 bytes or more (the author's `~/.cache/tmp/claude-1001`), and the full gate on `bffc76b5` (`/var/tmp/ck`) and Amadan CI on the same tip both failed on that fixture's own guard, as review seventeen (Opus P2-2) had worked out from the arithmetic. Corrected in the eighteenth build, below. |
| -## Review seventeen (reviewed `bffc76b5`; Opus accepted with fixes; Astra's report pending at the time of the eighteenth build) |
| +## Review seventeen (reviewed `bffc76b5`; Opus accepted with fixes, Astra rejected) |
| -Report: `OPUS-REVIEW-metered-replicator-review17.md` (Astra's is copied in when it lands, and answered in the next section if it names anything new). Opus found review sixteen's findings closed, Astra 5 closed in behaviour and the render parts of Astra 11 closed, and two new defects in the fix for Astra 5. |
| +Reports: `OPUS-REVIEW-metered-replicator-review17.md`, `CODEX-FINDINGS-metered-replicator-review17.md`. Both found review sixteen's findings closed, Astra 5 closed in behaviour and the render parts of Astra 11 closed, and the same two new defects in the fix for Astra 5 (Opus P2-1 and P2-2 are Astra's findings 5 and 6). Astra rejects on the deep items (its 1 to 4 at P1, 7 to 10 at P2, 11 at P3), open as before and listed as blocking wiring; its finding 12 (P3) names assertions that survive removing what they describe. |
| | Finding | Disposition | Where | Reproduction | |
| | --- | --- | --- | --- | |
| | Opus P2-1: `statTarget` opened the link's directory by path with Go's default flags: a FIFO a tenant swaps into the directory's place blocks the open until a writer appears (the watch goroutine hangs, Close waits for ever at `watching.Wait()`, `chainClosed` never lets the replacement open), and a device there would have had its driver's open run. | Fixed: the directory is opened with `O_RDONLY\|O_DIRECTORY\|O_NONBLOCK\|O_NOCTTY\|O_CLOEXEC`; anything but a directory is refused by the open itself with ENOTDIR, before any driver's open, and nothing can block on it or become its controlling terminal. `O_NOFOLLOW` is not added: the tree's own root is reached through systemd's state link. The descriptor is still a fresh open by path, not the one the walk listed from (EE2 stays open, stated in the code). | `linktarget_unix.go` `statTarget` | `TestStatTargetRefusesADirectorySwappedForAFIFO`: with Go's default flags the test fails after five seconds ("the open blocked"); with `O_DIRECTORY` alone dropped it fails because the refusal came from the stat through the handed-back descriptor, not the open (the test asserts the `PathError`'s op is `open`). | |
| | Opus P2-2: the long-target fixture's guard held only under a long temp root; the gate under `/var/tmp/ck` and CI failed on it. | Fixed: the target is sized from the directory the run's temp root gives (`n := (4097-len(deep))/2 + 1`), so the concatenation outgrows the limit under any root while the target alone stays under 4,095 bytes; the guard checks both. | `litestream_owners_test.go` | With the fixed 1,500 repetitions put back, the test fails under `/var/tmp/ck` ("the concatenation is 4074 bytes"); with the fix it passes under `/var/tmp/ck`, `~/.cache/tmp` and the author's root. | |
| -| Opus P3-1: wording wider than the code ("a descriptor for that directory"; "never a concatenation that could outgrow PATH_MAX"; this file's "pass under `/tmp`"). | Corrected: the comment says the directory is resolved again from its path at that moment and a link in its last component is followed; that the argument handed to the kernel is the target alone, the target itself bounded by what a link can hold; and the review-sixteen claim is marked false above. | `linktarget_unix.go`, `litestream_owners.go`, this file | | |
| +| Opus P3-1, Astra claims: wording wider than the code ("a descriptor for that directory", "the directory the link is in"; "never a concatenation that could outgrow PATH_MAX"; this file's "pass under `/tmp`"). | Corrected: the comment says the directory is resolved again from its path at that moment and a link in its last component is followed; that the argument handed to the kernel is the target alone, the target itself bounded by what a link can hold; and the review-sixteen claim is marked false above. | `linktarget_unix.go`, `litestream_owners.go`, this file | | |
| +| Astra 12 (P3): assertions that survive removing what they describe (shutdown's handles and workers, the watch's `lookAgain`, admission's pre-witness check, the latch's recheck, the shadow state's data-directory check, the inode comparisons, render's section separators, the 300 ms negative Close check, the clean-start test's clock; no fixture for the directory-substitution window). | Two closed this build: the directory-substitution window has `TestStatTargetRefusesADirectorySwappedForAFIFO`, and `render` is compared across sections (the same entry in two different lists, and a list entry spelt like the separator, render differently). The rest stay open as named: each needs a controlled interleaving or a fake the package does not have yet, and none is closed by a claim here. | `litestream_metered_test.go` `TestRenderQuotesEveryEntry`, `linktarget_unix_test.go` | | |
| +| Astra claims: the Accept fixture's comment still said "through a `..`" (the fixture is `sub/./live.sqlite`); this file's review-thirteen row described the sixteenth build's concatenation as current; the review-sixteen open-item list omitted Astra 7; `litestream_owners.go`'s "a tenant's file in a directory the platform owns is refused" was wider than the comparison (which is against the chain's root, not the file's immediate directory); `TestOwnershipWitnessMovesOnTheColumnsItReads` was named for a narrower property than it tests; `OUTSTANDING.md` 1 described a third build. | Corrected in place: the fixture comment, the row (bracketed with what the seventeenth and eighteenth builds do), the numbering, the owner-check comment (what it compares and the case it does not decide), the test's name (`TestOwnershipWitnessMovesOnTheColumnsItReads`, with its comment), and the outstanding item (on this branch and, in the same words, on the launch branch where it is maintained). | this file, `litestream_metered_test.go`, `litestream_owners.go`, `internal/registry/observe_ownership_test.go`, `OUTSTANDING.md` | | |
| | Claims the code contradicts: "a descriptor for the link's directory" (corrected as above); whether CI runs as root is not established by committed source (open, as before). | | | | |
| | Astra 1 to 4, 6 to 10, approval H's uid binding, the box-wide refusal decision: open as before; Astra 6 has a second instance closed by P2-1's fix and its first (the walk by path) open. | | | | |
diff --git a/implementation/carloku-paid-launch/OUTSTANDING.md b/implementation/carloku-paid-launch/OUTSTANDING.md| index fe90ff0b1..d598f0288 100644 |
| --- a/implementation/carloku-paid-launch/OUTSTANDING.md |
| +++ b/implementation/carloku-paid-launch/OUTSTANDING.md |
| @@ -32,7 +32,7 @@ Each is concrete. None is waived. |
| ### Meters (the complete metered model cannot invoice until both are proven) |
| -1. **Replication writes (`durability_writes_1k`) cannot be proven on any box.** The external replicator writes outside the ledger. Path A (scoped attestation) was built, reviewed in round six, and cannot prove an exclusive writer; it is off this branch and parked on `feat/carloku-scoped-attestation-20261001`. Path B (the platform's own replicator as the only writer) is **partial, unwired, rejected at both of its reviews**, on `feat/carloku-metered-replicator-20261001` (`9480fe3f`, on the forge; the full local gate passes on it). Its second review found the ownership fence built after the first did not hold: a refused request was still sent, and the registry witness was moved by the replicator's own writes. A third build, not yet reviewed, makes the two changes that needed: a request gate in `internal/lease` that stops an attempt before it is sent, and an ownership generation in the registry that only ownership moves; with them a fleet that holds every recorder's days in doubt and watches without a request, one strict inventory with file checks (one name, ordinary siblings, no link or mount, the tree owner's, under a platform directory) and a second look on a timer, and shadow state made private before litestream can make it. Still open there: a swap after a look and before litestream's next open by path (needs litestream to open beneath a pinned root), a process dying between a change and the rebuild, the prefix-lineage and shrink guards, and that any tenant can stop a rebuild for the box. `DISPOSITIONS-metered-replicator.md` on that branch has every finding and what each build did about it. What is there: a litestream store whose every replica client is metered with no unmetered fallback, an owner for each database the box replicates, and a fleet store that refuses anything it cannot meter, each with tests. **Absent**: the agent wiring that binds those meters to the box's ledger, a restore path that does not shell out to the external binary, the unit change, moving the replica credentials so nothing else on the box holds them (the unit runs as the same user as the agent today), and any attestation. Until all of that exists and is approved for a box, this meter is incomplete every month and nothing is invoiced. |
| +1. **Replication writes (`durability_writes_1k`) cannot be proven on any box.** The external replicator writes outside the ledger. Path A (scoped attestation) was built, reviewed in round six, and cannot prove an exclusive writer; it is off this branch and parked on `feat/carloku-scoped-attestation-20261001`. Path B (the platform's own replicator as the only writer) is **partial, unwired, and has not had a whole-branch pass from both reviewers** (eighteen builds; Opus accepts each with fixes, Astra rejects each on the deep items), on `feat/carloku-metered-replicator-20261001` (the forge has the tip; `READINESS.md` names it with its gate and CI state). What is built there: a request gate in `internal/lease` that stops an attempt before it is sent; an ownership generation in the registry moved by the six route columns the replicator reads (not by its own bookkeeping writes); a fleet that holds every recorder's days in doubt and watches without a request; one strict inventory with file checks (one name, ordinary siblings, a link named with its target as written and classified by that spelling through a directory-only descriptor, no mount, the chain root's owner, under a platform directory) and a second look on a timer; and shadow state made private before litestream can make it. Still open there, each named in its dispositions as blocking wiring: litestream's own opens by path after a look (needs litestream to open beneath a pinned root), replaceable ancestry above a protected directory, an admitted request delayed past the margin, a process dying between a change and the rebuild (the doubt is in memory only), device effects through an ancestor swap, hibernating aliases and claims on skipped files, required coverage that disappears without an entry, that any tenant can stop a rebuild for the box, route identity unbound to filesystem authority, and the uid binding under approval H. `DISPOSITIONS-metered-replicator.md` on that branch has every finding and what each build did about it. What is there: a litestream store whose every replica client is metered with no unmetered fallback, an owner for each database the box replicates, and a fleet store that refuses anything it cannot meter, each with tests. **Absent**: the agent wiring that binds those meters to the box's ledger, a restore path that does not shell out to the external binary, the unit change, moving the replica credentials so nothing else on the box holds them (the unit runs as the same user as the agent today), and any attestation. Until all of that exists and is approved for a box, this meter is incomplete every month and nothing is invoiced. |
| 2. **Store transfer (`store_egress_gb`) has no source.** No Cost and Usage Report export exists on the account. It needs the scoped infrastructure approval, a first real delivery, an IAM principal inventory in the AWS provisioner (not built), and the bucket ownership attestation for stores that already exist (designed, not built). |
| 3. **A store's partial first hour** with a charged line holds the month, and nothing releases it. It holds by default and nobody is asked to waive it. What can release it is evidence: an operator door that records, for one bucket-hour, who is shown to have owned it and on what evidence, which the fold then honours. **Partial, unwired into this branch**, on `feat/carloku-bucket-hour-evidence-20261001` (`b50f29bc`; code at `58c1870c`): the record, the fold change and three operator API doors, with tests and mutation checks. It accepts only the hour a complete ledger began vouching in and only the owner that ledger names. The full local gate passes on it. First review: Astra rejected it, Opus accepted with fixes. Both confirmed that through the fold it releases nothing but that hour for that owner. Both also found that the evidence is free text that the same operator records and that stands at once, which in practice is a waiver: what counts as evidence, and whether a second person must approve it, needs Paul's ruling (approval J) before more is built. Nothing fixed yet (`DISPOSITIONS-bucket-hour-evidence.md` on that branch). No CI run, no CLI verb, and its operator-facing sentences are unreviewed. |
| 4. **A box's first minutes** before its ledger opened hold that day for everyone. Same as item 3: it holds by default, and the release is an operator evidence door, not a waiver. **Absent**. |
diff --git a/internal/ops/litestream_metered_test.go b/internal/ops/litestream_metered_test.go| index 4760fdca7..c2f161b24 100644 |
| --- a/internal/ops/litestream_metered_test.go |
| +++ b/internal/ops/litestream_metered_test.go |
| @@ -1857,8 +1857,8 @@ func TestCloseHoldsWhenItsLastLookCannotBeTaken(t *testing.T) { |
| // Accept compares a link entry as written: the pair the operator accepted |
| // is the pair that must be there, and a target re-pointed to spell the |
| -// same file through a ".." (which cleaning would fold away) is not it. |
| -// (Review twelve, Opus P3-1.) |
| +// same file through a "./" (which cleaning would fold away) is not it. |
| +// (Review twelve, Opus P3-1; the fixture is the fifteenth build's.) |
| func TestAcceptComparesALinkEntryAsWritten(t *testing.T) { |
| newFakeBucket(t) |
| dir := platformDir(t) |
| @@ -1964,4 +1964,17 @@ func TestRenderQuotesEveryEntry(t *testing.T) { |
| t.Fatalf("a list is not quoted:\n%s", pair[0].render()) |
| } |
| } |
| + // The sections are told apart: the same entry in another list, and an |
| + // entry spelt like the separator, each render differently. (Review |
| + // seventeen, Astra 12.) |
| + for _, pair := range [][2]FleetInventory{ |
| + {{Unowned: []string{"x"}}, {Skipped: []string{"x"}}}, |
| + {{Skipped: []string{"x"}}, {Problems: []string{"x"}}}, |
| + {{Unowned: []string{"x", "--"}}, {Unowned: []string{"x"}, Skipped: []string{"--"}}}, |
| + {{Unowned: []string{"--"}}, {}}, |
| + } { |
| + if pair[0].render() == pair[1].render() { |
| + t.Fatalf("two sections render alike:\n%s", pair[0].render()) |
| + } |
| + } |
| } |
diff --git a/internal/ops/litestream_owners.go b/internal/ops/litestream_owners.go| index 5832e07cc..bb134b774 100644 |
| --- a/internal/ops/litestream_owners.go |
| +++ b/internal/ops/litestream_owners.go |
| @@ -550,12 +550,16 @@ func checkOwnedFile(root, path string, kind claimKind) (fileFacts, string) { |
| if facts.Nlink != 1 { |
| return fileFacts{}, "it has more than one name (a hard link)" |
| } |
| - // The file's owner must be the chain's, except that root and the |
| - // platform's user are one owner here as they are in platformOwned (a |
| - // root-owned file in the platform user's directory, or the reverse, |
| - // is the platform's either way). A tenant's file in a directory the |
| - // platform owns is refused by this: whether such a file is billable |
| - // to the tenant is not decided, and until it is, it is not. |
| + // The file's owner must be the chain's root's, except that root and |
| + // the platform's user are one owner here as they are in platformOwned |
| + // (a root-owned file in the platform user's directory, or the reverse, |
| + // is the platform's either way). The comparison is with the chain's |
| + // root, which the climb chose, not with the file's immediate |
| + // directory: a tenant's file directly under a root the platform owns |
| + // is refused by this, while a platform-owned directory lower in a |
| + // tenant's own chain is passed through by the climb and this does not |
| + // see it. Whether a file in the first case is billable to the tenant |
| + // is not decided, and until it is, it is not. |
| if (kind == tenantTree || kind == tenantFile) && facts.UID != rootFacts.UID && !(platformUID(facts.UID) && platformUID(rootFacts.UID)) { |
| return fileFacts{}, "it does not belong to the user its tenant's directory belongs to" |
| } |
diff --git a/internal/registry/observe_ownership_test.go b/internal/registry/observe_ownership_test.go| index e8f21ddaa..c7e1487c4 100644 |
| --- a/internal/registry/observe_ownership_test.go |
| +++ b/internal/registry/observe_ownership_test.go |
| @@ -9,12 +9,14 @@ import ( |
| ) |
| // 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. |
| -func TestOwnershipWitnessMovesOnlyWhenOwnershipChanges(t *testing.T) { |
| +// one of the six columns the replicator reads changes value (some of |
| +// those moves change no database's owner: a host rename, say; the witness |
| +// moves on them too, by design), and for nothing else. Every row of the |
| +// second table is a commit that the whole-database witness (Unchanged) |
| +// reports and that touches none of those columns: the writes a live box |
| +// makes all day, and the bookkeeping a replicator writes into the |
| +// registry it replicates. |
| +func TestOwnershipWitnessMovesOnTheColumnsItReads(t *testing.T) { |
| const host = "t.example.com" |
| moves := map[string]func(t *testing.T, w *Registry){ |
| "the route moves to another account": func(t *testing.T, w *Registry) { |