amadan
One holder inventory per storage pass, a ledger check around the sizing, and a departed store's record still holds
From the first review of this branch (Astra reject, Opus accept with fixes): - The inventory is read once per pass, by literal record keys, so a record under a name no build writes counts too. A record the store could not return fails the pass before the day is written; one that will not parse holds every store that day. - The bucket's ledger generation is read before and after the sizing. A holder that appeared, even one that left again, discards the size. - A store that left a bucket counts as a holder for storage until its retained teardown is finished: whose its leftover bytes are has not been decided, so the remaining store is held, not sized with them. OUTSTANDING 19i lists the holds that last until a person acts and what is not covered (a bucket adopted in two deployments). The ruling on leftover bytes is approval K. Each check has a reproduction and was mutation-tested in a scratch copy. Billing is off.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
7 files changed,
+462
−31
docs/superpowers/specs/CODEX-FINDINGS-storage-sole-holder-review1.md+81 −0docs/superpowers/specs/DISPOSITIONS-storage-sole-holder.md+20 −0docs/superpowers/specs/OPUS-REVIEW-storage-sole-holder-review1.md+77 −0implementation/carloku-paid-launch/OUTSTANDING.md+2 −1internal/console/economicspass.go+47 −27internal/console/economicspass_test.go+105 −3internal/objstore/walk.go+130 −0
CI passed — run details
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-storage-sole-holder-review1.md b/docs/superpowers/specs/CODEX-FINDINGS-storage-sole-holder-review1.md| new file mode 100644 |
| index 000000000..d7197dd27 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-FINDINGS-storage-sole-holder-review1.md |
| @@ -0,0 +1,81 @@ |
| +All locations below refer to **d5464eb1**. Conclusions are from source inspection; no tests were run. |
| + |
| +1. **P1 — A second holder can appear during sizing without invalidating the recorded bytes.** |
| + **Location:** `internal/console/economicspass.go:229`, `:247`, `:251`. |
| + |
| + A owns bucket X. `OtherHolders` finishes and returns nobody. While `sizeStore` lists X, an operator grants B access to X and B writes objects. A’s record remains unchanged, so the post-sizing `Get` and `SameStore` check succeed. The pass records the whole listing against A in the create-only day object. |
| + |
| + This also permits an inconsistent day: A is measured before B’s adoption, while B is subsequently recorded as shared. The inventory and ledger reads are sequential observations, not a snapshot protected against ownership changes. |
| + |
| + **Smallest acceptable fix:** protect the inventory and measurement with the bucket coordination used by Grant/Teardown, discarding the measurement if that protection expires. Alternatively, validate an ownership generation covering the entire measurement. Repeating only `OtherHolders` afterward would miss an intervening holder that appeared and left. |
| + |
| +2. **P1 — The filtered inventory misses persisted holders and cannot establish exclusivity across deployment namespaces.** |
| + **Location:** `internal/objstore/walk.go:1360`, `:1367`, `:1376`, `:1624`. |
| + |
| + Put a record naming X at `apps/legacy/shop/store.json`, alongside a valid provisioned A record naming X. `legacy` is not a valid account, so the inventory skips it without reading the record. The storage pass also skips its store read (`economicspass.go:207`). A is consequently sized and charged X’s full bytes. An invalid app segment produces the same result. |
| + |
| + Separately, a holder recorded under another deployment’s control prefix or release store is invisible: this inventory visits only `apps/` in the supplied store and reads only that store’s ledger. Two deployments adopting the same physical bucket can each conclude that their local app is its sole holder. **A different minted bucket-name prefix alone is not missed**—there is no bucket-prefix filter. |
| + |
| + **Smallest acceptable fix:** inspect literal store-record keys even when their owner names are invalid, retaining bucket-specific ambiguity rather than treating them as absent. For cross-deployment adoption, use ownership evidence shared across the participating deployments, or enforce exclusive deployment ownership before allowing measurement. A local inventory alone cannot establish that property. |
| + |
| +3. **P2 — A completed revocation on an unsupported bucket name can hold the remaining live store indefinitely.** |
| + **Location:** `internal/objstore/walk.go:1629`, `:1637`; underlying inventory at `:1421`. |
| + |
| + A and B share a legacy bucket name rejected by `ValidBucketName`. A’s teardown leaves the bucket for B, revokes A’s credentials, and retains A’s record (`walk.go:1064`, `:1183`, `:1240`). No ownership interval is closed for that name. |
| + |
| + `OtherHolders` continues counting A because its retained record names the bucket and has no bucket-deletion timestamp. B is therefore held every day even though A’s access has been revoked. Re-running A’s teardown while B remains live returns retained again; it does not clear this condition. |
| + |
| + **Smallest acceptable fix:** persist evidence of completed credential revocation that works for unsupported bucket names, and reconcile coherent retained records against that evidence. Do not simply exclude every record with a teardown block: revocation may not have happened yet. |
| + |
| + **19i does not disclose this shape.** Its “until one of them leaves” description is insufficient: A has already left operationally, but the remaining store still cannot be measured. |
| + |
| +4. **P2 — A transient failure reading another account’s record becomes a permanent hold on a healthy account’s day.** |
| + **Location:** `internal/objstore/walk.go:1385`, `:1624`; `internal/console/economicspass.go:229`, `:244`, `:269`. |
| + |
| + A and B have separate buckets. While checking A, the inventory encounters a transient read failure on B’s `store.json`. A is marked unswept. The failure clears before B’s own visit, so B can be measured, but A’s held result is written into the create-only day. Subsequent passes return immediately because that day exists. |
| + |
| + A persistently malformed record is worse: every otherwise eligible provisioned store encounters it during inventory, so unrelated accounts acquire holds every day. Per-account flags do not prevent this cross-account propagation. |
| + |
| + **Smallest acceptable fix:** propagate transient inventory failures out of the pass before creating the day, allowing a retry. Permanent unreadability needs an explicit repair path or independent bucket-scoped ownership evidence; silently ignoring the unreadable record would permit incorrect billing. |
| + |
| + **19i does not disclose either consequence.** |
| + |
| +5. **P2 — Holder discovery adds unbounded quadratic work to the synchronous economics pass.** |
| + **Location:** `internal/console/economicspass.go:229`; `internal/objstore/walk.go:1624`. |
| + |
| + With N valid app prefixes and all N stores eligible for checking, the new inventory performs **N(N−1) other-record GET attempts per daily pass**, plus N ledger reads and repeated account/app directory listings. More generally, S eligible stores among N apps cost **O(SN)** record reads, including attempts for apps without a store record. |
| + |
| + For 1,000 eligible apps, that is 999,000 inventory record reads before counting the pass’s existing work. Calls are sequential. There is no inventory budget, app-count cap, shared cache, or pass-specific deadline here. The daily existence check limits repetition after success; it does not bound the work needed to succeed. Statements and bill collection wait behind it. |
| + |
| + **Smallest acceptable fix:** build and reuse a bucket-to-holder inventory once per pass, while retaining ownership-change validation during sizing. Bound the work and retry incomplete collection rather than writing a misleading completed day. |
| + |
| +6. **P3 — Two new subtests pass with the entire storage holder check removed.** |
| + **Location:** `internal/console/economicspass_test.go:1785`, `:1806`. |
| + |
| + By source inspection, both **“a sole holder”** and **“the other store has left”** pass against the base behavior. The base already sizes the live store and holds the other store because its teardown block exists. |
| + |
| + These are useful positive controls, but they do not independently demonstrate that the new check runs. The latter *does* distinguish removing `ledger.left` reconciliation while leaving the new check installed. The **shared-record** and **ledger-only** subtests would fail without the new check, so the suite as a whole is not vacuous. |
| + |
| + **Smallest acceptable fix:** retain these controls and add cases that change ownership during `storeSizer`, inject inventory failures, and exercise grant-only, refused-record, invalid-owner, and same-account holders. Verify account-local completeness through the resulting day. |
| + |
| +**Claims checked against the code** |
| + |
| +- **19i, `OUTSTANDING.md:68`: “sizes a store only when it is the sole holder.”** Too strong. The race and inventory scope gaps above allow measurement without establishing that property. |
| +- **`walk.go:1621`: “reads every store record in the deployment.”** False for invalid account/app segments. It also excludes the requesting store, whose record the caller supplies separately. |
| +- **19i: “A store that left (closed ledger interval) does not count.”** Missing a required exception. A `Coherent`-refused record still holds every bucket it names even with closed intervals (`walk.go:1637`). Such holds persist until repair; 19g mentions manual repair, but 19i does not explain their effect on other stores. |
| +- **19i: held “until one of them leaves.”** Besides the unsupported-name case, already-written held days remain held after departure. That historical limitation is acknowledged in 19a, not in 19i. |
| +- **Grant-only and teardown-note-only holders are covered within valid owner paths.** Grant-only records are included at `walk.go:1413`. A bucket named only in a teardown note makes the record incoherent and is included at `:1400`, with the refusal flag preventing closed-ledger reconciliation. |
| +- **Grant’s take-back behavior is unchanged for reachable inputs.** The line-by-line comparison is: |
| + |
| + | Step | Base → d5464eb1 | |
| + |---|---| |
| + | Entry condition | Same: unfinished teardown and non-operator source. | |
| + | Record inventory | Same function, directory order, exclusions, and refused-record handling. | |
| + | Read order | Same: complete record inventory, then one ledger read. Inventory failure still prevents the ledger read. | |
| + | Ledger candidates | Previously appended every open foreign-account interval. Now includes other apps of the same account too and deduplicates; Grant subsequently skips same-account holders. | |
| + | Reconciliation | Previously applied during Grant’s candidate loop; now applied to record holders inside `OtherHolders`. Refused records still survive. An open ledger candidate cannot satisfy `left`, so omitting that redundant reconciliation changes no decision. | |
| + | First refusal | Surviving record holders still precede ledger-only holders. Deduplication does not change the first foreign holder reported. | |
| + | Errors | Inventory and ledger error wrappers retain the same text and `%w` behavior. The `ErrBucketHeld` message is unchanged. | |
| + | Bucket validation | The helper now conditionally reads the ledger, but Grant already rejects invalid bucket names at `walk.go:221`; its reachable behavior is unchanged. | |
| + |
| +**Verdict: reject.** |
| \ No newline at end of file |
diff --git a/docs/superpowers/specs/DISPOSITIONS-storage-sole-holder.md b/docs/superpowers/specs/DISPOSITIONS-storage-sole-holder.md| new file mode 100644 |
| index 000000000..03761816a |
| --- /dev/null |
| +++ b/docs/superpowers/specs/DISPOSITIONS-storage-sole-holder.md |
| @@ -0,0 +1,20 @@ |
| +# Storage sole holder: review dispositions |
| + |
| +Branch `feat/carloku-storage-sole-holder-20261001`, off the launch branch at `c68ecddd`. It is the fix for `OUTSTANDING.md` 19i. Billing is off. |
| + |
| +## Review one (reviewed `d5464eb1`; Astra rejected, Opus accepted with fixes) |
| + |
| +Reports: `CODEX-FINDINGS-storage-sole-holder-review1.md`, `OPUS-REVIEW-storage-sole-holder-review1.md`. Both found Grant's take-back check unchanged by the refactor. |
| + |
| +| Finding | Disposition | Where | Reproduction | |
| +| --- | --- | --- | --- | |
| +| Both: a holder that appears while the bucket is being sized is not seen, and one that appears and leaves would be missed by a second inventory | Fixed. The bucket's ledger generation is read before the sizing and again after it; a grant opens its interval on the ledger before it delivers a credential, so a changed generation discards the size. | `console/economicspass.go` `storagePass`; `objstore/walk.go` `OpenOnLedger` | `TestStoragePassSizesOnlyASoleHolderOfItsBucket/a_holder_appears_while_the_bucket_is_being_sized` | |
| +| Both: the inventory read every record once per store, with no bound; one transient read error held a healthy account's day for good | Fixed. One inventory per pass (`ReadHolderInventory`). A record the store could not return fails the pass before the day is written, and the pass runs again. A ledger that cannot be read does the same. | the same | covered by the cases below; no test injects a transient read error | |
| +| Astra P1: a record under an account or app name no build writes is skipped, so a bucket it names is sized to the other holder | Fixed for the storage pass. The inventory reads record objects by their literal keys; such a record holds every bucket it names. The walks' own holder scan (`storeHolders`) still skips such names; that is open and predates this branch. | `objstore/walk.go` `ReadHolderInventory` | `.../a_record_under_a_name_no_build_writes` | |
| +| Astra P2: a malformed record held every eligible store through repeated inventory errors, silently | Changed, not removed. A record that is there and will not parse cannot say which bucket it names, so no store can be shown to be the only holder of anything: every store is held that day, in one logged decision. It lasts until the record is repaired. | `storagePass` | `.../a_record_that_will_not_parse` | |
| +| Opus P2, needs a ruling: a store that left was not counted, so the objects it left in the bucket were billed to the remaining holder | Changed to hold. For storage, a departed store's record counts until its retained teardown has finished. Whose the leftover bytes are is Paul's decision (approval K on this branch's `OUTSTANDING.md`); until then the remaining store is held, not sized. | `ReadHolderInventory` | `.../the_other_store_has_left` | |
| +| Astra P2, Opus P3: holds that last for good, and 19i did not say so | 19i now lists them: a retained record (with or without a ledger) until its teardown is finished, which nothing runs (19a); a record `Coherent` refuses, until repaired by hand (19g); another store's grant that died after writing its grant block, until it is run again; two apps of one account on one bucket. | `OUTSTANDING.md` 19i | none | |
| +| Both, P1/P3: a holder under another deployment's control prefix is invisible | Accepted, not fixed. One bucket adopted in two deployments is sized in full in each. It needs shared ownership evidence or a refusal to adopt across deployments. Open, in 19i. | none | none | |
| +| Tests | The departed-store case now asserts the hold and the release once the record is gone; four cases added. | `economicspass_test.go` | above | |
| + |
| +Mutation checks in a scratch copy, each failing its test: the unparsed-record hold off; the ledger generation not compared; invalid names skipped; a record with a teardown block not counted; ledger-only holders ignored. |
diff --git a/docs/superpowers/specs/OPUS-REVIEW-storage-sole-holder-review1.md b/docs/superpowers/specs/OPUS-REVIEW-storage-sole-holder-review1.md| new file mode 100644 |
| index 000000000..00783a2f5 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/OPUS-REVIEW-storage-sole-holder-review1.md |
| @@ -0,0 +1,77 @@ |
| +I found no P1 issues. The grant refactor behaves exactly as before. Two P2s need fixing before merge: a race between the holder check and the sizing, and the cost of the holder check together with its failure mode. A third P2 needs a ruling on bytes a departed store leaves behind. The P3 hold-forever shape below also contradicts what 19i says. |
| + |
| +## Findings |
| + |
| +### P2: a holder that appears during the sizing is not seen |
| +`internal/console/economicspass.go:229` → `:247` → `:256`. `OtherHolders` runs once, before `sizeStore`. The re-read afterwards (`:256`) compares only this store's own record (`SameStore`). It never looks at the other holders again. |
| + |
| +**Scenario:** A/app is the sole holder of X. The pass checks X's holders and finds none, then starts a long `ListObjectsV2` of X. Meanwhile an operator adopts X for B/shop. The grant writes B's record and opens B's ledger interval, B gets credentials and writes objects. The listing picks those objects up, and A's create-only day records all of X, B's bytes included. The rule says an ambiguous attribution holds; this one is measured to A. |
| + |
| +**Smallest fix:** keep the ledger etag read in `OtherHolders` (or read it in the pass) before sizing. After sizing, re-read the ledger and hold if the etag changed or a foreign interval is open. Every grant this build makes opens its interval before it creates any credential (`walk.go` around `:452`), so the ledger alone catches a new holder. No second record walk is needed. A bucket name the build would refuse has no ledger, but `Grant` refuses those names, so no new holder can appear on one. |
| + |
| +### P2: the inventory is O(stores × apps), synchronous, has no deadline, and one transient read error permanently holds a store's day |
| +`economicspass.go:229`, `walk.go:1623` / `:1348`. Each sized store costs: |
| +- `ListDirs("apps/")`, plus one `ListDirs` per account directory; |
| +- one `Inspect` GET per app directory (every app, store or not); |
| +- one ledger GET. |
| + |
| +With S stores, A accounts and N apps, that is about S·(A+N+2) requests a day. For example, 200 stores, 1,000 apps and 300 accounts come to about 260k sequential requests. Request cost is small, but at roughly 20 ms each that is about 90 minutes of wall time. |
| + |
| +Nothing bounds it: |
| +- `StartEconomicsPass` calls `economicsPass(ctx, …)` with the process context and no timeout. |
| +- `storagePass` runs inline ahead of `statementsPass` and `billPass` (`economicspass.go:59-72`), so the inventory delays the statements and the bill pull. |
| +- Each of those requests is a fresh chance to fail. Any failure in `OtherHolders` holds that store's day (`:229`, `herr != nil`). Days are create-only, so a single flaky GET leaves that account's storage month incomplete for good. |
| + |
| +**Smallest fix:** |
| +1. Build the inventory once per pass: one walk of the records into a map from bucket to holders, plus one ledger read per distinct bucket. |
| +2. If that walk fails, return the error before `CreateDayObject` (as a `ListSizes` failure already does), so the next hourly tick retries instead of writing a permanent hold. |
| + |
| +The per-store post-sizing ledger check from the finding above still applies. |
| + |
| +### P2 (needs a ruling): a store that left is excluded, so the bytes it left behind are billed to the remaining holder |
| +`walk.go:1637`. The third subtest at `economicspass_test.go:1806` asserts this. When B's teardown finds that A still holds the bucket, it revokes B's access, closes B's interval and keeps B's record. When A is another account's store, that teardown also reports `ErrBucketRetained`, whose own doc says the bucket "may still contain this app's objects… a person decides". The pass then sizes all of X to A, including B's leftovers. |
| + |
| +That is an attribution nobody has decided, and it falls under "an ambiguous attribution holds". Before this branch A was sized the same way, so this is not a regression. But 19i ("A store that left … does not count") presents it as settled without addressing the retained-bytes clause. |
| + |
| +**Smallest fix:** either count a left store whose record still exists with an unfinished teardown, for the storage pass only (a parameter on `OtherHolders`, so grant is untouched), or put the ruling in 19i in so many words. |
| + |
| +### P3: holds that last forever |
| +`OUTSTANDING.md:68` (19i) says a shared store is held "every day, until one of them leaves". The code contradicts that for the first shape below, and 19i does not mention the others: |
| +1. **No ledger (a name the build would refuse).** B's retained teardown keeps B's record with a teardown block and no deletion noted. `OtherHolders` counts it, because `otherStoreHolders` passes `countLeaving=true`, and with no ledger, `left()` cannot excuse it. So A is held for good. Teardown itself reads the same shape with `countLeaving=false` (`walk.go:1064-1074`), and `storeHolders`' doc warns about exactly this forever-wait. **Fix:** in `OtherHolders`, use `countLeaving=false` when `!ValidBucketName(bucket)`. Grant never reaches `OtherHolders` with such a name (`walk.go:221`), so grant is unaffected. |
| +2. **Another store's record that `Coherent` refuses names the bucket.** The hold lasts until someone repairs the record by hand, and 19g says there is no verb for that. 19i should cross-reference it. |
| +3. **Another store's grant died after writing its grant block** (a provider error, not a refusal). The hold lasts until someone re-runs that grant. Nothing retries it. |
| +4. **Two apps of the same account on one bucket.** Both are held. This is conservative and consistent with "another store", but worth one sentence in 19i. |
| + |
| +### P3: a cross-deployment holder is invisible |
| +`walk.go:1623`. The records and the ledger are both read from this deployment's release store. If an operator adopts one bucket in two deployments (dev and flagship, or two regions), each side sees a sole holder and sizes it in full. **Fix:** note it in 19i as unsupported, or refuse adoption of a bucket another deployment's prefix minted. |
| + |
| +### Holders the inventory does not miss |
| +- A record with only a grant block is counted (`walk.go:1395-1399`). |
| +- A bucket named only in a teardown note is counted for refused records. For coherent records the note must equal the record's own bucket. |
| +- App prefixes and account directories the pass skips cannot carry records any build writes. |
| +- A store whose own teardown deleted the bucket is rightly not counted. |
| + |
| +## Grant's take-back check (item 3): identical |
| +`walk.go:303` against the base: |
| +- **Records first, then the ledger:** same order. |
| +- **Reconciliation:** the same `left()` exclusion, skipped for refused records. Old code dropped open intervals only when they belonged to the same account; new code drops them only when they are this exact app, and dedups. The loop skips `h.Account == acct` anyway, so the first foreign holder named is the same. |
| +- **Ledger read:** now skipped for a name the build would refuse, but `grant` already returned on that at `:221`. |
| +- **Errors:** both messages are byte-identical (`objstore: list the stores holding %s: %w`, `objstore: read ownership of %s: %w`), and herr is now returned unwrapped, exactly as before. |
| + |
| +## Tests (item 5) |
| +- **"the other store has left" (`economicspass_test.go:1806`):** passes with the new branch removed. B is held by the existing teardown branch, and A is sized because nothing checks. It only guards the `left()` exclusion once the check exists. |
| +- **"a sole holder":** also passes without the code. That is fine for a regression test. |
| +- **"two stores on one bucket" and "another holder on the ledger only":** both fail without the code, so they are genuine. |
| +- **Missing tests:** |
| + - a refused record that holds whatever the ledger shows; |
| + - an `OtherHolders` error that holds the store; |
| + - a holder appearing during sizing, which would expose the first P2; |
| + - the bucket with no ledger whose other store has left (P3 shape 1). |
| + |
| +## Claims the code contradicts |
| +- **19i says "held … until one of them leaves".** On a bucket with no ledger, B leaving (a retained teardown) never releases A. See P3 shape 1. |
| +- **19i says "It reads every store record once per store per day".** That is accurate but understated. It is S×N reads plus a `ListDirs` per account for every store. They run synchronously before statements and the bill pull, with no deadline, and one failure holds that store-day permanently. |
| +- **`OtherHolders` doc (`walk.go:1610`) says "a record exists from its grant until its teardown has finished … holds".** A record whose teardown noted the bucket deleted is not counted (`storeHolders`, `:1415`). That is minor and correct in behaviour, but the sentence overstates it. |
| +- **The comment above the take-back check (`walk.go:300`) says the check is made "as a minting grant checks them".** A minting grant uses `otherStoreHolders` plus an exclusive `openBucketOwner`, not `OtherHolders`. That sentence predates this branch, but it now points at a different function. |
| + |
| +**Verdict:** accept with fixes. Before merge: add the post-sizing ledger check and build the inventory once per pass (failing the whole pass rather than holding a store when the walk fails). Correct 19i on the hold-forever shapes and get a ruling on whether a store's leftover bytes should be billed to the remaining holder. |
diff --git a/implementation/carloku-paid-launch/OUTSTANDING.md b/implementation/carloku-paid-launch/OUTSTANDING.md| index 7d2c3679b..1deb617bc 100644 |
| --- a/implementation/carloku-paid-launch/OUTSTANDING.md |
| +++ b/implementation/carloku-paid-launch/OUTSTANDING.md |
| @@ -65,7 +65,7 @@ Each is concrete. None is waived. |
| 19. **No review of the whole branch has passed.** Astra rejected rounds one to sixteen; Opus accepted each with fixes. Round seventeen read only the diff since round sixteen: Opus accepted, Astra accepted with fixes (one P2, fixed since and unconfirmed). That is not a pass of the branch: the last whole-branch reading was round sixteen's, and it was rejected. Every round has closed the shapes the round before reported and found neighbouring ones, in the store walks since round seven. Some concern records no walk of this build writes; others need only this build's ordinary walks (a failed rotation, a retained teardown, a scanning door racing a teardown), and two were faults in the previous round's own fixes. Round eleven found one P1 and two P2 in the store walks (a grant could move an older record to another bucket name; a waiting rotation ignored a grant that ran; the user and its policy were written before the grant was counted). Its fixes are on the launch branch (`1395f7bf`). Round twelve read them: every round-eleven finding is closed for its reported shape, Astra rejected again (one P1, two P2) and Opus accepted with fixes (one P2, three P3). The new findings were about records no walk of this build writes (a teardown note for another bucket, a record naming two buckets) being acted on by teardown, rotation and the scanning doors; a rotation started after a grant that failed at its last write; and a take-back that returned access to a bucket another account holds, through the member's own door. Round thirteen read those fixes: closed for their reported shapes, Astra rejected again (two P1, four P2) and Opus accepted with fixes (one P2, five P3). The new findings were paths that read such a record and acted without the check (the holder inventory, the storage pass, the declare door), a rotation's own failed last write, and a retained record blocking another account's take-back. The check now sits in the record's reader and writer. Round fourteen read those: closed for their reported shapes; Astra rejected (two P1, two P2), Opus accepted with fixes (two P2, two P3). Round fifteen read those: all closed; Astra rejected (one P1, one P2), Opus accepted with fixes (one P2, two P3), all about readers of a bucket that a retained teardown left to another account (a sweep in flight, the scan status, the storage pass). Round sixteen read those: all closed, and one of them had made a single store under teardown hold storage for every account. Astra rejected (one P1, two P2), Opus accepted with fixes (one P1, one P2, two P3). The storage hold is now per account; two findings are left open as 19f and 19i. Those fixes are on the launch branch and unconfirmed until the next round reads them. Each round's fixes are on the launch branch, gated locally, and unconfirmed until the next round reads them. |
| 19g. **Refusals whose doors say the wrong thing, and records only a person can repair.** A record that names two buckets (`ErrTwoBuckets`) or whose teardown note is about another bucket (`ErrUnfinishedTeardown`) is refused by the record's one reader and one writer, so nothing acts on it and nothing writes it; the settings page shows such a store as unreadable. No walk of this build writes either; whether an earlier build left such records in production has not been checked. There is no operator verb to repair one. The doors that meet these refusals answer with sentences written for other failures: the grant and provision doors say to run the grant again, the destroy door and the purge say to try again, the scanning doors say to try again or to re-run the grant. The same goes for a rotation refused because a grant must be run again first (`ErrGrantUnfinished`) and for a member's take-back refused because another account holds the bucket (`ErrBucketHeld`). Correct copy for each needs approval (A). |
| -19i. **A live bucket shared by two stores was sized in full for each of them.** Fixed on this branch (`feat/carloku-storage-sole-holder-20261001`), not on the launch branch: the daily storage pass sizes a store only when it is the sole holder of its bucket, by the store records and the ownership ledger (`objstore.OtherHolders`, the same inventory the grant's take-back check now uses). A store on a bucket another store holds is held for its own account, every day, until one of them leaves: a person has to decide how such a bucket is billed, and there is no door for that. A store that left (closed ledger interval) does not count. It reads every store record once per store per day. Not reviewed. |
| +19i. **A live bucket shared by two stores was sized in full for each of them.** Fixed on this branch (`feat/carloku-storage-sole-holder-20261001`), not on the launch branch: the daily storage pass sizes a store only when it is the sole holder of its bucket, by one inventory of the store records per pass and the bucket's ownership ledger, read before and after the sizing. Otherwise the store is held for its own account. Holds that last until a person acts: a store that left the bucket keeps its record, and holds the remaining store, until its retained teardown is finished, which nothing runs (19a); a record no walk can follow, until it is repaired by hand (19g); another store's grant that died after writing its grant block, until it is run again; two apps of one account on one bucket, always; any store record that will not parse holds every store. Whose bytes a departed store's leftover objects are needs Paul's ruling (approval K). Not covered: one bucket adopted in two deployments is invisible to each and is sized in full in both. The walks' own holder scan still skips records under names no build writes. Reviewed once (`DISPOSITIONS-storage-sole-holder.md`); the fixes since are unread. |
| 19h. **A rotation whose delivery fails is repaired by a grant, which swaps the app's key with no overlap.** A failed delivery may still land, so the record stays marked, the next rotation is refused, and the grant run again replaces whichever key the app holds. Hosts hold a dead key until they adopt the new config. Before this branch such a rotation was resumed by running it again, which kept the overlap when the delivery had not landed and broke the app when it had. A repair that waits out the signing window and then reads which key was delivered would keep the overlap where the window is enforced (observed on AWS only, 19d); it is not built. |
| 19f. **A store that leaves a bucket with an unsupported name does not hand on its scanner debt.** There is no ledger for such a name. The last holder would delete the bucket and leave a scanning plan behind. Today's scanner (ClamAV) has no plan to leave, so this is latent. Also latent for the same reason: the scan grant door checks that no teardown has begun and then enables scanning as two steps, so a retained teardown landing between them would leave scanning enabled on the other account's bucket with nothing recording that it is owed. And the scan queue's drain checks permission once per batch, so a drain admitted before a retained teardown could return the other account's object metadata; nothing calls the drain or enqueues today, and it must be fixed before the queue is wired. |
| 19e. **A purge can end with a store that was granted during it.** A declare and a grant admitted while the app was live can land between the purge's last check and the removal of its marker. The purge now reports that as an error and never deletes the record, but it does not prevent it: the doors check liveness when they admit a request, not when they write. And after that error the estate is named only by its record and a log line. The app's console row is still trashed, so the settings page and the store-destroy door answer 404 and a second purge answers "not trashed", until the console restarts and restores the row from the record. The flash the member sees says "Nothing was deleted", which is false in this case; correct copy needs approval (A). |
| @@ -91,6 +91,7 @@ Nothing below was assumed overnight. None of it has been done. No row asks anyon |
| | G | AWS, read-write, scoped | Create the Cost and Usage Report data export in the collector plan. Separately: write, conditionally delete and remove one scratch object in the deployment bucket, to check that S3 honours `If-Match` on a delete (item 19c) | The store transfer meter's source; the conditional delete the store walks rely on | |
| | H | Box change | Path B on a dev box: replace the external replicator with the platform's own, including which user holds the replica credentials. Not ready to ask for: the code is partial and unwired (item 1) | The replication meter | |
| | J | What counts as evidence for a held bucket-hour | Whether an operator's entry must carry a retained artifact (which trail, which query, which interval) and whether a second person must approve it before it stands. The door as built takes free text from one operator | Whether the evidence door can ever release a store's first hour. Until ruled, it releases nothing | |
| +| K | Storage on a bucket a store has left | When a store leaves a bucket to another store, its objects may stay in it. Whether the remaining store's storage is billed with them, without them (which needs the bucket split by prefix), or held until the departed store's teardown is finished. As built on the storage branch: held | The storage meter for any account on an operator-adopted shared bucket | |
| | I | Merge | None requested. No review round has passed | | |
| ## Tested on this branch, not proven |
diff --git a/internal/console/economicspass.go b/internal/console/economicspass.go| index e048798a8..cf8dd9f03 100644 |
| --- a/internal/console/economicspass.go |
| +++ b/internal/console/economicspass.go |
| @@ -181,6 +181,19 @@ func (s *Server) storagePass(ctx context.Context, date string) error { |
| if err != nil { |
| return err |
| } |
| + // Who holds which bucket, read once for the whole pass. A record the |
| + // store could not return fails the pass before the day is written: the |
| + // day is create-only, and a store held on a reading that failed would |
| + // stay held. The pass runs again on the next tick. A record that is |
| + // there and will not parse is different: it will not parse next tick |
| + // either, so the day is written with every store held (below). |
| + inventory, err := objstore.ReadHolderInventory(ctx, s.cfg.Releases) |
| + if err != nil { |
| + return fmt.Errorf("storage pass: %w", err) |
| + } |
| + if len(inventory.Unparsed) > 0 { |
| + log.Printf("console: storage pass: store record(s) that will not parse: %v; no store can be shown to be the only holder of its bucket, so every store is held today", inventory.Unparsed) |
| + } |
| day := usage.StorageDay{V: 1, Date: date, Apps: map[string]usage.AppStorage{}} |
| for _, acctPrefix := range acctPrefixes { |
| acct := strings.TrimSuffix(strings.TrimPrefix(acctPrefix, "apps/"), "/") |
| @@ -226,26 +239,34 @@ func (s *Server) storagePass(ctx context.Context, date string) error { |
| // (and only this account), not measured. |
| st.StoreProvisioned = true |
| st.StoreUnswept = true |
| - } else if others, herr := storeSharedWith(ctx, s.cfg.Releases, rec, acct, app); herr != nil || len(others) > 0 { |
| - // The sizer lists the whole bucket. A bucket another store |
| - // also holds (an operator adopted it for both) has no line |
| - // between this store's bytes and the other's, so its size |
| - // is not this app's storage: recorded in full for each of |
| - // them, two accounts would each carry all of it. Whose |
| - // bytes are whose cannot be told from here, so the day is |
| - // held for this account, not measured. The same when the |
| - // holders cannot be read. |
| - if herr != nil { |
| - log.Printf("console: storage pass: holders of %s/%s's bucket: %v", acct, app, herr) |
| - } else { |
| - log.Printf("console: storage pass: %s/%s shares its bucket with %d other store(s); held", acct, app, len(others)) |
| - } |
| - st.StoreProvisioned = true |
| - st.StoreUnswept = true |
| } else if rec != nil && rec.Provisioned != nil { |
| st.StoreProvisioned = true |
| - n, serr := s.sizeStore(ctx, rec.Provisioned) |
| - if serr != nil { |
| + // The sizer lists the whole bucket. A bucket another store |
| + // also holds has no line between this store's bytes and |
| + // the other's, so its size is not this app's storage: |
| + // recorded in full for each, two accounts would each carry |
| + // all of it. "Holds" is by the store records (inventory, |
| + // read once for this pass) and by the ledger's open |
| + // intervals. A store that left the bucket to another keeps |
| + // its record until its retained teardown is finished, and |
| + // counts until then: its objects may still be in the |
| + // bucket, and whose those bytes are has not been decided. |
| + bucket := rec.Provisioned.Bucket |
| + onLedger, generation, lerr := objstore.OpenOnLedger(ctx, s.cfg.Releases, bucket, acct, app) |
| + if lerr != nil { |
| + // Not a hold for this one store: the day is not written |
| + // on a reading that failed, and the pass runs again. |
| + return fmt.Errorf("storage pass: %s/%s: %w", acct, app, lerr) |
| + } |
| + if len(inventory.Unparsed) > 0 { |
| + // Some store record in the deployment will not parse, |
| + // so which bucket it names is unknown, and no store |
| + // can be shown to be the only holder of its bucket. |
| + st.StoreUnswept = true |
| + } else if others := len(inventory.Others(bucket, acct, app)) + len(onLedger); others > 0 { |
| + log.Printf("console: storage pass: %s/%s shares its bucket with %d other store(s); held", acct, app, others) |
| + st.StoreUnswept = true |
| + } else if n, serr := s.sizeStore(ctx, rec.Provisioned); serr != nil { |
| log.Printf("console: storage pass: store %s/%s: %v", acct, app, serr) |
| st.StoreUnswept = true |
| } else if after, _, aerr := objstore.Get(ctx, s.cfg.Releases, acct, app); aerr != nil || |
| @@ -259,6 +280,14 @@ func (s *Server) storagePass(ctx context.Context, date string) error { |
| // size is discarded and the day held for this account. |
| log.Printf("console: storage pass: store %s/%s changed while it was being sized; held", acct, app) |
| st.StoreUnswept = true |
| + } else if _, now, gerr := objstore.OpenOnLedger(ctx, s.cfg.Releases, bucket, acct, app); gerr != nil || now != generation { |
| + // The bucket's ledger moved while it was being sized: a |
| + // grant opens its interval there before it delivers a |
| + // credential, so somebody may have been given the |
| + // bucket, and may have written to it, in between (and |
| + // may since have left). The size is discarded. |
| + log.Printf("console: storage pass: ownership of %s/%s's bucket changed while it was being sized; held", acct, app) |
| + st.StoreUnswept = true |
| } else { |
| st.StoreBytes = n |
| } |
| @@ -2425,12 +2454,3 @@ func (s *Server) handleAPIEconomicsBackhaul(w http.ResponseWriter, r *http.Reque |
| } |
| writeJSON(w, http.StatusOK, buildBackhaulReport(v)) |
| } |
| - |
| -// storeSharedWith names the other stores that hold a provisioned store's |
| -// bucket. A record with nothing provisioned holds no bucket to size. |
| -func storeSharedWith(ctx context.Context, store release.Store, rec *objstore.Record, acct, app string) ([]objstore.OwnerRef, error) { |
| - if rec == nil || rec.Provisioned == nil || rec.Provisioned.Bucket == "" { |
| - return nil, nil |
| - } |
| - return objstore.OtherHolders(ctx, store, rec.Provisioned.Bucket, acct, app) |
| -} |
diff --git a/internal/console/economicspass_test.go b/internal/console/economicspass_test.go| index 6269f6800..875b858c9 100644 |
| --- a/internal/console/economicspass_test.go |
| +++ b/internal/console/economicspass_test.go |
| @@ -1823,15 +1823,117 @@ func TestStoragePassSizesOnlyASoleHolderOfItsBucket(t *testing.T) { |
| if err := objstore.CloseBucketOwner(ctx, fx.rel, shared, other, "shop", at.Add(time.Hour)); err != nil { |
| t.Fatal(err) |
| } |
| + // The store that left keeps its record until its retained |
| + // teardown is finished, and its objects may still be in the |
| + // bucket. Whose those bytes are has not been decided, so the |
| + // remaining store is held too, not sized with them. |
| apps, sized := run(t, fx) |
| + if len(sized) != 0 { |
| + t.Fatalf("sized %v; want nothing sized while the departed store's record stands", sized) |
| + } |
| + for _, key := range []string{mine, theirs} { |
| + if app := apps[key]; !app.StoreUnswept || app.StoreBytes != 0 { |
| + t.Errorf("%s: %+v; want held", key, app) |
| + } |
| + } |
| + // Once the departed store's teardown has finished, its record is |
| + // gone, and the remaining store is the only holder. |
| + _, etag, err := fx.rel.Get(ctx, objstore.Key(other, "shop")) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := release.DeleteIfMatch(ctx, fx.rel, objstore.Key(other, "shop"), etag); err != nil { |
| + t.Fatal(err) |
| + } |
| + _, dayTag, err := fx.rel.Get(ctx, usage.StorageDayKey("2026-08-12")) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := release.DeleteIfMatch(ctx, fx.rel, usage.StorageDayKey("2026-08-12"), dayTag); err != nil { |
| + t.Fatal(err) |
| + } |
| + apps, sized = run(t, fx) |
| if len(sized) != 1 || sized[0] != shared { |
| - t.Fatalf("sized %v; want the remaining store's bucket once", sized) |
| + t.Fatalf("after the departed store's record is gone: sized %v; want the remaining store's bucket once", sized) |
| } |
| if app := apps[mine]; app.StoreUnswept || app.StoreBytes != 7<<30 { |
| t.Errorf("the remaining store: %+v; want measured", app) |
| } |
| - if app := apps[theirs]; !app.StoreUnswept || app.StoreBytes != 0 { |
| - t.Errorf("the store that left: %+v; want held", app) |
| + }) |
| + |
| + t.Run("a record under a name no build writes", func(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + ctx := context.Background() |
| + if _, err := objstore.Put(ctx, fx.rel, fixtureAcct, fixtureApp, provisioned(shared), ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + raw, _ := json.Marshal(provisioned(shared)) |
| + if _, err := fx.rel.Put(ctx, "apps/Not-An-Account/shop/store.json", raw, ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + apps, sized := run(t, fx) |
| + if len(sized) != 0 { |
| + t.Fatalf("a bucket a record under an invalid name also names was sized: %v", sized) |
| + } |
| + if app := apps[mine]; !app.StoreUnswept || app.StoreBytes != 0 { |
| + t.Errorf("%+v; want held", app) |
| + } |
| + }) |
| + |
| + t.Run("a record that will not parse", func(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + ctx := context.Background() |
| + seedOther(t, fx) |
| + if _, err := objstore.Put(ctx, fx.rel, fixtureAcct, fixtureApp, provisioned(shared), ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := fx.rel.Put(ctx, objstore.Key(other, "shop"), []byte("{not json"), ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + apps, sized := run(t, fx) |
| + if len(sized) != 0 { |
| + t.Fatalf("a store was sized while another record names nobody knows which bucket: %v", sized) |
| + } |
| + for _, key := range []string{mine, theirs} { |
| + if app := apps[key]; !app.StoreUnswept || app.StoreBytes != 0 { |
| + t.Errorf("%s: %+v; want held", key, app) |
| + } |
| + } |
| + }) |
| + |
| + t.Run("a holder appears while the bucket is being sized", func(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + ctx := context.Background() |
| + if _, err := objstore.Put(ctx, fx.rel, fixtureAcct, fixtureApp, provisioned(shared), ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := objstore.OpenBucketOwner(ctx, fx.rel, shared, fixtureAcct, fixtureApp, at, at, false); err != nil { |
| + t.Fatal(err) |
| + } |
| + fx.srv.storeSizer = func(_ context.Context, p *objstore.Provisioned) (int64, error) { |
| + // An operator grants the bucket to another store, which |
| + // writes to it and leaves again, all inside the listing. |
| + if err := objstore.OpenBucketOwner(ctx, fx.rel, shared, other, "shop", at.Add(time.Hour), at.Add(time.Hour), false); err != nil { |
| + t.Error(err) |
| + } |
| + if err := objstore.CloseBucketOwner(ctx, fx.rel, shared, other, "shop", at.Add(2*time.Hour)); err != nil { |
| + t.Error(err) |
| + } |
| + return 9 << 30, nil |
| + } |
| + if err := fx.srv.storagePass(ctx, "2026-08-12"); err != nil { |
| + t.Fatal(err) |
| + } |
| + raw, _, err := fx.rel.Get(ctx, usage.StorageDayKey("2026-08-12")) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + var day usage.StorageDay |
| + if err := json.Unmarshal(raw, &day); err != nil { |
| + t.Fatal(err) |
| + } |
| + if app := day.Apps[mine]; !app.StoreUnswept || app.StoreBytes != 0 { |
| + t.Errorf("%+v; want the size discarded and the store held", app) |
| } |
| }) |
diff --git a/internal/objstore/walk.go b/internal/objstore/walk.go| index 47e3dab4a..36f952476 100644 |
| --- a/internal/objstore/walk.go |
| +++ b/internal/objstore/walk.go |
| @@ -7,6 +7,7 @@ package objstore |
| import ( |
| "context" |
| "crypto/ecdh" |
| + "encoding/json" |
| "errors" |
| "fmt" |
| "log" |
| @@ -1651,3 +1652,132 @@ func OtherHolders(ctx context.Context, s release.Store, bucket, acct, app string |
| } |
| return out, nil |
| } |
| + |
| +// HolderInventory is one reading of every store record in the deployment, |
| +// arranged by the buckets the records name. It is for a pass that asks |
| +// about many buckets (the daily storage pass): one walk, not one per store. |
| +// |
| +// It reads the record objects by their literal keys, so a record that sits |
| +// under an account or app name no build would write is read too, and |
| +// counts: it names a bucket, whoever put it there. A record that cannot be |
| +// read (the store answered with an error) fails the whole reading, and the |
| +// caller tries again. A record that is there and will not parse is listed |
| +// in Unparsed: nobody can say which bucket it names, so while one exists no |
| +// store can be shown to be the only holder of anything. |
| +type HolderInventory struct { |
| + byBucket map[string][]OwnerRef |
| + // Unparsed lists the keys of store records that will not parse. |
| + Unparsed []string |
| +} |
| + |
| +// ReadHolderInventory walks apps/<account>/<app>/store.json. |
| +func ReadHolderInventory(ctx context.Context, s release.Store) (*HolderInventory, error) { |
| + inv := &HolderInventory{byBucket: map[string][]OwnerRef{}} |
| + accounts, err := s.ListDirs(ctx, "apps/") |
| + if err != nil { |
| + return nil, err |
| + } |
| + for _, adir := range accounts { |
| + a := strings.TrimSuffix(strings.TrimPrefix(adir, "apps/"), "/") |
| + apps, err := s.ListDirs(ctx, adir) |
| + if err != nil { |
| + return nil, err |
| + } |
| + for _, pdir := range apps { |
| + name := strings.TrimSuffix(strings.TrimPrefix(pdir, adir), "/") |
| + key := pdir + "store.json" |
| + raw, _, err := s.Get(ctx, key) |
| + if errors.Is(err, os.ErrNotExist) { |
| + continue |
| + } |
| + if err != nil { |
| + return nil, fmt.Errorf("objstore: holder inventory: %s: %w", key, err) |
| + } |
| + var rec Record |
| + if err := json.Unmarshal(raw, &rec); err != nil { |
| + inv.Unparsed = append(inv.Unparsed, key) |
| + continue |
| + } |
| + since := time.Time{} |
| + if rec.Grant != nil { |
| + since = rec.Grant.At |
| + } |
| + add := func(bucket string, refused bool) { |
| + if bucket == "" { |
| + return |
| + } |
| + for _, h := range inv.byBucket[bucket] { |
| + if h.Account == a && h.App == name { |
| + return |
| + } |
| + } |
| + inv.byBucket[bucket] = append(inv.byBucket[bucket], OwnerRef{Account: a, App: name, Since: since, refused: refused}) |
| + } |
| + if !release.ValidAccount(a) || !release.ValidApp(name) || Coherent(&rec, a, name) != nil { |
| + // A record nothing may act on holds every bucket it names. |
| + if rec.Provisioned != nil { |
| + add(rec.Provisioned.Bucket, true) |
| + } |
| + if rec.Grant != nil { |
| + add(rec.Grant.Bucket, true) |
| + } |
| + if rec.Teardown != nil { |
| + add(rec.Teardown.DeletedBucket, true) |
| + } |
| + continue |
| + } |
| + // A store whose own teardown noted its bucket deleted holds |
| + // nothing by that name any more. Any other record that names |
| + // a bucket holds it here, a teardown that left the bucket to |
| + // another store included: its objects may still be in it. |
| + if rec.Teardown != nil && !rec.Teardown.BucketDeletedAt.IsZero() { |
| + continue |
| + } |
| + switch { |
| + case rec.Provisioned != nil: |
| + add(rec.Provisioned.Bucket, false) |
| + case rec.Grant != nil: |
| + add(rec.Grant.Bucket, false) |
| + } |
| + } |
| + } |
| + return inv, nil |
| +} |
| + |
| +// Others names the stores, other than acct/app, whose records name bucket. |
| +func (inv *HolderInventory) Others(bucket, acct, app string) []OwnerRef { |
| + var out []OwnerRef |
| + for _, h := range inv.byBucket[bucket] { |
| + if h.Account == acct && h.App == app { |
| + continue |
| + } |
| + out = append(out, h) |
| + } |
| + return out |
| +} |
| + |
| +// OpenOnLedger names the apps, other than acct/app, with an open interval |
| +// on bucket's ownership ledger, and returns the ledger's generation as read |
| +// ("" when there is no ledger, or the name is one this build would refuse |
| +// and so has none). A caller that measures the bucket afterwards reads the |
| +// generation again: a grant opens its interval on the ledger before it |
| +// delivers a credential, so an unchanged generation means nobody was given |
| +// the bucket in between. |
| +func OpenOnLedger(ctx context.Context, s release.Store, bucket, acct, app string) (others []OwnerRef, generation string, err error) { |
| + if !ValidBucketName(bucket) { |
| + return nil, "", nil |
| + } |
| + ledger, etag, err := ReadBucketOwnership(ctx, s, bucket) |
| + if err != nil { |
| + return nil, "", fmt.Errorf("objstore: read ownership of %s: %w", bucket, err) |
| + } |
| + if ledger == nil { |
| + return nil, "", nil |
| + } |
| + for _, iv := range ledger.Intervals { |
| + if iv.open() && !(iv.Account == acct && iv.App == app) { |
| + others = append(others, OwnerRef{Account: iv.Account, App: iv.App, Since: iv.From}) |
| + } |
| + } |
| + return others, etag, nil |
| +} |