amadan
Twentieth replicator build: a tree claim refuses a tenant's file in a platform directory too; the fixture's guard reachable; the no-recorder refusal quoted
Review nineteen (Opus accept; Astra reject on the deep items). The walk keeps the owner of the file's immediate directory so a tree claim refuses what a file claim refuses; the long-target fixture checks the file's own path before making anything; the new test's last check asserts no refusal; the no-recorder message quotes the path; the swapped-directory comment says what a swap can change. Dispositions separate these bounded changes from the deferred items that block wiring. One mutant killed; ops and registry pass under /var/tmp/ck and ~/.cache/tmp.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
8 files changed,
+379
−24
docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review19.md+209 −0docs/superpowers/specs/DISPOSITIONS-metered-replicator.md+21 −4docs/superpowers/specs/OPUS-REVIEW-metered-replicator-review19.md+111 −0internal/ops/linktarget_unix.go+3 −2internal/ops/litestream_metered.go+1 −1internal/ops/litestream_owners.go+16 −4internal/ops/litestream_owners_test.go+8 −7internal/ops/litestream_owners_unix_test.go+10 −6
CI passed — run details
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review19.md b/docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review19.md| new file mode 100644 |
| index 000000000..76cadc43e |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-FINDINGS-metered-replicator-review19.md |
| @@ -0,0 +1,209 @@ |
| +| Review-sixteen finding | Closed at `53e3dcd8`? | Would its test pass without the fix? | |
| +|---|---|---| |
| +| **Opus P3-1 — fleet versus tenant mount namespace** | **Closed as a wording request.** Namespace divergence remains explicitly open. | Wording only; no behavioral test. | |
| +| **Opus T-1 — aliased tree root** | **Closed.** The fixture includes `state -> private/state`, with `cur -> ../d`. | **No** with lexical joining; **yes** with the previous uncleaned concatenation, which handles this case correctly. | |
| +| **Opus T-1 — rendering coverage** | **Closed for the reported mutations.** Owner fields, `Platform`, individual lists and section boundaries have assertions. | **No** with the raw owner triple, omitted `Platform`, raw list entries or removed section separators. | |
| +| **Opus claims** | Namespace wording, fixture descriptions and historical qualifications are corrected. New overstatements remain below. | Prose changes have no behavioral test. | |
| +| **Astra 1 — Litestream pathname opens** | **Open.** | Existing tests permit the defect; none controls substitution at Litestream’s actual open. | |
| +| **Astra 2 — replaceable ancestry** | **Open.** | Existing endpoint checks and fixtures do not establish protected ancestry. | |
| +| **Astra 3 — delayed admission escapes holds** | **Open.** | No test suspends an admitted attempt beyond the margin across midnight. | |
| +| **Astra 4 — process exit loses uncertainty** | **Open.** | No durable handover or recovery test. | |
| +| **Astra 5 — concatenation exceeds `PATH_MAX`** | **Closed on supported Unix platforms with stable directories.** The revised fixture also fixes the reported short-root, 3,200-byte-root and XFS-target cases. | **No** when restoring concatenation, provided fixture construction succeeds. The new budget diagnostic has a limitation below. | |
| +| **Astra 6 — header-open device effects** | **Open.** The separate directory-open FIFO/device defect is closed. | Header tests permit ancestor substitution. The directory FIFO test fails without `O_DIRECTORY`. | |
| +| **Astra 7 — hibernating aliases and skipped-file claims** | **Open.** | No distinguishing identity-based claimant 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 expect this 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.** Rendering and directory-open refusal are now distinguished. Shutdown, watch, interleaving and timing gaps remain. | **No** for the repaired mutations; **yes** for the surviving counterfactuals below. | |
| +| **Astra claims** | Witness wording, rendering, historical numbering, the Accept fixture description and inventory-list quoting are corrected. Some broader claims still exceed the implementation. | Author-reported test and mutation results were not independently executed. | |
| + |
| +All locations below refer to **`53e3dcd8`**. This is a source-only review against `72b4f59a`, including the nineteenth-build diff from `6249af63`. No files were changed, tests run, or network commands issued. Counterfactual test results are deductions from source. |
| + |
| +The prompt’s `*review19.md` reports are absent at this commit. The dispositions correctly reference `*review18.md`; I read those and the review-sixteen reports. |
| + |
| +**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, then restores the original pathname before the next look. Litestream can retain substituted handles while subsequent inventories—including Close’s final look—see the original inode. Initialization enables WAL and writes bookkeeping tables. Later WAL opens independently follow mutable paths. |
| + |
| +The open item includes initialization effects, retained handles and subsequent database/sidecar opens. It is larger than “a swap during the first open”; the current owner-check comment correctly describes that larger scope. |
| + |
| +**Smallest acceptable fix:** constrain the actual database, SQLite and sidecar opens through verified, pinned directory authority, including subsequent opens. Checking identity after an unrestricted open cannot undo its effects. |
| + |
| +**2. P1 — Protected directories can still have tenant-replaceable ancestors.** |
| + |
| +**Locations:** `internal/ops/litestream_owners.go:520`, `:592`, `:828`, `:856`; `internal/ops/litestream_metered.go:988`. |
| + |
| +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 containing an approved platform link. The nineteenth-build start check does not affect this example: A’s file starts in A’s directory. |
| + |
| +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. Endpoint ownership and modes are insufficient. |
| + |
| +**3. P1 — An admitted request delayed beyond the margin escapes the inherited hold.** |
| + |
| +**Locations:** `internal/ops/litestream_metered.go:240`, `:325`, `:789`; `internal/lease/requestgate.go:58`. |
| + |
| +A concrete UTC sequence: |
| + |
| +1. A admits an attempt on October 2 at **23:58:00**, then pauses before recording. |
| +2. Ownership changes at **23:58:10**. A detects it and holds through **23:59:10**. |
| +3. B is built for the new owner and closes unopened while A remains live. |
| +4. A’s admitted attempt resumes, records and completes at **00:02 on October 3**. |
| +5. A closes. Already stale, it skips the final checks and does not extend its window. |
| +6. C replaces B. B and C inherit A’s window correctly, but **October 3 is absent from their ownership holds**. |
| + |
| +`goStaleFrom` correctly takes the earlier lower bound. Passing a window through unopened fleets preserves that window; it cannot supply 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`, `:834`. |
| + |
| +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 the completed attempt. |
| + |
| +This includes routine restarts, not only crashes. It also includes inheritance awaiting delivery to successor 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 — Directory-only opening still permits classification against a substituted directory.** |
| + |
| +**Locations:** `internal/ops/litestream_owners.go:659`, `:688`, `:707`, `:714`; `internal/ops/linktarget_unix.go:45`. |
| + |
| +The walker lists `D/cache` and captures `current`’s relative target. Before `statTarget` opens `D/cache`, the tenant replaces it with another directory—or a link to another directory—where that target is absent. `Fstatat` returns `ENOENT`; the non-database-named link disappears from that inventory. A replacement containing a regular file instead of the original directory can turn an unconditional problem into an acceptable skipped pair. |
| + |
| +A bind mount visible in the fleet’s namespace can produce the same mismatch; ordinary tenant mount authority inside a separate namespace is insufficient. |
| + |
| +`O_DIRECTORY` prevents the FIFO/device-open defect. It does not establish directory identity. The new comment acknowledges omission or misclassification, but “nothing more” understates the resulting build decision: missing an entry can let the replacement build without the acknowledgement otherwise required. |
| + |
| +**Smallest acceptable fix:** list, inspect entries, read links and classify captured targets through the same safely acquired directory descriptor. Resolve the approved systemd root link explicitly when acquiring that root. |
| + |
| +**6. P2 — Header inspection can still cause device-open effects before rejection.** |
| + |
| +**Location:** `internal/ops/sqliteopen_unix.go:26`. |
| + |
| +The walker observes regular file `T/d/watchdog`. T replaces `d` with a link to `/dev`; a privileged header 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` and `O_NOCTTY` address particular effects, not ancestor confinement. |
| + |
| +**Smallest acceptable fix:** constrain traversal through pinned directory descriptors before opening the file. Preserve the refusing unsupported-platform fallback. |
| + |
| +**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 spelling supplies no candidate, and textual claimant matching misses the shared inode. |
| + |
| +Two unit claims on a tree containing only skipped files, or competing claims on `.cache/app.db` or `main.sqlite`, also bypass listed-file ownership reconciliation. Two hibernating claims can bypass it altogether. |
| + |
| +Exact-spelling live/hibernating conflicts and a unit tree over a listed platform database are reported correctly. |
| + |
| +**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:648`, `:725`. |
| + |
| +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 changes can refuse replacement replication or hold every account.** |
| + |
| +**Locations:** `internal/ops/litestream_owners.go:712`, `:717`; `internal/ops/litestream_metered.go:660`, `:880`, `:906`. |
| + |
| +Installing packages or a virtual environment introduces ordinary file and directory links. File links require acceptance; directory links are unconditional problems. The current fleet reports drift, but its replacement refuses. A subsequent ownership change can leave every account waiting for that tenant’s inventory to be resolved. |
| + |
| +A legitimate concurrent replacement 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. |
| + |
| +**10. P3 — Route identity remains unbound to independently established filesystem authority.** |
| + |
| +**Locations:** `internal/ops/litestream_owners.go:176`, `:435`, `:503`, `:568`; `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 acceptance. |
| + |
| +Root-run units and tenants sharing the platform UID can likewise claim otherwise-unclaimed qualifying platform files. This is not acceptance “from anywhere”: directory modes, links, filesystem boundaries, regular-file status, link count and sidecars still constrain it. |
| + |
| +**Smallest acceptable fix:** bind route claims to independently established roots and identities, with explicit treatment of trusted platform workloads. Deployment approval H remains outside this review. |
| + |
| +**11. P3 — Several tests still permit removal of the behavior they describe.** |
| + |
| +**Locations:** `internal/ops/litestream_metered_test.go:689`, `:807`, `:1062`, `:1380`, `:1496`; `internal/ops/litestream_owners.go:194`, `:217`; `internal/ops/litestream_owners_unix_test.go:542`. |
| + |
| +Surviving counterfactuals: |
| + |
| +- Clearing the store pointer and refusing later requests does not establish that retained handles and workers stopped. Those shutdown assertions survive removing `store.Close`. |
| +- Removing the watch’s `lookAgain` call leaves the direct-call file tests intact. |
| +- 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. |
| +- Removing `prepareShadowState`’s data-directory check survives the fixture refused earlier by inventory. |
| +- Neither inode comparison has a controlled distinguishing interleaving. |
| +- The new platform-owned-file assertion only rejects an error containing `"the file is not"`. An unrelated refusal passes; it does not assert that the valid file remains accepted. |
| +- The 300 ms negative Close assertion can pass before the second goroutine enters Close. The clean-look-start test reconstructs time from Unix nanoseconds and depends on wall-clock stability. |
| + |
| +The opened-store test checks positive counts, but the asymmetric direct-client assertions at `litestream_metered_test.go:334` and `:337` **do catch swapped fleet recorders**. |
| + |
| +**Smallest acceptable fix:** assert retained-handle/worker shutdown, drive the watch’s file tick, add controlled interleavings, synchronize the second Close caller, and require `why == ""` for the new valid-file case. Use a controlled clock for boundary assertions. |
| + |
| +**12. P3 — The fixture’s no-budget guard is unreachable, and its final guard runs after creation.** |
| + |
| +**Location:** `internal/ops/litestream_owners_test.go:764`. |
| + |
| +The target is 999 bytes. The loop runs only while `len(deep) <= 3096`; inside it, the condition `len(deep)+201 >= 4000` therefore cannot hold. |
| + |
| +For ordinary roots, the loop finishes with directory length between 3097 and 3297. A 3,200-byte root needs no additions. Both fix the reported defect. |
| + |
| +For a valid resolved temporary directory of 4,085 bytes, however, the loop is skipped and `writeFile` attempts a 4,097-byte pathname before the final guard runs. The fixture fails during construction rather than through its stated budget check. |
| + |
| +**Smallest acceptable fix:** check the directory and child-path budgets before creating either child. Keep an explicit fixture failure when the supplied root leaves insufficient room. |
| + |
| +The remaining requested checks hold with these qualifications: |
| + |
| +| Area | Result at `53e3dcd8` | |
| +|---|---| |
| +| **Tenant-file chain — A/J/P/U/KK2** | The start check at `litestream_owners.go:475` now refuses a distinct tenant’s file in a platform-owned immediate directory in both reported layouts. Foreign tenant starting directories and stopping parents are refused. Every accepting `tenantFile` path reaches the top check at `:508`; earlier refusals need not. Tenant-made links are refused on stable paths. Higher ancestry remains finding 2. | |
| +| **Root and platform UID** | Only root and the process UID are equivalent at `:568`. A root/platform file directly inside a distinct tenant’s directory is refused at `:490`; the reverse mismatch is now refused at `:487`. Root-run units stop climbing at platform ownership and can accept qualifying platform files. Sharing the process UID also lets tenant-made links look platform-made; finding 10 remains. | |
| +| **Normal platform layouts — P/S/KK2** | The DynamicUser fixture at `litestream_owners_unix_test.go:85` resolves the platform state link to the dynamic user’s directory, so the new platform-directory refusal does not apply. Isolated-instance files use their own directory chain. Ordinary activator `dataDir/*.db` exec claims use `namedFile`; a unit claimant adds `tenantFile`. Protected operator-chosen platform files still pass. A custom tenant-owned file in a platform-owned immediate directory is deliberately refused by `tenantFile`. | |
| +| **Every claimant — S/W/Z** | `claimChecks` at `litestream_owners.go:278` retains each distinct root/kind, including two trees and tree-plus-file claims. All must pass. Keeping the first successful check’s facts is correct because later successful checks must match its device/inode. The kept-facts duplicate pass at `:217` closes the later duplicate window. Stable distinct files are not confused; legitimate concurrent changes can conservatively refuse and hold everyone. | |
| +| **Admission — B/I** | Checks at `litestream_metered.go:226` and `:240` refuse staleness encountered while waiting for the lock or during the witness read. Staleness after the final atomic load can still precede return/dispatch; it remains the admitted-request gap, not synchronization with ownership writers. | |
| +| **Witness retries — B/I** | `witnessUnchanged` at `:257` makes at most three reads, accepts recovered errors, stops immediately on a witnessed change and treats exhausted errors as uncertainty. Caller cancellation does not become an ownership change. Three five-second contexts and two delays do **not** impose a hard bound on mutex waiting or filesystem `Stat`. | |
| +| **Registry generation — C/II2** | `registry.go:569`, `:602`, `:612` cover the six consumed columns, inserts/deletes and rowid immutability. Older-build raw statements still fire the triggers. Same-value updates, unrelated columns and Litestream bookkeeping do not move it. Database-less insertions/deletions and semantically irrelevant changes to the six columns still invalidate conservatively. The new host-only raw update at `observe_ownership_test.go:32` closes that test gap. | |
| +| **Replacement chain — 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 use it. Within the documented single-chain contract, a replacement cannot open beside an open/draining predecessor, and an unopened successor cannot latch through one. Inheritance is applied once per fleet, including intermediate recorders that must outlive Close. Reading a closed stale predecessor’s saved window is intentional. | |
| +| **Close’s final look — M** | Order is store stopped (`:763`) → watch stopped (`:767`) → file look (`:790`) → witness (`:793`) → inheritance (`:799`) → closed (`:800`). A persistent changed/missing owned file on a current fleet holds from its last clean look. Already-stale fleets skip the checks, leaving finding 3. Harmless changes after replication stops can over-hold; filesystem operations can block Close. `closeOnce` prevents a second caller from publishing closure while draining. | |
| +| **Lineage — E** | `ownedReplicaKey` at `:965` 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 previous lineage; changing app within one account does not create a new key. Separation from an external replicator’s prefix remains unenforced. | |
| +| **Strict walk — F/K/L/N** | 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, so a linked base cannot suppress inspection. Unreadable directories are problems. Missing/relative unit claims, units without `DBPath`, empty explicit claims and discovered files failing their final look are reported. Findings 7–8 bound completeness. | |
| +| **Explicit claims and external config — N/P/Q** | Live explicit exec claims outside the globs receive ownership checks. The external config path still passes a nil strict walker and retains its previous discovery behavior. Hibernating exec claims outside the candidate set remain absent by design; aliases and skipped-file conflicts remain finding 7. | |
| +| **Leading `..` — K** | `walkWithoutLinks` at `litestream_owners.go:770` permits systemd’s leading `../private/...` shape and rejects ascents after named components. It does not clean away intervening links. Stable accepted paths resolve consistently within its checked assumptions; subsequent substitution and higher ancestry remain separate gaps. | |
| +| **Captured targets — R/BB2/CC2/EE2/GG2** | `Fstatat(fd, dest, …, 0)` follows middle links and resolves `..` physically from the opened directory, including an aliased tree root. Absolute targets use `Stat` unchanged. The second `Readlink` is gone. Non-`ENOENT` target failures report the original path and quoted captured target; ordinary dangling non-database links remain omitted. Finding 5 limits directory identity. | |
| +| **Unsupported platforms — K/EE2** | `linktarget_other.go:17` retains concatenation, but unsupported ownership facts and `sqliteopen_other.go:13` refuse the fleet/header read. This fallback does not enable unrestricted header opening on those platforms. | |
| +| **Link entries and Accept — T/V/Y/DD2/MM2** | `linkEntry` at `litestream_owners.go:901` distinguishes absolute plain paths and every path/target pair. Relative targets retain literal `Readlink` spelling. Cleaning a plain absolute path cannot forge the prefixed entry. Unchanged accepted pairs remain accepted while classification stays unchanged; target identity is not pinned. The flat impostor fixture distinguishes the old encoding. | |
| +| **Rendering — AA2/FF2/II2** | `litestream_metered.go:679`, `:686` quote strings, include `Platform` and preserve sections. Reported inventory contents are unambiguous, with list order normalized. Private inode facts are intentionally excluded and compared separately. Production compares rendering at `:666` and `:956`; it does not parse it or use it for Accept. | |
| +| **Quoted refusal lists — MM2** | For fleet-generated entries, `quotedList` at `:697` keeps plain entries quoted and genuine link pairs unambiguous. Tenant filenames cannot begin the link-entry prefix because their inventory paths are absolute. These messages are diagnostics, not acceptance keys. Other raw error paths remain below. | |
| +| **FIFO regression — GG2** | `linktarget_unix_test.go:37` requires `ENOTDIR` in an `open` `PathError`. Restoring the blocking default open times out; removing only `O_DIRECTORY` produces a stat-stage refusal and fails. The fixture establishes the open’s refusal, not directory identity across listing and reopening. | |
| +| **Fixture portability — H/L/X/HH2/LL2** | `platformDir` resolves linked temporary roots; explicit chmods preserve intended modes under umask `077`. The 999-byte target addresses XFS’s reported limit. When construction succeeds, the length guard and skipped-link assertion distinguish concatenation. Finding 12 describes the remaining budget diagnostic defect. Basename-based fake ownership and fixed fake UIDs remain assumptions. | |
| +| **Root and CI — X** | Root skips unreadable-directory/target cases and the root-versus-platform-user distinction. Fake-owner tests still exercise distinct UIDs through `factsOf`. `.amadan/ci.d/10-gate:52` and `Makefile:18` establish the commands, not CI’s UID. | |
| +| **Other open items and reports — G/O** | One same-filesystem bind alias remains undetected; two inventoried aliases are rejected by identity. Mount visibility in the fleet’s namespace matters. External-inventory comparison remains absent. Both reviewers’ review-eight, review-nine and review-seventeen reports are present. | |
| + |
| +**Claims the code contradicts** |
| + |
| +- **“A tenant’s file in a platform-owned directory never reaches here.”** `litestream_owners.go:565` is true for `tenantFile`, not for the shared comparison. A `tenantTree` rooted at tenant-owned `/srv/T` accepts T’s `bridge/a.db` beneath root-owned `bridge`: it never executes the start check. The same unqualified claim remains in dispositions `:255` and the new test’s introduction at `litestream_owners_unix_test.go:518`. Scope the wording to `tenantFile`; if the refusal is intended as universal policy, apply it to tree claims too. |
| + |
| +- **“A plain entry is quoted in every refusal message.”** Dispositions `:360` is broader than the change. Inventory lists are fixed, but the no-recorder refusal at `litestream_metered.go:938` still formats `owned.Path` with `%s`. An owned filename containing a newline can still split that diagnostic. Quote that path or narrow the claim to inventory-list refusals. This is display ambiguity, not an Accept bypass. |
| + |
| +- **The fixture’s explicit no-budget diagnostic.** `litestream_owners_test.go:766` cannot execute under its enclosing loop condition. Near-limit roots fail before the final guard, as finding 12 describes. |
| + |
| +- **Directory substitution “and nothing more.”** `linktarget_unix.go:26` correctly limits the direct operation to classification, but omitted or weakened reports can change whether construction refuses or succeeds. No database is directly streamed through that classified link; the inventory decision can nevertheless be wrong. |
| + |
| +- **Per-database attribution remains listed too broadly as untested.** Dispositions `:256` still carries that open item without distinguishing the live-replication test from the direct-client test. The latter already rejects swapped fleet recorders. |
| + |
| +- **Verification claims remain author reports.** Dispositions `:349` now records a green gate and CI on `6249af63`; `:366` reports package runs and leaves this tip’s full gate “next.” This review confirms neither execution nor CI’s 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 5c6189488..020000d74 100644 |
| --- a/docs/superpowers/specs/DISPOSITIONS-metered-replicator.md |
| +++ b/docs/superpowers/specs/DISPOSITIONS-metered-replicator.md |
| @@ -252,7 +252,7 @@ Reports: `OPUS-REVIEW-metered-replicator-review11.md`, `CODEX-FINDINGS-metered-r |
| | Opus P2-1, Astra 9 (P2): a link to a file was skipped by its own name alone, so a re-pointed accepted link read the same; a `Stat` error other than ENOENT was treated as nothing; and every link tenant software makes becomes an unaccepted skip that refuses the whole box's next build. | Fixed for the first two: a link entry is named with its target, so a re-pointed link reads as a change; a link whose target cannot be looked at is a problem. [Review twelve: `Accept` cleaned the whole entry as a path, so a target that folds or pops under cleaning passed as the accepted pair; the thirteenth build quotes the target and compares a link entry as written.] The third is stated in `strictTreeDBs`'s comment and here as a decision before wiring: whether one tenant's problems and unaccepted skips refuse the whole box or that tenant's databases alone; today they refuse the box, through `Accept`. | `litestream_owners.go` `strictTreeDBs` | `TestStrictTreeNamesALinkToAFile` (the target named; re-pointed reads differently; a target that cannot be looked at is a problem); each fails with its case removed (mutation runs) | |
| | Astra 6 (P2): a hibernating exec route over a hibernating unit's file dropped the file (the unit was not counted a live claim). | Fixed: a unit route's database is this fleet's whether or not the unit hibernates, so only a hibernating exec route is no live claim. | `litestream.go` `liveClaim` | `TestExplicitClaimsOutsideTheGlobsAreInventoried` (a hibernating unit under a hibernating exec claim: a problem); fails with units skipped when hibernating (mutation run) | |
| | Opus P3-3, Astra 7 (P2): a unit tree above a platform file was silently platform-owned; a hibernating path spelled through an alias, or on a skipped file, is unseen. | Fixed for the first: `routeClaiming` uses the inventory's own claim predicate (a unit tree counts). The alias and the skipped-file cases are open: comparing hibernating paths by identity against listed and skipped files is not built. | `litestream_owners.go` `routeClaiming` | the same test (a unit route over the console's directory: unowned); fails with exact spellings alone (mutation run) | |
| -| Opus P3-2: root and the platform's user were two owners at the end of a tenant chain; `instances/*/*.db` rested on a premise isolated exec contradicts; a tenant's file in a platform-owned directory is refused [from the nineteenth build in both layouts; before it the climb accepted such a file under a tenant's own tree: review eighteen]. | Fixed for the first two: root and the platform's user are one owner there as in `platformOwned` (`platformUID`); `activatorFile` is `dataDir/*.db` alone, so an instance's database is checked by its chain. The third is stated as not decided (billable or not), and so refused until it is. | `litestream_owners.go` | `TestRootAndThePlatformUserAreOneOwner` (skipped under root, where the two owners are one and the test would say nothing); `TestAnInstancesDatabaseIsCheckedByItsChain` (an instance directory that is a tenant's link: refused); each fails with its fix removed (mutation runs, not as root) | |
| +| Opus P3-2: root and the platform's user were two owners at the end of a tenant chain; `instances/*/*.db` rested on a premise isolated exec contradicts; a tenant's file in a platform-owned directory is refused [for a file claim from the nineteenth build in both layouts (before it the climb accepted such a file under a tenant's own tree: review eighteen); for a tree claim from the twentieth (before it the walk accepted it: review nineteen)]. | Fixed for the first two: root and the platform's user are one owner there as in `platformOwned` (`platformUID`); `activatorFile` is `dataDir/*.db` alone, so an instance's database is checked by its chain. The third is stated as not decided (billable or not), and so refused until it is. | `litestream_owners.go` | `TestRootAndThePlatformUserAreOneOwner` (skipped under root, where the two owners are one and the test would say nothing); `TestAnInstancesDatabaseIsCheckedByItsChain` (an instance directory that is a tenant's link: refused); each fails with its fix removed (mutation runs, not as root) | |
| | Opus P3-4, Astra 11: the "other user's directory" fixture keyed by base name was a foreign ancestor, not a coherent tree; the ranking untested. | Fixed: B's directory and file are named apart, and the case now shows what the code does (a coherent other-user tree named by a route is accepted: Astra 9, open with approval H; the test's introduction said "refused" until the thirteenth build); the every-claim test above. Still open as stated: retained handles and worker exit, the watch's own call, per-database attribution, the re-check under the lock, the 300 ms check, `withOwners` by base name, the admission's under-lock check, the shadow-state directory check. | tests | | |
| | Astra 1 to 4, 8, 10 (P1s, the device open, the uid binding): litestream's opens by path; tenant-replaceable ancestry; a delayed admitted request past the margin; the crash and restart gap; the header open through a swapped ancestor; a route naming another user's coherent tree. | Open, as before, stated where each applies; required before wiring, with approval H for the last. | | | |
| | Claims (both): the strict walk's "two things on purpose" and "never followed"; `activatorFile`'s premise; "strictest kind"; the hibernation and platform-conflict rows above; `Close` "asks the witness once more"; the close test's comment; `fleetInventory`'s comment. | Corrected in place, bar the leftovers review twelve found (the old ranking paragraph above the new one, "the two things", `strictestClaim` in `litestream.go`, the witness sentence at `inheritHold` and `Open`), corrected in the thirteenth build. | | | |
| @@ -355,12 +355,29 @@ Reports: `OPUS-REVIEW-metered-replicator-review18.md`, `CODEX-FINDINGS-metered-r |
| | Finding | Disposition | Where | Reproduction | |
| | --- | --- | --- | --- | |
| | Opus P3-1, Astra 10: the long-target fixture needed a link target of about 3 KB (XFS refuses one over 1,024 bytes), and added five directories unconditionally (a root of about 3,200 bytes could not build it). | Fixed: the target is 999 bytes; directories of 200 bytes are added until the concatenation outgrows the limit, within the pathname budget, and a root that leaves no budget is reported as the fixture's failure, not worked around. | `litestream_owners_test.go` | The guard checks the concatenation, the target's length and the file's own path. Passes under `/var/tmp/ck`, `~/.cache/tmp` and the author's root. | |
| -| Opus P3-2 (JJ2), Astra's claim: a tenant's file in a platform-owned directory was refused under the platform's tree and accepted under the tenant's own tree (the climb took the tenant's directory as the chain root). | Fixed: the start check refuses a file that is not the platform's in a directory that is; both layouts are now the undecided case, refused. | `litestream_owners.go` `checkOwnedFile` | `TestATenantsFileInAPlatformDirectoryIsRefusedInBothLayouts`; with the refusal removed, the test fails on the tenant's-tree layout. | |
| +| Opus P3-2 (JJ2), Astra's claim: a tenant's file in a platform-owned directory was refused under the platform's tree and accepted under the tenant's own tree (the climb took the tenant's directory as the chain root). | Fixed for a file claim: the start check refuses a file that is not the platform's in a directory that is; both layouts are the undecided case, refused. [A tree claim still accepted it until the twentieth build: review nineteen.] | `litestream_owners.go` `checkOwnedFile` | `TestATenantsFileInAPlatformDirectoryIsRefusedInBothLayouts`; with the refusal removed, the test fails on the tenant's-tree layout. | |
| | Opus P3-3 (GG2), Astra 5: what a swap of the link's directory still does (another directory, or a link to one, in its place between the listing and the open: the one link omitted or misread for that one look; never a database streamed or attributed through it). | Stated in the code as open under EE2, with what it can and cannot do; the walk by descriptor stays the fix and stays open (Astra 1, 5). | `linktarget_unix.go` | | |
| -| Astra's claim: refusal messages joined raw entries, so a name with a line break could mislead a displayed diagnostic. | Fixed: a plain entry is quoted in every refusal message; a link entry is already the quoted pair. | `litestream_metered.go` `quotedList` | With plain entries unquoted again, the impostor case of `TestAcceptComparesALinkEntryAsWritten` fails (it wants the quoted name). | |
| +| Astra's claim: refusal messages joined raw entries, so a name with a line break could mislead a displayed diagnostic. | Fixed for the inventory lists (problems, unseen, unowned): a plain entry is quoted; a link entry is already the quoted pair. [The no-recorder refusal still printed the owned path raw until the twentieth build: review nineteen.] | `litestream_metered.go` `quotedList` | With plain entries unquoted again, the impostor case of `TestAcceptComparesALinkEntryAsWritten` fails (it wants the quoted name). | |
| | Opus II2: the witness test had no host-rename case. | Added: a raw `UPDATE routes SET host` moves the witness. | `observe_ownership_test.go` | | |
| | Claims (both): "a link in its last component is followed" (any component is); `OUTSTANDING.md` "no mount" (no crossing onto another filesystem; a same-filesystem bind alias is not detected) and "Opus accepts each with fixes" (Opus rejected the first, second and eighth builds); this file's review-eight row, review-thirteen row (`x/..`), the review-seventeen numbering, "under any TMPDIR", the FIFO test's description, and the missing record of the green gate on `6249af63`. | Corrected in place. | this file, `linktarget_unix.go`, `OUTSTANDING.md` (this branch and the launch branch) | | |
| | Astra 12 (P3): tests that survive removing what they describe. | Open as named; two were closed in the eighteenth build. | | | |
| | Astra 1 to 4, 6 to 9, 11; approval H; the box-wide refusal decision. | Open as before; each blocks wiring. | | | |
| -Package tests pass under `/var/tmp/ck` and `~/.cache/tmp` in the author's runs, not as root. The full gate on this tip is next; the nineteenth build has not been read by a reviewer. |
| +Package tests pass under `/var/tmp/ck` and `~/.cache/tmp` in the author's runs, not as root. The full gate on `53e3dcd8` passed (exit 0, three cross builds, `gate-replicator-53e3dcd8.txt`, 09:38Z) and Amadan CI passed on it. |
| + |
| +## Review nineteen (reviewed `53e3dcd8`; Opus accepted, Astra rejected) |
| + |
| +Reports: `OPUS-REVIEW-metered-replicator-review19.md`, `CODEX-FINDINGS-metered-replicator-review19.md`. Opus found review eighteen's P3s and Astra 10 closed, each with a test that fails without its fix, no new P1 or P2, and two P3s. Astra found the same closed and rejected on the deep items (its 1 to 4 at P1, 5 to 9 at P2, 10 and 11 at P3), with the fixture's guard as its 12. |
| + |
| +**Bounded changes, made in the twentieth build:** |
| + |
| +| Finding | Disposition | Where | Reproduction | |
| +| --- | --- | --- | --- | |
| +| Opus P3-1, Astra's claim: a tree claim still accepted a tenant's file in a platform-owned directory (the walk reaches the owner comparison without the start check), so the kind of claim decided the undecided case. | Fixed: the walk keeps the owner of the file's immediate directory, and a tree claim refuses a file that is not the platform's in a directory that is. The comparison's comment says what it is for each kind (for a file claim, a check against a change between the climb and the walk). | `litestream_owners.go` `checkOwnedFile` | `TestATenantsFileInAPlatformDirectoryIsRefusedInBothLayouts` (the tree layout added): with the refusal removed, the test fails on the tree claim. | |
| +| Opus P3-2, Astra 12: the fixture's "no budget" guard could not run, and a root too long failed at the file's creation with an OS error; the new test's third check accepted any refusal that lacked one phrase. | Fixed: the dead guard is gone; the file's own path length is checked before anything is made; the third check asserts no refusal at all. | `litestream_owners_test.go`, `litestream_owners_unix_test.go` | | |
| +| Claims (both): "a plain entry is quoted in every refusal message" (the no-recorder refusal printed the owned path raw); "and nothing more" for a swapped directory (an omitted or misread link can change whether a build is refused); this file's rows on both. | Fixed and corrected: the no-recorder refusal quotes the path; the comment says what a swap can change; the rows are scoped above. | `litestream_metered.go`, `linktarget_unix.go`, this file | | |
| +| Astra's claim: per-database attribution is listed as untested without telling the live-replication test from the direct-client test, which already rejects swapped recorders. | Stated: the direct-client test (`litestream_metered_test.go`, the asymmetric assertions) tells swapped fleet recorders apart; the live-replication test checks positive counts only. | this file | | |
| + |
| +**Deferred, each blocking wiring, none changed by this build:** Astra 1 (litestream's own opens by path), 2 (replaceable ancestry above a protected directory), 3 (an admitted request delayed past the margin), 4 (uncertainty lost across process exit), 5 (the link's directory resolved again by path: the walk by descriptor), 6 (device effects through an ancestor swap at the header open), 7 (hibernating aliases and claims on skipped files), 8 (required coverage that disappears without an entry), 9 (one tenant's changes refusing or holding the whole box: a decision), 10 (route identity unbound to filesystem authority; approval H's uid binding), 11 (tests that survive removing what they describe: shutdown, watch, interleavings, timing). These need a design (the walk and litestream's opens beneath a pinned root; durable transition state) or a decision (box-wide refusal; approval H), not another build of the walk. Whether CI runs as root is not established by committed source. |
| + |
| +Package tests pass under `/var/tmp/ck` and `~/.cache/tmp` in the author's runs, not as root. The full gate on this tip is next; the twentieth build has not been read by a reviewer. |
diff --git a/docs/superpowers/specs/OPUS-REVIEW-metered-replicator-review19.md b/docs/superpowers/specs/OPUS-REVIEW-metered-replicator-review19.md| new file mode 100644 |
| index 000000000..7255b1e74 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/OPUS-REVIEW-metered-replicator-review19.md |
| @@ -0,0 +1,111 @@ |
| +# Review nineteen: feat/carloku-metered-replicator-20261001 at `53e3dcd8` |
| + |
| +**Verdict: accept.** Review eighteen's three Opus P3s and Astra 10 are closed. Each has a test, and the tests that can fail without their fix do. The nineteenth build adds no P1 or P2. There are two new P3s and three claims the code contradicts. Astra 1–4, approval H and the box-wide-refusal decision still block wiring. |
| + |
| +I read source with `git show 53e3dcd8:` only and ran nothing. The brief says "review sixteen" and names report files `…review19.md`. At this commit the reports are `…review18.md` and the findings are review eighteen's, so I read it as the previous round. |
| + |
| +## Review eighteen's findings, one by one |
| + |
| +| Finding | At `53e3dcd8` | Would its test pass without the fix? | |
| +|---|---|---| |
| +| **Opus P3-1 / Astra 10**: the fixture's target length on XFS, and its fixed depth | **Closed.** `litestream_owners_test.go:764-779`: the target is 999 bytes. Directories of 200 bytes are added only while the concatenation is ≤ 4,096. A resolved root of 3,200 bytes adds none: the concatenation is 4,200 and the file's path 3,212. A linked root is resolved by `platformDir`. Two caveats are in P3-2 below. | **No.** The concatenation is always > 4,096, so a concatenating `statTarget` gets `ENAMETOOLONG`, files a problem, and the test fails. | |
| +| **Opus P3-2 (JJ2)**: one tenant file, opposite answers by layout | **Closed for `tenantFile`** (`litestream_owners.go:475-490`). **Not closed for a tree claim** (P3-1 below). | **No.** Without `:487`, `/srv/T/bridge/a.db` climbs to `T` and is accepted, so `:529` fails. The `/srv/bridge` layout also fails without the fix, because the test asks for the new message. | |
| +| **Opus P3-3 (GG2)**: a swap of the link's directory | **Closed as stated.** `linktarget_unix.go:22-26` now says what the swap does (omits or misreads one link for one look), under EE2. The walk by descriptor stays open in Astra 1 and 5. | Wording only. | |
| +| **Opus II2**: the witness test had no host case | **Closed** (`observe_ownership_test.go:32`). | **No**, if the trigger's column list drops `host`. | |
| +| **Opus claims 1–3** ("last component", "no mount", the FIFO test's description) | **Closed.** `OUTSTANDING.md:35` now says no crossing onto another filesystem and that a same-filesystem bind alias is not detected. | n/a | |
| +| **Opus claim 4**: whether CI runs as root | **Open.** Still not established by committed source. | n/a | |
| +| **Astra 1–4 (P1), 5–9 (P2), 11 (P3)** | **Open**, unchanged, listed as blocking wiring. | Yes, as before. | |
| +| **Astra 10** | **Closed** (see the first row). | No. | |
| +| **Astra 12** | **Open** as named, apart from the two parts closed in build eighteen. | Yes, as before. | |
| +| **Astra's claims** | | | |
| +| – "Under any TMPDIR" | **Closed:** the wording is gone. | | |
| +| – Undecided case stated inconsistently | **Partly closed:** still wrong for tree claims (P3-1 below). | | |
| +| – `x/..` row (dispositions :283) | **Closed:** bracketed. | | |
| +| – Review-seventeen numbering | **Closed.** | | |
| +| – "Opus accepts each" | **Closed.** | | |
| +| – Raw refusal messages | **Closed** for the three inventory messages; one message remains (claim 2 below). | | |
| +| – Recorded gate | **Still not recorded** for this tip. Dispositions say "the full gate on this tip is next". | | |
| + |
| +**Code changes in this build:** the start check in `checkOwnedFile` (`:475-490`) and `quotedList` (`litestream_metered.go:692-707`, used at `:881`, `:907`, `:910`). Everything else is tests, comments and docs. Areas A–JJ2 stand as review eighteen found them. Line numbers in `litestream_metered.go` after `:689` move by +17. |
| + |
| +## Findings |
| + |
| +### P3-1. A tree claim still accepts a tenant's file in a platform-owned directory; the comment and dispositions say nothing reaches the comparison |
| + |
| +**Where:** `litestream_owners.go:409-423` (the `tenantTree` case), `:536-556` (the walk), `:561-568` (the comparison and its comment). Dispositions `:255` and `:358`. |
| + |
| +**Scenario:** |
| +- A unit route has `DBPath=/srv/A` (a tree claim). `/srv/A` is owned by A, `/srv/A/bridge` by root with mode 0755, and `/srv/A/bridge/a.db` by A. |
| +- The strict walk finds the file. `claimChecks` gives one check, `{"/srv/A", tenantTree}`. |
| +- `platformOwned("/srv")` passes, and the walk sees no link and no device change. |
| +- At `:568`, `facts.UID == rootFacts.UID` (A == A), so the file is accepted and billed to A. |
| +- Named by a unit route as a file instead (`tenantFile`), the same file is refused at `:487`. |
| + |
| +So the case this build calls undecided, and refused "whatever the directory above", is accepted when the claim is a tree. The comment at `:565-567` ("never reaches here: the start check above refuses it") is attached to a comparison that `tenantTree` also reaches. |
| + |
| +For `tenantFile` the comparison can now only fail on a race: the climb guarantees the chain root's uid equals `ownerFacts.UID`. The comment does not say that either. |
| + |
| +Reachability is low. A tenant cannot make a root-owned directory itself, so this needs a platform-made directory inside a unit tree. It is still a decision taken by the kind of claim. |
| + |
| +**Smallest fix:** |
| +- In the walk at `:536`, for `tenantTree`, refuse when the file's immediate directory is platform-owned and the file is not (`platformUID(dirUID) && !platformUID(facts.UID)`). |
| +- Add the tree layout to `TestATenantsFileInAPlatformDirectoryIsRefusedInBothLayouts`. |
| +- Make the comment say that, for `tenantFile`, the comparison is a race check only. |
| + |
| +### P3-2. The fixture's "no budget" guard is unreachable, and the third check of the new test proves less than it says |
| + |
| +**Where:** `litestream_owners_test.go:766-768`; `litestream_owners_unix_test.go:538-543`. |
| + |
| +**The guard:** |
| +- Inside the loop, `len(deep) ≤ 3,096`, so `len(deep)+201 ≤ 3,297 < 4,000`. The `t.Fatalf` at `:767` can never fire. |
| +- A root of 4,084 bytes or more, which is the only kind with "no budget", skips the loop and fails at `writeFile` (`:772`) with an OS error before the guard at `:776`. The comment's "a root that leaves no budget … is reported" therefore holds only as an OS error. |
| +- No reachable root makes the fixture fail or mislead. It holds under short, long and linked roots, on XFS, and under umask 077 (`mkdirs` and `writeFile` chmod explicitly). It is unaffected by root, since no ownership is involved. |
| + |
| +**The new test's third check:** |
| +- `:542` only asserts that `why` lacks "the file is not". `own.db` refused for any other reason (the `srv` mode, a link, the `chmod` at `:539`, which is redundant after `mkdirs`) would still pass. |
| +- It should assert `why == ""`. |
| +- The first two checks are sound under root and under any umask: owners come through `withOwners`, and modes are explicit. |
| + |
| +**Smallest fix:** |
| +- Delete the dead guard, or move a reachable length check before `mkdirs`. |
| +- Make `:542` assert `why == ""`. |
| + |
| +## Areas KK2, LL2, MM2 |
| + |
| +### KK2: the new start-check refusal |
| + |
| +It refuses a newly refused layout only when the file's directory is platform-owned and the directory above it is the file owner's (the old climb's accepting case). For the layouts the brief asks about: |
| + |
| +- **DynamicUser state directories:** `/var/lib/private/<name>` is owned by the dynamic uid, so the `else` branch at `:491` applies. Unaffected. |
| +- **An isolated instance's `dataDir/instances/<id>`:** if the instance owns it, unaffected. If the activator makes it root-owned and the instance writes as its own uid, it was already refused before this build (the climb stops at once, then the comparison fails against the root-owned chain root). No change. |
| +- **The activator's `dataDir/*.db`:** an exec route is `namedFile`, so unaffected. A unit route naming such a file with a tenant owner was already refused against `dataDir`'s owner. No change. |
| +- **Units run as root, or a tenant sharing the platform uid:** the file is `platformUID`, so it passes as before. |
| +- **What is newly refused:** a root-owned directory inside a tenant's own directory, which no platform layout I can find makes. |
| + |
| +`platformUID(sf.UID)` at `:475` is the old `sf.UID == 0 || Geteuid` expression exactly. |
| + |
| +The comment at `:561-567` is exact for `tenantFile` and wrong for `tenantTree` (P3-1). |
| + |
| +### LL2: the fixture's budget loop |
| + |
| +It does not fail or mislead under any root: short, long, linked or XFS. Its guard at `:776` still establishes Astra 5's property: concatenation > 4,096, target < 1,024, file path < 4,096. Its other guard is dead (P3-2). |
| + |
| +### MM2: `quotedList` |
| + |
| +`isLinkEntry` is a prefix test (`"link \""`). Every entry that reaches `quotedList` otherwise starts with `/` or with a host: |
| +- problems: `litestream.go:371`, `:393`, `litestream_owners.go:664-740`; |
| +- `Unowned`: the absolute `p`; |
| +- `unseen`: absolute skipped paths. |
| + |
| +So only real `linkEntry` values pass through unquoted, and both halves of those are already `strconv.Quote`d. Quoted strings delimit themselves, so no entry can read as another or as two in the joined list. |
| + |
| +Nothing parses these messages. `MeteredFleetStore` has no production caller, and tests match with `strings.Contains`. |
| + |
| +## Claims the code contradicts |
| + |
| +1. **`litestream_owners.go:565-567`, "A tenant's file in a platform-owned directory never reaches here", and dispositions `:255` and `:358` ("refused … in both layouts", "both layouts are now the undecided case, refused"):** true for `tenantFile` only. A tree claim accepts it (P3-1). |
| +2. **Dispositions review-eighteen row, "a plain entry is quoted in every refusal message":** `litestream_metered.go:938` still formats `owned.Path` with `%s`. A tenant tree's file name with a line break prints raw there. It is display only and needs a recorder missing as well. The data-directory messages at `:1002-1006` use platform-chosen paths. |
| +3. **`litestream_owners_test.go:762-763`, "A root that leaves no budget for the fixture is reported":** only through `writeFile`'s OS error. The guard written for it is unreachable (P3-2). |
| +4. **Open as before:** whether CI runs as root is not established, and no full gate is recorded on this tip. |
| + |
| +**Verdict: accept.** P3-1 (a tree claim decides the undecided case) and the claims above are follow-ups. Astra 1–4, approval H and the box-wide-refusal decision still block wiring. |
diff --git a/internal/ops/linktarget_unix.go b/internal/ops/linktarget_unix.go| index d429b93c2..f3bcc5aa1 100644 |
| --- a/internal/ops/linktarget_unix.go |
| +++ b/internal/ops/linktarget_unix.go |
| @@ -23,8 +23,9 @@ type targetKind struct { |
| // any component of it is followed, as the tree's own root is reached |
| // through one; a directory, or a link to one, put in its place between the |
| // listing and this open is looked at instead, which can omit or misread |
| -// this one link for this one look and nothing more: stated open as EE2), |
| -// with |
| +// this one link for this one look, and so can change whether a build is |
| +// refused for it; no database is streamed or attributed through the link |
| +// either way: stated open as EE2), with |
| // fstatat, so the kernel resolves every ".." physically, from wherever the |
| // component before it led, and the argument handed to it is the target |
| // alone, so the link's directory adds nothing to the length the kernel |
diff --git a/internal/ops/litestream_metered.go b/internal/ops/litestream_metered.go| index f484b6a1c..96e287677 100644 |
| --- a/internal/ops/litestream_metered.go |
| +++ b/internal/ops/litestream_metered.go |
| @@ -935,7 +935,7 @@ func MeteredFleetStore(o MeteredFleetOptions) (fleet *MeteredFleet, err error) { |
| for _, owned := range inv.Owned { |
| rec := o.Meter(owned) |
| if rec == nil { |
| - return nil, fmt.Errorf("%w: no recorder for %s (%s/%s)", ErrUnmeterable, owned.Path, owned.Account, owned.App) |
| + return nil, fmt.Errorf("%w: no recorder for %s (%s/%s)", ErrUnmeterable, strconv.Quote(owned.Path), owned.Account, owned.App) |
| } |
| built.recorders = append(built.recorders, rec) |
| key := ownedReplicaKey(owned) |
diff --git a/internal/ops/litestream_owners.go b/internal/ops/litestream_owners.go| index 24b839a5c..a636065ca 100644 |
| --- a/internal/ops/litestream_owners.go |
| +++ b/internal/ops/litestream_owners.go |
| @@ -533,6 +533,9 @@ func checkOwnedFile(root, path string, kind claimKind) (fileFacts, string) { |
| } |
| at := root |
| var last os.FileInfo |
| + // dirUID is the owner of the file's immediate directory: the root's |
| + // until the walk has passed a directory beneath it. |
| + dirUID, lastUID := rootFacts.UID, rootFacts.UID |
| for _, part := range strings.Split(rel, string(filepath.Separator)) { |
| at = filepath.Join(at, part) |
| fi, lerr := os.Lstat(at) |
| @@ -549,6 +552,7 @@ func checkOwnedFile(root, path string, kind claimKind) (fileFacts, string) { |
| if f.Dev != rootFacts.Dev { |
| return fileFacts{}, "its path crosses onto another filesystem (" + at + ")" |
| } |
| + dirUID, lastUID = lastUID, f.UID |
| last = fi |
| } |
| if !last.Mode().IsRegular() { |
| @@ -561,13 +565,21 @@ func checkOwnedFile(root, path string, kind claimKind) (fileFacts, string) { |
| // 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. A tenant's file in a platform-owned |
| - // directory never reaches here: the start check above refuses it, |
| - // whatever is above the directory. |
| + // is the platform's either way). For a file claim the root is the one |
| + // the climb chose, whose owner is the file's by construction, so this |
| + // can fail there only when something changed between the climb and |
| + // the walk; for a tree claim it is the real comparison. |
| 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" |
| } |
| + // A tenant's file in a directory the platform owns is the undecided |
| + // case, refused whatever kind of claim reaches it: the start check |
| + // refuses it for a file claim, and this refuses it for a tree claim, |
| + // whose walk can pass a platform-made directory inside the tenant's |
| + // tree (review nineteen, Opus P3-1: the kind of claim decided it). |
| + if kind == tenantTree && platformUID(dirUID) && !platformUID(facts.UID) { |
| + return fileFacts{}, "its directory is the platform's and the file is not" |
| + } |
| for _, suffix := range []string{"-wal", "-shm", "-journal"} { |
| fi, lerr := os.Lstat(path + suffix) |
| if errors.Is(lerr, fs.ErrNotExist) { |
diff --git a/internal/ops/litestream_owners_test.go b/internal/ops/litestream_owners_test.go| index cc3ead9c5..74955c894 100644 |
| --- a/internal/ops/litestream_owners_test.go |
| +++ b/internal/ops/litestream_owners_test.go |
| @@ -756,18 +756,19 @@ func TestStrictTreeNamesALinkToAFile(t *testing.T) { |
| // length is put in the directories, not the target: the target stays |
| // under a kilobyte (XFS refuses a longer link target: review eighteen, |
| // Opus P3-1), and directories are added only until the concatenation |
| - // would outgrow the limit, within the pathname budget the temp root |
| - // leaves (review seventeen, Opus P2-2, and eighteen, Astra 10: a fixed |
| - // depth passed only under a root of the right length). A root that |
| - // leaves no budget for the fixture is reported, not worked around. |
| + // would outgrow the limit (review seventeen, Opus P2-2, and eighteen, |
| + // Astra 10: a fixed depth passed only under a root of the right |
| + // length). A root so long that the file's own path would not fit is |
| + // said before anything is made (review nineteen, both: the earlier |
| + // guard for it could not run). |
| deep := platformDir(t) |
| dest := strings.Repeat("./", 494) + "live.sqlite" // 999 bytes, under XFS's limit |
| for len(deep)+1+len(dest) <= 4096 { |
| - if len(deep)+1+200 >= 4000 { |
| - t.Fatalf("fixture: the temp root %q leaves no room for the fixture (%d bytes)", deep, len(deep)) |
| - } |
| deep = filepath.Join(deep, strings.Repeat("d", 200)) |
| } |
| + if len(deep)+1+len("live.sqlite") >= 4096 { |
| + t.Fatalf("fixture: the temp root leaves no room for the fixture (the directory would be %d bytes)", len(deep)) |
| + } |
| mkdirs(t, deep) |
| long := writeFile(t, filepath.Join(deep, "live.sqlite")) |
| if err := os.Symlink(dest, filepath.Join(deep, "current")); err != nil { |
diff --git a/internal/ops/litestream_owners_unix_test.go b/internal/ops/litestream_owners_unix_test.go| index 745a6444e..c00ac6cf1 100644 |
| --- a/internal/ops/litestream_owners_unix_test.go |
| +++ b/internal/ops/litestream_owners_unix_test.go |
| @@ -516,8 +516,9 @@ func TestOpenRegularNoFollowRefusesALinkToTheSameFile(t *testing.T) { |
| } |
| // A tenant's file in a directory the platform owns is refused whatever is |
| -// above that directory: under the platform's own tree and under the |
| -// tenant's own tree alike. Before this the climb decided it one way in each |
| +// above that directory and whatever kind of claim reaches it: a file claim |
| +// under the platform's own tree and under the tenant's own tree, and a |
| +// tree claim on the tenant's tree. Before this the climb decided it one way in each |
| // layout (review eighteen, Opus P3-2); the case itself (whether such a file |
| // is billable to the tenant) is not decided, and until it is, it is not. |
| func TestATenantsFileInAPlatformDirectoryIsRefusedInBothLayouts(t *testing.T) { |
| @@ -534,12 +535,15 @@ func TestATenantsFileInAPlatformDirectoryIsRefusedInBothLayouts(t *testing.T) { |
| if _, why := checkOwnedFile(filepath.Dir(above), above, tenantFile); !strings.Contains(why, "its directory is the platform's and the file is not") { |
| t.Fatalf("a tenant's file in a platform directory under the platform's tree: %q", why) |
| } |
| + // A tree claim on the tenant's directory reaches the same file by its |
| + // walk, and refuses it too (review nineteen, Opus P3-1: a tree claim |
| + // accepted what a file claim refused). |
| + if _, why := checkOwnedFile(filepath.Join(top, "srv", "T"), under, tenantTree); !strings.Contains(why, "its directory is the platform's and the file is not") { |
| + t.Fatalf("a tenant's file in a platform directory, by a tree claim: %q", why) |
| + } |
| // The platform's own file in its own directory is still the platform's. |
| own := writeFile(t, filepath.Join(top, "srv", "bridge", "own.db")) |
| - if err := os.Chmod(filepath.Join(top, "srv", "bridge"), 0o755); err != nil { |
| - t.Fatal(err) |
| - } |
| - if _, why := checkOwnedFile(filepath.Dir(own), own, tenantFile); strings.Contains(why, "the file is not") { |
| + if _, why := checkOwnedFile(filepath.Dir(own), own, tenantFile); why != "" { |
| t.Fatalf("the platform's own file: %q", why) |
| } |
| } |