amadan
Close round ten: every creating grant is counted before it calls the provider, a deletion note names its bucket, and two sharers of an unsupported name cannot wait on each other
Round ten (dispositions, "Round ten"): Astra three P1, Opus one P2 and one P3. - A grant that goes past the point of creating anything is counted on the record before it calls the provider, and the count is part of the estate a waiting walk compares. Round nine counted take-backs only; an ordinary retry of a failed grant could still create and deliver, fail at its last write, and leave the record equal to what a waiting teardown had read. - A teardown's note that it deleted a bucket names the bucket. A grant for another name over that unfinished teardown is refused, and never carries the note to another bucket's ledger. - For a bucket name this build would refuse, a store whose own teardown has begun is not counted as a holder, so the last sharer destroys the bucket and the first can then finish. Round nine's holder check had made two such stores wait on each other for good. Recorded as open in OUTSTANDING.md: the purge's late refusal leaves an estate reachable only after a console restart, with wrong member copy; a leaving store with an unsupported bucket name hands on no scanner debt. Includes the round eleven review brief.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
12 files changed,
+534
−22
docs/superpowers/specs/2026-10-01-stripe-idempotency-evidence.md+1 −1docs/superpowers/specs/CODEX-DISPOSITIONS-carloku-paid-launch-overnight.md+21 −0docs/superpowers/specs/CODEX-FINDINGS-carloku-paid-launch-overnight-round10.md+115 −0docs/superpowers/specs/CODEX-REVIEW-BRIEF-carloku-paid-launch-overnight-round11.md+28 −0docs/superpowers/specs/OPUS-REVIEW-carloku-paid-launch-overnight-round10.md+126 −0implementation/carloku-paid-launch/AGENT_HANDOFF.md+2 −2implementation/carloku-paid-launch/OUTSTANDING.md+3 −2internal/billingstripe/sandbox_fence_test.go+5 −1internal/objstore/fence.go+7 −4internal/objstore/objstore.go+12 −0internal/objstore/ownership_test.go+144 −3internal/objstore/walk.go+70 −9
diff --git a/docs/superpowers/specs/2026-10-01-stripe-idempotency-evidence.md b/docs/superpowers/specs/2026-10-01-stripe-idempotency-evidence.md| index 9aead313b..568dac63d 100644 |
| --- a/docs/superpowers/specs/2026-10-01-stripe-idempotency-evidence.md |
| +++ b/docs/superpowers/specs/2026-10-01-stripe-idempotency-evidence.md |
| @@ -6,7 +6,7 @@ Date: 2026-10-01. Status: **documentation evidence only. Not verified against St |
| The team quantity worker (`internal/console/billingquantity.go`) retires an operation that did not run and replaces it with a newer one. A request for the retired operation may still be on its way: its sender was paused after its last check, or the request was held in transit. A timed lease in the console's database cannot stop that request. Only the provider can. |
| -The fence: before an operation nobody can show ran is acknowledged, its idempotency key is consumed at Stripe with a metadata-only update of the same subscription item (`billingstripe.FenceQuantityKey`). The claim is that Stripe then refuses, or answers from memory, any later quantity update under that key. |
| +The fence: before an operation nobody can show ran is acknowledged, its idempotency key is consumed at Stripe with a metadata-only update of the same subscription item (`billingstripe.FenceQuantityKey`). The claim is that Stripe then refuses, or answers from memory, any later quantity update under that key. One acknowledgement consumes no key: when the fence cannot be taken, the operation is held with a review, and a person's resolution of that review stands in for the fence. |
| That claim is about Stripe's behaviour. The tests in `internal/console` use a fake provider written from the rules quoted below. They prove the console's logic given those rules. They do not prove the rules. |
diff --git a/docs/superpowers/specs/CODEX-DISPOSITIONS-carloku-paid-launch-overnight.md b/docs/superpowers/specs/CODEX-DISPOSITIONS-carloku-paid-launch-overnight.md| index 64cada8fb..e52b3622b 100644 |
| --- a/docs/superpowers/specs/CODEX-DISPOSITIONS-carloku-paid-launch-overnight.md |
| +++ b/docs/superpowers/specs/CODEX-DISPOSITIONS-carloku-paid-launch-overnight.md |
| @@ -347,3 +347,24 @@ Also changed: the lease reads its clock after it reads the lease object, so a le |
| Mutation checks in a scratch copy, each failing its test: no take-back count; grant comparing names only; an unsupported name destroyed regardless of holders; an open review not stopping a send. |
| Not done from this round: the simulations' remaining limits Astra lists (ownership sampled outside nested walks, a user's existence standing in for working credentials, the account-wide dating review, the accepted-held return skipping the final equality check); counting only walks that took the lease in the storage model. |
| + |
| +## Round ten (reviewed `366bd583`, code `94ca7482`; Astra rejected, Opus accepted with fixes) |
| + |
| +Reports: `CODEX-FINDINGS-carloku-paid-launch-overnight-round10.md`, `OPUS-REVIEW-carloku-paid-launch-overnight-round10.md`. |
| + |
| +Astra: three P1. Opus: one P2, one P3. Astra's third and Opus's first are the same fault. Nothing that attests, reaches Stripe with a switch unset, or makes an open late-request defect reachable. |
| + |
| +| Finding | Disposition | Where | Reproduction | |
| +| --- | --- | --- | --- | |
| +| Astra F1: an ordinary retry of a failed grant creates a bucket and delivers credentials, fails at its last write, and leaves the record equal to what a waiting teardown read | Accepted. Round nine counted take-backs only. Every grant that goes past the point of creating anything is now counted on the record (`GrantBlock.Walks`) before it calls the provider, and the count is part of the estate. An idempotent re-grant of a healthy store writes nothing. | `walk.go` `Grant`; `objstore.go` | `TestStaleTeardownDoesNotDestroyWhatAFailedGrantRetryMade` | |
| +| Astra F2: a take-back records one bucket's deletion on another bucket's ledger | Accepted. The note now names the bucket it is about (`TeardownBlock.DeletedBucket`; older records fall back to the record's own coordinates). A grant for another name over an unfinished teardown is refused until that teardown is finished, and the ledger that is told is the deleted bucket's. | `walk.go` `Grant`, `Teardown`; `objstore.go` | `TestTakeBackDoesNotRecordOneBucketsDestructionOnAnother` | |
| +| Astra F3, Opus N1: two accounts sharing an unsupported bucket name each wait on the other for good | Accepted. Introduced by round nine's holder check. For such a name a record whose own teardown has begun is not counted as a holder: its owner has asked for it to go, Grant refuses the name so the record cannot be taken back, and there is no ledger to break the tie. The last of the sharers destroys the bucket and the first can then finish. | `walk.go` `storeHolders` | `TestTeardownOfAnUnsupportedNameLeavesABucketAnotherStoreHolds` (both stores torn down in turn, then the first run again) | |
| +| Opus N1, second effect: a leaving store with scanning enabled records no scanner debt for an unsupported name | Accepted, not fixed. There is no ledger for such a name to carry the debt. Open. | none | none | |
| +| Opus N2: the purge's late refusal shows "Nothing was deleted" and leaves an estate no door reaches until the console restarts | Accepted, not fixed in code. The member copy is wrong for that case and new copy needs approval. `OUTSTANDING.md` 19e now says the estate is reachable only after a console start. Open. | `OUTSTANDING.md` | none | |
| +| Opus, claims: `fence.go`'s exception; the evidence document's opening; the sandbox check that accepted a failed re-read | Corrected. | `fence.go`, evidence document, `sandbox_fence_test.go` | none | |
| +| Astra, claims: the handoff on what unsupported-name teardowns decide from, and on same-app retries taking an ended lease | Corrected in the handoff. | `AGENT_HANDOFF.md` | none | |
| +| Opus: a grant that loses to a concurrent identical grant now gets a conflict instead of the idempotent answer | Noted. A spurious failure for the member, not a hold. Not changed. | none | none | |
| + |
| +The walk count is written with the same tolerance the grant's last write has: if the console's declare door wrote the record in between and the estate is unchanged, the count goes on the record as it now stands (`TestGrantRetriesLostCAS`). |
| + |
| +Mutation checks in a scratch copy, each failing its test: the walk not counted; a take-back allowed to carry a note to another bucket; leaving stores counted as holders of an unsupported name. Storage simulation: 3,000 seeds on this code. |
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-carloku-paid-launch-overnight-round10.md b/docs/superpowers/specs/CODEX-FINDINGS-carloku-paid-launch-overnight-round10.md| new file mode 100644 |
| index 000000000..e4d56b038 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-FINDINGS-carloku-paid-launch-overnight-round10.md |
| @@ -0,0 +1,115 @@ |
| +# Round-ten review: `366bd583` |
| + |
| +## 1. Round-nine dispositions |
| + |
| +Reviewed `366bd583`, base `aed42815`, concentrating on `3d4104cb..366bd583`. Static review only: no files modified, tests run, or network commands executed. Counterfactual test assessments below come from reading the code. |
| + |
| +All references identify the pinned commit. **O** = `internal/objstore`, **C** = `internal/console`, **R** = `internal/release`, **B** = `internal/billingstripe`, **D** = `implementation/carloku-paid-launch`. |
| + |
| +| Round-nine finding | Disposition | Fix and regression | Would the regression pass without the fix? | |
| +|---|---|---|---| |
| +| Astra F1: failed take-back leaves an unchanged estate identity | **Closed for the reported take-back.** Ordinary failed-grant retries still escape the comparison; F1 below. | Count persisted before effects at `O/walk.go:252`; comparison includes count and deletion state at `:1125`. Test: `O/ownership_test.go:2267`. | **No** without the count. The `EmptyBucket` case leaves the deletion flag unchanged and needs the count to distinguish estates. | |
| +| Astra F2: unsupported name destroyed without checking holders | **Closed for the reported deletion.** The new retention branch introduces F3 below. | Holder check: `O/walk.go:872`. Test: `O/ownership_test.go:2403`. | **No.** Restoring unconditional destruction loses the other store’s object. | |
| +| Astra F3: final purge race reports success with estate remaining | **Partly closed**, as dispositioned; prevention remains open under 19e. | Final read: `R/trash.go:263`. Test: `R/trash_test.go:515`, error assertion at `:567`. | **No.** The fifth landing point now requires `ErrStoreProvisioned`. It does not cover writes after the new final read. | |
| +| Astra F4: hashed leases collide with valid bucket names | **Closed.** | Disjoint namespace: `O/walklock.go:92`. Test: `O/ownership_test.go:2444`. | **No.** The former key fails the subdirectory assertion. | |
| +| Astra F5: young operation sends despite an open review | **Closed for the reported routing.** | Operation-specific review checked before sending: `C/billingquantity.go:1227`. Test: `C/billingquantity_test.go:2488`. | **No.** The old path sends during the open review instead of waiting. The test also exercises resolution and replacement. | |
| +| Astra F6: legacy creation-second reviews become disconnected | **Accepted and left open; no migration**, under the stated premise that this unmerged worker never ran. | Dispositions document `:341`; no code fix or regression. | Not applicable. The repository records the deployment premise; this review cannot independently establish it. | |
| +| Opus N1: stale grant resurrects an interveningly destroyed store | **Closed for the reported schedule.** | First-read estate captured at `O/walk.go:191`, compared at `:223`. Test: `O/ownership_test.go:2321`. | **No.** Comparing only the bucket permits the stale take-back. | |
| +| Opus N2: purge leaves estate without reporting it | **Partly closed**, same scope as Astra F3. | `R/trash.go:263`; `R/trash_test.go:515`. | **No** for the fifth tested landing point. | |
| +| Opus N3: destruction evidence lost between record and ledger writes | **Closed for the reported same-bucket replay.** The broadened replay introduces F2 below. | Replay before take-back at `O/walk.go:255`, on teardown retry at `:898`; marker-independent helper at `O/ownership.go:518`. Test: `O/ownership_test.go:2359`. | **No** without replay. **Yes** if only the former own-marker requirement is restored: this fixture retains that marker. | |
| +| Opus N4: unsupported name lacks holder protection | **Closed for the reported deletion**, same scope as Astra F2. | `O/walk.go:879`; `O/ownership_test.go:2403`. | **No.** | |
| +| Opus N5: sandbox pass leaves declined-card behavior undecided | **Accepted and left open**, with the requested decision added to the switch gate. | `D/OUTSTANDING.md:35` explicitly requires reading and deciding check 5b’s log. No new test. | The sandbox procedure could still pass without that decision. This is a documented human gate, not an automated assertion. | |
| + |
| +The round-nine claim items have these dispositions: |
| + |
| +| Claim item | Disposition and evidence | Regression/counterfactual | |
| +|---|---|---| |
| +| Every waiting walk recognizes the same estate | **Partly closed.** Grant now compares the captured estate; handoff describes rotation’s narrower comparison. F1 below still defeats the broader identity claim. | `O/ownership_test.go:2267`, `:2321` fail without their respective fixes, but omit F1’s no-teardown shape. | |
| +| Teardown decides through the ledger and protects other holders | **Partly closed.** Unsupported names now consult records, not the ledger (`O/walk.go:879`). The universal ledger wording remains inaccurate. | `O/ownership_test.go:2403` catches destruction, not eventual completion. | |
| +| A retained teardown finishes after destruction when rerun | **Partly closed.** Same-bucket evidence replay is repaired. F2 and F3 below undermine the broader claim. | `O/ownership_test.go:2359` checks only a nonnil ledger timestamp; it does not rerun a previously retained foreign store. | |
| +| Estate appearing during purge causes refusal | **Partly closed.** The fifth tested landing point now refuses. A read cannot cover later writes. | `R/trash_test.go:515` fails without the final read; no post-final-read schedule is asserted. | |
| +| Unsettled operations have a review and a resolution path | **Closed for the reported young-operation defect; legacy migration accepted open.** `C/billingquantity.go:1227`; dispositions `:341`. | `C/billingquantity_test.go:2488` distinguishes the routing fix. | |
| +| Every unconfirmed acknowledgement consumes the provider key | **Not closed as documentation.** The code explicitly permits human resolution instead (`C/billingquantity.go:658`); the handoff and evidence introduction omit it. | No documentation regression. | |
| +| Failed leases cannot overlap earlier accepted requests | **Partly closed as wording.** `O/fence.go:39` now states the same-kind exception; `walklock.go:47` also states it. The handoff remains unconditional. | No new regression; the underlying exception remains accepted open. | |
| +| Conditional record deletion still lacks CAS protection | **Closed as wording.** `O/walk.go:1214` now describes conditional deletion correctly. | Existing implementation uses `DeleteIfMatch` at `:1243`; no new behavioral fix. | |
| +| A grant cannot follow declaration deletion during purge | **Closed as wording.** `R/trash.go:218` now names the delayed re-declaration exception. | The fifth case at `R/trash_test.go:515` exercises that exception. | |
| +| Every prior finding received a disposition | **Closed for the omitted N13 remainder.** Explicitly covered by dispositions `:337`. | Not applicable. | |
| +| Storage oracle detects actual re-grant data loss | **Closed for the injected-loss check.** `O/walk_sim_test.go:297` now deletes the object; `:299` checks its contents. | Test at `:215` would also pass with the former forced-error implementation. With the new injection retained, removing the actual data check defeats the test. | |
| +| Canceled-subscription reread proves unchanged quantity | **Accepted and left open within the sandbox limitations.** The permissive reread at `B/sandbox_fence_test.go:421` remains. | An unrun procedure; a failed reread still does not establish unchanged quantity. | |
| + |
| +The destruction-replay test’s name is broader than its assertions: it synthesizes the persistence gap by clearing `DestroyedAt`, leaves the destroying marker intact, and checks only that a timestamp reappears. It does not establish correct bucket attribution or release of the original retained teardown. |
| + |
| +## 2. New findings |
| + |
| +### F1. P1 — An ordinary failed-grant retry can create usable estate without changing the waiting teardown’s identity |
| + |
| +**Where:** `O/walk.go:231`, `:262`, `:416`, `:437`, `:448`, `:802`, `:1114`. |
| + |
| +**Scenario:** |
| + |
| +1. Grant G0 records its grant and opens ownership, then fails at `EnsureUser`, before creating the bucket. The record has `Grant`, but neither `Provisioned` nor `Teardown`. |
| +2. After G0’s lease expires, teardown T reads that record and pauses before acquiring its lease. |
| +3. Grant G1 retries. Both `wroteGrant` and `takeBack` are false, so it makes no preliminary store-record write. |
| +4. G1 creates the bucket, mints and delivers credentials, then its final record write fails before taking effect. |
| +5. The app adopts the delivered configuration and writes data. After G1’s lease expires, T resumes. |
| +6. The record still has exactly the grant, empty key and absent teardown that T first read. `estateOf` compares equal, so T empties and deletes the newly usable bucket. |
| + |
| +This requires neither clock disagreement nor a request outside its lease. Unlike round nine’s take-back, there is no teardown block whose counter changes. |
| + |
| +**Smallest acceptable fix:** Persist an attempt generation before any grant retry can create or deliver usable estate, including a retry with an existing grant and no teardown. Include it in the expected estate identity. Add the schedule above with failure of G1’s final write and preservation of newly written data. |
| + |
| +### F2. P1 — Take-back can record one bucket’s destruction against another bucket’s ledger |
| + |
| +**Where:** `O/walk.go:176`, `:233`, `:255`, `:827`, `:1040`; `O/ownership.go:518`. |
| + |
| +**Scenario:** |
| + |
| +1. An accepted legacy or hand-repaired record has `Declared` and `Provisioned` naming bucket X, but no `Grant`. This is not a shape produced by a successful current grant, but teardown explicitly handles it through the provisioned coordinates. |
| +2. Teardown deletes X, persists its deletion time D, then fails at `DeleteUser`. |
| +3. The app’s currently minted bucket name is Y, different from X. Y already has an ownership ledger and a live foreign holder C. Another account’s store A previously left Y and remains retained, with its interval closed before D. Y and A’s objects have never been deleted. |
| +4. Grant takes back the record. `bucketFor` ignores `Provisioned` and chooses Y; it creates a grant block for Y. |
| +5. Before opening ownership or creating anything, it calls `markBucketDestroyed(Y, D)` using X’s deletion note. |
| +6. The helper now requires neither a matching destroying marker nor evidence binding D to Y. It writes Y’s `DestroyedAt`, even if the subsequent exclusive ownership check refuses the grant. |
| +7. A’s retained teardown sees the false destruction time after its leaving time. At `:1040` it clears `foreign` and can remove its record successfully, although C can still read A’s objects in Y. |
| + |
| +The new replay converts an inconsistent-but-readable record into false deletion authority over an unrelated ledger. |
| + |
| +**Smallest acceptable fix:** Bind deletion evidence to the bucket actually deleted. Before replaying it, derive the old bucket from the provisioned/grant coordinates and reject disagreement or an attempted bucket change. Add an X/Y regression proving that Y’s destruction timestamp stays unchanged and A remains retained. |
| + |
| +### F3. P1 — Unsupported-name stores can retain each other permanently after both credentials are revoked |
| + |
| +**Where:** `O/walk.go:879`, `:883`, `:988`, `:1001`, `:1045`, `:1189`. |
| + |
| +**Scenario:** |
| + |
| +1. Two records in different accounts name the same backend-supported bucket `Legacy_Bucket`. |
| +2. Teardown A sees B’s record, leaves the bucket, deletes A’s user, and returns `ErrBucketRetained`, keeping A’s record. |
| +3. Teardown B sees A’s retained record. It likewise leaves the bucket, deletes B’s user, and retains B’s record. |
| +4. Every subsequent invocation sees the other record as a holder. `otherStoreHolders` excludes only records with a bucket-deletion note; neither teardown has deleted the bucket. |
| +5. Unsupported names never enter the ledger’s leaving/closed-interval decision. Neither teardown can become the last holder, delete the bucket, or remove its own record. |
| + |
| +This worsens open item 19a: manually rerunning both teardowns cannot finish, even though both principals are gone. It follows directly from the newly added retention branch. |
| + |
| +**Smallest acceptable fix:** Preserve revocation and shared destruction evidence for unsupported names, using a collision-free identity, so the last departing holder can destroy the bucket and earlier retained teardowns can verify that destruction. Extend `O/ownership_test.go:2403` to tear down both foreign-account holders and rerun the first to completion. |
| + |
| +### Other requested checks |
| + |
| +Both direct purge callers honor the error. The console returns at `C/ownership.go:505`, before relay aggregate publication at `:516` and app-row removal at `:532`. Box auto-purge logs and continues at `internal/domains/reconcile.go:232`; it does not report success or perform success-dependent cleanup afterward. This does not undo objects already removed inside `PurgeApp`. |
| + |
| +I found no additional operation-specific review routing failure in the inspected quantity paths. Missing reviews fall through; open reviews stop sends; resolved reviews enter settlement. |
| + |
| +Reading the clock after the lease object at `O/walklock.go:143` preserves the CAS dependency and avoids judging the fetched lease with an older timestamp. I found no dependency requiring the former ordering. |
| + |
| +## 3. Claims the code contradicts |
| + |
| +| Location | Claim | Contradiction | |
| +|---|---|---| |
| +| `D/AGENT_HANDOFF.md:35`; `D/OUTSTANDING.md:85` | Waiting walks recognize the same estate. | F1 creates and delivers usable estate while the persisted identity remains equal. The take-back fix covers only records carrying a teardown block. | |
| +| `D/AGENT_HANDOFF.md:35` | Teardown decides by CAS on the ledger. | Unsupported-name teardown decides from store-record enumeration at `O/walk.go:879`, without the ledger’s departure or destruction state. | |
| +| `D/AGENT_HANDOFF.md:35`; `D/OUTSTANDING.md:63` | A retained teardown can finish when someone reruns it after destruction. | F2 permits false completion without destruction. F3 prevents the participating teardowns from reaching destruction at all. | |
| +| `O/ownership.go:511` | `markBucketDestroyed` records a fact about the named bucket taken from its teardown. | Its caller can supply a different bucket’s deletion note, and the helper accepts it; F2. | |
| +| `D/OUTSTANDING.md:59`; dispositions `:340` | A purge with surviving estate reports an error and keeps the app row. | This holds only for estate visible to `R/trash.go:263`. Delayed declaration/grant writes after that read still permit success and row removal. This qualifies the already-open 19e race, rather than adding a separate finding. | |
| +| `D/AGENT_HANDOFF.md:38`; Stripe evidence document `:9` | Every unconfirmed acknowledgement first consumes the provider key. | Resolved human review substitutes for provider fencing; `C/billingquantity.go:658` states the exception explicitly. | |
| +| `D/AGENT_HANDOFF.md:35` | Failed walks reserve the lease until expiry, so earlier requests are refused before takeover. | Same-app, same-kind retries can take an ended lease immediately at `O/walklock.go:155`. The code comments now name the exception; the handoff still needs it. | |
| + |
| +**Verdict: reject.** |
| \ No newline at end of file |
diff --git a/docs/superpowers/specs/CODEX-REVIEW-BRIEF-carloku-paid-launch-overnight-round11.md b/docs/superpowers/specs/CODEX-REVIEW-BRIEF-carloku-paid-launch-overnight-round11.md| new file mode 100644 |
| index 000000000..b3635e390 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-REVIEW-BRIEF-carloku-paid-launch-overnight-round11.md |
| @@ -0,0 +1,28 @@ |
| +# Review brief, round 11: Carloku paid launch, overnight changes of 2026-10-01 |
| + |
| +Read the round 1 brief (`CODEX-REVIEW-BRIEF-carloku-paid-launch-overnight.md`) for the fixed rulings and the output rules, the round 3 to 10 briefs for rulings 6 to 12, then `CODEX-DISPOSITIONS-carloku-paid-launch-overnight.md` (its last section covers round 10) and `implementation/carloku-paid-launch/OUTSTANDING.md`. |
| + |
| +Review the commit named in the prompt. Read source only with `git show <commit>:<path>`. The changes since round 10 are `git diff 94ca7482 <commit>`. They are in `internal/objstore` only, apart from documents and one sandbox test line. |
| + |
| +Still the fixed snapshot: round 10's findings, nothing else. Nothing may attest. The quantity worker has two switches, both unset. Billing is off. |
| + |
| +Do not report again, as findings, what `OUTSTANDING.md` lists as open: items 6 and 6a, 7 and 9, 19a to 19f. Do report anything that makes one of them worse than the document says, and any claim about them the code contradicts. |
| + |
| +## First: a disposition table |
| + |
| +For every round 10 finding (Astra F1 to F3; Opus N1, N2 and the claims): closed, partly closed, not closed, or accepted and left open as the dispositions say. File and line of the fix and of the test, and whether the test would pass without the fix. |
| + |
| +## Then: new findings only |
| + |
| +- **The walk count** (`internal/objstore/walk.go` `Grant`, `GrantBlock.Walks`, `estateOf`). The claim: any grant that creates or delivers anything has changed the record before it calls the provider, so a walk that read the record earlier and waited for its lease does not take what now stands for the store it was asked about. Find a provider call that can create, mint, deliver or delete before the count is written. Find a path where the count is written and a legitimate walk is then refused for good. Check the count's own conflict retry: can it put the count on a record that is a different estate? |
| +- **The deletion note's bucket** (`TeardownBlock.DeletedBucket`, the refusal in `Grant`, `markBucketDestroyed`). Find a note carried to a ledger it is not about, by any path. Find a record that can now never be granted and never be torn down. |
| +- **Unsupported names** (`storeHolders` with leaving stores left out). Find a bucket destroyed while a store that has not asked to go still holds it. |
| +- **Rotation.** It does not write the walk count or compare it. Say whether a rotation can now act on a store other than the one it was asked about. |
| +- **Claims** in `AGENT_HANDOFF.md` item 5 and `OUTSTANDING.md`. |
| + |
| +## What to return |
| + |
| +1. The disposition table. |
| +2. New findings, most severe first, in the round 1 format. |
| +3. Claims the code contradicts. |
| +4. A one-line verdict: accept, accept with fixes, or reject. |
diff --git a/docs/superpowers/specs/OPUS-REVIEW-carloku-paid-launch-overnight-round10.md b/docs/superpowers/specs/OPUS-REVIEW-carloku-paid-launch-overnight-round10.md| new file mode 100644 |
| index 000000000..ede4eeb33 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/OPUS-REVIEW-carloku-paid-launch-overnight-round10.md |
| @@ -0,0 +1,126 @@ |
| +# Opus production and authority review, round 10: Carloku paid launch, `aed42815..366bd583` |
| + |
| +I read `3d4104cb..366bd583` and only used `git show`. This was a read-only pass: I changed nothing, ran no tests and made no network calls. Every "would the test fail without the fix?" answer comes from reading the test against the code. The code tip is `94ca7482`; `366bd583` adds documents only. |
| + |
| +Abbreviations: **O** = `internal/objstore`, **C** = `internal/console`, **R** = `internal/release`, **B** = `internal/billingstripe`. |
| + |
| +**Production authority** |
| + |
| +- **Nothing attests.** `SetAttestation` and `SetAllClientsInstrumented` (`usage/durabilityledger.go:1455`, `:1475`) have no non-test caller. Nothing under `internal/usage`, `internal/host` or `cmd` changed this round. |
| +- **The quantity worker cannot reach Stripe with either switch unset.** |
| + - There are still two non-test provider calls: `UpdateQuantityWith` (`C/billingquantity.go:1059`) and `FenceQuantityKey` (`:1301`). |
| + - Both are reachable only through `billingQuantityPass`. That function returns before any read unless `quantityWorker()` (`:133-139`) sees a proration policy and `CONSOLE_BILLING_QUANTITY_FENCE=stripe-sandbox-verified`. |
| + - `billingservice.go`, `B/client.go` and `cmd/console/billing.go` did not change this round. |
| +- **The sandbox verification test** (`B/sandbox_fence_test.go`) did not change and has not been run. `OUTSTANDING.md` item 6 now states that check 5b must be read and decided on before the switch is set. |
| +- **Billing a customer wrongly.** I found no path. The only quantity change is a new gate that makes the worker send less: an operation with a review of its own is never sent. |
| +- **Leaks.** No new log line or error carries a key, a secret or a Stripe identifier. |
| +- **Customer data.** |
| + - I found no path that destroys a customer's data. |
| + - I found one new path that keeps a customer's data forever after both owners asked to destroy it (N1). |
| + - The purge's new refusal tells the member something false (N2). |
| + |
| +## 1. Disposition table (round 9) |
| + |
| +| Finding | Status | Fix | Test; fails without the fix? | |
| +|---|---|---|---| |
| +| Astra F1: a failed take-back leaves a record that still compares equal | **Closed** | `TakeBacks` counted in the take-back's first write, before anything is created (`O/walk.go:252-266`). `estateOf` adds `bucketDeleted` and `takeBacks` (`:1106-1129`). | `O/ownership_test.go` `TestStaleTeardownDoesNotDestroyWhatAFailedTakeBackMade`. **Fails.** The `EmptyBucket` shape changes nothing on the record except the count. The `DeleteUser` shape also fails without `bucketDeleted`. | |
| +| Astra F2 / Opus N4: an unsupported name is destroyed with no holder check | **Closed for the reported case.** The fix opens a permanent wedge when both holders are of different accounts and both tear down (N1). | `O/walk.go:872-894` | `TestTeardownOfAnUnsupportedNameLeavesABucketAnotherStoreHolds`. **Fails.** It has only one teardown and does not test the mutual case. | |
| +| Astra F3 / Opus N2: a purge reports success with estate remaining | **Partly closed, as the dispositions say; open as 19e.** Both callers treat the error as "not purged" (§2, "The purge's last read"). The member is told "Nothing was deleted" and sent to a page that answers 404 (N2). | `R/trash.go:251-273` | `R/trash_test.go` fifth case now requires `ErrStoreProvisioned`. **Fails** (it returned nil before). | |
| +| Astra F4: a hashed lease key is a name a bucket could have | **Closed** | `O/walklock.go:90-93` (`by-hash/`). A valid bucket name has no `/`. | `TestHashedLeaseKeysCannotCollideWithABucketName`. **Fails.** | |
| +| Astra F5: a young operation keeps being sent while its review is open | **Closed** | `C/billingquantity.go:1222-1235` | `TestQuantityYoungOperationWithAnOpenReviewIsNotSentAgain`. **Fails**: `q.posts` would not be empty. | |
| +| Astra F6: legacy creation-second reviews are orphaned | **Accepted, left open, no migration.** The reasoning holds: that reason existed only between two commits of an unmerged branch, and the worker has never run. | — | — | |
| +| Opus N1: a stale grant takes back a store torn down after its request | **Closed** | `O/walk.go:191`, `:223-225` | `TestStaleGrantDoesNotTakeBackAStoreTornDownAfterItsRequest`. **Fails.** The name check alone passes, because the minted name is the same. | |
| +| Opus N2: a purge reports success and leaves an orphan | See Astra F3 above. | | | |
| +| Opus N3 (N13 remainder): a crash between the two writes, then a take-back, loses the destruction | **Closed** | Take-back marks the ledger first (`O/walk.go:253-258`). A `bucketDone` re-run marks again (`:895-900`). `markBucketDestroyed` no longer needs the marker (`O/ownership.go:518-530`). | `TestDestructionReachesTheLedgerAfterACrashBetweenTheTwoWrites`. **Fails** in each branch without its own mark. It asserts `DestroyedAt != nil`; it does not run the retained teardown that this exists to release. | |
| +| Opus N5: a sandbox pass leaves the worker's decline outcome undecided | **Accepted, left open**, now written into item 6 | `OUTSTANDING.md` item 6 | n/a | |
| +| Opus claims: stale delete comment, purge comment, `fence.go` sentence, oracle test that did not delete | **Closed.** One wording remainder in `fence.go` (§3). | `O/walk.go` (`deleteRecordIfUnchanged` comment), `R/trash.go:211-224`, `O/fence.go:39-43`, `O/walk_sim_test.go` | The oracle now deletes the object, so the check has to detect the loss. | |
| + |
| +## 2. New findings, most severe first |
| + |
| +### N1. P2: two accounts sharing an unsupported bucket name each wait for the other forever, and the bucket is never destroyed |
| + |
| +**Where:** |
| +- The unsupported-name branch, `O/walk.go:879-894`. |
| +- `otherStoreHolders` counts any record whose own teardown has not noted a deleted bucket (`:1189`). |
| +- The release by `DestroyedAt` exists only for valid names (`:1025`). |
| + |
| +**Scenario:** |
| +1. Records of accounts X (app A) and Y (app B) both name `Legacy_Bucket`. |
| +2. A's member destroys A. |
| + - `otherStoreHolders` returns B, so `destroy = false` and `foreign = true`. |
| + - A's user is deleted, and the walk returns `ErrBucketRetained` with A's record kept (`:1045`). |
| +3. Later B's member destroys B. |
| + - `otherStoreHolders` returns A, because A's record has a teardown block and `BucketDeletedAt` is zero. |
| + - So `destroy = false` and `foreign = true`. B's user is deleted, and the walk returns `ErrBucketRetained`. |
| +4. Every re-run of either teardown repeats step 3 against the other's record. Grant refuses invalid names, so neither record can ever change. |
| + |
| +**Result:** |
| +- Both owners asked for destruction, and the bucket and both accounts' objects stay forever. |
| +- Team Tito keeps paying for the bucket. |
| +- Nobody is told beyond an error string that has no member copy. |
| +- For a valid name the ledger breaks this tie: A is `known` with a closed interval, so B destroys. The unsupported branch has no tie-break. |
| + |
| +**Second, smaller effect:** a leaving store with scanning enabled records no scanner debt for an unsupported name (`:917`). The last holder then deletes the bucket and leaves its Malware Protection plan behind, counting against the 25-per-region quota. |
| + |
| +**Smallest fix:** |
| +- In the unsupported branch, do not count a holder whose record carries a `Teardown` block. Grant refuses these names, so such a record can never be taken back, and its owner has already asked for destruction. |
| +- Test two cross-account records, both torn down in turn. The second must destroy the bucket, and a re-run of the first must then finish. |
| +- Optionally, refuse an unsupported-name leave while that record has scanning enabled. |
| + |
| +### N2. P3: the purge's new late refusal tells the member "Nothing was deleted" and sends them to a page that answers 404 |
| + |
| +**Where:** |
| +- `R/trash.go:262-271` returns `ErrStoreProvisioned` after the sweep and after the marker's delete. |
| +- `C/ownership.go:502-505` shows `purgeAdvice` (`:563-573`): "tear it down on the app's Settings page … Nothing was deleted." |
| + |
| +**Scenario:** |
| +1. The fifth landing point of 19e: a declare and a grant land between the last check and the marker's delete. |
| +2. By then every other object under the prefix is swept and the marker is gone. Saying "Nothing was deleted" is false. |
| +3. The console row is still trashed (`deleted_at ≠ 0`), so `HasAppIn` (`C/store.go:1452`) is false and the Settings page and store-destroy door both answer 404. |
| +4. A second purge gets `ErrNotTrashed`. |
| +5. The row comes back only when `MaterializeApps` restores it (`C/materialize.go:376-377`, because `store.json` remains). That runs only at console start (`C/server.go:1289`). |
| +6. Until a restart, the live bucket and IAM user are named only in a log line. `OUTSTANDING.md` 19e says "named and reachable". |
| + |
| +**What is right:** the console returns before the relay republish and before `PurgeAppRow`. The box-side auto-purge (`domains/reconcile.go:232`) logs the error and removes nothing. No caller treats the error as a purge. |
| + |
| +**Smallest fix:** |
| +- Return a distinct sentinel for the after-marker case, and give it its own flash: "the app's files were deleted, but an object store was created during the deletion and still exists". Say how to reach it. |
| +- On that error, either restore the console row (as `MaterializeApps` would) or file a review. |
| +- Reword 19e to say "reachable after the next console start" until then. |
| + |
| +### Asked and not found |
| + |
| +- **Estate and take-backs.** |
| + - I tried teardown after a take-back, teardown after a failed re-teardown, a stale rotation over a take-back, and a stale grant over a teardown. In each, either the key changed (keys are unique), a teardown block appeared, `bucketDeleted` flipped, or `TakeBacks` grew before anything was created. |
| + - The only "equal but something usable was made" schedules are repairs of the same store: a grant re-run on the crashed-grant shape, or a rotation whose final `Put` failed. A waiting teardown then destroys the store it was asked to destroy, which is correct. |
| + - No legitimate re-run is refused for good. Each comparison is between two reads of one invocation, and a re-run reads afresh. |
| + - A grant that loses to a concurrent identical grant now gets `ErrCASConflict` instead of the idempotent short-circuit. That is a spurious failure for the member, not a hold. |
| +- **`markBucketDestroyed`.** |
| + - Every caller passes a `BucketDeletedAt` that was written only after `DeleteBucket` succeeded (`O/walk.go:963-975`), read under the lease. |
| + - `DeleteBucket` treats `NoSuchBucket` as success, so a mark can record a deletion of an already-absent bucket. "Everything before `d` is gone" is still true. |
| + - The lease and the `Destroying` marker block any other app's recreation while a note stands. |
| + - I found no false `DestroyedAt`. A false one would only release a retained teardown (`:1040`), which deletes that store's record and never the bucket. |
| +- **The review gate.** |
| + - It is reached only for unfenced operations, because the fenced and aged cases return earlier. |
| + - Open: nothing is sent, and a person has been told by the open review. |
| + - Resolved: `abandon` calls `markQuantityReviewed` (kind `reviewed`), and the next pass retires the operation on a fresh read. An operation that ran after all hits the `item.Quantity == op.Target` case first and is recorded as landed and unverified. |
| + - I found no operation left unsent, unsettled and untold. I found no resolved review used by the wrong path: reasons are per serial. |
| +- **The lease clock after the read.** The holder's `Until` is judged against a later clock reading, which is more accurate. `SignedAt` is still no later than the `Put`, and `bound()` computes from `SignedAt`. Nothing relied on the earlier order. |
| +- **Fence, send lease, second switch, `aws.go`:** unchanged since my round 9 reading. Nothing new. |
| + |
| +## 3. Claims the code contradicts |
| + |
| +| Location | Claim | What the code supports | |
| +|---|---|---| |
| +| `AGENT_HANDOFF.md` item 5 | A retained teardown "can finish once that bucket has been destroyed, when someone runs it again" | Not for an unsupported name shared across accounts: neither can ever be destroyed (N1). | |
| +| `OUTSTANDING.md` 19e | "the estate is named and reachable" | Reachable only after a console restart; until then every door answers 404 or `ErrNotTrashed` (N2). | |
| +| `C/ownership.go:573` (member copy) | "Nothing was deleted." | False for the after-marker refusal: the prefix and the marker are gone (N2). | |
| +| `O/fence.go:39-43` | The same-kind retry exception is about "the user", which "belongs to the app and not to the name" | The exception lets any request of the earlier attempt arrive beside the re-run, on the bucket as well as the user. That is harmless because it is the same app and the same decision. The cross-name user case is the separate 19b item. Say both. | |
| +| Evidence document, opening fence description (Astra round 9, not corrected) | Every unconfirmed acknowledgement first consumes the key | A resolved review stands in for the fence (`C/billingquantity.go:658-660`, `:745`). The code says so; the evidence document's introduction still does not. | |
| +| Dispositions, sandbox-procedure row (Astra round 9, not corrected) | The canceled-subscription check rereads quantity | `B/sandbox_fence_test.go:421` still accepts a failed reread. | |
| + |
| +Nothing in `walklock.go`'s header, "The fence" in `billingquantity.go`, or `OUTSTANDING.md` items 6, 6a, 7, 9 or 19a–19d is contradicted by the code. Nothing this round makes item 7 or 9 reachable with both switches unset and billing off. |
| + |
| +## 4. Verdict |
| + |
| +**Accept with fixes.** Nothing attests, the worker cannot reach Stripe with either switch unset, and billing stays off. N1 should land with the storage changes. N2's copy and the 19e wording should be fixed before the purge claims are relied on. |
diff --git a/implementation/carloku-paid-launch/AGENT_HANDOFF.md b/implementation/carloku-paid-launch/AGENT_HANDOFF.md| index a6e2e9d91..fdf7fe6a7 100644 |
| --- a/implementation/carloku-paid-launch/AGENT_HANDOFF.md |
| +++ b/implementation/carloku-paid-launch/AGENT_HANDOFF.md |
| @@ -32,7 +32,7 @@ When a review is approved, apply each string byte for byte: the site by replacin |
| 2. Console GET `/a/{sqid}/billing` (`billingpage.go`, `templates/billing.html`): current plan, owner-only chooser posting to the existing checkout door, `?country=` price preview computed from the plans terms, error and pending banners, portal button once a customer exists, read-only for members, byte-identical 404 for non-members, closed state when billing is off. `plans.Countries()` exposes the embedded country list. |
| 3. Durability ledger: the epoch read race is fixed (epoch and created-at are immutable after recover) with a `-race` regression. An unmetered replication fallback on a box whose ledger opened now files an escape (`DurabilityLedger.Escaped`): the box-day is incomplete and the owner is excluded for the run. A box whose ledger failed to open has no publisher and files nothing. The console fold asks each box's published day to prove the union of that box's durability, metrics and edge windows; a box with no ledger directory, no day file, or a ledger whose coverage starts after the box began serving holds the day for everyone. There is no tolerance. Durability days are counted per account: a box-day attested with an exclusion set is complete for every account the box did not exclude. The move worker's checkpoint is recorded against the route's owner. Nothing attests: attestation is false everywhere, in this code and in production. The scoped attestation (path A) was built, reviewed in round six, found unable to prove an exclusive writer, and removed from this branch. It is kept on `feat/carloku-scoped-attestation-20261001` with its findings open. Replication-write completeness is a launch blocker, not a waived requirement. |
| 4. `internal/awsbilling`: Cost and Usage Report reader (versioned deliveries only), newest-delivery selection, digests, charged S3 transfer classifier. It holds on a charged usage line it does not count and on any non-usage line (fee, refund, credit, discount) with a cost on a transfer usage type. Every transfer usage type the flagship account wrote from July to September 2026 is pinned in a test. |
| -5. `internal/objstore/ownership.go`: append-only bucket ownership ledger, opened at grant, closed at teardown. It resolves an owner only for a ledger that claims completeness, and only for hours that begin at or after the instant it began vouching, so a store's partial first hour never resolves (a ledger that began vouching exactly on the hour answers for that hour). Completeness needs a grant block recorded as minting, nothing delivered to the app yet, a probe showing the bucket does not exist, and an inventory showing no principal's policy names the bucket (`GranteeProber`), and is dated when both probes had answered. The AWS provisioner cannot list principals yet, so on AWS no ledger claims completeness, new stores included, and store transfer holds. A minting grant is refused while another app holds the name by ledger interval or by store record, and for a bucket that already exists and it never held. Grant, teardown and both rotation verbs are walks. A walk that spends backend authority takes a lease (`walklock.go`) on the bucket name, or on a hash of it when the name is one this build would refuse, or on a hash of the app when the record names no bucket: an object taken by compare-and-swap, bound to the invocation, thirty minutes long, standing until it runs out if the walk failed or any of its requests needed a second attempt. The record is read again under the lease and must still be the same estate, not only the same name: teardown and grant compare the grant instant, the key, whether a teardown had begun and how many take-backs it has seen; rotation compares the bucket, the key and that no teardown has begun. A declaration-only teardown and the app purge take no lease; they delete the store record only as the generation they read. The user belongs to the app and the lease to the name, so a late request of a failed teardown can still remove the policy of a store granted afterwards under another name (round eight, Opus N2, open). Every request of a walk, to S3, to IAM and to the deployment bucket that holds the records and ledgers, is signed with the instant its lease was taken (`internal/sigfence`), so AWS refuses a request a paused process sends after its lease could be taken over. The record and its scan sidecar are deleted as the generation that was read. The signing-time rule was checked read-only against S3 and IAM with a list and two gets (`TestFenceAgainstAWS`); no delete was sent. A declaration-only teardown takes no lease (there is no bucket name) and relies on the conditional delete alone. A teardown writes a block on its own record before it touches anything and decides by compare-and-swap on the ledger: if nobody else holds the bucket it marks the bucket as being destroyed; otherwise it leaves the bucket and its scanner, revokes its own credentials, and records that the scanner is owed. A grant by the same app over its own unfinished teardown takes the store back, runs in full, and voids the teardown's note that the bucket was deleted. A teardown that leaves a bucket to another account keeps its record and returns `ErrBucketRetained` (discussion 191). It can finish once that bucket has been destroyed, when someone runs it again; nothing runs it again by itself. `ErrBucketRetained` has no member copy yet; it needs Paul's review. A randomized simulation (`walk_sim_test.go`) checks the invariants over sequential walks with provider and store faults, including failures after the effect, and walks frozen past their lease while another takes the name. It does not generate a second bucket name, rotations, stores with no ledger interval or disagreeing clocks; those have direct tests. |
| +5. `internal/objstore/ownership.go`: append-only bucket ownership ledger, opened at grant, closed at teardown. It resolves an owner only for a ledger that claims completeness, and only for hours that begin at or after the instant it began vouching, so a store's partial first hour never resolves (a ledger that began vouching exactly on the hour answers for that hour). Completeness needs a grant block recorded as minting, nothing delivered to the app yet, a probe showing the bucket does not exist, and an inventory showing no principal's policy names the bucket (`GranteeProber`), and is dated when both probes had answered. The AWS provisioner cannot list principals yet, so on AWS no ledger claims completeness, new stores included, and store transfer holds. A minting grant is refused while another app holds the name by ledger interval or by store record, and for a bucket that already exists and it never held. Grant, teardown and both rotation verbs are walks. A walk that spends backend authority takes a lease (`walklock.go`) on the bucket name, or on a hash of it when the name is one this build would refuse, or on a hash of the app when the record names no bucket: an object taken by compare-and-swap, bound to the invocation, thirty minutes long, standing until it runs out if the walk failed or any of its requests needed a second attempt. The record is read again under the lease and must still be the same estate, not only the same name: teardown and grant compare the grant instant, the key, how many grants have gone as far as creating anything, whether a teardown had begun, whether it noted the bucket deleted and how many take-backs it has seen; rotation compares the bucket, the key and that no teardown has begun. Every grant that creates anything is counted on the record before it calls the provider. A failed walk's lease stands until it runs out, with one exception: the same app re-running the same kind of walk may take it at once. A declaration-only teardown and the app purge take no lease; they delete the store record only as the generation they read. The user belongs to the app and the lease to the name, so a late request of a failed teardown can still remove the policy of a store granted afterwards under another name (round eight, Opus N2, open). Every request of a walk, to S3, to IAM and to the deployment bucket that holds the records and ledgers, is signed with the instant its lease was taken (`internal/sigfence`), so AWS refuses a request a paused process sends after its lease could be taken over. The record and its scan sidecar are deleted as the generation that was read. The signing-time rule was checked read-only against S3 and IAM with a list and two gets (`TestFenceAgainstAWS`); no delete was sent. A declaration-only teardown takes no lease (there is no bucket name) and relies on the conditional delete alone. A teardown writes a block on its own record before it touches anything and decides by compare-and-swap on the ledger (for a bucket name this build would refuse there is no ledger, and it decides from the store records alone): if nobody else holds the bucket it marks the bucket as being destroyed; otherwise it leaves the bucket and its scanner, revokes its own credentials, and records that the scanner is owed. A grant by the same app over its own unfinished teardown takes the store back, runs in full, and voids the teardown's note that the bucket was deleted. A teardown that leaves a bucket to another account keeps its record and returns `ErrBucketRetained` (discussion 191). It can finish once that bucket has been destroyed, when someone runs it again; nothing runs it again by itself. For a bucket name this build would refuse there is no ledger: a store whose teardown has begun is not counted as a holder there, so the last of two sharers destroys the bucket. `ErrBucketRetained` has no member copy yet; it needs Paul's review. A randomized simulation (`walk_sim_test.go`) checks the invariants over sequential walks with provider and store faults, including failures after the effect, and walks frozen past their lease while another takes the name. It does not generate a second bucket name, rotations, stores with no ledger interval or disagreeing clocks; those have direct tests. |
| 6. Statement folds: `console/durabilityfold.go` (`durability_writes_1k`, policy in `usage.BillableReplicationWrites`) and `console/storeegressfold.go` (`store_egress_gb`). `usage.MeterSource` names a source for both meters. Month evidence pins ownership ledgers by their month projection and knows the durability box directory. Statements carry the five billed meters by UTC day (`Statement.Days`), summing to the monthly totals; the other meters are not recorded by day. With a transfer report configured, the statements catch-up walks a 36-month window and sweeps every month that has an open statement, is missing a statement an account is owed (the platform's included), or is a gap between metered months. An account's statement that will not parse, or that cannot be read three passes running, is reported once per process and the month is classified without it. The platform's own statement is different: while it cannot be read the month stays unknown, for as long as that lasts. A month seen with no statement is remembered for six hours, a closed statement for the life of the process. |
| 7. Operator API doors (`billingoperator.go`): list and resolve billing review holds, list and set the house marker. No CLI verb yet. |
| 8. Team quantity worker (`billingquantity.go`) behind two switches, both unset: `CONSOLE_BILLING_PRORATION` (`immediate` or `next_invoice`, Paul's decision) and `CONSOLE_BILLING_QUANTITY_FENCE=stripe-sandbox-verified` (set only after `TestSandboxQuantityFence` has passed against a Stripe test-mode account; it has not been run). With either unset it makes no provider call. An operation is recorded by a compare-and-set on the billing row's revision, subscription, period, quantity and status, on the target and newest pending addition recomputed from the database, and on its own instant lying inside the period and not before the addition it charges for. The request dates its proration at that instant (`proration_date`). Sending and retiring exclude each other through a send lease on the operation's row, owned by one sender's token; an operation dated ahead of the observing clock is left alone. Every key ends in the account's operation serial. An operation nobody can show ran is acknowledged only after its idempotency key has been tried at the provider with a metadata-only request (the fence). If the fence request took the key, the operation never ran and may be retired, on a provider read taken after the fence when the subscription is current. If the key is held by the operation's own request, that request began and its outcome is not known: the operation is recorded as landed if the provider shows its target, and otherwise held with a review (`billing_quantity_operation_unsettled`) and never replaced until a person resolves it. A fence the provider refuses is held the same way. The fence rests on Stripe's documented key rules, is modelled by a fake in the tests, is not verified against Stripe, and lasts only as long as Stripe remembers a key (documented as at least 24 hours): `docs/superpowers/specs/2026-10-01-stripe-idempotency-evidence.md`. A quantity that goes down inside a period files `billing_quantity_lowered`. An operation is confirmed only by the provider's success answer to its own request. A confirmed landing takes effect at the operation's instant, so a seat's charge and its allowance start together; a quantity that merely matches an unconfirmed operation's target is recorded as `landed_unverified`, dated when observed, with a review. If the provider refuses a request and names `proration_date`, the operation's key is fenced at once and it is retired on the next read; if it does not name it, the operation waits out the 22-hour replay limit. None of the provider's side is confirmed: test mode is not authorized. Only additions of the subscription's current period can raise it; older ones are closed uncharged with a review; additions and operations of an ended subscription are closed. It does not yet lower the quantity at renewal. |
| @@ -77,7 +77,7 @@ Merchant `acct_1UD7LXI4Nrr6AEsO`, Team Tito Limited, Ireland, EUR; Irish standar |
| ## Next actions, in order |
| 1. Apply approved copy from both reviews when `result.json` arrives. |
| -2. Independent review: Astra rejected rounds 1 to 9 (ten, eight, five, five, eleven, fourteen, nine, five and six findings); Opus accepted with fixes eight times (eight, eight, eight, nine, nine, thirteen, eight, five). Rounds seven to nine attacked the lease and the fences at their edges; every finding was accepted, and those not fixed are named as open (`docs/superpowers/specs/CODEX-DISPOSITIONS-carloku-paid-launch-overnight.md`, "Round eight" and "Round nine"). A further round of both must pass on the exact tip before anything lands; do not read "fixed" as "accepted". |
| +2. Independent review: Astra rejected rounds 1 to 10 (ten, eight, five, five, eleven, fourteen, nine, five, six and three findings); Opus accepted with fixes nine times (eight, eight, eight, nine, nine, thirteen, eight, five, two). Rounds seven to nine attacked the lease and the fences at their edges; every finding was accepted, and those not fixed are named as open (`docs/superpowers/specs/CODEX-DISPOSITIONS-carloku-paid-launch-overnight.md`, "Round eight" to "Round ten"). A further round of both must pass on the exact tip before anything lands; do not read "fixed" as "accepted". |
| 3. Scoped approvals: Cost and Usage Report export; Stripe catalog, portal, webhook and the `rk_live` key; then a test-mode rehearsal and a real purchase. |
| 4. Build the principal inventory and the bucket ownership attestation (designed in `docs/superpowers/plans/`), quantity reduction at renewal, a `carlos` verb and documentation for the operator doors, then Free-tier enforcement. |
| 5. Build path B of the attestation plan (the platform's own replicator as sole writer). Until a box can prove its replication writes, that meter holds every month. |
diff --git a/implementation/carloku-paid-launch/OUTSTANDING.md b/implementation/carloku-paid-launch/OUTSTANDING.md| index 67c7ca12b..879b4b7c4 100644 |
| --- a/implementation/carloku-paid-launch/OUTSTANDING.md |
| +++ b/implementation/carloku-paid-launch/OUTSTANDING.md |
| @@ -55,8 +55,9 @@ Each is concrete. None is waived. |
| ### Process |
| -19. **No review round has passed.** Astra rejected rounds one to nine; Opus accepted each with fixes. The fixes to round nine's findings are unreviewed. |
| -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, so the estate is named and reachable, but it does not prevent it. The doors check liveness when they admit a request, not when they write. |
| +19. **No review round has passed.** Astra rejected rounds one to ten; Opus accepted each with fixes. The fixes to round ten's findings are unreviewed. |
| +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. |
| +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). |
| 19b. **Two round-eight findings are accepted and not fixed.** A late request of a failed teardown can remove the policy of a store the same app is granted afterwards under another name (the store goes dark; nothing is lost or leaked). A console whose clock is far ahead reserves a bucket name until that time, with no operator verb to clear it. |
| 19c. **S3's conditional delete is unverified on a real backend.** The store walks and the app purge delete a record only as the generation they read. That has run against the directory store and an HTTP stub. No `If-Match` delete has been sent to AWS or to an S3-compatible store, and a backend that ignored the header would delete unconditionally. It needs one write to a scratch object, which is a production-adjacent write nobody has approved. |
| 19d. **The signing-time fence has been observed on AWS only.** An S3-compatible deployment or store endpoint has not been probed for its window. |
diff --git a/internal/billingstripe/sandbox_fence_test.go b/internal/billingstripe/sandbox_fence_test.go| index 6ec9e3c19..c298b605f 100644 |
| --- a/internal/billingstripe/sandbox_fence_test.go |
| +++ b/internal/billingstripe/sandbox_fence_test.go |
| @@ -418,7 +418,11 @@ func TestSandboxQuantityFence(t *testing.T) { |
| if u.status != 400 && u.status != 404 { |
| t.Fatalf("a quantity update on a canceled subscription's item: %s, want it refused as a request (400 or 404)", errOf(u)) |
| } |
| - if s, err := c.GetSubscription(ctx, subID); err == nil && len(s.Items) == 1 && s.Items[0].Quantity != before { |
| + s, err := c.GetSubscription(ctx, subID) |
| + if err != nil || len(s.Items) != 1 { |
| + t.Fatalf("read the canceled subscription again: %v", err) |
| + } |
| + if s.Items[0].Quantity != before { |
| t.Fatalf("the quantity of a canceled subscription moved from %d to %d", before, s.Items[0].Quantity) |
| } |
| t.Logf("OBSERVED: a quantity update on a canceled subscription's item is answered %s", errOf(u)) |
diff --git a/internal/objstore/fence.go b/internal/objstore/fence.go| index 8b4b99831..1ab66dd7e 100644 |
| --- a/internal/objstore/fence.go |
| +++ b/internal/objstore/fence.go |
| @@ -37,10 +37,13 @@ import ( |
| // the name, every request the earlier walk could ever send is one the |
| // backend refuses. fenceSkew is how much longer than the window the lease |
| // lasts. One exception, stated in walklock.go: the same app re-running the |
| -// same kind of walk may take an ended lease at once. The name stays |
| -// reserved, but the user belongs to the app and not to the name, so an |
| -// earlier attempt's request on that user can still arrive beside the |
| -// re-run (an open finding, OUTSTANDING.md 19b). |
| +// same kind of walk may take an ended lease at once, so a request of the |
| +// earlier attempt can arrive beside the re-run, on the bucket or on the |
| +// user. That is the same app making the same decision twice, and the name |
| +// stays reserved for everyone else. Separately, the user belongs to the |
| +// app and not to the name, so such a request on the user can also arrive |
| +// beside a later walk of that app on ANOTHER name, which is an open |
| +// finding (OUTSTANDING.md 19b). |
| // |
| // How much longer: a whole second window. Whoever takes the lease over |
| // judges "run out" by its own clock, and nothing here can check that clock |
diff --git a/internal/objstore/objstore.go b/internal/objstore/objstore.go| index 7a5678f5e..8068a4059 100644 |
| --- a/internal/objstore/objstore.go |
| +++ b/internal/objstore/objstore.go |
| @@ -61,6 +61,13 @@ type GrantBlock struct { |
| // ownership ledger may claim completeness only for a minted bucket |
| // (ownership.go), so the absence of the field must never read as minted. |
| Minted bool `json:"minted,omitempty"` |
| + // Walks counts the grants that went past the point of creating |
| + // anything. It is written to the record before such a grant calls the |
| + // provider, so a grant that creates a bucket and delivers credentials |
| + // and then fails before it records them has still changed the record: |
| + // a walk that read the record earlier and waited for its lease can |
| + // tell that the store it was asked about is not the one standing now. |
| + Walks int `json:"walks,omitempty"` |
| } |
| type Provisioned struct { |
| @@ -160,6 +167,11 @@ type TeardownBlock struct { |
| // may create the bucket itself, so it voids this note before it does |
| // anything else, and a later teardown decides again from the ledger. |
| BucketDeletedAt time.Time `json:"bucket_deleted_at,omitzero"` |
| + // DeletedBucket names the bucket BucketDeletedAt is about. The note is |
| + // evidence about that bucket and no other: a grant that takes the |
| + // store back may be for a different name, and must not put this |
| + // deletion on that name's ledger. |
| + DeletedBucket string `json:"deleted_bucket,omitempty"` |
| // TakeBacks counts the grants that have begun over this teardown. It |
| // is written before such a grant creates or delivers anything, so that |
| // from that write on the record is a different estate to any walk that |
diff --git a/internal/objstore/ownership_test.go b/internal/objstore/ownership_test.go| index 16c346db1..094d62b47 100644 |
| --- a/internal/objstore/ownership_test.go |
| +++ b/internal/objstore/ownership_test.go |
| @@ -2287,9 +2287,10 @@ func TestStaleTeardownDoesNotDestroyWhatAFailedTakeBackMade(t *testing.T) { |
| advance(walkLease + time.Minute) |
| s := &hookStore{Store: dir, at: "get:" + walkLockKey(ledgerBucket)} |
| s.hook = func() { |
| - // The take-back, failing at its final record write. |
| - failing := &failingPutStore{Store: dir, key: Key("bab", "titogo"), nth: 2} |
| - if _, err := Grant(ctx, failing, p, testDeployment(), pub, "bab", "titogo", "op", GrantSourceOperator, "", t0.Add(time.Hour)); err == nil || failing.seen < 2 { |
| + // The take-back, failing at its final record write (its third: |
| + // the take-back is counted, then the walk, then the result). |
| + failing := &failingPutStore{Store: dir, key: Key("bab", "titogo"), nth: 3} |
| + if _, err := Grant(ctx, failing, p, testDeployment(), pub, "bab", "titogo", "op", GrantSourceOperator, "", t0.Add(time.Hour)); err == nil || failing.seen < 3 { |
| t.Errorf("%s: fixture: the take-back was meant to fail at its last write (err %v, writes %d)", failAt, err, failing.seen) |
| } |
| if _, ok := p.Buckets[ledgerBucket]; !ok { |
| @@ -2436,6 +2437,24 @@ func TestTeardownOfAnUnsupportedNameLeavesABucketAnotherStoreHolds(t *testing.T) |
| if _, ok := p.Users[UserName(otherAcct, "other")]; !ok { |
| t.Fatal("the other store lost its user") |
| } |
| + // Then the other store is torn down too. Both owners have now asked |
| + // for the bucket to go. The store that left first is not counted |
| + // as holding it, so the second destroys it, and the first can then |
| + // finish. Neither waits on the other for good. |
| + if err := Teardown(ctx, s, p, nil, otherAcct, "other"); err != nil { |
| + t.Fatalf("other account %s: the last store's teardown: %v", otherAcct, err) |
| + } |
| + if _, exists := p.Buckets[legacy]; exists { |
| + t.Fatalf("other account %s: both stores were torn down and the bucket is still there", otherAcct) |
| + } |
| + if err := Teardown(ctx, s, p, nil, "bab", "titogo"); err != nil { |
| + t.Fatalf("other account %s: the first store's teardown run again: %v", otherAcct, err) |
| + } |
| + for _, o := range []struct{ acct, app string }{{"bab", "titogo"}, {otherAcct, "other"}} { |
| + if rec, _, _ := Get(ctx, s, o.acct, o.app); rec != nil { |
| + t.Fatalf("other account %s: %s/%s still has a record: %+v", otherAcct, o.acct, o.app, rec) |
| + } |
| + } |
| } |
| } |
| @@ -2458,3 +2477,125 @@ func TestHashedLeaseKeysCannotCollideWithABucketName(t *testing.T) { |
| t.Fatal("a valid bucket name is not leased under itself") |
| } |
| } |
| + |
| +// No teardown is involved, and nothing was taken back. A grant records its |
| +// grant block and fails before it creates anything. A teardown request |
| +// reads that record and is held before its lease. The grant is retried: it |
| +// creates the bucket, mints a key and delivers it, and fails at its last |
| +// write. The record shows the same grant block and still no key. The app |
| +// writes data. The held teardown must still see that the store is not the |
| +// one it was asked about, because a grant that creates anything is counted |
| +// on the record before it calls the provider. |
| +func TestStaleTeardownDoesNotDestroyWhatAFailedGrantRetryMade(t *testing.T) { |
| + ctx := context.Background() |
| + dir := release.NewDirStore(t.TempDir()) |
| + _, pub := testSealKeypair(t) |
| + p := NewMemProvisioner("eu-west-1") |
| + advance := leaseClock(t) |
| + declare(t, dir, "bab", "titogo", "TITO_S3") |
| + p.FailNext = map[string]error{"EnsureBucket": errors.New("injected EnsureBucket")} |
| + if _, err := Grant(ctx, dir, p, testDeployment(), pub, "bab", "titogo", "op", GrantSourceOperator, "", t0); err == nil { |
| + t.Fatal("fixture: the first grant was meant to fail before creating anything") |
| + } |
| + p.FailNext = nil |
| + if rec, _, _ := Get(ctx, dir, "bab", "titogo"); rec == nil || rec.Grant == nil || rec.Provisioned != nil || rec.Teardown != nil { |
| + t.Fatalf("fixture: want a grant block and nothing else: %+v", rec) |
| + } |
| + advance(walkLease + time.Minute) |
| + s := &hookStore{Store: dir, at: "get:" + walkLockKey(ledgerBucket)} |
| + s.hook = func() { |
| + // The retry: counted (first write), then failing at its result. |
| + failing := &failingPutStore{Store: dir, key: Key("bab", "titogo"), nth: 2} |
| + if _, err := Grant(ctx, failing, p, testDeployment(), pub, "bab", "titogo", "op", GrantSourceOperator, "", t0.Add(time.Hour)); err == nil || failing.seen < 2 { |
| + t.Errorf("fixture: the retry was meant to fail at its last write (err %v, writes %d)", err, failing.seen) |
| + } |
| + if _, ok := p.Buckets[ledgerBucket]; !ok { |
| + t.Error("fixture: the retry did not create the bucket") |
| + return |
| + } |
| + if u, ok := p.Users[UserName("bab", "titogo")]; !ok || len(u.Keys) == 0 { |
| + t.Error("fixture: the retry did not mint a key") |
| + } |
| + p.Buckets[ledgerBucket]["invoice.pdf"] = []byte("written with the delivered key") |
| + advance(walkLease + time.Minute) |
| + } |
| + err := Teardown(ctx, s, p, nil, "bab", "titogo") |
| + if !s.done { |
| + t.Fatal("fixture: the held teardown never reached its lease") |
| + } |
| + if !errors.Is(err, release.ErrCASConflict) { |
| + t.Fatalf("a teardown asked about the store before the retry made it usable: %v, want it to stop", err) |
| + } |
| + if got := string(p.Buckets[ledgerBucket]["invoice.pdf"]); got != "written with the delivered key" { |
| + t.Fatalf("what a failed grant retry made was destroyed by a stale teardown (object now %q)", got) |
| + } |
| + // A teardown asked now, about the store as it stands, goes through. |
| + if err := Teardown(ctx, dir, p, nil, "bab", "titogo"); err != nil { |
| + t.Fatalf("a fresh teardown: %v", err) |
| + } |
| + if _, ok := p.Buckets[ledgerBucket]; ok { |
| + t.Fatal("a fresh teardown left the bucket") |
| + } |
| +} |
| + |
| +// A teardown's note that it deleted a bucket is evidence about that bucket. |
| +// Here an older record is provisioned on bucket X with no grant block, its |
| +// teardown deletes X and fails, and the app is then granted again. By the |
| +// grant's own rule its bucket is the minted name Y, which is another |
| +// bucket, with a ledger and holders of its own. The grant must not carry |
| +// X's deletion to Y's ledger, where it would tell a store that left Y that |
| +// its objects are gone. It is refused until X's teardown is finished. |
| +func TestTakeBackDoesNotRecordOneBucketsDestructionOnAnother(t *testing.T) { |
| + ctx := context.Background() |
| + s := release.NewDirStore(t.TempDir()) |
| + _, pub := testSealKeypair(t) |
| + p := NewMemProvisioner("eu-west-1") |
| + advance := leaseClock(t) |
| + const legacyX = "carlos-store-legacy-x" |
| + // Y has a ledger: another account's store holds it. |
| + declare(t, s, "bac", "other", "OTHER_S3") |
| + if _, err := Grant(ctx, s, p, testDeployment(), pub, "bac", "other", "op", GrantSourceOperator, ledgerBucket, t0); err != nil { |
| + t.Fatal(err) |
| + } |
| + // The older record: provisioned on X, no grant block. |
| + user := UserName("bab", "titogo") |
| + if _, err := p.EnsureUser(ctx, user, "/carlos/", "arn:aws:s3:::"+legacyX, nil); err != nil { |
| + t.Fatal(err) |
| + } |
| + p.Buckets[legacyX] = map[string][]byte{} |
| + rec := &Record{Declared: &Declared{EnvPrefix: "TITO_S3", At: t0, By: "member@example.com"}, |
| + Provisioned: &Provisioned{Bucket: legacyX, Region: "eu-west-1", IAMUser: user, KeyID: "LEGACYKEY", At: t0}} |
| + if _, err := Put(ctx, s, "bab", "titogo", rec, ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + p.FailNext = map[string]error{"DeleteUser": errors.New("injected DeleteUser")} |
| + if err := Teardown(ctx, s, p, nil, "bab", "titogo"); err == nil { |
| + t.Fatal("fixture: the teardown was meant to fail after deleting X") |
| + } |
| + p.FailNext = nil |
| + if _, exists := p.Buckets[legacyX]; exists { |
| + t.Fatal("fixture: X was not deleted") |
| + } |
| + if cur, _, _ := Get(ctx, s, "bab", "titogo"); cur == nil || cur.Teardown == nil || cur.Teardown.DeletedBucket != legacyX { |
| + t.Fatalf("fixture: the record does not say which bucket was deleted: %+v", cur) |
| + } |
| + advance(walkLease + time.Minute) |
| + _, err := Grant(ctx, s, p, testDeployment(), pub, "bab", "titogo", "op", GrantSourceOperator, "", t0.Add(time.Hour)) |
| + if err == nil { |
| + t.Fatal("a grant for Y went ahead over an unfinished teardown of X") |
| + } |
| + ledger, _, lerr := ReadBucketOwnership(ctx, s, ledgerBucket) |
| + if lerr != nil || ledger == nil { |
| + t.Fatalf("Y's ledger: %v", lerr) |
| + } |
| + if ledger.DestroyedAt != nil { |
| + t.Fatalf("X's deletion was recorded on Y's ledger: %v", ledger.DestroyedAt) |
| + } |
| + if cur, _, _ := Get(ctx, s, "bab", "titogo"); cur == nil || cur.Grant != nil || cur.Teardown == nil || cur.Teardown.BucketDeletedAt.IsZero() { |
| + t.Fatalf("the refused grant changed the record of the unfinished teardown: %+v", cur) |
| + } |
| + // Finishing X's teardown is the way forward. |
| + if err := Teardown(ctx, s, p, nil, "bab", "titogo"); err != nil { |
| + t.Fatalf("finishing the teardown of X: %v", err) |
| + } |
| +} |
diff --git a/internal/objstore/walk.go b/internal/objstore/walk.go| index c19446c0a..5554e95db 100644 |
| --- a/internal/objstore/walk.go |
| +++ b/internal/objstore/walk.go |
| @@ -228,6 +228,26 @@ func Grant(ctx context.Context, s release.Store, p Provisioner, d Deployment, |
| if rec.Provisioned != nil { |
| startKeyID = rec.Provisioned.KeyID |
| } |
| + // The bucket an unfinished teardown of this record deleted, if it noted |
| + // one. Read before this call writes a grant block of its own. |
| + deletedBucket := "" |
| + if rec.Teardown != nil && !rec.Teardown.BucketDeletedAt.IsZero() { |
| + switch { |
| + case rec.Teardown.DeletedBucket != "": |
| + deletedBucket = rec.Teardown.DeletedBucket |
| + case rec.Provisioned != nil: |
| + deletedBucket = rec.Provisioned.Bucket |
| + case rec.Grant != nil: |
| + deletedBucket = rec.Grant.Bucket |
| + } |
| + // The note is about that bucket. A take-back for another name |
| + // would carry it to a ledger it says nothing about, and leave the |
| + // old name's teardown unfinished behind a record that no longer |
| + // names it. The teardown is finished first. |
| + if deletedBucket != bucket { |
| + return nil, fmt.Errorf("objstore: %s/%s has an unfinished teardown of bucket %s; finish it before granting %s: %w", acct, app, deletedBucket, bucket, release.ErrCASConflict) |
| + } |
| + } |
| wroteGrant := rec.Grant == nil |
| if wroteGrant { |
| rec.Grant = &GrantBlock{Backend: d.Backend, Bucket: bucket, Source: source, At: now, By: actor, Minted: adoptBucket == ""} |
| @@ -252,10 +272,12 @@ func Grant(ctx context.Context, s release.Store, p Provisioner, d Deployment, |
| takeBack := rec.Teardown != nil |
| if takeBack { |
| if at := rec.Teardown.BucketDeletedAt; !at.IsZero() { |
| - if err := markBucketDestroyed(ctx, s, bucket, at); err != nil { |
| - return nil, fmt.Errorf("objstore: record the destruction of %s: %w", bucket, err) |
| + // deletedBucket is this grant's bucket (checked above), and |
| + // it is the deleted bucket's own ledger that is told. |
| + if err := markBucketDestroyed(ctx, s, deletedBucket, at); err != nil { |
| + return nil, fmt.Errorf("objstore: record the destruction of %s: %w", deletedBucket, err) |
| } |
| - rec.Teardown.BucketDeletedAt = time.Time{} |
| + rec.Teardown.BucketDeletedAt, rec.Teardown.DeletedBucket = time.Time{}, "" |
| } |
| rec.Teardown.TakeBacks++ |
| } |
| @@ -412,6 +434,28 @@ func Grant(ctx context.Context, s release.Store, p Provisioner, d Deployment, |
| return rec, nil |
| } |
| } |
| + // From here the walk creates things. It is counted on the record first |
| + // (GrantBlock.Walks): if this grant makes a bucket and delivers |
| + // credentials and then fails before its last write, the record must |
| + // still differ from what any waiting walk read before it began. |
| + // The console's declare door writes this same object. If it wrote in |
| + // between, the count is put on the record as it now stands, provided |
| + // it is still the same estate; anything else is a conflict. |
| + before := estateOf(rec) |
| + rec.Grant.Walks++ |
| + if etag, err = Put(ctx, s, acct, app, rec, etag); errors.Is(err, release.ErrCASConflict) { |
| + fresh, freshEtag, gerr := Get(ctx, s, acct, app) |
| + if gerr != nil || fresh == nil || fresh.Declared == nil || fresh.Grant == nil || estateOf(fresh) != before { |
| + return nil, err |
| + } |
| + fresh.Grant.Walks++ |
| + if etag, err = Put(ctx, s, acct, app, fresh, freshEtag); err != nil { |
| + return nil, err |
| + } |
| + rec = fresh |
| + } else if err != nil { |
| + return nil, err |
| + } |
| // 3. the bucket |
| region, err := p.EnsureBucket(ctx, bucket) |
| if err != nil { |
| @@ -875,8 +919,11 @@ func Teardown(ctx context.Context, s release.Store, p Provisioner, sc Scanner, a |
| // say whose the bucket is. The store records do: if another record |
| // names this bucket, this teardown leaves it, revokes its own |
| // access, and (when that record is another account's) reports the |
| - // bucket as retained, exactly as for a name with a ledger. |
| - holders, herr := otherStoreHolders(ctx, s, bucket, acct, app) |
| + // bucket as retained, exactly as for a name with a ledger. A record |
| + // whose own teardown has begun is not counted (storeHolders): its |
| + // owner asked for it to go, and with no ledger to break the tie two |
| + // such stores would wait on each other for ever. |
| + holders, herr := storeHolders(ctx, s, bucket, acct, app, false) |
| if herr != nil { |
| return fmt.Errorf("objstore: list the stores holding %s: %w", bucket, herr) |
| } |
| @@ -969,7 +1016,7 @@ func Teardown(ctx context.Context, s release.Store, p Provisioner, sc Scanner, a |
| // against a fresh read: a record that has changed since describes |
| // some other estate, and saying ITS bucket was deleted would be |
| // false. If it has changed the write loses, and the walk stops. |
| - rec.Teardown.BucketDeletedAt = ownershipNow() |
| + rec.Teardown.BucketDeletedAt, rec.Teardown.DeletedBucket = ownershipNow(), bucket |
| if etag, err = Put(ctx, s, acct, app, rec, etag); err != nil { |
| return fmt.Errorf("objstore: record the deleted bucket for %s/%s: %w", acct, app, err) |
| } |
| @@ -1106,6 +1153,7 @@ func Teardown(ctx context.Context, s release.Store, p Provisioner, sc Scanner, a |
| type storeEstate struct { |
| bucket, keyID string |
| grantedAt int64 |
| + grantWalks int |
| tearingDown bool |
| bucketDeleted bool |
| takeBacks int |
| @@ -1117,7 +1165,7 @@ func estateOf(rec *Record) storeEstate { |
| return e |
| } |
| if rec.Grant != nil { |
| - e.bucket, e.grantedAt = rec.Grant.Bucket, rec.Grant.At.UnixNano() |
| + e.bucket, e.grantedAt, e.grantWalks = rec.Grant.Bucket, rec.Grant.At.UnixNano(), rec.Grant.Walks |
| } |
| if rec.Provisioned != nil { |
| e.bucket, e.keyID = rec.Provisioned.Bucket, rec.Provisioned.KeyID |
| @@ -1147,6 +1195,18 @@ var grantElapsed = func(started time.Time) time.Duration { return time.Since(sta |
| // what says an app was given access: it exists from the grant until the |
| // teardown has finished, ledger or no ledger. |
| func otherStoreHolders(ctx context.Context, s release.Store, bucket, acct, app string) ([]OwnerRef, error) { |
| + return storeHolders(ctx, s, bucket, acct, app, true) |
| +} |
| + |
| +// storeHolders is otherStoreHolders with a choice about stores that are |
| +// themselves being torn down. countLeaving false leaves them out: a record |
| +// that carries a teardown block belongs to a store whose owner has already |
| +// asked for it to go. For a name with a ledger the ledger breaks the tie |
| +// between two such stores (the one that left has a closed interval). A name |
| +// this build would refuse has no ledger, and counting leaving stores there |
| +// would make two accounts that share the bucket each wait for the other |
| +// for good: neither record can ever change, because Grant refuses the name. |
| +func storeHolders(ctx context.Context, s release.Store, bucket, acct, app string, countLeaving bool) ([]OwnerRef, error) { |
| accounts, err := s.ListDirs(ctx, "apps/") |
| if err != nil { |
| return nil, err |
| @@ -1186,9 +1246,10 @@ func otherStoreHolders(ctx context.Context, s release.Store, bucket, acct, app s |
| } |
| // A store whose own teardown already deleted its bucket holds |
| // nothing by that name any more. |
| - if held == bucket && (rec.Teardown == nil || rec.Teardown.BucketDeletedAt.IsZero()) { |
| - out = append(out, OwnerRef{Account: a, App: name, Since: since}) |
| + if held != bucket || (rec.Teardown != nil && (!countLeaving || !rec.Teardown.BucketDeletedAt.IsZero())) { |
| + continue |
| } |
| + out = append(out, OwnerRef{Account: a, App: name, Since: since}) |
| } |
| } |
| return out, nil |