amadan
An older grant that names no bucket holds the name the deployment mints for it; the whole-diff review's small items
From the sixth review, the first of the whole change (Astra reject on one P1, Opus accept with fixes): - The holder inventory left out a record with a grant block that names no bucket and nothing provisioned. Its grant may have delivered a credential for the minted name. It now counts as a holder of that name; if the name cannot be resolved, every store is held. - Tests: the inventory's ledger read failing alone; clocks pinned where a test asserted a hold in last-hour mode or depended on the day. - Comments and 19i brought up to date, including that storage days written by the previous build on a shared bucket are not billable. Billing is off. This branch is not accepted and not merged.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
8 files changed,
+242
−18
docs/superpowers/specs/CODEX-FINDINGS-storage-sole-holder-review6.md+38 −0docs/superpowers/specs/DISPOSITIONS-storage-sole-holder.md+17 −0docs/superpowers/specs/OPUS-REVIEW-storage-sole-holder-review6.md+73 −0implementation/carloku-paid-launch/OUTSTANDING.md+1 −1internal/console/economicspass.go+12 −5internal/console/economicspass_test.go+69 −7internal/objstore/walk.go+28 −3internal/usage/storage.go+4 −2
CI passed — run details
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-storage-sole-holder-review6.md b/docs/superpowers/specs/CODEX-FINDINGS-storage-sole-holder-review6.md| new file mode 100644 |
| index 000000000..42d085855 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-FINDINGS-storage-sole-holder-review6.md |
| @@ -0,0 +1,38 @@ |
| +All locations refer to `09520a62`. Test and mutation outcomes below are inferred from source; no tests were run. |
| + |
| +1. **P1 — A legacy granted store with an unknown bucket is silently omitted from ownership evidence.** |
| + `internal/objstore/walk.go:1758`, `:1792`. |
| + |
| + Consider a surviving pre-ledger grant that delivered credentials for bucket X but failed before writing `Provisioned`. Its older grant block omits `Bucket`—a supported legacy shape explicitly described at `walk.go:499` and exercised at `ownership_test.go:3422`. Another account’s provisioned store now names X. |
| + |
| + The inventory successfully decodes the older record, `Coherent` accepts it, and `add("")` silently returns. It records neither a holder nor unresolved ownership. If X’s ledger contains only the newer store—or is absent—the generation checks and `SameStore` check pass, and X’s entire size is recorded against the newer store despite the surviving credential-bearing holder. |
| + |
| + **Smallest acceptable fix:** treat granted/provisioned estate with no resolvable bucket as unknown ownership, preventing sole-holder measurement until resolved. Keep declaration-only records exempt. Add a regression with an unnamed legacy grant, no interval for it, and another provisioned store naming its bucket. The existing grant-only inventory test at `economicspass_test.go:2190` always supplies a bucket name. |
| + |
| +2. **P3 — The baseline-ledger failure test does not protect `LedgerUnread`.** |
| + `internal/console/economicspass_test.go:2027`; implementation at `internal/console/economicspass.go:2479`. |
| + |
| + The inventory failure test makes **every** GET of the ledger fail. Removing the `LedgerUnread` guard would still pass that case: the subsequent `OpenOnLedger` fails and supplies the expected retry. |
| + |
| + The missed scenario is a failed baseline GET followed by a successful GET. Without the guard, the recovered generation differs from the inventory’s empty generation, so the pass writes an irreversible held day at midday instead of retrying. |
| + |
| + **Smallest acceptable fix:** add a first-ledger-GET-only failure to `TestStoragePassRetriesEachOfAStoresReadings`, alongside its existing second- and third-GET cases. |
| + |
| +3. **P3 — An existing sizing-failure test now depends on the wall clock.** |
| + `internal/console/economicspass_test.go:301`. |
| + |
| + `TestStoragePassMarksUnreachableStoreUnswept` expects an endpoint error to produce a written, held day, but never pins `storageClock`. It passes today because its requested date differs from today. With the clock on `2026-08-11` before 23:00 UTC, the same code returns a retry error and the test fails. |
| + |
| + **Smallest acceptable fix:** pin this test to its date’s last hour. The separate midday retry tests already cover the other outcome. |
| + |
| +Outside storage, I found no changed caller decision from the decoding wrapper: error text changes, while `errors.Is`/`errors.As` retain access to the original cause. The multi-error wrapper changes single-error `errors.Unwrap` behavior, but no repository caller relies on that. Grant’s take-back decision remains equivalent. |
| + |
| +The day object remains v1 with unchanged fields. Statements and the economics view read old and new days together correctly: bytes fold identically, and existing account-scoped completeness handling consumes the additional `StoreUnswept` cases. Previously recorded shared-bucket overcounts remain unchanged. The read-hook tests depend on explicit GET ordinals; the between-stores race test accommodates either traversal order. |
| + |
| +**Claims the code contradicts** |
| + |
| +- `implementation/carloku-paid-launch/OUTSTANDING.md:68` promises sole-holder sizing; finding 1 leaves an unresolved legacy holder outside both checks. The inventory argument in `DISPOSITIONS-storage-sole-holder.md:43` needs the same qualification: finding a record does not help when its unknown bucket is silently discarded. |
| +- `internal/console/economicspass_test.go:1725` says a departed store’s retained record does not count. The implementation and that test’s assertions count it until removal. Correct the comment. |
| +- `internal/console/economicspass_test.go:2407` says an app’s record is read by the inventory and its own visit “and by nothing else.” Successfully sized stores also receive the post-sizing reread. Restrict that statement to the declaration-only probe. |
| + |
| +**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| index 6bd45df07..407a57a1f 100644 |
| --- a/docs/superpowers/specs/DISPOSITIONS-storage-sole-holder.md |
| +++ b/docs/superpowers/specs/DISPOSITIONS-storage-sole-holder.md |
| @@ -77,3 +77,20 @@ Reports: `CODEX-FINDINGS-storage-sole-holder-review5.md`, `OPUS-REVIEW-storage-s |
| Mutation checks in a scratch copy, each failing its test: the ledger's validation removed; the last hour decided once at the start of the reading. |
| This branch is a fix for one open item, read five times; the last reading accepted it with small fixes, which are in the commit that adds this section and are unread. It is not merged, and it is not the launch branch's approval. |
| + |
| +## Review six (the whole diff `c68ecddd..09520a62`; Astra rejected, Opus accepted with fixes) |
| + |
| +Reports: `CODEX-FINDINGS-storage-sole-holder-review6.md`, `OPUS-REVIEW-storage-sole-holder-review6.md`. The first reading of the change as it would merge. Both found no behaviour change for callers outside the storage pass, the day object's shape and meaning unchanged for its readers, and Grant's take-back decision equivalent. Opus: four P3, none blocking. Astra: one P1, two P3. |
| + |
| +| Finding | Disposition | Where | Reproduction | |
| +| --- | --- | --- | --- | |
| +| Astra P1: an older grant block that names no bucket, with nothing provisioned, was left out of the inventory. Its grant may have delivered a credential for the deployment's minted name before it failed, and another store naming that bucket was sized as its only holder. | Fixed. The inventory resolves such a record to the name the deployment mints for it and counts it as a holder of that bucket. With no rule, or no answer, the record is unresolved, and then, as for a record that will not parse, every store is held. | `objstore/walk.go` `ReadHolderInventory` (`minted`, `Unresolved`); `console/economicspass.go` `storagePass` | `TestStoragePassCountsAGrantThatNamesNoBucket`, `TestHolderInventoryRules` | |
| +| Astra P3: the baseline-ledger failure was only tested with every read failing | Added: the inventory's read alone failing, mid-day and in the last hour. | `economicspass_test.go` | `TestStoragePassRetriesEachOfAStoresReadings/its ledger at the inventory` | |
| +| Astra P3, Opus P3-3: a sizing-failure test depended on the wall clock; the sole-holder cases and one race test ran in last-hour mode | Fixed. Each pins the clock: the sizing-failure tests to the last hour, the others to mid-day. | `economicspass_test.go` | the tests named in the reports | |
| +| Opus P3-1: three comments still gave `StoreUnswept` its old meaning | Corrected. | `usage/storage.go`, `economicspass.go` | none | |
| +| Opus P3-2: storage days written by the previous build on a shared bucket read as complete, and during a rolling deploy a console on the old build can write the day first | 19i says so: storage days written before every console runs this build are not billable for any store on a shared bucket. No code. | `OUTSTANDING.md` | none | |
| +| Astra, comments: a test comment said a departed store does not count; another said a record is read "by nothing else" | Corrected. | `economicspass_test.go` | none | |
| + |
| +Mutation checks in a scratch copy, each failing its test: the unnamed grant not counted; the unread-baseline guard off. |
| + |
| +Six readings. The whole-diff reading found one more P1, fixed in the commit that adds this section and not read by anyone. This branch is not accepted, and it is not merged. |
diff --git a/docs/superpowers/specs/OPUS-REVIEW-storage-sole-holder-review6.md b/docs/superpowers/specs/OPUS-REVIEW-storage-sole-holder-review6.md| new file mode 100644 |
| index 000000000..9718ff130 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/OPUS-REVIEW-storage-sole-holder-review6.md |
| @@ -0,0 +1,73 @@ |
| +## Whole-diff review: `feat/carloku-storage-sole-holder-20261001` at `09520a62` against `c68ecddd` |
| + |
| +I found nothing at P1 or P2. Every finding below is P3. |
| + |
| +### Question 1: bytes recorded for a store that could have another holder, or a hold on the wrong account |
| + |
| +I found no such path. Sizing happens only in `sizeSoleHolder` (`internal/console/economicspass.go:2464`). It is reached only when the inventory was read successfully and contains no record that fails to parse, and only for a store that is provisioned and not being torn down. It records bytes only when all of these hold: |
| + |
| +- **Ledger read at the start:** the store's ledger was read when the inventory was taken, and the ledger is valid now. |
| +- **Nothing changed since the inventory:** the ledger generation is the same as it was then. |
| +- **No other holder:** no other store's record or open ledger interval names the bucket. |
| +- **Same store afterwards:** the record read after the sizing has no teardown and `SameStore` matches. |
| +- **Nothing changed during the sizing:** the ledger generation is still the same. |
| + |
| +What makes the ordering argument work: |
| + |
| +- **Ledgers before records.** The inventory reads every ledger before any record (`walk.go:1709`). A grant writes its grant block, then the ledger, then the credentials. So a grant that finished before the ledgers were read leaves a record for the walk to find, and a grant after that moves a ledger generation the pass recorded. |
| +- **The generation is a reliable witness.** Ledgers are never deleted (the only writer is `writeBucketOwnership`, append-only). Every grant or departure changes the ledger's bytes, so it also changes the ETag. |
| +- **A departing store is still caught.** A store of another account that leaves the bucket keeps its record (retained teardown), or closes its interval, which also moves the generation. |
| + |
| +Each hold lands on the account it concerns: |
| + |
| +- **Shared bucket:** each store is held for its own account. |
| +- **Unreadable ledger:** only the stores on that bucket are held. |
| +- **Unparsed or unreadable record:** every store is held, as the dispositions and 19i rule. |
| + |
| +### Question 2: behaviour for callers outside the pass |
| + |
| +No caller's behaviour changes. |
| + |
| +- **`objstore.Get` and `Inspect`.** Every caller either logs the error or returns it. The console doors that answer users (`apistore.go:295,393`, `apiscan.go:155`, `appsettings.go:382,990`) send a fixed sentence. Nothing tests the decode error's type or text, and `mail/walk.go` has its own `Get`. The only visible change is a longer log message (`objstore: store record does not decode: …`). |
| +- **`OtherHolders` in Grant's take-back** (`walk.go:304`) gives the same result as the code it replaced: |
| + - holders come back in the same order; |
| + - the `left` filter makes no difference for ledger-derived holders, because their intervals are open, so `left()` is false; |
| + - the caller still skips holders from the same account; |
| + - the error strings are kept; |
| + - the new `ValidBucketName` gate is unreachable from Grant, which refuses invalid names earlier, at `walk.go:222`. |
| +- **The new exports** (`ReadHolderInventory`, `OpenOnLedger`, `ErrLedgerMalformed`, `storageClock`) have no callers outside the storage pass. |
| +- **The store-count reader** (`storepage.go:239`) is unaffected: the new holds set `StoreProvisioned: true`. |
| + |
| +### Question 3: the day object |
| + |
| +The shape has not changed: `usage.StorageDay` and `AppStorage` are untouched. A day written by either build reads the same way in the fold (`economicspass.go:923`) and in `storepage`. What `StoreUnswept` means has widened: it now also covers "shared, or the holder cannot be told". The fold already treats it as a hold for that account, so that is the right behaviour. One difference across builds remains (finding 2). |
| + |
| +### Findings |
| + |
| +**P3-1: comments still describe the old meaning of `StoreUnswept`.** |
| +- **Where:** `internal/usage/storage.go:6-7` ("could not size (bad credentials, endpoint down)"), `internal/console/economicspass.go:547` ("sizing failed, or the store was being torn down") and `economicspass.go:162-165` ("sizes … its tenant object store when one is provisioned"). |
| +- **Scenario:** a store on a shared bucket is now held with `StoreUnswept`, and its credentials are fine. Someone reading the dashboard or the fold will diagnose it as a sizing failure. |
| +- **Fix:** amend the three comments to add "or not shown to be the bucket's only holder". |
| + |
| +**P3-2: days written by the previous build on a shared bucket look complete.** |
| +- **Where:** `economicspass.go:923`, and 19i. |
| +- **Scenario:** the old build sized a shared bucket in full for each account, and those days read as complete. A month that spans the roll folds them into `storage_gb_month` for both accounts. During a rolling deploy, a sibling console still on the old build can also write that day's object first, and it is create-only. |
| +- **Fix:** no code. Add a sentence to 19i saying that storage days written before every console runs this build are not billable for any store on a shared bucket. Alternatively, confirm before the roll that no shared bucket exists in production. |
| + |
| +**P3-3: tests that assert holds run in last-hour mode.** |
| +- **Where:** `economicspass_test.go:1728` (`TestStoragePassSizesOnlyASoleHolderOfItsBucket`: every subtest except "a record that will not parse") and `:2108` (`TestStoragePassSeesAHolderThatCameAndWentBeforeAStoresTurn`). Neither pins `storageClock`, and the date is in the past, so `storageLastChance` is always true there. Nothing depends on the wall clock, so they are not flaky; the problem is what they can prove. |
| +- **Scenario:** a regression makes `OpenOnLedger` return a temporary error on every call. In last-hour mode that error becomes a hold. "two stores on one bucket", "another holder on the ledger only", the first half of "the other store has left" and "a record under a name no build writes" would all still pass. Only "a sole holder" catches it. |
| +- **Related:** `storagePass` ignores the `now` that `economicsPass` is given. Tests that drive `economicsPass` with a fixed `now` (`:93`) therefore always run storage in last-hour mode. |
| +- **Fix:** call `storageMidDay(t, "2026-08-12")` at the start of each subtest and of the second test. |
| + |
| +**P3-4: 19i's closing sentence will be stale.** |
| +- **Where:** `OUTSTANDING.md:68`, "Those last fixes are unread." |
| +- **Fix:** record this reading's outcome when it is filed. |
| + |
| +### Claims the code contradicts |
| + |
| +- **`usage/storage.go:6-7` and `economicspass.go:547`:** they describe `StoreUnswept` as a sizing failure or a teardown only (P3-1). |
| +- **`economicspass.go:162`:** it says the pass sizes every provisioned store. It now sizes only stores that are the sole holder of their bucket. |
| +- **The other claims hold:** I checked the new comments, the five disposition tables and the rest of 19i against the code, and they match. That includes the take-back being unchanged, the ledgers being read first, the record-only inventory, the permanent holds for a malformed ledger or a bucket name with no ledger, and the last-hour question being asked at the moment of failure. |
| + |
| +**Verdict: accept with fixes.** All four are P3, comment and test edits only, and none blocks the merge. |
diff --git a/implementation/carloku-paid-launch/OUTSTANDING.md b/implementation/carloku-paid-launch/OUTSTANDING.md| index 436c5aa3f..6397ba494 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 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 and the sharing it then creates is itself resolved, or, for a grant that was refused and whose undo write failed, until the block is removed by hand; a record under an account or app name no build writes, which no walk can touch, until it is removed by hand; an open ledger interval of a store whose record is gone, until the ledger is reconciled by hand; 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. A store on a bucket name this build keeps no ledger for is held, for good: its ownership evidence cannot be checked, and nothing short of moving the store to a valid name changes that. A ledger that will not parse holds its bucket's stores until it is repaired by hand. A grant block written after the inventory's walk, by a grant whose ledger write then failed, is not seen; that store has no credential, so the bytes sized are the bucket's own store's. A reading the store fails to return (a record, a ledger, a bucket listing) fails the pass for a retry until the day's last hour (23:00 UTC), when the day is written with the affected stores held; a pass that keeps failing repeats every listing and sizing each hour until then, and a reading that never succeeds (a bucket deleted out of band, a ledger the console may not read) does that every day. Open: the pass runs hourly, so the last hour has about one tick, and a failure of the listings it starts with, of the day's own write, or a pass that runs past that tick, still leaves the day unwritten, which holds storage for every account; it needs a short retry timer after 23:00. Reviewed five times (`DISPOSITIONS-storage-sole-holder.md`): Astra rejected the first four, Opus accepted each with fixes; the fifth reading, of the last diff, was accepted with fixes by both, P3 only. Those last fixes are unread. |
| +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 and the sharing it then creates is itself resolved, or, for a grant that was refused and whose undo write failed, until the block is removed by hand; a record under an account or app name no build writes, which no walk can touch, until it is removed by hand; an open ledger interval of a store whose record is gone, until the ledger is reconciled by hand; 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. A store on a bucket name this build keeps no ledger for is held, for good: its ownership evidence cannot be checked, and nothing short of moving the store to a valid name changes that. A ledger that will not parse holds its bucket's stores until it is repaired by hand. A grant block written after the inventory's walk, by a grant whose ledger write then failed, is not seen; that store has no credential, so the bytes sized are the bucket's own store's. A reading the store fails to return (a record, a ledger, a bucket listing) fails the pass for a retry until the day's last hour (23:00 UTC), when the day is written with the affected stores held; a pass that keeps failing repeats every listing and sizing each hour until then, and a reading that never succeeds (a bucket deleted out of band, a ledger the console may not read) does that every day. Open: the pass runs hourly, so the last hour has about one tick, and a failure of the listings it starts with, of the day's own write, or a pass that runs past that tick, still leaves the day unwritten, which holds storage for every account; it needs a short retry timer after 23:00. An older grant block that names no bucket counts as holding the name the deployment mints for it. Storage days written before every console runs this build are not billable for any store on a shared bucket: the previous build sized such a bucket in full for each, and those days read as complete. Reviewed six times (`DISPOSITIONS-storage-sole-holder.md`): the fifth reading, of a diff, was accepted with fixes by both reviewers; the sixth, of the whole change, was accepted with fixes by Opus and rejected by Astra on one P1, which is fixed since and 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). |
diff --git a/internal/console/economicspass.go b/internal/console/economicspass.go| index 3e66a4042..be17afbbe 100644 |
| --- a/internal/console/economicspass.go |
| +++ b/internal/console/economicspass.go |
| @@ -160,7 +160,8 @@ func (s *Server) censusPass(ctx context.Context, date string) error { |
| } |
| // storagePass sizes every app's deployment-bucket prefix, plus its tenant |
| -// object store when one is provisioned, create-only per day. Requires the |
| +// object store when one is provisioned and can be shown to be the only |
| +// holder of its bucket (sizeSoleHolder), create-only per day. Requires the |
| // bucket to support sized listings (release.SizeLister); a store that |
| // cannot answer marks the day's sweep absent rather than lying with zeros. |
| func (s *Server) storagePass(ctx context.Context, date string) error { |
| @@ -197,7 +198,9 @@ func (s *Server) storagePass(ctx context.Context, date string) error { |
| // from the start: it will not parse next tick either. It holds what it |
| // makes uncertain, at once: a record, every store (nobody can say which |
| // bucket it names); a ledger, the stores on that bucket. |
| - inventory, err := objstore.ReadHolderInventory(ctx, s.cfg.Releases) |
| + inventory, err := objstore.ReadHolderInventory(ctx, s.cfg.Releases, func(acct, app string) string { |
| + return objstore.MintedBucket(s.cfg.StoreDeployment, acct, app) |
| + }) |
| if err != nil { |
| if !storageLastChance(date) { |
| return fmt.Errorf("storage pass: %w", err) |
| @@ -208,7 +211,10 @@ func (s *Server) storagePass(ctx context.Context, date string) error { |
| if inventory != nil && 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) |
| } |
| - holdEveryStore := inventory == nil || len(inventory.Unparsed) > 0 |
| + if inventory != nil && len(inventory.Unresolved) > 0 { |
| + log.Printf("console: storage pass: granted store record(s) that name no bucket: %v; no store can be shown to be the only holder of its bucket, so every store is held today", inventory.Unresolved) |
| + } |
| + holdEveryStore := inventory == nil || len(inventory.Unparsed) > 0 || len(inventory.Unresolved) > 0 |
| day := usage.StorageDay{V: 1, Date: date, Apps: map[string]usage.AppStorage{}} |
| for _, acctPrefix := range acctPrefixes { |
| acct := strings.TrimSuffix(strings.TrimPrefix(acctPrefix, "apps/"), "/") |
| @@ -544,8 +550,9 @@ type usageSweep struct { |
| DurabilityExcluded map[string]int |
| // StorageExcluded counts, per account, the days SourceDays counts for |
| // usage.SourceStorage on which a store of that account could not be |
| - // measured (StoreUnswept: sizing failed, or the store was being torn |
| - // down). Its zero is not a measurement, so that account's own |
| + // measured (StoreUnswept: sizing failed, the store was being torn |
| + // down, or it was not shown to be the only holder of its bucket). Its |
| + // zero is not a measurement, so that account's own |
| // statement carries SourceDays[SourceStorage] less this number. Every |
| // other account's storage for the day was measured and stays complete. |
| StorageExcluded map[string]int |
diff --git a/internal/console/economicspass_test.go b/internal/console/economicspass_test.go| index f54898644..0946f897f 100644 |
| --- a/internal/console/economicspass_test.go |
| +++ b/internal/console/economicspass_test.go |
| @@ -205,6 +205,11 @@ func TestStoragePassSizesAppsAndDegradesStores(t *testing.T) { |
| fx.srv.storeSizer = func(ctx context.Context, p *objstore.Provisioned) (int64, error) { |
| return 0, errors.New("endpoint down") |
| } |
| + // The day's last hour: a sizing that fails earlier in the day fails the |
| + // pass for a retry (TestStoragePassRetriesEachOfAStoresReadings). |
| + prevClock := storageClock |
| + storageClock = func() time.Time { return time.Date(2026, 8, 11, 23, 5, 0, 0, time.UTC) } |
| + t.Cleanup(func() { storageClock = prevClock }) |
| if err := fx.srv.storagePass(ctx, "2026-08-11"); err != nil { |
| t.Fatal(err) |
| @@ -297,6 +302,11 @@ func TestStoragePassMarksUnreachableStoreUnswept(t *testing.T) { |
| fx.srv.storeSizer = func(ctx context.Context, p *objstore.Provisioned) (int64, error) { |
| return 0, errors.New("endpoint down") |
| } |
| + // The day's last hour: a sizing that fails earlier in the day fails the |
| + // pass for a retry (TestStoragePassRetriesEachOfAStoresReadings). |
| + prevClock := storageClock |
| + storageClock = func() time.Time { return time.Date(2026, 8, 11, 23, 5, 0, 0, time.UTC) } |
| + t.Cleanup(func() { storageClock = prevClock }) |
| if err := fx.srv.storagePass(ctx, "2026-08-11"); err != nil { |
| t.Fatal(err) |
| @@ -1722,9 +1732,10 @@ func TestStoragePassDiscardsASizeTakenAcrossAReplacementOfTheStore(t *testing.T) |
| // The sizer lists a whole bucket. A bucket two stores hold has no line |
| // between one store's bytes and the other's, so neither is sized: recorded |
| // in full for each, both would carry all of it. Each is held for its own |
| -// account. A store that is the only holder is sized, and a store that left |
| -// the bucket (its ledger interval is closed, its record is kept until its |
| -// retained teardown is run again) does not count as holding it. |
| +// account. A store that is the only holder is sized. A store that left the |
| +// bucket still counts as holding it for as long as its record stands (its |
| +// objects may still be there); once its record is gone the remaining store |
| +// is sized. |
| func TestStoragePassSizesOnlyASoleHolderOfItsBucket(t *testing.T) { |
| at := time.Date(2026, 8, 10, 9, 0, 0, 0, time.UTC) |
| const shared, other = "carlos-store-shared-x", "bcd" |
| @@ -1737,6 +1748,9 @@ func TestStoragePassSizesOnlyASoleHolderOfItsBucket(t *testing.T) { |
| run := func(t *testing.T, fx *consoleFixture) (map[string]usage.AppStorage, []string) { |
| t.Helper() |
| ctx := context.Background() |
| + // Mid-day, so every hold below is a hold on the evidence and not |
| + // the last-hour fallback, which holds on anything. |
| + storageMidDay(t, "2026-08-12") |
| var sized []string |
| fx.srv.storeSizer = func(_ context.Context, p *objstore.Provisioned) (int64, error) { |
| sized = append(sized, p.Bucket) |
| @@ -2110,6 +2124,7 @@ func TestStoragePassSeesAHolderThatCameAndWentBeforeAStoresTurn(t *testing.T) { |
| const mineBucket, theirBucket, other = "carlos-store-mine", "carlos-store-theirs", "bcd" |
| fx := newConsoleFixture(t) |
| ctx := context.Background() |
| + storageMidDay(t, "2026-08-12") |
| if _, err := fx.rel.Put(ctx, release.AppPrefix(other, "shop")+"marker", []byte("x"), ""); err != nil { |
| t.Fatal(err) |
| } |
| @@ -2191,20 +2206,30 @@ func TestHolderInventoryRules(t *testing.T) { |
| // Its own bucket noted deleted: holds nothing. |
| put("bcd", "gone", &objstore.Record{Grant: grant("carlos-store-d"), Provisioned: prov("carlos-store-d"), |
| Teardown: &objstore.TeardownBlock{StartedAt: at, BucketDeletedAt: at, DeletedBucket: "carlos-store-d"}}) |
| - inv, err := objstore.ReadHolderInventory(ctx, fx.rel) |
| + // An older grant block that names no bucket: resolved by the minted |
| + // name when there is a rule, unresolved when there is none. |
| + put("bcd", "unnamed", &objstore.Record{Grant: &objstore.GrantBlock{Backend: "aws", Source: objstore.GrantSourceOperator, At: at, By: "op"}}) |
| + if bare, err := objstore.ReadHolderInventory(ctx, fx.rel, nil); err != nil || len(bare.Unresolved) != 1 { |
| + t.Fatalf("with no minted-name rule: %+v %v; want the unnamed grant listed as unresolved", bare, err) |
| + } |
| + inv, err := objstore.ReadHolderInventory(ctx, fx.rel, func(acct, app string) string { return "carlos-store-minted-" + acct + "-" + app }) |
| if err != nil { |
| t.Fatal(err) |
| } |
| for bucket, want := range map[string]int{ |
| "carlos-store-x": 1, "carlos-store-y": 1, |
| "carlos-store-p": 1, "carlos-store-q": 1, |
| - "carlos-store-g": 1, |
| - "carlos-store-d": 0, |
| + "carlos-store-g": 1, |
| + "carlos-store-d": 0, |
| + "carlos-store-minted-bcd-unnamed": 1, |
| } { |
| if got := len(inv.Others(bucket, "bab", "asking")); got != want { |
| t.Errorf("%s: %d holder(s), want %d", bucket, got, want) |
| } |
| } |
| + if len(inv.Unresolved) != 0 { |
| + t.Errorf("with a minted-name rule the unnamed grant is still unresolved: %v", inv.Unresolved) |
| + } |
| } |
| // nthGet fails, or runs a hook before, the nth Get of one key. It still |
| @@ -2275,6 +2300,7 @@ func TestStoragePassRetriesEachOfAStoresReadings(t *testing.T) { |
| // the sizing, and after it. |
| "its own record": {objstore.Key(fixtureAcct, fixtureApp), 2}, |
| "its record after the sizing": {objstore.Key(fixtureAcct, fixtureApp), 3}, |
| + "its ledger at the inventory": {objstore.BucketOwnershipKey(bucket), 1}, |
| "its ledger before the sizing": {objstore.BucketOwnershipKey(bucket), 2}, |
| "its ledger after the sizing": {objstore.BucketOwnershipKey(bucket), 3}, |
| "the sizing of its bucket fails": {"", 0}, |
| @@ -2406,7 +2432,9 @@ func TestStoragePassHoldsAtOnceOnARecordThatDoesNotDecode(t *testing.T) { |
| // The holder inventory is read once for the whole pass, not once for each |
| // store: an app's record is read by the inventory and by the pass's own |
| -// visit to that app, and by nothing else, however many stores there are. |
| +// visit to that app, and, for an app that is never sized, by nothing else, |
| +// however many stores there are. (A store that is sized has its record read |
| +// once more, after the sizing.) |
| func TestStoragePassReadsItsInventoryOnce(t *testing.T) { |
| at := time.Date(2026, 8, 10, 9, 0, 0, 0, time.UTC) |
| fx := newConsoleFixture(t) |
| @@ -2494,3 +2522,37 @@ func TestStoragePassAsksForTheLastHourWhenAReadingFails(t *testing.T) { |
| t.Fatalf("day written %v, %+v; want the day written with the store held", written, app) |
| } |
| } |
| + |
| +// An older grant block names no bucket, and its grant may have delivered a |
| +// credential before it failed to record the store. The bucket it holds is |
| +// the one the deployment mints for it. Another store that names that bucket |
| +// is not its only holder, and is held. |
| +func TestStoragePassCountsAGrantThatNamesNoBucket(t *testing.T) { |
| + at := time.Date(2026, 8, 10, 9, 0, 0, 0, time.UTC) |
| + fx := newConsoleFixture(t) |
| + ctx := context.Background() |
| + storageMidDay(t, "2026-08-12") |
| + fx.srv.cfg.StoreDeployment = objstore.Deployment{Backend: "mem", Region: "eu-west-1", BucketPrefix: "carlos-"} |
| + legacy := objstore.MintedBucket(fx.srv.cfg.StoreDeployment, "bcd", "old") |
| + if legacy == "" { |
| + t.Fatal("fixture: the deployment mints no name") |
| + } |
| + if _, err := fx.rel.Put(ctx, release.AppPrefix("bcd", "old")+"marker", []byte("x"), ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + putRawStore(t, fx, "bcd", "old", &objstore.Record{ |
| + Declared: &objstore.Declared{EnvPrefix: "S3", At: at, By: "m@example.com"}, |
| + Grant: &objstore.GrantBlock{Backend: "mem", Source: objstore.GrantSourceOperator, At: at, By: "op"}}) |
| + putRawStore(t, fx, fixtureAcct, fixtureApp, &objstore.Record{ |
| + Grant: &objstore.GrantBlock{Backend: "mem", Bucket: legacy, Source: objstore.GrantSourceOperator, At: at, By: "op"}, |
| + Provisioned: &objstore.Provisioned{Bucket: legacy, Region: "eu-west-1", IAMUser: "u", KeyID: "K", At: at}}) |
| + sized := 0 |
| + fx.srv.storeSizer = func(context.Context, *objstore.Provisioned) (int64, error) { sized++; return 5 << 30, nil } |
| + if err := fx.srv.storagePass(ctx, "2026-08-12"); err != nil { |
| + t.Fatal(err) |
| + } |
| + day, _ := storageDay(t, fx, "2026-08-12") |
| + if app := day.Apps[fixtureAcct+"/"+fixtureApp]; sized != 0 || !app.StoreUnswept || app.StoreBytes != 0 { |
| + t.Fatalf("sized %d time(s), %+v; want held: an older grant with no bucket named holds the name the deployment mints for it", sized, app) |
| + } |
| +} |
diff --git a/internal/objstore/walk.go b/internal/objstore/walk.go| index 89cb089a5..611462f2e 100644 |
| --- a/internal/objstore/walk.go |
| +++ b/internal/objstore/walk.go |
| @@ -1676,6 +1676,13 @@ type HolderInventory struct { |
| ledgerUnread map[string]bool |
| // Unparsed lists the keys of store records that will not parse. |
| Unparsed []string |
| + // Unresolved lists the keys of records that carry a grant block and |
| + // name no bucket anywhere, which the caller's minted-name rule could |
| + // not resolve either. Such a grant may have created a bucket and |
| + // delivered a credential for it, so, as with an unparsed record, no |
| + // store can be shown to be the only holder of anything while one |
| + // exists. |
| + Unresolved []string |
| } |
| // Generation is bucket's ownership ledger generation as the inventory found |
| @@ -1697,7 +1704,14 @@ func (inv *HolderInventory) LedgerUnread(bucket string) bool { |
| } |
| // ReadHolderInventory walks apps/<account>/<app>/store.json. |
| -func ReadHolderInventory(ctx context.Context, s release.Store) (*HolderInventory, error) { |
| +// |
| +// minted gives the bucket name the deployment mints for an app. An older |
| +// grant block names no bucket (the name was derived each time), and a grant |
| +// that died after delivering a credential and before recording the store |
| +// leaves exactly that: a record that holds a bucket and does not say |
| +// which. minted resolves it. With no rule (nil), or an empty answer, the |
| +// record goes in Unresolved. |
| +func ReadHolderInventory(ctx context.Context, s release.Store, minted func(acct, app string) string) (*HolderInventory, error) { |
| inv := &HolderInventory{byBucket: map[string][]OwnerRef{}, generation: map[string]string{}, ledgerUnread: map[string]bool{}} |
| // The ledgers first, every one there is, and only then the records. |
| // A grant opens its interval on the ledger before it writes anything |
| @@ -1786,10 +1800,21 @@ func ReadHolderInventory(ctx context.Context, s release.Store) (*HolderInventory |
| continue |
| } |
| switch { |
| - case rec.Provisioned != nil: |
| + case rec.Provisioned != nil && rec.Provisioned.Bucket != "": |
| add(rec.Provisioned.Bucket, false) |
| - case rec.Grant != nil: |
| + case rec.Grant != nil && rec.Grant.Bucket != "": |
| add(rec.Grant.Bucket, false) |
| + case rec.Grant != nil || rec.Provisioned != nil: |
| + // Granted, and no bucket named anywhere in the record. |
| + name2 := "" |
| + if minted != nil { |
| + name2 = minted(a, name) |
| + } |
| + if name2 == "" { |
| + inv.Unresolved = append(inv.Unresolved, key) |
| + } else { |
| + add(name2, false) |
| + } |
| } |
| } |
| } |
diff --git a/internal/usage/storage.go b/internal/usage/storage.go| index ceda9a973..b0d63e1e8 100644 |
| --- a/internal/usage/storage.go |
| +++ b/internal/usage/storage.go |
| @@ -3,8 +3,10 @@ package usage |
| // AppStorage is one app's occupancy on one day: deployment-bucket bytes |
| // under the app's prefix, plus its tenant object-store bucket when one is |
| // provisioned. StoreProvisioned distinguishes an empty provisioned store from |
| -// no store; StoreUnswept marks a store the pass could not size (bad |
| -// credentials, endpoint down) — the dashboard must show "unswept", never 0. |
| +// no store; StoreUnswept marks a store the pass did not measure: it could |
| +// not size it (bad credentials, endpoint down), or the store was not shown |
| +// to be the only holder of its bucket, or its teardown had begun — the |
| +// dashboard must show "unswept", never 0. |
| type AppStorage struct { |
| BucketBytes int64 `json:"bucket_bytes"` |
| StoreBytes int64 `json:"store_bytes,omitempty"` |