carlos / platform Public

Close round seven: the lease covers what a walk acts on, the fence reaches every client, and a held key is two facts

Round seven attacked the two mechanisms at their edges. Every finding is
accepted (dispositions, "Round seven").

Storage:
- A walk reads its record again under the lease and stops if it no longer
  leads to the leased name (teardown, grant, and now both rotation verbs,
  which are walks: they take the lease and run inside the fence).
- The fence is its own package (internal/sigfence) and is installed on the
  deployment bucket's S3 client too, so a walk's record, ledger and config
  writes carry the lease's stamp like its provider calls.
- The record and its scan sidecar are deleted as the generation that was
  read, never unconditionally.
- A request that needed a second attempt leaves a doubt on its walk, and a
  walk with a doubt keeps its lease until it runs out. Both attempts carry
  the lease's stamp.
- The lease lasts two backend windows, so no check of the taker's clock is
  needed.
- A store that held a name and left is not a holder. The ledger records a
  bucket's destruction when it is deleted.
- The replacement S3 signer does not escape the key twice.
- The simulation checks invariants mid-walk, freezes walks past their lease,
  refuses stale store requests, and fails calls after their effect.

Quantity worker (still off; both switches unset):
- The fence reports what holds the key. Held by the fence request: the
  operation never ran and may be retired. Held by the operation's own
  request: it began and its outcome is unknown (Stripe calls a 500
  indeterminate and may reconcile it). Such an operation is never retired
  or replaced on that evidence; it is held with a review.
- A server error answered from memory holds the operation at once. The
  simulation found it was otherwise replayed every pass for 22 hours.
- Resolving an operation's review is the way out, recorded on the operation.
  A 404 is no longer read as fenced.
- The send lease is one sender's, by token. Every key ends in the account's
  operation serial.
- The sandbox verification covers both proration policies, requires exactly
  one non-replayed answer among concurrent copies, races a fence against an
  update, and sends the late update ninety seconds after the fence. Not run.

Read-only against AWS, rerun: S3 and IAM accept stamps 1 and 13 minutes old
with the operation's own answer and refuse 16 and 20 minutes, with the SDK's
retries on as well; a key that needs escaping is signed correctly.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

claude 3 weeks ago 6fcccecff9c154547ed82529845bd06486857be2 parent b669254

24 files changed, +2612 −450

docs/superpowers/specs/2026-10-01-stripe-idempotency-evidence.md +16 −10
docs/superpowers/specs/CODEX-DISPOSITIONS-carloku-paid-launch-overnight.md +60 −0
docs/superpowers/specs/CODEX-FINDINGS-carloku-paid-launch-overnight-round7.md +193 −0
docs/superpowers/specs/CODEX-REVIEW-BRIEF-carloku-paid-launch-overnight-round8.md +32 −0
docs/superpowers/specs/OPUS-REVIEW-carloku-paid-launch-overnight-round7.md +239 −0
implementation/carloku-paid-launch/AGENT_HANDOFF.md +4 −4
implementation/carloku-paid-launch/OUTSTANDING.md +9 −7
internal/billingstripe/client.go +44 −26
internal/billingstripe/client_test.go +19 −19
internal/billingstripe/sandbox_fence_test.go +261 −177
internal/console/billingquantity.go +241 −62
internal/console/billingquantity_sim_test.go +7 −4
internal/console/billingquantity_test.go +267 −38
internal/objstore/aws.go +12 −3
internal/objstore/fence.go +22 −38
internal/objstore/fence_aws_test.go +69 −27
internal/objstore/ownership.go +25 −0
internal/objstore/ownership_test.go +474 −1
internal/objstore/walk.go +143 −16
internal/objstore/walk_sim_test.go +173 −14
internal/objstore/walklock.go +13 −4
internal/release/store.go +11 −0
internal/sigfence/sigfence.go +155 −0
internal/sigfence/sigfence_test.go +123 −0

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 ec94121b4..9aead313b 100644

--- a/docs/superpowers/specs/2026-10-01-stripe-idempotency-evidence.md

+++ b/docs/superpowers/specs/2026-10-01-stripe-idempotency-evidence.md

@@ -31,7 +31,8 @@ Source D: <https://docs.stripe.com/error-codes>

Consequences for the design:

-- A fence request answered 2xx has taken the key.

+- A fence request answered 2xx has taken the key (`FenceTaken`): the operation's own request never ran under it.

+- A fence request refused as a parameter mismatch found the key held by the operation's own request (`FenceHeldByRequest`). That request began executing. How it ended is not known from this: it may have succeeded, failed for good, or ended in a 500. Such an operation may be recorded as landed when the provider shows its target. It is never retired as unsent on this evidence; it is held with a review.

- A fence request answered 429, 401, or a validation 400 has taken nothing. The code treats every such answer as not fenced.

- A fence request answered 500 is indeterminate. The code treats it as not fenced and asks again; if the key was in fact saved, the next ask replays the saved 500 and stays not fenced. An operation in that state is held, not acknowledged. That is safe and can stick for as long as Stripe remembers the key.

- A quantity update refused for its `proration_date` is a validation failure and saves nothing, so the worker fences the key itself afterwards. Which 400s count as "most" is not specified. Sandbox check 3 covers this one.

@@ -45,7 +46,7 @@ Consequences for the design:

> `idempotency_error`: Idempotency errors occur when an `Idempotency-Key` is re-used on a request that does not match the first request's API endpoint and parameters. (C)

-The documentation names the error type. It does not state the HTTP status for a mismatch. The code requires status 400 with type `idempotency_error` before it counts a key as held. If Stripe answers a mismatch with another status, the code reads it as not fenced, which holds the operation and never acknowledges it wrongly. Sandbox checks 1 and 2 establish the real status.

+The documentation names the error type. It does not state the HTTP status for a mismatch. The code requires status 400 with type `idempotency_error` before it counts a key as held by another request, and a 2xx before it counts the fence as taken. A missing item (404) counts as nothing: a 404 saves no result, and the same answer comes back for an item of another account or mode. If Stripe answers a mismatch with another status, the code reads it as not fenced, which holds the operation and never acknowledges it wrongly. Sandbox checks 1 and 2 establish the real status.

The comparison is against "the original request". If the original saved nothing (validation failure), there is nothing to compare with and the later request runs as new. That is why the fence is taken explicitly and not inferred from an earlier refusal.

@@ -77,7 +78,7 @@ The schedule the fence does not cover:

What bounds it:

-- The sender's request carries a context deadline tied to its lease. A process stopped by a signal wakes to an expired deadline and sends nothing, because the clock the deadline runs on keeps counting. A suspended virtual machine is different: that clock stops with it. The deadline is therefore not a bound for a suspended host, and this design does not claim it is.

+- The sender's request carries a context deadline tied to its lease. A process stopped by a signal wakes to an expired deadline and starts no new request. A request already handed to the transport can still leave. A suspended virtual machine is worse: the clock the deadline runs on stops with it. A context deadline is therefore not a fence, for either kind of pause, and this design does not claim it is.

- The request carries `proration_date`, which Stripe refuses outside the subscription's current period. On an annual plan that is a wide window. It stops a charge into a later period and little else.

- Detection. The console never lowers a quantity inside a period, so a quantity observed to have gone down files `billing_quantity_lowered`, which holds the account's invoices until a person has looked (`TestQuantityLoweredAtTheProviderFilesAReview`). Before 2026-10-01 04:50 this did not exist: the console followed the provider's number down with no review. An earlier comment in the code said a mismatch review covered this case. It did not, and the comment was wrong.

@@ -99,15 +100,20 @@ CARLOS_STRIPE_SANDBOX_KEY=<rk_test_ key, from the environment, never on a comman

go test ./internal/billingstripe/ -run TestSandboxQuantityFence -count=1 -v

```

-It needs a test-mode restricted key that can write customers, products, prices and subscriptions and read invoices. It refuses any key that is not `rk_test_`. It has **not been run**: test mode is not authorized for this overnight run.

+It needs a test-mode restricted key that can write customers, payment methods, products, prices and subscriptions and read invoices. It refuses any key that is not `rk_test_`. It has **not been run**: test mode is not authorized for this overnight run.

| Check | What it establishes | Status |

| --- | --- | --- |

-| 1 | A fence request is accepted, changes neither the quantity nor any billing object, and a quantity update arriving after it is refused with 400 `idempotency_error` | not run |

-| 2 | A second copy of a quantity update is answered from memory and creates no second proration; a fence under that key is refused with 400 `idempotency_error` | not run |

+| 1 | A fence request runs, changes neither the quantity nor any billing object, and a quantity update sent ninety seconds after it is refused with 400 `idempotency_error` | not run |

+| 2 | A second copy of a quantity update is answered from memory and bills nothing; a fence under that key is refused with 400 `idempotency_error` | not run |

| 3 | A `proration_date` outside the period is refused naming the parameter and saves no key | not run |

-| 4 | Concurrent copies of one request run it once; the 409 answer if it is observed | not run |

-| 5 | A canceled subscription refuses a quantity update; what a fence on its item answers | not run |

-| none | Behaviour after the key is forgotten | cannot be checked in a run; documented as "runs as new" and treated that way |

+| 4 | Of six concurrent copies of one request exactly one is run (one answer that is not a replay) | not run |

+| 5 | A fence and an update racing for one key, six times: exactly one takes it each time | not run |

+| 6 | What a canceled subscription's item answers to a quantity update and to a fence. Logged, not asserted | not run |

+| none | Behaviour after the key is forgotten; Stripe's later reconciliation of a request it answered with a 500 | cannot be checked in a run |

-If check 5 shows Stripe refusing the fence on a canceled subscription's item, the worker's ended-subscription path would file `billing_quantity_fence_refused` for every canceled team subscription with an unconfirmed operation. That is safe and noisy, and needs a code decision before the worker is switched on.

+Checks 1 to 6 run twice: under `next_invoice` on an invoiced subscription, and under `immediate` on a subscription with a test card, because the two differ in what a quantity update does at the provider.

+If check 6 shows Stripe refusing the fence on a canceled subscription's item, the worker's ended-subscription path files `billing_quantity_operation_unsettled` for every canceled team subscription with an unconfirmed operation, and each waits for a person to resolve it. That holds the account's quantity work until then, including under a later subscription. It needs a code decision before the worker is switched on: if the same check shows Stripe refuses a quantity update on a canceled subscription, a canceled subscription can be treated as its own fence.

+Not verified and not stated by Stripe: whether a key remembered under one API key is still remembered after that key is rotated. A late request signed with the old key against a fence taken with the new one is outside what has been checked.

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 b1db452cd..f1c4bc624 100644

--- a/docs/superpowers/specs/CODEX-DISPOSITIONS-carloku-paid-launch-overnight.md

+++ b/docs/superpowers/specs/CODEX-DISPOSITIONS-carloku-paid-launch-overnight.md

@@ -223,3 +223,63 @@ Each removes one piece of a mechanism. All fail a test except one equivalent mut

- The invoice engine's lease has no provider-side fence beyond identical replays. Its operations are never replaced, so within Stripe's key memory a late request is a replay. Past it a late request runs as new and is detected and held, not prevented. One schedule leaves an open invoice with a wrong total that the engine refuses to charge; whether the customer could pay it from Stripe's hosted page is not determined.

- `ErrBucketRetained` has no member copy. It needs Paul's review.

- A fence request answered 500 leaves its operation outstanding for as long as Stripe remembers the key.

+## Round seven (reviewed `43383b93`; Astra rejected, Opus accepted with fixes)

+Reports: `CODEX-FINDINGS-carloku-paid-launch-overnight-round7.md`, `OPUS-REVIEW-carloku-paid-launch-overnight-round7.md`.

+Both reviewers attacked the two mechanisms at their edges, as the brief asked, and both were right. The lease covered a bucket name while the thing read before it was an app's record. The fence covered this package's clients and not the deployment bucket's. "The key is held" was treated as one fact when it is two. Every finding is accepted. None is disputed.

+### Storage

+| --- | --- | --- | --- |

+| Astra F1: a declaration-only teardown deletes after a re-read, outside any lease | Accepted. The delete names the generation it read (`release.DeleteIfMatch`). A grant that landed in the gap changed the generation and the backend refuses the delete. | `walk.go` `deleteRecordIfUnchanged` | `TestDeclarationOnlyTeardownCannotEraseAStoreGrantedMeanwhile` |

+| Astra F2, Opus N1: teardown takes bucket A's lease and acts on a record that now names B | Accepted. After the lease the record's bucket is compared with the leased name, before anything is written or called. | `walk.go` `Teardown` | `TestTeardownStopsWhenItsRecordNamesAnotherBucketThanItsLease` |

+| Opus N2: an adopting grant writes a bucket it only saw in a stale grant block | Accepted. One rule says which bucket a grant is for, applied to the first read and again to the record read under the lease. A difference stops the grant. | `walk.go` `Grant` `bucketFor` | `TestGrantDoesNotAdoptABucketItOnlySawInAStaleRecord` |

+| Opus N3: `Rotate` and `RotateFinish` call IAM outside any lease or fence | Accepted. Both are walks now: the bucket's lease, the fenced context, and the record as read under the lease, which must still be the store the rotation was asked for. | `walk.go` `leaseForRotation` | `TestRotationStopsWhenTheStoreWasReplacedWhileItWaited` |

+| Astra F3, Opus N8: the deployment bucket's writes are not fenced | Accepted. The fence moved to its own package and is installed on the deployment bucket's S3 client as well. Under a walk's context its requests carry the lease's stamp. The record and sidecar deletes are also conditional on the generation read. | `internal/sigfence`, `release/store.go`, `walk.go` `deleteGeneration` | `TestTeardownDoesNotDeleteASidecarWrittenAfterItLooked`; the simulation's store now refuses a stale walk as AWS would |

+| Astra F4, Opus N7: a retried request succeeds and the walk frees the name with the first attempt unaccounted for | Accepted. A middleware counts attempts per operation and raises a doubt on the walk at the second. A walk with a doubt ends like a failed one: the name stays reserved until the lease runs out. The retryer itself is untouched. | `sigfence.WatchAttempts`, `walklock.go` | `TestRetriedRequestRaisesDoubtAndKeepsItsStamp` (a real SDK client against a server that fails once: both attempts carry the lease stamp), `TestWalkThatNeededASecondAttemptKeepsItsLease` |

+| Opus N11: takeover trusts the taker's clock to within five minutes | Accepted, without a clock check. The lease now outlasts the backend's window by a whole window (thirty minutes in all). A taker whose clock runs ahead by less than the window cannot take over early; one that runs ahead by more stamps its own requests beyond what AWS accepts. | `fence.go` | `TestLeaseIsNotTakenOverEarlyByAClockThatRunsAhead` |

+| Opus N13: a take-back clears the destroying marker without recording the destruction | Accepted. The ledger records the destruction when the bucket is deleted, not only when the interval closes. | `ownership.go` `markBucketDestroyed` | `TestTakeBackAfterDeletingTheBucketStillRecordsItsDestruction` |

+| Astra F8, Opus N6: the simulation never checks mid-walk, generates no pause and no failure after the effect | Accepted. It now checks every invariant the moment a nested walk returns (and has a test that the check sees a violation that exists only then). A walk is frozen past its lease one time in three while another takes the name over. The store refuses stale walks as the provisioner does. Provider calls and store writes can fail after they ran. A busy refusal no longer marks a store broken. | `walk_sim_test.go` | `TestSimulationOracleSeesATransientViolation`; the sweep's seed count is in the commit |

+| Astra, AWS evidence: the probe's acceptance checks are weak | Accepted. It now requires the operation's own answer for an accepted request on both services, checks a key that needs escaping, and runs once with the SDK's retries on. Rerun 2026-10-01, read-only. | `fence_aws_test.go` | the run's log lines are in the handoff |

+Still outside the simulation, each with a direct test and not a random schedule: a second bucket name, rotations, records with no ledger interval, the SDK's retries. Not modelled at all: two clocks that disagree (one test covers the takeover case).

+A fix that had a latent fault of its own: the replacement signer was built without S3's rule that the object key is not escaped twice. The SDK passes that rule per request, so nothing had failed, but a replacement must not depend on it. `sigfence.NewS3Signer` sets it, a test pins it, and the AWS probe signs such a key.

+### Quantity

+| --- | --- | --- | --- |

+| Astra F5: a key held by the operation's own request is treated as settled, though that request may have ended in a 500 Stripe still reconciles | Accepted. The fence now reports which request holds the key. Recording a landing accepts either. Retiring does not: an operation whose key is held by its own request, and whose effect the provider does not show, is held with a review and is never replaced on that evidence. | `billingstripe.FenceQuantityKey`, `billingquantity.go` `quantityOperationRetirable`, `settleQuantityOperation` | `TestQuantityIndeterminateRequestIsHeldNotRetired` (the fake now remembers a 500 and replays it) |

+| Found by the simulation after that fix: a remembered 500 is replayed every pass for 22 hours before anyone is told | Fixed. A server error answered from memory (Stripe's `Idempotent-Replayed` header) holds the operation at once. | `billingquantity.go` `sendQuantityOperation` | `TestQuantityReplayedServerErrorHoldsAtOnce` |

+| Opus N4: an operation whose fence is refused wedges the account for good, and resolving the review re-files it | Accepted. Resolving the operation's review is the way out. It is recorded on the operation as its fence and the operation is retired. While the review is open the provider is not asked again. | `markQuantityReviewed`, `fenceQuantityOperation` | `TestQuantityRefusedFenceHoldsForAPersonAndIsRetiredOnceReviewed` |

+| Astra F6, Opus N9: the send lease is not owned | Accepted. A token per sender. The lease is taken only when none is live and released only by the token that took it. | `claimQuantitySend`, `releaseQuantitySend` | `TestQuantitySendLeaseBelongsToOneSender` |

+| Astra F7, Opus N10: key uniqueness remembers one operation back | Accepted. The key ends in the account's operation serial, which only goes up and survives on the row. | `beginQuantityOperation` | `TestQuantityOperationKeysNeverRecur` |

+| Astra F9, Opus N12: the sandbox procedure can pass without establishing what the gate needs | Accepted. It runs under both proration policies (the immediate one with a test card). Concurrent copies must show exactly one answer that is not a replay. A fence and an update race for one key six times and exactly one must take it. The late update is sent ninety seconds after the fence. The canceled-subscription check logs raw statuses and asserts nothing it has not seen. | `sandbox_fence_test.go` | not run: test mode is not authorized |

+### Claims corrected

+- "A process stopped by a signal sends nothing": it starts no new request. One already handed to the transport can leave. The design on `feat/carloku-late-request-fencing-20261001` is what closes that, at the connection.

+- "A retirement is decided on a provider read taken after the fence": true of an operation of the current subscription. An ended subscription's operation is acknowledged with no provider read, on the fence alone.

+- "Never used twice", "one walk at a time across processes" as a finding of the simulation, "accepts one thirteen minutes old on IAM": each now says what the test shows.

+- The `FenceQuantityKey` comment said the fence lasts 24 hours from the call. It lasts 24 hours from the request that first used the key, which for a key held by the operation's own request may be nearly over.

+### Open after this round

+- Both late-request defects (`OUTSTANDING.md` items 7 and 9) and their design.

+- Whether a key survives an API key rotation is unknown and unmentioned by Stripe.

+- A retained teardown finishes only when someone runs it again. Nothing runs it.

+- `ErrBucketBusy`, `ErrBucketRetained` and `ErrBucketHeld` reach the console as plain errors. Member copy for them needs Paul's review.

+### Mutation checks for round seven's fixes (run in a scratch copy of the tree)

+Each removes one fix. Each fails the named test, except the last.

+- Storage: the backend never refuses a stale walk (`TestPausedWalkCannotActAfterItsLeaseIsTaken` and the simulation); the record delete unconditional (`TestDeclarationOnlyTeardown…`); teardown not comparing its record with its lease; grant not recomputing its bucket; a doubtful walk freeing the name; a store that left counted as holding (its test, and the simulation at seed 1360); rotation ignoring a replaced store; rotation leasing another name; destruction not recorded at the deletion; the lease margin shortened; the sidecar delete unconditional.

+- Quantity: a key held by its own request made retirable; the send lease taken while live; any sender releasing it; the key without its serial; a replayed 500 replayed again (its test, and the simulation at seed 4); an unresolved review letting the operation be retired.

+- Equivalent: removing the "review is open, do not ask the provider again" branch. The branch after it returns without asking as well.

diff --git a/docs/superpowers/specs/CODEX-FINDINGS-carloku-paid-launch-overnight-round7.md b/docs/superpowers/specs/CODEX-FINDINGS-carloku-paid-launch-overnight-round7.md

new file mode 100644

index 000000000..5e7795869

--- /dev/null

+++ b/docs/superpowers/specs/CODEX-FINDINGS-carloku-paid-launch-overnight-round7.md

@@ -0,0 +1,193 @@

+## 1. Round-six dispositions

+Reviewed `43383b93`, with base `aed42815` and round-seven changes `058ea506..43383b93`. This was a static review: no files modified, tests run, or network commands executed. Counterfactual test assessments below come from reading code and assertions, not executing mutations.

+Path abbreviations: **O** = `internal/objstore`, **C** = `internal/console`, **H** = `internal/host`, **B** = `internal/billingstripe`.

+|---|---|---|---|

+| Astra R1 | **Partly closed.** Ordinary competing invocations are excluded. Lease coverage and early release remain unsafe; F1–F4 below. | `O/walklock.go:104`, `:124`; `O/walk.go:188`, `:662`. Tests: `O/ownership_test.go:1252`, `:1316`, `:1364`. | **No** without the corresponding lease/fence protection. These tests omit the new schedules. |

+| Astra R2 | **Closed for the reported stale-marker write.** The broader serialization guarantee remains incomplete. | Carried ETag: `O/walk.go:826`; invalidating the deleted-bucket note before rebuilding: `:223`. Test: `O/ownership_test.go:1579`. | **No** without invalidating the note. **Yes** if only the carried-ETag protection is reverted: this fixture does not interleave another record generation at that write. |

+| Astra R3 | **Closed for the reported mixed-account holders.** | `O/ownership.go:458`, `:469`; final check `O/walk.go:879`. Simulation assertion: `O/walk_sim_test.go:202`. | Removing only the first foreign-holder protection can still **pass**, because the final check remains. Removing both exposes the adverse schedule. The reported seed was not rerun. |

+| Astra R4 | **Partly closed.** Inventory is skipped when the app has any historical interval, including a closed one. | Inventory: `O/walk.go:308`; bypass: `:292`. Test: `O/ownership_test.go:942`, assertion `:974`. | **No** without inventory. It does not cover historical ownership followed by a legacy adopter. |

+| Astra R5 | **Partly closed.** The original live-sender schedule is guarded; the send lease is shared between invocations, and provider settlement is overstated. | `C/billingquantity.go:557`, `:579`, `:1091`. Tests: `C/billingquantity_test.go:1241`, `:1799`. | **No** without the corresponding guards. The provider test establishes the fake’s behavior, not Stripe’s; F5–F6 remain. |

+| Astra R6 | **Closed for the reported two-webhook schedule.** | Transactional dating hold: `C/billingquantity.go:328`. Test: `C/billingquantity_test.go:1311`. | **No.** The test checks the specific operation’s dating review, so the separate lowering review cannot satisfy it. |

+| Astra R7 | **Parked with the code removed.** | Production publisher now seals without an attestation callback: `H/durability.go:154`; opens with empty options at `:49`. Scoped implementation and tests deleted. | **N/A:** removal, not a behavioral repair. |

+| Astra R11 | **Closed for write-ahead recovery.** | Teardown block: `O/walk.go:691`; healthy shortcut: `:366`; clearing only after successful rebuilding: `:410`, `:445`. Tests: `O/ownership_test.go:866`, `:1579`. | The named successful-recovery test at `:866` would **pass** with early clearing. The failed-recovery test at `:1579` would **fail**, including its assertion that the block remains. |

+| Astra R13 | **Partly closed.** A single sender’s lease delays reconciliation, but another sender can release that lease; F6. | `C/billingservice.go:546`; `C/billingquantity.go:1046`. Test: `C/billingquantity_test.go:1200`. | **No** without the wait. The fixture manually claims one lease and cannot reproduce another sender clearing it. |

+| Opus N2 | **Partly closed.** Dead-key recovery and retained-teardown completion are repaired. The requested member copy remains unfinished. | Rebuild protection above; `O/ownership.go:405`; `O/walk.go:886`. Test: `O/ownership_test.go:1468`. | **No** without rebuild/destruction-history handling. It does not test member copy. |

+R4’s remaining scenario is concrete: A previously held its deterministic bucket and has a closed interval. A legacy B store subsequently adopts that name without an interval. A’s new minting Grant treats its historical interval as `held=true`, skips both the existing-bucket refusal and record inventory, and passes the exclusive ledger check because B has no open interval. A receives access to B’s data. This is the remaining portion of R4, not a new finding.

+Four listed mutation checks were verified by reading:

+- **Signer timestamp pinning:** `O/ownership_test.go:1441` requires the lease timestamp rather than the supplied timestamp forty minutes later. Removing the override in `O/fence.go:72` fails that assertion.

+- **Backend expired-stamp refusal:** `O/ownership_test.go:1364` requires `ErrFenced` immediately beyond the acceptance window. Removing `memFenced`’s refusal fails it.

+- **Failed-walk reservation:** `O/ownership_test.go:1316` requires competing grants to remain busy, including after a successful retry carrying the earlier reservation. Freeing the failed lease fails it.

+- **Destructive re-confirmation:** `O/ownership_test.go:1534` removes the destroying marker during `EmptyBucket` and requires that `DeleteBucket` never run. Removing that confirmation fails it.

+## 2. New findings

+### F1. P1 — Declaration-only teardown bypasses the lease and can erase a newly provisioned store

+**Where:** `O/walk.go:636`, `:1040`, `:1058`.

+**Scenario:** Teardown reads a declaration-only record. Its early return calls `deleteRecordIfUnchanged` before acquiring any lease. The helper rereads the declaration, passes its checks, and pauses before deletion.

+Another console process grants the store, creates its bucket, user and credentials, and records success. The original teardown resumes and unconditionally deletes the new record. It reports success while leaving a live, credentialed estate without its cleanup record.

+**Smallest acceptable fix:** Make declaration deletion conditional on the exact record generation at the backend. Include declaration-only teardown in cross-process app coordination. Another client-side reread is insufficient.

+### F2. P1 — Teardown can acquire bucket A’s lease and destroy bucket B

+**Where:** `O/walk.go:654`, `:662`, `:671`, `:703`.

+**Scenario:** T1 reads an app’s bucket A and pauses before acquiring its lease. T2 completes teardown. The app is redeclared and granted bucket B.

+T1 resumes, acquires A’s lease, rereads the now-B record, and uses its new bucket and user without checking that the effective bucket still equals `leased`. It can empty and delete B while a walk holding B’s actual lease runs concurrently.

+Grant has the corresponding changed-bucket refusal at `O/walk.go:202`; Teardown does not.

+**Smallest acceptable fix:** Compare the reread record’s effective bucket with the acquired lease before writing teardown state or calling the provider. Refuse or restart with the correct lease on mismatch. Cover same-app recreation across two bucket names.

+### F3. P1 — Deployment-bucket mutations bypass the backend fence

+**Where:** `O/aws.go:81`, `:92`; `internal/release/store.go:538`, `:769`; `O/walk.go:940`, `:1058`.

+**Scenario:** The provisioner’s S3 and IAM clients install `fencedSigner`; the separate `release.S3Store` client does not.

+Teardown issues its final `DeleteObject(store.json)` near minute fourteen. That request is signed at minute fourteen rather than the walk’s start. Its response times out, leaving the request uncertain and the lease reserved until minute twenty.

+After expiry, another teardown finishes and a new grant recreates the record. The old deletion then reaches AWS with a still-valid ordinary signature and deletes the new estate’s record.

+**Smallest acceptable fix:** Extend backend fencing to every remote mutation made by the walk, including release-store operations. Make record and sidecar deletion conditional on the generation being removed.

+### F4. P1 — Successful SDK retries can release a lease while an earlier request remains uncertain

+**Where:** `O/walklock.go:178`, `:194`; `O/walk.go:952`; `O/aws.go:80`.

+**Scenario:** A destructive SDK request suffers an ambiguous transport failure. The SDK retries and the second attempt succeeds. The enclosing method returns success, so teardown finishes and releases the name immediately.

+The first attempt was never proved unable to execute. A new grant reuses the name while that attempt’s pinned signature remains valid. A delayed `DeleteBucket` can remove the recreated empty bucket; a delayed object-delete request can delete newly written objects with reused keys.

+The production constructors retain SDK retry behavior. `TestFenceAgainstAWS` explicitly disables retries at `O/fence_aws_test.go:37` and `:48`.

+**Smallest acceptable fix:** Preserve uncertainty from individual transport attempts through the walk’s outcome. A successful retry must not permit early release after an ambiguous earlier attempt. Conservatively retaining the reservation through the acceptance window is preferable to treating method-level success as proof that every attempt finished.

+### F5. P1 — A cached indeterminate quantity request can be retired as settled

+**Where:** `B/client.go:680`, `:710`; `C/billingquantity.go:1098`, `:1111`, `:994`, `:1074`.

+**Scenario:** A quantity update returns a cached 500 while its effects remain indeterminate. A metadata fence receives `400 idempotency_error`, proving that the key already has different parameters. `FenceQuantityKey` returns true.

+On the next pass, the subscription still shows the original quantity. The worker treats the operation as settled, retires it, and permits a replacement. However, the evidence document explicitly allows Stripe to reconcile the original failed request later: `2026-10-01-stripe-idempotency-evidence.md:38`. The mismatch prevents a new execution under that key; it does not establish that the original request’s effects have finished.

+This uncertainty exists before key expiry. The repository does not establish the eventual ordering of Stripe’s reconciliation, so it cannot justify replacing the operation on this evidence.

+The fake hides the distinction: `C/billingquantity_test.go:125` records a successful result before returning the injected 500 at `:131`; subsequent identical requests receive success at `:106`.

+**Smallest acceptable fix:** Distinguish “metadata fence executed” from “some original request owns the key.” Preserve and hold indeterminate original outcomes, including lost responses, until authoritative evidence settles their effects. Do not automatically retire them solely on a mismatch and one unchanged subscription read.

+### F6. P2 — One sender can release another sender’s lease and discard its valid confirmation

+**Where:** `C/billingquantity.go:559`, `:570`, `:529`; `C/billingservice.go:546`.

+**Scenario:** A claims the lease and begins sending an operation. B reads the same outstanding operation before its effect is visible and also claims it: `claimQuantitySend` neither excludes a live holder nor identifies an invocation.

+B’s duplicate receives a conflict or transport error and calls `releaseQuantitySend`, clearing the shared lease while A remains active. A’s update succeeds, but its response is delayed. Another pass now sees the target with no live lease, fences and acknowledges an unverified landing, and files a dating hold.

+When A receives its successful response, confirmation updates only `acked_at=0`, changes no row, and returns success. Valid evidence is lost, reproducing R13 through the new lease mechanism.

+**Smallest acceptable fix:** Bind send ownership and release to an invocation, or otherwise account for every live sender. Coordinate fencing against current persisted send ownership. Preserve a valid late confirmation rather than silently discarding it.

+### F7. P2 — Key uniqueness remembers only the immediately preceding operation

+**Where:** `C/billingquantity.go:477`, `:515`, `:874`; test `C/billingquantity_test.go:2061`.

+**Scenario:** An unchanged account has pending additions. Operation K1 is fenced and retired without changing quantity. K2 is then fenced and retired. Recording K2 overwrites the account’s sole operation row, erasing K1 from the uniqueness check.

+A proposal using K1 again now passes the database check. Deterministic key construction makes recurrence possible when a clock returns to an earlier operation second while the subscription, generation, target and behavior remain unchanged.

+Stripe can still remember K1 as a metadata fence. The newly accepted operation therefore does not own the key the database assigned it.

+**Smallest acceptable fix:** Give each new operation a durable unique identity independent of wall-clock recurrence, persisting its key for retries. Alternatively retain all consumed keys. Extend the test from K1→K1 to K1→K2→K1.

+### F8. P2 — The storage simulation never performs its claimed intermediate invariant check

+**Where:** `O/walk_sim_test.go:307`, `:312`, `:329`, `:354`.

+**Scenario:** The comment says invariants are checked when the nested walk returns. `midProblem` is initialized but never assigned; the callback invokes neither `check()` nor `owner()`.

+A nested walk can violate an invariant and the outer walk can repair the observable state before the eventual check. For example, an erroneous inner teardown can remove an object that the outer grant writes again at `:153`.

+**Smallest acceptable fix:** Check and retain invariant failures immediately after nested actions and at the modeled mutation boundaries. Add a deliberately transient violation to establish that the oracle detects it.

+The model also cannot generate:

+- Two bucket names or same-app nested walks; `:319` excludes the latter.

+- Lease expiry during an outstanding call, independent clock skew, delayed requests, or SDK retries.

+- Store read/CAS/delete failures, scanner failures, or crashes between persistence steps.

+- Ambiguous failures after a side effect: `MemProvisioner.takeFailure` fails before mutation.

+- Transient ownership changes between its samples.

+Its **Data** assertion checks bucket existence and one object, not usable recorded credentials or delivered configuration (`:218`). Its credential snapshots record user existence, not valid keys (`:271`). Its **Owner** property is sampled rather than continuous. These are weaker than the properties needed for R11/N2 and ownership completeness.

+The quantity simulation has related limits: one shared clock, no key eviction, and the simplified 500 behavior described in F5. Its **Held** check accepts any open dating review for the account (`C/billingquantity_sim_test.go:223`), rather than the review belonging to each operation.

+### F9. P2 — The sandbox procedure can pass without establishing concurrent fencing

+**Where:** `B/sandbox_fence_test.go:230`, `:255`, `:258`, `:264`.

+**Scenario:** The concurrency test submits six identical absolute-quantity updates. It checks final quantity 12 and at most two additional invoice items.

+A provider that executes all six requests can satisfy those assertions: the first changes 11→12; subsequent 12→12 executions need not create further prorations. The test does not require replay evidence on the successful concurrent responses.

+All metadata-fence versus quantity-update checks are sequential. A provider that rejects mismatches against completed requests but incorrectly admits differently parameterized concurrent requests could pass the procedure.

+**Smallest acceptable fix:** Exercise concurrent metadata-fence and quantity requests sharing a key, verify which request won and how the loser was refused or replayed, and inspect replay evidence for duplicate successes. Do not label an unobserved concurrency outcome verified.

+No concrete live-mode or secret-output exposure was found: the procedure requires `rk_test_`, uses the fixed API host, refuses redirects, and avoids printing credentials. Cleanup is less complete than its comment promises: the product is archived, the price remains, and cleanup response statuses are ignored.

+### Other requested boundaries

+- **Attestation:** No production path setting attestation true was found. The host opens the ledger with empty options at `H/durability.go:49`; the scoped caller and reader are removed.

+- **Worker gates:** No production bypass was found. `C/billingquantity.go:709` checks both exact configuration values before account discovery, ended-subscription handling, or fencing. Invalid nonempty values are rejected by `cmd/console/billing.go:38` and `:41`.

+- **Acknowledgment:** Every quantity-operation acknowledgment statement includes `quantityOperationSettled`. F5 concerns what qualifies as settled, not a missing SQL guard.

+- **AWS evidence:** `TestFenceAgainstAWS` checks `ListObjectsV2` and `GetUser`, using clients with retries disabled. That is consistent with timestamp rejection at the shared signing boundary, but insufficient to establish the complete walk invariant: it excludes release-store mutations, destructive retries and outstanding attempts. Its positive checks are weaker still: a young S3 request passes on any error other than the recognized skew error, and young IAM age refusals are merely logged. A pass does not itself prove that both services accepted the thirteen-minute-old requests.

+## 3. Claims contradicted or overstated

+| Location | Claim | What the implementation supports |

+|---|---|---|

+| `implementation/carloku-paid-launch/AGENT_HANDOFF.md:35` | One walk per bucket name, with every request stamped at acquisition. | Declaration-only teardown bypasses the lease; teardown can use the wrong bucket’s lease; release-store requests use another signer. F1–F3. |

+| Same, `:35` | Successful walk completion permits safe reuse of the name. | SDK success can conceal an ambiguous earlier attempt. F4. |

+| Same, `:35` | Minting is refused while another store record holds the name. | Historical ownership bypasses record inventory. Astra R4 remains partly closed. |

+| Same, `:36` | A statement unreadable for three passes is classified without it. | The rule excludes the platform statement at `C/economicspass.go:1435`. This round-six wording defect remains. |

+| Same, `:38` | Sending and retiring exclude each other through the send lease. | The lease is not invocation-owned, and one sender can clear another’s protection. F6. |

+| Same, `:38` | A provider key fence settles an unconfirmed operation for retirement. | A mismatch can identify an indeterminate original request. F5. |

+| `implementation/carloku-paid-launch/OUTSTANDING.md:62`, `:68` | The AWS premise is proven, including acceptance at thirteen minutes. | The probe’s positive assertions do not establish acceptance, and its coverage does not establish whole-walk fencing. |

+| Same, `:63` | Never-reused keys and the stated invariants are tested. | Only immediate key reuse is tested; historical keys are overwritten. Intermediate storage checks are absent. F7–F8. |

+| Same, `:35`–`:38` | Stripe blockers cover the fence’s unresolved limits. | Cached-original-500 reconciliation is missing as a separate unresolved retirement hazard. Passing the current sandbox test would not close it or F9. |

+| `docs/superpowers/specs/2026-10-01-stripe-idempotency-evidence.md:80` | A signal-stopped sender resumes with an expired deadline and sends nothing. | A pause after the transport’s final cancellation check can resume the write before cancellation takes effect. A context deadline supplies no backend fence; this limitation is not confined to suspended VMs. |

+| Same, `:95`, `:109` | The sandbox pass establishes the concurrency behavior required by the gate. | Its concurrency oracle cannot establish single execution and never races the fence against the update. F9. |

+| `B/client.go:688` | The fence lasts at least twenty-four hours “from this call.” | A mismatch or replay does not restart retention. The key may have been consumed nearly twenty-four hours earlier. The evidence document correctly measures from the consuming request at `:68`. |

+| `B/client.go:680` | A mismatch means the original quantity update has run and is settled. | It establishes retained parameters, not successful or finally resolved effects. F5. |

+The evidence document appropriately distinguishes fake-provider tests from live verification and acknowledges key expiry and undocumented mismatch status. Those qualifications do not repair the separate settlement and concurrency gaps above.

+## 4. Verdict

+**Reject.** Bucket-walk authority still has destructive bypasses, and quantity retirement treats indeterminate provider effects as settled.

\ No newline at end of file

diff --git a/docs/superpowers/specs/CODEX-REVIEW-BRIEF-carloku-paid-launch-overnight-round8.md b/docs/superpowers/specs/CODEX-REVIEW-BRIEF-carloku-paid-launch-overnight-round8.md

new file mode 100644

index 000000000..ba61e7cf7

--- /dev/null

+++ b/docs/superpowers/specs/CODEX-REVIEW-BRIEF-carloku-paid-launch-overnight-round8.md

@@ -0,0 +1,32 @@

+# Review brief, round 8: 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, and the round 3 to 7 briefs for rulings 6 to 12. Then read `CODEX-DISPOSITIONS-carloku-paid-launch-overnight.md`, whose last section covers round 7, `docs/superpowers/specs/2026-10-01-stripe-idempotency-evidence.md`, 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 7 are `git diff 43383b93 <commit>`.

+This is still the fixed snapshot: round 7's findings fixed, nothing else added. Nothing on the branch may attest (ruling 10). The quantity worker has two switches and both are unset.

+Two documents on the branch describe work that is **not** in this snapshot: `docs/superpowers/plans/2026-10-01-late-request-fencing.md` (a design, built on another branch) and `OUTSTANDING.md` items 7 and 9 (two open defects whose paths are disabled). Do not report those two defects again as findings. Do report anything that makes either one reachable with the switches as they are, and any claim about them that the code contradicts.

+## First: a disposition table

+For every round 7 finding (Astra F1 to F9 and the remaining part of R4, Opus N1 to N13): closed, partly closed or not closed, with the file and line of the fix and of the test, and whether that test would pass without the fix.

+## Then: new findings only

+- **The lease's scope** (`internal/objstore/walk.go` `Grant`, `Teardown`, `Rotate`, `RotateFinish`, `leaseForRotation`, `deleteRecordIfUnchanged`, `deleteGeneration`; `walklock.go`). Every walk now reads its record, takes the lease, and reads the record again. Find a walk that acts on something the lease does not cover: a record that changed in a way the re-read comparison misses (same bucket, different estate), a verb or a console door that reaches the provisioner with no lease at all, a declaration-only path that the conditional delete does not protect, a store with an invalid bucket name. Find a lease left held for good.

+- **The fence's reach** (`internal/sigfence`; `internal/objstore/aws.go`; `internal/release/store.go` `NewS3Store`). Find a client a walk reaches that does not carry the signer; a request under a walk's context that is not SigV4-signed by it; a consequence of installing the signer and the attempt watcher on every user of `release.NewS3Store` (the edge, the host agent, vet, ops) for requests that carry no fence. `NewS3Signer` disables the second escaping of the key: say whether that matches what the SDK's own S3 signer does for every request type this store sends.

+- **Doubt** (`sigfence.WatchAttempts`, `walklock.go` `release`). Find a retried attempt that raises no doubt (a retry the middleware does not see, a paginator, a waiter, a multipart call), and a path that frees the name while doubt stands.

+- **The thirty-minute lease.** The argument is that no clock check is needed. Attack it: a taker whose clock is behind, a holder whose clock is ahead, both wrong at once.

+- **Fence kinds and the review way out** (`internal/console/billingquantity.go` `settleQuantityOperation`, `fenceQuantityOperation`, `markQuantityReviewed`, `quantityOperationRetirable`, `quantityOperationSettled`; `sendQuantityOperation`'s handling of a replayed server error). Find an operation retired or replaced while its own request could still take effect; an operation recorded as landed that did not land; a review that a person resolves and the worker files again; an operation that can never leave the outstanding state and tells nobody; a way a resolved review of one operation lets another be retired.

+- **The owned send lease and the serial.** Find two senders live at once, a lease released by a sender that did not hold it, a key that recurs.

+- **The simulations.** State what each now generates and what it still cannot. Find a bookkeeping rule that hides a real violation (the storage one has had three such faults).

+- **`TestSandboxQuantityFence`.** It has not been run. Could it pass against a provider that does not fence? Would a pass under `immediate` establish what the gate needs? Could it leave objects behind in the sandbox or touch live mode?

+- **Claims.** `AGENT_HANDOFF.md`, `OUTSTANDING.md`, the evidence document, and the comments at the top of `walklock.go`, `fence.go`, `sigfence.go` and "The fence" in `billingquantity.go`. Anything stated as proven, never, cannot or always that rests on a fake, on an inference, or on one read-only observation.

+## 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-round7.md b/docs/superpowers/specs/OPUS-REVIEW-carloku-paid-launch-overnight-round7.md

new file mode 100644

index 000000000..0244fd23f

--- /dev/null

+++ b/docs/superpowers/specs/OPUS-REVIEW-carloku-paid-launch-overnight-round7.md

@@ -0,0 +1,239 @@

+# Opus production and authority review, round 7: Carloku paid launch, `aed42815..43383b93`

+I read `058ea506..43383b93` as whole files at `43383b93`, using `git show` only. As the brief asked, this was a read-only pass: I ran no tests, made no mutations and made no network calls. That differs from round 6, where I ran mutations. Every "would the test fail without the fix?" answer below comes from reading the code.

+**Production authority**

+- **Nothing attests.** `durabilityscope.go`, `durabilityreplicator.go`, the agent switch and `ops/litestream.go`'s reader are gone. No non-test code calls `SetAttestation` or `SetAllClientsInstrumented`. `newDurabilityPublisher` opens the ledger with `DurabilityLedgerOptions{}` (`host/durability.go:49`).

+- **The quantity worker cannot reach Stripe with either switch unset.** All provider calls in `billingquantity.go` (`GetSubscription`, `UpdateQuantityWith`, `FenceQuantityKey`) go through `processQuantityAdditions`. Its only caller is `billingQuantityPass`, which returns before any work unless `quantityWorker()` sees both values exactly (`billingquantity.go:115-121`, `:709-712`). The ended-subscription fence and the close are on that same path. `cmd/console/billing.go:39-42` and `billingservice.go:94-96` refuse a malformed fence value, with no trimming. The webhook reconcile reads and never writes.

+- **Billing.** Billing is off. I found no reachable path that bills a customer wrongly. Every acknowledgement (`:234`, `:272`, `:579`, `:671`) carries `quantityOperationSettled`.

+- **Customer data and leaks.** Two narrow races remain, both inside the new lease, and both need fixing before the storage changes land. A stale teardown can destroy a store that was re-created after it was asked for (N1). A racing adopt-grant can be handed another app's bucket (N2). I found no secret or identifier leak. The sandbox test cannot touch live mode: it checks the `rk_test_` prefix, then checks `livemode` on the first object it creates. It never logs the key or a response body.

+## 1. Disposition table (round 6)

+Abbreviations: **O** = `internal/objstore`, **C** = `internal/console`.

+|---|---|---|---|

+| Astra R1: duplicate teardowns destroy a newly granted bucket | **Closed for provider calls on the leased name.** Not closed for a teardown whose record changed before it took the lease (N1), for Rotate (N3), or for store writes (N8). | `O/walklock.go:104-149` (CAS lease), `:154-159` (bound); `O/walk.go:188-194`, `:661-687`; `O/fence.go:71-76`; `O/aws.go:81`, `:92` | `TestWalkLeaseIsOneInvocationAtATime` (`ownership_test.go:1252`) **fails** without the lease. `TestPausedWalkCannotActAfterItsLeaseIsTaken` (`:1364`) **fails** with a short lease or no backend refusal, but tests the fake's `memFenced`, not a walk end to end. |

+| Astra R2: a stale teardown marks a recreated bucket deleted | **Closed** | Note written against the carried etag (`O/walk.go:826-829`); grant voids the note (`:223-231`) | `TestTeardownAfterAFailedTakeBackDeletesTheBucketTheGrantRecreated` (`:1579`) **fails** without the void. |

+| Astra R3: a same-account holder hides another account | **Closed in code; no deterministic reproduction** | `O/ownership.go:458-459`, `:470-475`, `:480-482`; `O/walk.go:871-893` (re-read) | Only the simulation's Quiet invariant (dispositions: seed 5). No fixed-order mixed-holder test. |

+| Astra R4: exclusive minting ignores a legacy adopter | **Closed** | `O/walk.go:300-315` | `TestRetriedLegacyTeardownLeavesABucketAnotherAppNowHolds` (`:942`) asserts `ErrBucketHeld`; **fails** without the record check. |

+| Astra R5: retirement overtakes a sender after its re-read | **Closed** | `C/billingquantity.go:557-565` (claim), `:578-585` (retire under no live lease), `:1091-1094` | `…NotRetiredAcrossTheReplayLimitUnderItsSender` (`:1241`) and `…NothingIsConcludedWhileASenderHoldsTheLease` (`:1147`) **fail** without the lease condition. |

+| Supervision: a request issued inside the lease that arrives after it | **Partly closed: bounded, unverified, as stated.** Fence semantics have two gaps (N4, N5). | `C/billingquantity.go:635`, `:1091-1112`; `billingstripe/client.go:690-719` | `…PausedSenderIsRefusedByTheProvider…` (`:1799`) and `…NeverAcknowledgedUnfenced` (`:1881`): **fail** without the fence. Fake provider only. |

+| Astra R11: recovery clears its rebuild requirement early | **Closed** | `O/walk.go:164`, `:205`, `:366`, `:410`; TeardownBlock written first `:691-696` | `TestGrantOverItsOwnUnfinishedTeardownTakesTheStoreBack` (`:866`) **fails** without it. |

+| Astra R13: a retry loses its confirmation | **Partly closed.** Two concurrent senders can still clear each other's lease (N9). | `C/billingservice.go:546` | `TestQuantityReconcileWaitsWhileASenderHoldsTheLease` (`:1200`) **fails** without the lease wait. |

+| Opus N2: re-grant after a retained teardown reports a dead key healthy | **Closed.** A retained teardown finishes only when re-run (N13). | `O/walk.go:691-696`, `:871-893`; `O/ownership.go:404-405` | `TestRetainedTeardownFinishesOnceTheBucketItLeftIsDestroyed` (`:1468`) **fails** without `DestroyedAt`. |

+| Opus N6: every restart day holds | **Open, moot.** Nothing attests, so every day holds anyway; path B inherits it. | — | — |

+**Mutation checks verified by reading:**

+1. "Signer not pinning the lease instant": `TestFencedSignerStampsTheLeaseInstant` (`:1441`) asserts `X-Amz-Date`, so it fails.

+2. "A failed walk freeing the name": `TestWalkLeaseStandsAfterAWalkThatFailed` (`:1316`) fails.

+3. "Lease shorter than the window", "backend never refusing", "live lease taken over": all three are caught by `:1364`.

+4. "Fenced operation claimable" and "acknowledged without the fence": both are caught by `:1881`.

+**Not confirmed:** "walk calls carrying no fence". Only `:1252` inspects `bound()`. If the mutation drops `walk.bound` at the call sites in `Grant` or `Teardown`, no test I could find fails.

+## 2. New findings, most severe first

+### N1. P2: Teardown acts on whatever record it re-reads inside the lease, even one naming another bucket or a re-created store

+**Where:** `O/walk.go:654-686` takes the lease on the name from the first read. `:703-709` then takes `bucket` from the re-read, and nothing compares the two.

+**Scenario:**

+1. Teardown T1 reads `bab/x` (bucket A) and stalls before `acquireBucketWalk`.

+2. T2, from another process, finishes the teardown.

+3. The member re-declares, and the operator grants adopting B. That grant completes and frees B's lease.

+4. T1 takes lease A and re-reads. The record now names B, so T1 empties and deletes B under lease A.

+5. If the member re-minted A instead, T1 destroys the new store's data on the same name. The lease is satisfied either way. Only the request is stale.

+**Fix:**

+- After the re-read, refuse with `ErrCASConflict` when the bucket differs from `leased`.

+- Have the console door pass the estate it decided on (`Grant.At` and `KeyID`), and refuse when the re-read record differs.

+### N2. P2: a grant whose re-read finds no grant block adopts the stale first-read bucket

+**Where:** `O/walk.go:172-178` takes `bucket` from the first read. `:202` only checks a non-nil block. `:210-213` then writes `Grant{Bucket: <first read>, Minted: adoptBucket==""}`.

+**Scenario:**

+1. Minting grant G0 has written `Grant{A}`.

+2. An operator's adopt-B grant G1 reads `Grant{A}`, so `bucket=A`.

+3. G0 is refused (`ErrBucketHeld`: account C's legacy store holds A) and `refused()` clears the block.

+4. G1 takes lease A and re-reads: no block. It writes `Grant{A, Minted:false}`.

+5. Adopt grants skip every holder check (`:276`), so G1 hands `bab/x` credentials to account C's data.

+**Fix:** recompute the bucket from the fresh record and the arguments. If it differs from the leased name, release and refuse.

+### N3. P2: `Rotate` and `RotateFinish` call IAM outside any lease or fence

+**Where:** `O/walk.go:495-593`.

+**Scenario:**

+1. A rotate in process P stalls after reading the record.

+2. The app is torn down and re-granted. The user name is deterministic, and the new grant delivers key K2.

+3. P wakes. `ReapKeys(user, keep=[old KeyID])` deletes K2. P mints and delivers K3, then its CAS fails and the retry refuses (`:549`).

+4. The record names the deleted K2 while the app runs on K3. The next rotate or grant reaps K3, and the store goes dark.

+This contradicts "no provider call of a walk executes outside that walk's lease" (`walklock.go:28`).

+**Fix:** take the walk lease (kind `rotate`) and the bound context in both verbs. When a GuardDuty scanner is built, give its client the fenced signer too. Today `EnsureDisabled` is a ClamAV no-op.

+### N4. P2: an operation on a canceled or replaced subscription wedges the account's quantity work for good

+**Where:** `C/billingquantity.go:759-767`, `:994-996`, `:1100-1106`, and `beginQuantityOperation` `:465-470`.

+**Scenario:**

+1. An operation's response is lost, and the customer cancels.

+2. The fence on the canceled item is refused with a 4xx. That files `billing_quantity_fence_refused`, and the operation stays outstanding.

+3. Every pass calls Stripe again. Resolving the review re-files it.

+4. If the account later subscribes again, `resolveQuantityOperation` abandons, fences, is refused, and returns. `beginQuantityOperation` refuses while any operation is outstanding, so the new subscription's additions are never raised and hold for good.

+No operator verb retires an operation. The evidence document calls this "safe and noisy". It is a permanent hold with no way out.

+**Fix (before the second switch is set):**

+- If check 5 shows Stripe refuses quantity updates on a canceled subscription, treat a provider read showing the operation's subscription canceled as settled.

+- Otherwise, add an operator door to retire an operation, with a review on the record.

+### N5. P2: `FenceQuantityKey` reads 404 `resource_missing` as fenced, and check 5 would report that as verified

+**Where:** `billingstripe/client.go:712-713`.

+**Why it matters:**

+- The evidence document says the code counts a key as held only on 400 `idempotency_error`. Nothing documents or tests the 404 branch.

+- Whether a 404 saves the key is unknown.

+- A wrong-account or wrong-mode key also answers `resource_missing`. On the ended path (`:759-767`, then `closeEndedQuantityAdditions` `:234`), acknowledgement needs no provider read, so a misread fence retires an unfenced operation.

+- `TestSandboxQuantityFence` check 5 calls `FenceQuantityKey`. If Stripe 404s a canceled subscription's item, the check logs "VERIFIED: the fence … is taken" when no key was taken.

+**Fix:**

+- Return `fenced=false` on a 404 and decide the missing-item case from a provider read.

+- In check 5, use the raw request, log the status, and assert what the worker relies on.

+### N6. P2: the storage simulation generates no concurrent walk and no paused walk

+**Where:** `O/walk_sim_test.go:255-261`, `:311-331`.

+**What it cannot generate:**

+- **Concurrency.** The inner walk runs at the same simulated instant on the one bucket name. It always meets the outer walk's 20-minute lease and returns `ErrBucketBusy` before any call. Since the lease landed, the "interleaving of two console processes" is vacuous.

+- **Pauses.** Time never advances within a walk, so the fence path is never exercised.

+- **Fault kinds.** It has no store (`release.Store`) faults or CAS conflicts, and no "executed, then answered an error" faults (`FailNext` fails before the effect).

+- **Verbs and shapes.** It has no Rotate, no second bucket name, no re-declaration, and no legacy record without an interval.

+- **Weak invariants.** Owner samples two instants per hour, never inside a walk. Quiet checks that other-account users exist, not that their policy reaches the bucket.

+- **Bookkeeping weakness.** An inner grant refused as busy turns a `none` app into `broken` (`:133-135`), which switches off its Gone checks until settle. That is the same class as seed 778, on the grant side.

+The 30,000 seeds show sequential walks with provider faults, nothing more. N1 and N2 are outside the model.

+**Fix:**

+- Let the inner walk run with `walkClock` advanced past `walkLease`, then resume the outer walk on its stale context.

+- Add store, CAS and lost-response faults, and a second name.

+- Add one end-to-end Teardown that crosses `fenceWindow` mid-walk.

+**Quantity simulation:** it runs one clock for both processes, so the lease's clock-tolerance argument is never exercised. Its pauses stay under 24 hours, and it never produces a saved 500 or a 404.

+### N7. P3: a walk that "ended cleanly" can still have a retried attempt in flight

+**Where:** `O/walk.go:952-955`, `walklock.go:194-198`. The fenced clients use the SDK's default retryer (`aws.go:81`, `:92`).

+**Scenario:** an attempt times out client-side and the retry succeeds. The walk returns nil and frees the name at once. The first attempt carries the same stamp and is still acceptable for up to 15 minutes.

+**Fix:** set `RetryMaxAttempts = 1` on both fenced clients, or never free before `SignedAt + walkLease`.

+### N8. P3: a walk's store writes are not fenced

+**Where:** `O/walk.go:930`, `:940`, `:944` (`deleteRecordIfUnchanged`'s unconditional `Delete`, `:1040`).

+**Scenario:**

+1. A VM suspend leaves the monotonic deadline unexpired, so `confirm()` and the record delete still run.

+2. If the suspend falls after the re-read and the same app is re-granted after the lease ends, the delete removes a live estate's record.

+**Fix:** use a conditional (If-Match) delete, or check `walk.held()` on wall-clock time before each store mutation. State in the docs that the fence covers provider calls only.

+### N9. P3: send leases are not owned

+**Where:** `C/billingquantity.go:557-572`.

+Claim extends any live lease. Release zeroes it by key. Two processes resolving the same operation both claim it. The first to return clears the other's live lease, and a webhook (`billingservice.go:546`) then records the second sender's landing as unconfirmed, with a review. This is R13's hold again.

+**Fix:** a lease token. Claim only if the lease has expired or is your own. Release only your own.

+### N10. P3: "a key is never used twice" checks only the account's current row

+**Where:** `C/billingquantity.go:476-482`. The table keeps one row per account.

+A console whose clock is a few seconds behind can regenerate a fenced key from two operations back. Stripe answers `idempotency_error` until the 22-hour limit, so the cost is liveness only.

+**Fix:** a monotonic per-account counter in the key, or a used-keys table.

+### N11. P3: lease takeover relies on the taker's clock being within 5 minutes of AWS

+**Where:** `walklock.go:124`, `fence.go:37-41`.

+The taker's clock is never checked. **Fix:** at acquire, compare the S3 response `Date` header and refuse if the skew is over a minute.

+### N12. P3: a passing sandbox test covers `next_invoice` only

+**Where:** `billingstripe/sandbox_fence_test.go`.

+- **Behaviour covered.** Checks 1-4 use `create_prorations` on a `send_invoice` subscription. `CONSOLE_BILLING_PRORATION=immediate` (`always_invoice`, charging a card) is never exercised, including a declined payment's saved result.

+- **Check 4.** It treats "> 2 invoice items" as running twice, which depends on the proration-line count. Assert exactly one non-replayed 200 instead.

+- **Check 5.** It accepts any error from the update, including a network error, and never re-reads the quantity.

+- **Late arrival.** No check sends the late request minutes after the fence. Every check is immediate.

+A pass can establish what the gate needs for `next_invoice`. It cannot for `immediate`.

+### N13. P3: a same-app take-back clears `Destroying` without stamping `DestroyedAt`

+**Where:** `O/ownership.go:340-345`.

+Retained teardowns of other apps then wait until that app is torn down, although their objects went with the deleted bucket. Separately, a retained teardown only finishes when someone re-runs it, and nothing re-runs it. Both are holds.

+## 3. Claims the code contradicts

+**Evidence document (`2026-10-01-stripe-idempotency-evidence.md`)**

+- "The code requires status 400 with type `idempotency_error` before it counts a key as held." `client.go:712` also counts 404 `resource_missing` (N5).

+- On check 5: "safe and noisy." It wedges every later operation on the account, including under a new subscription (N4).

+- "A process stopped by a signal … sends nothing." A request already handed to the transport's write loop can still leave after the deadline. Say "starts no new request".

+- The other Stripe claims match Stripe's documentation as I know it: results saved once execution begins, validation refusals and in-flight conflicts saved nothing, 409 `idempotency_key_in_use`, keys pruned after at least 24 hours, a 500 indeterminate.

+- **Unverified and unmentioned:** whether a key's scope survives an API key rotation (an old-key late request against a new-key fence).

+**`AGENT_HANDOFF.md` item 8 and the round 6 dispositions**

+- "A retirement is decided on a provider read taken after that." The ended path acknowledges with no read (`:234`, via `:759-767`). A changed-subscription abandon retires with no read in that pass (`:1000-1001`). The cases I traced are held by the cancel-time reconcile, but the sentence is not true as written.

+- "A key is never used for two operations" (N10).

+**`AGENT_HANDOFF.md` item 5 and `walklock.go:28-36`**

+- "No provider call of a walk executes outside that walk's lease": `Rotate` and `RotateFinish` (N3), and Teardown on a re-read bucket (N1).

+- "A walk that ends cleanly had every call answered" (`walklock.go:48-49`, `walk.go:947-951`): SDK retries (N7).

+- "Finishes once that bucket has been destroyed": only when re-run, and not when the destroyer was the app's own take-back (N13).

+**`OUTSTANDING.md`**

+- **Line 62 / dispositions, "Simulation: 30,000 seeds"** sits under "one at a time across processes", and `walk_sim_test.go:38-41` describes "the interleaving of two console processes". The simulation generates neither (N6).

+- **Line 68, "accepts one thirteen minutes old, on S3 and on IAM".** `TestFenceAgainstAWS` asserts acceptance only for S3. For IAM it logs and passes either way. The refusal was observed at 16 and 20 minutes, for one list and one get, in one region.

+- **Missing launch blockers:** the ended-subscription fence decision (N4, and the evidence document's own "needs a code decision"); the sandbox test's lack of `immediate` coverage (N12); no end-to-end test of a walk crossing its fence (N6).

+**Dispositions, mutation list:** "walk calls carrying no fence" is caught only if the mutation is made inside `bound()`.

+## 4. Verdict

+**Accept with fixes.** Nothing attests, the quantity worker cannot reach Stripe with either switch unset, and billing stays off.

+- **Before the storage changes land:** N1, N2 and N3, each with a reproduction.

+- **Before `CONSOLE_BILLING_QUANTITY_FENCE` is set:** N4, N5 and N12.

+- **Before the simulation is cited as evidence of safety across processes:** N6.

diff --git a/implementation/carloku-paid-launch/AGENT_HANDOFF.md b/implementation/carloku-paid-launch/AGENT_HANDOFF.md

index d31ecaa81..84e7916b7 100644

--- a/implementation/carloku-paid-launch/AGENT_HANDOFF.md

+++ b/implementation/carloku-paid-launch/AGENT_HANDOFF.md

@@ -32,10 +32,10 @@ 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 and teardown are walks, and one walk at a time runs on a bucket name across every process: a lease object taken by compare-and-swap (`walklock.go`), bound to the invocation, standing until it runs out if the walk failed. Every request of a walk is signed with the instant its lease was taken (`fence.go`), so AWS refuses a request a paused process sends after its lease could be taken over. That was checked read-only against S3 and IAM (`TestFenceAgainstAWS`). 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), and finishes once that bucket has been destroyed. `ErrBucketRetained` has no member copy yet; it needs Paul's review. The invariants are checked by a randomized simulation (`walk_sim_test.go`; 30,000 seeds on 2026-10-01).

-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. A 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. A month seen with no statement is remembered for six hours, a closed statement for the life of the process.

+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 takes a lease on the bucket name (`walklock.go`): 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 lead to the leased name. 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), and finishes once that bucket has been destroyed. `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; an operation dated ahead of the observing clock is left alone. An operation nobody can show ran is acknowledged only after its idempotency key has been consumed at the provider with a metadata-only request (the fence), and a retirement is decided on a provider read taken after that. 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.

+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.

9. Stripe provisioning plan `stripe-provisioning.md` and print-only `stripe-provision.sh` (verified never to invoke the CLI).

10. Partial-month billing: `plans.AllowanceForMonth` returns the paid spans of a month and the allowance weighted over them. A partly covered month (a first month, a month a subscription ends in, a lapse) bills only the UTC days wholly inside a paid span, against that allowance, and records the days it counted. Completeness is still whole-month. **The whole-paid-day rule is not in the approved spec, which speaks only of prorating the allowance. It is a proposal that needs Paul's approval; the plan document states what it rounds away and the tests that pin its boundary.** A month with a paid span and no whole paid day bills nothing. A month with no paid span is refused quietly. Two subscriptions in one month price and then hold on `billing_invoice_identity_mixed`.

11. Plans: `docs/superpowers/plans/2026-10-01-store-transfer-cur-collector.md`, `2026-10-01-durability-attestation-path.md`, `2026-10-01-bucket-ownership-attestation.md`, `2026-10-01-partial-month-billing.md` (built).

@@ -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 6 (ten, eight, five, five, eleven and fourteen findings); Opus accepted with fixes five times (eight, eight, eight, nine, nine). The storage and quantity findings were fixed at the mechanism with simulations and mutation checks; the attestation findings are open against the parked branch (`docs/superpowers/specs/CODEX-DISPOSITIONS-carloku-paid-launch-overnight.md`, "Round six"). Round seven of both must pass on the exact tip before anything lands; do not read "fixed" as "accepted". The walk lease, the backend fence, the send lease and the key fence have had no independent review yet.

+2. Independent review: Astra rejected rounds 1 to 7 (ten, eight, five, five, eleven, fourteen and nine findings); Opus accepted with fixes six times (eight, eight, eight, nine, nine, thirteen). Round seven attacked the lease and the fences at their edges and every finding was accepted (`docs/superpowers/specs/CODEX-DISPOSITIONS-carloku-paid-launch-overnight.md`, "Round seven"). Round eight 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 7b71654f5..7e3dab127 100644

--- a/implementation/carloku-paid-launch/OUTSTANDING.md

+++ b/implementation/carloku-paid-launch/OUTSTANDING.md

@@ -32,7 +32,8 @@ Each is concrete. None is waived.

### Stripe

5. **No catalog, portal configuration, webhook endpoint or runtime key exists.** The plan and a print-only script are written. Needs scoped approval, then a test-mode rehearsal, then a real purchase.

-6. **The quantity worker's fence is unverified against Stripe.** It rests on documented idempotency rules, modelled by a fake. `TestSandboxQuantityFence` must pass in a Stripe sandbox before `CONSOLE_BILLING_QUANTITY_FENCE` is set. Test mode is not authorized, so it has not been run.

+6. **The quantity worker's fence is unverified against Stripe.** It rests on documented idempotency rules, modelled by a fake. `TestSandboxQuantityFence` must pass in a Stripe sandbox, under both proration policies, before `CONSOLE_BILLING_QUANTITY_FENCE` is set. Test mode is not authorized, so it has not been run. Two things it cannot settle and that stay open after it: what Stripe does with a key across an API key rotation, and how Stripe later reconciles a request it answered with a 500.

+6a. **An ended subscription's operation may need a person every time.** If Stripe refuses the fence request on a canceled subscription's item (the sandbox run logs what it answers), every canceled team subscription with an unconfirmed operation waits on a resolved review, and until then the account's quantity work is held, under a later subscription too. That needs a code decision from the sandbox log before the worker is switched on.

7. **Defect, open: an old quantity request can run after Stripe has forgotten its key.** Stripe forgets keys after 24 hours. A sender frozen longer than that can still land a request for an operation that was retired and replaced. Today that is only detected after the write (`billing_quantity_lowered`), which is not a fix. It is being designed out on `feat/carloku-late-request-fencing-20261001` (`docs/superpowers/plans/2026-10-01-late-request-fencing.md`). The path that could cause it, the quantity worker, stays disabled until the design is built, reviewed and verified in a Stripe sandbox. Nobody is asked to accept this risk.

8. **Stripe's treatment of a proration date outside the period** is assumed from documentation. Same sandbox run.

9. **Defect, open: a late invoice-line request can leave a finalized invoice with a wrong total.** Past Stripe's key memory a stale line request runs as new. If it lands between the engine's line check and the finalize, the invoice is finalized with a duplicate line. The engine flags it and will not charge it, but a finalized invoice is open at Stripe, and an open invoice can be paid by the customer from Stripe's hosted page or the portal. That is customer-payable exposure and it is treated as such. It is being designed out on the same branch (a draft whose operations are not all confirmed inside one key-retention window is deleted, never finalized, and a deleted draft refuses every late request for good). Billing stays OFF, so the path is disabled.

@@ -42,7 +43,7 @@ Each is concrete. None is waived.

10. Copy approval for both reviews, and the HALF code word.

11. The proration policy (`CONSOLE_BILLING_PRORATION` stays unset until then).

12. The whole-paid-day rule for partial months. The approved spec only says the allowance is prorated. The rule is tested at its boundaries and is a proposal.

-13. Member copy for a bucket retained by another account (`ErrBucketRetained`).

+13. Member copy for the three ways a store walk can be refused: the bucket is retained by another account (`ErrBucketRetained`), held by another store (`ErrBucketHeld`), or busy with another walk (`ErrBucketBusy`). They reach the console as plain errors today.

### Not built

@@ -54,18 +55,19 @@ Each is concrete. None is waived.

### Process

-19. **Round seven review has not passed.** Astra rejected rounds one to six. The lease, fence and simulation work since round six is unreviewed.

+19. **Round eight review has not passed.** Astra rejected rounds one to seven; Opus accepted each with fixes. The fixes to round seven's findings are unreviewed.

+19a. **A retained teardown finishes only when someone runs it again.** Nothing runs it. A store that left a bucket to another account keeps its record until then.

20. **The branch tip is not on the forge.** From 03:30 on 2026-10-01 the hub's `carlos/platform` repository answered 500 or 502 after a ten-second wait, to reads and pushes alike, while the hub's front page and other repositories answered. Bundles are in the run directory. Amadan CI has not run on the tip.

21. **A scanner backend that calls a provider must be fenced like the walks.** Today's scanner (ClamAV) makes no provider call at teardown. A native one would, and its client would need the fenced signer.

## Tested on this branch, not proven

-- Bucket-name walks: one at a time across processes, fenced at AWS by signing time. The fence's premise is proven read-only against S3 and IAM, with a list and a get. No delete was sent, and no S3-compatible backend other than AWS was checked. The walks themselves have run only against the in-memory provisioner. Simulation: 30,000 seeds.

-- Quantity worker: send lease, key fence, never-reused keys, the hold on every unverified addition. All against a fake provider. Simulation: 500 seeds.

+- Bucket-name walks (grant, teardown, rotation): a lease per bucket name across processes, the record re-read under it, every request to S3, IAM and the deployment bucket stamped with the lease instant, deletes conditional on the generation read, the name held if any request needed a second attempt. All of it has run only against the in-memory provisioner and a directory store. The simulation covers sequential walks, walks frozen past their lease while another takes the name, and failures before and after the effect. It does not cover a second bucket name, rotations, stores with no ledger interval, or clocks that disagree; those have one direct test each.

+- Quantity worker: an owned send lease, the key fence and its two kinds, keys that never recur, the hold on every unverified addition and on every operation that cannot be settled, the way out through a resolved review. All against a fake provider written from Stripe's documentation.

- Partial-month billing, statement folds, the transfer classifier (pinned against the flagship's real usage types), the invoice engine's recovery paths.

## Proven

-- AWS refuses a request whose signing time is more than fifteen minutes old, on S3 and on IAM, and accepts one thirteen minutes old (read-only calls, 2026-10-01).

+- On S3 and on IAM, AWS accepts a request stamped one minute and thirteen minutes old (the operation's own answer came back) and refuses one stamped sixteen and twenty minutes old, including with the SDK's retries on. A key that needs escaping is signed correctly by the replacement signer. Read-only calls in one region, 2026-10-01: a list, a get of an object that does not exist, a get of a user that does not exist. No delete or write was sent; that the window applies to them is an inference from where AWS checks the signature.

- The account's billing from July to September 2026, read-only: no export exists, S3 carried only usage lines, no free transfer allowance reaches the account.

-- Nothing else. No purchase, webhook, entitlement write or meter has been verified live.

+- Nothing else. No purchase, webhook, entitlement write or meter has been verified live. Nothing about Stripe is proven.

diff --git a/internal/billingstripe/client.go b/internal/billingstripe/client.go

index c239834ef..22d8696e1 100644

--- a/internal/billingstripe/client.go

+++ b/internal/billingstripe/client.go

@@ -68,6 +68,10 @@ type Error struct {

// StripeParam is Stripe's error "param" field: the request parameter

// the refusal is about. Present only for CodeAPIError.

StripeParam string

+ // Replayed reports that the provider answered from its memory of an

+ // earlier request under the same idempotency key (its

+ // Idempotent-Replayed header), and ran nothing now.

+ Replayed bool

}

// RefusedParam reports whether Stripe refused the request outright as

@@ -219,6 +223,7 @@ func (c *Client) do(ctx context.Context, method, path string, form url.Values, i

StripeType: safeErrorEnum(env.Error.Type),

StripeCode: safeErrorEnum(env.Error.Code),

StripeParam: safeErrorParam(env.Error.Param),

+ Replayed: resp.Header.Get("Idempotent-Replayed") == "true",

}

@@ -666,33 +671,48 @@ func (c *Client) UpdateQuantityWith(ctx context.Context, itemID string, quantity

return &item, nil

}

-// FenceQuantityKey makes the provider refuse any later quantity update sent

-// under idempotencyKey, without changing anything a customer is billed for.

+// What a fence request found (FenceQuantityKey).

+const (

+ // FenceTaken: the fence request ran. The key now belongs to it, so the

+ // operation's own request never ran under this key, and cannot while

+ // the provider remembers the key.

+ FenceTaken = "fence"

+ // FenceHeldByRequest: the provider refused the fence request because

+ // the key already belongs to a request with other parameters, which is

+ // the operation's own. That request began executing. It says nothing

+ // about how it ended: it may have succeeded, failed for good, or ended

+ // in an error the provider itself calls indeterminate (a 500, which it

+ // may still reconcile). A second copy of it runs nothing.

+ FenceHeldByRequest = "request"

+// FenceQuantityKey tries to make the provider refuse any later quantity

+// update sent under idempotencyKey, without changing anything a customer is

+// billed for.

// The provider remembers an idempotency key with the parameters it was

// first used with, and refuses the key with any other parameters. This

-// sends a metadata-only update of the item under the key. One of three

-// things is then true, and each means a quantity update still on its way

-// under that key can no longer take effect:

-//

-// - the update ran (2xx): the key now belongs to this harmless request,

-// and the quantity update will be refused as a parameter mismatch;

-// - the provider refused it as a parameter mismatch (idempotency_error):

-// the key already belongs to the quantity update, which has run, and a

-// second copy of it is answered from memory and runs nothing;

-// - the item no longer exists (resource_missing): nothing can update it.

+// sends a metadata-only update of the item under the key. It returns

+// FenceTaken or FenceHeldByRequest (above), or "" with no error when a

+// request under the key is executing at this moment (a 409); the caller

+// asks again later.

-// fenced is false with no error when a request under the key is executing

-// at this moment (idempotency_key_in_use); the caller asks again later.

+// Every other answer is an error and means nothing is known to hold the

+// key. That includes a missing item: a 404 saves no result, and the same

+// answer comes back for an item id that belongs to another account or

+// mode.

-// The provider's memory of a key is not permanent (Stripe documents at

-// least 24 hours). The fence holds for that long from this call.

-func (c *Client) FenceQuantityKey(ctx context.Context, itemID string, generation int64, idempotencyKey string) (fenced bool, err error) {

+// The provider's memory of a key is not permanent. Stripe documents that a

+// key may be removed once it is at least 24 hours old, counted from the

+// request that first used it. FenceTaken therefore holds for 24 hours from

+// this call, and FenceHeldByRequest only until 24 hours after the

+// operation's own request, which may be nearly over.

+func (c *Client) FenceQuantityKey(ctx context.Context, itemID string, generation int64, idempotencyKey string) (kind string, err error) {

if !validID(itemID) {

- return false, &Error{Code: CodeInvalidID}

+ return "", &Error{Code: CodeInvalidID}

}

if generation < 0 {

- return false, &Error{Code: CodeInvalidParams}

+ return "", &Error{Code: CodeInvalidParams}

}

form := url.Values{}

form.Set("metadata[carloku_quantity_fence]", strconv.FormatInt(generation, 10))

@@ -700,22 +720,20 @@ func (c *Client) FenceQuantityKey(ctx context.Context, itemID string, generation

err = c.do(ctx, http.MethodPost, "/v1/subscription_items/"+itemID, form, idempotencyKey, &item)

if err == nil {

if item.ID != itemID {

- return false, &Error{Code: CodeDecodeError}

+ return "", &Error{Code: CodeDecodeError}

}

- return true, nil

+ return FenceTaken, nil

}

var refused *Error

if errors.As(err, &refused) && refused.Code == CodeAPIError {

switch {

case refused.HTTPStatus == http.StatusBadRequest && refused.StripeType == "idempotency_error":

- return true, nil

- case refused.HTTPStatus == http.StatusNotFound && refused.StripeCode == "resource_missing":

- return true, nil

+ return FenceHeldByRequest, nil

case refused.HTTPStatus == http.StatusConflict:

- return false, nil

+ return "", nil

}

- return false, err

+ return "", err

}

type subscriptionListResponse struct {

diff --git a/internal/billingstripe/client_test.go b/internal/billingstripe/client_test.go

index c414e13ca..234934023 100644

--- a/internal/billingstripe/client_test.go

+++ b/internal/billingstripe/client_test.go

@@ -881,25 +881,25 @@ func TestDo_PinnedAPIVersionOnEveryRequest(t *testing.T) {

}

// The fence request carries the operation's key and nothing that is billed:

-// no quantity, no proration. What the provider answers decides whether the

-// key is held.

+// no quantity, no proration. What the provider answers decides what is

+// known about the key, and only two answers say anything holds it.

func TestFenceQuantityKey(t *testing.T) {

cases := []struct {

- name string

- status int

- body string

- wantFenced bool

- wantErr bool

+ name string

+ status int

+ body string

+ wantKind string

+ wantErr bool

}{

- {"the update ran: the key is now this request's", 200, `{"id":"si_1","quantity":10,"price":{"id":"price_1"}}`, true, false},

- {"refused as a parameter mismatch: the key is the quantity update's", 400, `{"error":{"type":"idempotency_error","message":"Keys for idempotent requests can only be used with the same parameters they were first used with."}}`, true, false},

- {"the item is gone: nothing can update it", 404, `{"error":{"type":"invalid_request_error","code":"resource_missing","message":"No such subscription item"}}`, true, false},

- {"a request under the key is executing: ask again", 409, `{"error":{"type":"idempotency_error","code":"idempotency_key_in_use"}}`, false, false},

- {"a server error holds nothing", 500, `{"error":{"type":"api_error"}}`, false, true},

- {"another refusal holds nothing", 400, `{"error":{"type":"invalid_request_error","code":"parameter_missing"}}`, false, true},

- {"a 404 that is not a missing item holds nothing", 404, `{"error":{"type":"invalid_request_error"}}`, false, true},

- {"rate limited holds nothing", 429, `{"error":{"type":"rate_limit_error"}}`, false, true},

- {"an answer for another item holds nothing", 200, `{"id":"si_other","quantity":10,"price":{"id":"price_1"}}`, false, true},

+ {"the update ran: the key is now this request's", 200, `{"id":"si_1","quantity":10,"price":{"id":"price_1"}}`, FenceTaken, false},

+ {"refused as a parameter mismatch: the key is the quantity update's", 400, `{"error":{"type":"idempotency_error","message":"Keys for idempotent requests can only be used with the same parameters they were first used with."}}`, FenceHeldByRequest, false},

+ {"a request under the key is executing: ask again", 409, `{"error":{"type":"idempotency_error","code":"idempotency_key_in_use"}}`, "", false},

+ {"a missing item holds nothing: a 404 saves no result", 404, `{"error":{"type":"invalid_request_error","code":"resource_missing","message":"No such subscription item"}}`, "", true},

+ {"a server error holds nothing", 500, `{"error":{"type":"api_error"}}`, "", true},

+ {"another refusal holds nothing", 400, `{"error":{"type":"invalid_request_error","code":"parameter_missing"}}`, "", true},

+ {"a mismatch answered with another status holds nothing", 402, `{"error":{"type":"idempotency_error"}}`, "", true},

+ {"rate limited holds nothing", 429, `{"error":{"type":"rate_limit_error"}}`, "", true},

+ {"an answer for another item holds nothing", 200, `{"id":"si_other","quantity":10,"price":{"id":"price_1"}}`, "", true},

}

for _, tc := range cases {

t.Run(tc.name, func(t *testing.T) {

@@ -912,9 +912,9 @@ func TestFenceQuantityKey(t *testing.T) {

_, _ = w.Write([]byte(tc.body))

}))

defer srv.Close()

- fenced, err := testClient(t, srv.URL).FenceQuantityKey(context.Background(), "si_1", 7, "op-key")

- if fenced != tc.wantFenced || (err != nil) != tc.wantErr {

- t.Fatalf("fenced=%v err=%v, want fenced=%v err=%v", fenced, err, tc.wantFenced, tc.wantErr)

+ kind, err := testClient(t, srv.URL).FenceQuantityKey(context.Background(), "si_1", 7, "op-key")

+ if kind != tc.wantKind || (err != nil) != tc.wantErr {

+ t.Fatalf("kind=%q err=%v, want kind=%q err=%v", kind, err, tc.wantKind, tc.wantErr)

}

if path != "/v1/subscription_items/si_1" || key != "op-key" {

t.Fatalf("sent to %q under key %q", path, key)

diff --git a/internal/billingstripe/sandbox_fence_test.go b/internal/billingstripe/sandbox_fence_test.go

index 522823452..4858432a8 100644

--- a/internal/billingstripe/sandbox_fence_test.go

+++ b/internal/billingstripe/sandbox_fence_test.go

@@ -103,190 +103,274 @@ func TestSandboxQuantityFence(t *testing.T) {

return s

}

run := strconv.FormatInt(time.Now().UnixNano(), 36)

- k := func(name string) string { return "carloku-fence-check-" + run + "-" + name }

- // Setup: one annual subscription, quantity 10, invoiced (no card).

- customer := raw("POST", "/v1/customers", url.Values{"name": {"carloku fence check " + run}, "metadata[carloku_fence_check]": {run}}, k("customer"))

- customerID := id(customer, "customer")

- if live, _ := customer.body["livemode"].(bool); live {

- t.Fatal("the key is live-mode; refusing to continue")

+ // late is how long the "late" request waits after the fence before it

+ // is sent. Every other check sends at once; this one shows the fence

+ // holding minutes later, which is the case it exists for.

+ late := 90 * time.Second

+ if d, err := time.ParseDuration(os.Getenv("CARLOS_STRIPE_SANDBOX_LATE")); err == nil && d >= 0 {

+ late = d

}

- t.Cleanup(func() { raw("DELETE", "/v1/customers/"+customerID, nil, "") })

- productID := id(raw("POST", "/v1/products", url.Values{"name": {"carloku fence check " + run}}, k("product")), "product")

- t.Cleanup(func() { raw("POST", "/v1/products/"+productID, url.Values{"active": {"false"}}, k("product-off")) })

- priceID := id(raw("POST", "/v1/prices", url.Values{"product": {productID}, "currency": {"eur"}, "unit_amount": {"1000"}, "recurring[interval]": {"year"}}, k("price")), "price")

- subID := id(raw("POST", "/v1/subscriptions", url.Values{"customer": {customerID}, "items[0][price]": {priceID}, "items[0][quantity]": {"10"},

- "collection_method": {"send_invoice"}, "days_until_due": {"30"}}, k("subscription")), "subscription")

- canceled := false

+ productID := id(raw("POST", "/v1/products", url.Values{"name": {"carloku fence check " + run}}, "carloku-fence-check-"+run+"-product"), "product")

t.Cleanup(func() {

- if !canceled {

- raw("DELETE", "/v1/subscriptions/"+subID, nil, "")

- }

+ raw("POST", "/v1/products/"+productID, url.Values{"active": {"false"}}, "carloku-fence-check-"+run+"-product-off")

})

- sub, err := c.GetSubscription(ctx, subID)

- if err != nil || len(sub.Items) != 1 || sub.Items[0].Quantity != 10 {

- t.Fatalf("setup: read the subscription: %v", err)

- }

- item := sub.Items[0].ID

- at := sub.CurrentPeriodStart.Add(time.Minute)

- quantity := func() int64 {

- t.Helper()

- s, err := c.GetSubscription(ctx, subID)

- if err != nil || len(s.Items) != 1 {

- t.Fatalf("read the subscription: %v", err)

- }

- return s.Items[0].Quantity

- }

- count := func(path string) int {

- t.Helper()

- a := raw("GET", path, url.Values{"customer": {customerID}, "limit": {"100"}}, "")

- data, _ := a.body["data"].([]any)

- if a.status != 200 {

- t.Fatalf("list %s: %s", path, errOf(a))

- }

- return len(data)

- }

- update := func(target int64, date time.Time, idem string) error {

- _, err := c.UpdateQuantityWith(ctx, item, target, ProrationCreateProrations, date, idem)

- return err

- }

- fenceForm := url.Values{"metadata[carloku_quantity_fence]": {"1"}}

- t.Run("1 a fence taken first refuses the quantity update that arrives after it", func(t *testing.T) {

- items, invoices := count("/v1/invoiceitems"), count("/v1/invoices")

- a := raw("POST", "/v1/subscription_items/"+item, fenceForm, k("one"))

- if a.status != 200 {

- t.Fatalf("the fence request was not accepted: %s", errOf(a))

- }

- if q := quantity(); q != 10 {

- t.Fatalf("the fence request changed the quantity to %d", q)

- }

- if i, v := count("/v1/invoiceitems"), count("/v1/invoices"); i != items || v != invoices {

- t.Fatalf("the fence request created billing objects: invoice items %d -> %d, invoices %d -> %d", items, i, invoices, v)

- }

- err := update(11, at, k("one"))

- var refused *Error

- if !errors.As(err, &refused) || refused.HTTPStatus != 400 || refused.StripeType != "idempotency_error" {

- t.Fatalf("the late quantity update under the fenced key: %v, want a 400 idempotency_error", err)

- }

- if q := quantity(); q != 10 {

- t.Fatalf("the late quantity update ran: quantity %d", q)

- }

- if fenced, err := c.FenceQuantityKey(ctx, item, 1, k("one")); err != nil || !fenced {

- t.Fatalf("FenceQuantityKey on a key it already holds: fenced=%v err=%v", fenced, err)

- }

- t.Logf("VERIFIED: fence 200, quantity and billing objects unchanged; late update refused with 400 idempotency_error")

+ priceID := id(raw("POST", "/v1/prices", url.Values{"product": {productID}, "currency": {"eur"}, "unit_amount": {"1000"}, "recurring[interval]": {"year"}}, "carloku-fence-check-"+run+"-price"), "price")

+ t.Cleanup(func() {

+ raw("POST", "/v1/prices/"+priceID, url.Values{"active": {"false"}}, "carloku-fence-check-"+run+"-price-off")

})

- t.Run("2 a fence taken after the update ran finds the key taken, and a second copy of the update runs nothing", func(t *testing.T) {

- if err := update(11, at, k("two")); err != nil {

- t.Fatalf("the quantity update: %v", err)

- }

- items := count("/v1/invoiceitems")

- form := url.Values{"quantity": {"11"}, "proration_behavior": {ProrationCreateProrations}, "proration_date": {strconv.FormatInt(at.Unix(), 10)}}

- again := raw("POST", "/v1/subscription_items/"+item, form, k("two"))

- if again.status != 200 || !again.replayed {

- t.Fatalf("the second copy of the update: %s replayed=%v, want 200 answered from memory (Idempotent-Replayed: true)", errOf(again), again.replayed)

- }

- if i := count("/v1/invoiceitems"); i != items {

- t.Fatalf("the second copy of the update ran: invoice items %d -> %d", items, i)

- }

- a := raw("POST", "/v1/subscription_items/"+item, fenceForm, k("two"))

- if e, _ := a.body["error"].(map[string]any); a.status != 400 || e["type"] != "idempotency_error" {

- t.Fatalf("the fence under a key the update holds: %s, want 400 idempotency_error", errOf(a))

- }

- if fenced, err := c.FenceQuantityKey(ctx, item, 2, k("two")); err != nil || !fenced {

- t.Fatalf("FenceQuantityKey under a key the update holds: fenced=%v err=%v", fenced, err)

- }

- if q := quantity(); q != 11 {

- t.Fatalf("quantity %d, want 11", q)

- }

- t.Logf("VERIFIED: replay answered from memory with no new proration; fence under the update's key refused with 400 idempotency_error")

- })

+ // Both policies the worker can run under. They differ in what a

+ // quantity update does at the provider (an invoice and a card charge

+ // now, or a proration on the next invoice), so a pass under one says

+ // nothing about the other.

+ for _, mode := range []struct{ name, behaviour string }{

+ {"next_invoice", ProrationCreateProrations},

+ {"immediate", ProrationAlwaysInvoice},

+ } {

+ t.Run(mode.name, func(t *testing.T) {

+ k := func(name string) string { return "carloku-fence-check-" + run + "-" + mode.name + "-" + name }

+ customer := raw("POST", "/v1/customers", url.Values{"name": {"carloku fence check " + run + " " + mode.name}, "metadata[carloku_fence_check]": {run}}, k("customer"))

+ customerID := id(customer, "customer")

+ if live, _ := customer.body["livemode"].(bool); live {

+ t.Fatal("the key is live-mode; refusing to continue")

+ }

+ t.Cleanup(func() { raw("DELETE", "/v1/customers/"+customerID, nil, "") })

+ subForm := url.Values{"customer": {customerID}, "items[0][price]": {priceID}, "items[0][quantity]": {"10"}}

+ if mode.behaviour == ProrationAlwaysInvoice {

+ // A test card, so an immediate proration is charged.

+ pm := id(raw("POST", "/v1/payment_methods/pm_card_visa/attach", url.Values{"customer": {customerID}}, k("card")), "payment method")

+ subForm.Set("default_payment_method", pm)

+ } else {

+ subForm.Set("collection_method", "send_invoice")

+ subForm.Set("days_until_due", "30")

+ }

+ subID := id(raw("POST", "/v1/subscriptions", subForm, k("subscription")), "subscription")

+ canceled := false

+ t.Cleanup(func() {

+ if !canceled {

+ raw("DELETE", "/v1/subscriptions/"+subID, nil, "")

+ }

+ })

+ sub, err := c.GetSubscription(ctx, subID)

+ if err != nil || len(sub.Items) != 1 || sub.Items[0].Quantity != 10 {

+ t.Fatalf("setup: read the subscription: %v", err)

+ }

+ item := sub.Items[0].ID

+ at := sub.CurrentPeriodStart.Add(time.Minute)

+ quantity := func() int64 {

+ t.Helper()

+ s, err := c.GetSubscription(ctx, subID)

+ if err != nil || len(s.Items) != 1 {

+ t.Fatalf("read the subscription: %v", err)

+ }

+ return s.Items[0].Quantity

+ }

+ count := func(path string) int {

+ t.Helper()

+ a := raw("GET", path, url.Values{"customer": {customerID}, "limit": {"100"}}, "")

+ data, _ := a.body["data"].([]any)

+ if a.status != 200 {

+ t.Fatalf("list %s: %s", path, errOf(a))

+ }

+ return len(data)

+ }

+ billed := func() [2]int { return [2]int{count("/v1/invoiceitems"), count("/v1/invoices")} }

+ updateForm := func(target int64, date time.Time) url.Values {

+ return url.Values{"quantity": {strconv.FormatInt(target, 10)}, "proration_behavior": {mode.behaviour}, "proration_date": {strconv.FormatInt(date.Unix(), 10)}}

+ }

+ update := func(target int64, date time.Time, idem string) error {

+ _, err := c.UpdateQuantityWith(ctx, item, target, mode.behaviour, date, idem)

+ return err

+ }

+ mismatch := func(a answer) bool {

+ e, _ := a.body["error"].(map[string]any)

+ return a.status == 400 && e["type"] == "idempotency_error"

+ }

+ fenceForm := url.Values{"metadata[carloku_quantity_fence]": {"1"}}

+ itemPath := "/v1/subscription_items/" + item

- t.Run("3 a request refused as invalid does not take its key", func(t *testing.T) {

- err := update(12, sub.CurrentPeriodStart.Add(-48*time.Hour), k("three"))

- var refused *Error

- if !errors.As(err, &refused) || !refused.RefusedParam("proration_date") {

- t.Fatalf("an update dated before the period: %v, want a 400 invalid_request_error naming proration_date", err)

- }

- if q := quantity(); q != 11 {

- t.Fatalf("the refused update ran: quantity %d", q)

- }

- a := raw("POST", "/v1/subscription_items/"+item, fenceForm, k("three"))

- if a.status != 200 || a.replayed {

- t.Fatalf("the fence under the refused request's key: %s replayed=%v, want a fresh 200 (the refusal saved nothing)", errOf(a), a.replayed)

- }

- err = update(12, at, k("three"))

- if !errors.As(err, &refused) || refused.HTTPStatus != 400 || refused.StripeType != "idempotency_error" {

- t.Fatalf("the update under the now fenced key: %v, want a 400 idempotency_error", err)

- }

- if q := quantity(); q != 11 {

- t.Fatalf("quantity %d, want 11", q)

- }

- t.Logf("VERIFIED: a proration date outside the period is refused naming proration_date and saves no key; the fence then takes the key")

- })

+ t.Run("1 a fence taken first refuses the quantity update that arrives after it, minutes later", func(t *testing.T) {

+ before := billed()

+ a := raw("POST", itemPath, fenceForm, k("one"))

+ if a.status != 200 || a.replayed {

+ t.Fatalf("the fence request was not run: %s replayed=%v", errOf(a), a.replayed)

+ }

+ if q := quantity(); q != 10 {

+ t.Fatalf("the fence request changed the quantity to %d", q)

+ }

+ if after := billed(); after != before {

+ t.Fatalf("the fence request created billing objects: invoice items and invoices %v -> %v", before, after)

+ }

+ t.Logf("waiting %v before the late update", late)

+ time.Sleep(late)

+ err := update(11, at, k("one"))

+ var refused *Error

+ if !errors.As(err, &refused) || refused.HTTPStatus != 400 || refused.StripeType != "idempotency_error" {

+ t.Fatalf("the late quantity update under the fenced key: %v, want a 400 idempotency_error", err)

+ }

+ if q := quantity(); q != 10 {

+ t.Fatalf("the late quantity update ran: quantity %d", q)

+ }

+ if after := billed(); after != before {

+ t.Fatalf("the late quantity update billed something: %v -> %v", before, after)

+ }

+ if kind, err := c.FenceQuantityKey(ctx, item, 1, k("one")); err != nil || kind != FenceTaken {

+ t.Fatalf("FenceQuantityKey on a key the fence already holds: kind=%q err=%v, want %q", kind, err, FenceTaken)

+ }

+ t.Logf("VERIFIED: fence 200, quantity and billing objects unchanged; update %v later refused with 400 idempotency_error", late)

+ })

- t.Run("4 concurrent copies of one request run it once", func(t *testing.T) {

- const copies = 6

- items := count("/v1/invoiceitems")

- form := url.Values{"quantity": {"12"}, "proration_behavior": {ProrationCreateProrations}, "proration_date": {strconv.FormatInt(at.Unix(), 10)}}

- answers := make([]answer, copies)

- var wg sync.WaitGroup

- for i := range answers {

- wg.Add(1)

- go func() {

- defer wg.Done()

- answers[i] = raw("POST", "/v1/subscription_items/"+item, form, k("four"))

- }()

- }

- wg.Wait()

- ok, conflict := 0, 0

- for _, a := range answers {

- switch a.status {

- case 200:

- ok++

- case 409:

- conflict++

- default:

- t.Fatalf("a concurrent copy was answered %s, want 200 or 409", errOf(a))

- }

- if ok == 0 || quantity() != 12 {

- t.Fatalf("no copy ran: %d ok, %d conflict", ok, conflict)

- }

- if i := count("/v1/invoiceitems"); i-items > 2 {

- t.Fatalf("the update ran more than once: invoice items %d -> %d", items, i)

- }

- if conflict == 0 {

- t.Logf("NOT OBSERVED: no copy was answered 409 in this run (%d answered 200); the in-use answer is documented, and FenceQuantityKey treats 409 as not fenced", ok)

- } else {

- t.Logf("VERIFIED: %d copies answered 200, %d answered 409; the update ran once", ok, conflict)

- }

- })

+ t.Run("2 a fence taken after the update ran finds the key taken, and a second copy of the update runs nothing", func(t *testing.T) {

+ if err := update(11, at, k("two")); err != nil {

+ t.Fatalf("the quantity update: %v", err)

+ }

+ before := billed()

+ again := raw("POST", itemPath, updateForm(11, at), k("two"))

+ if again.status != 200 || !again.replayed {

+ t.Fatalf("the second copy of the update: %s replayed=%v, want 200 answered from memory (Idempotent-Replayed: true)", errOf(again), again.replayed)

+ }

+ if after := billed(); after != before {

+ t.Fatalf("the second copy of the update ran: %v -> %v", before, after)

+ }

+ if a := raw("POST", itemPath, fenceForm, k("two")); !mismatch(a) {

+ t.Fatalf("the fence under a key the update holds: %s, want 400 idempotency_error", errOf(a))

+ }

+ if kind, err := c.FenceQuantityKey(ctx, item, 2, k("two")); err != nil || kind != FenceHeldByRequest {

+ t.Fatalf("FenceQuantityKey under a key the update holds: kind=%q err=%v, want %q", kind, err, FenceHeldByRequest)

+ }

+ if q := quantity(); q != 11 {

+ t.Fatalf("quantity %d, want 11", q)

+ }

+ t.Logf("VERIFIED: replay answered from memory and billed nothing; fence under the update's key refused with 400 idempotency_error")

+ })

- t.Run("5 a canceled subscription", func(t *testing.T) {

- if a := raw("DELETE", "/v1/subscriptions/"+subID, nil, ""); a.status != 200 {

- t.Fatalf("cancel: %s", errOf(a))

- }

- canceled = true

- err := update(13, at, k("five-update"))

- if err == nil {

- t.Fatal("a quantity update on a canceled subscription's item was accepted")

- }

- t.Logf("VERIFIED: a quantity update on a canceled subscription's item is refused (%v)", err)

- fenced, ferr := c.FenceQuantityKey(ctx, item, 5, k("five-fence"))

- switch {

- case ferr == nil && fenced:

- t.Logf("VERIFIED: the fence on a canceled subscription's item is taken")

- default:

- // The worker would file billing_quantity_fence_refused and

- // hold the account here. That is safe and it is not what

- // anyone wants for every canceled subscription: it needs a

- // code decision before the worker is switched on.

- t.Fatalf("the fence on a canceled subscription's item: fenced=%v err=%v; the worker's ended-subscription path needs a decision", fenced, ferr)

- }

- })

+ t.Run("3 a request refused as invalid does not take its key", func(t *testing.T) {

+ err := update(12, sub.CurrentPeriodStart.Add(-48*time.Hour), k("three"))

+ var refused *Error

+ if !errors.As(err, &refused) || !refused.RefusedParam("proration_date") {

+ t.Fatalf("an update dated before the period: %v, want a 400 invalid_request_error naming proration_date", err)

+ }

+ if q := quantity(); q != 11 {

+ t.Fatalf("the refused update ran: quantity %d", q)

+ }

+ a := raw("POST", itemPath, fenceForm, k("three"))

+ if a.status != 200 || a.replayed {

+ t.Fatalf("the fence under the refused request's key: %s replayed=%v, want a fresh 200 (the refusal saved nothing)", errOf(a), a.replayed)

+ }

+ err = update(12, at, k("three"))

+ if !errors.As(err, &refused) || refused.HTTPStatus != 400 || refused.StripeType != "idempotency_error" {

+ t.Fatalf("the update under the now fenced key: %v, want a 400 idempotency_error", err)

+ }

+ if q := quantity(); q != 11 {

+ t.Fatalf("quantity %d, want 11", q)

+ }

+ t.Logf("VERIFIED: a proration date outside the period is refused naming proration_date and saves no key; the fence then takes the key")

+ })

+ t.Run("4 concurrent copies of one request run it exactly once", func(t *testing.T) {

+ const copies = 6

+ answers := make([]answer, copies)

+ var wg sync.WaitGroup

+ for i := range answers {

+ wg.Add(1)

+ go func() {

+ defer wg.Done()

+ answers[i] = raw("POST", itemPath, updateForm(12, at), k("four"))

+ }()

+ }

+ wg.Wait()

+ ran, replayed, conflict := 0, 0, 0

+ for _, a := range answers {

+ switch {

+ case a.status == 200 && !a.replayed:

+ ran++

+ case a.status == 200:

+ replayed++

+ case a.status == 409:

+ conflict++

+ default:

+ t.Fatalf("a concurrent copy was answered %s, want 200 or 409", errOf(a))

+ }

+ // The evidence that it ran once is the provider's own: one

+ // answer that was not a replay. A final quantity proves

+ // nothing, since running an absolute update six times leaves

+ // the same number.

+ if ran != 1 {

+ t.Fatalf("%d copies were run (answered 200 and not from memory), %d replayed, %d conflicted: want exactly one run", ran, replayed, conflict)

+ }

+ if q := quantity(); q != 12 {

+ t.Fatalf("quantity %d, want 12", q)

+ }

+ t.Logf("VERIFIED: 1 copy ran, %d were answered from memory, %d were refused with 409", replayed, conflict)

+ })

+ t.Run("5 a fence and an update racing for one key: exactly one takes it", func(t *testing.T) {

+ fenceWon, updateWon := 0, 0

+ for round := 0; round < 6; round++ {

+ key := k("five-" + strconv.Itoa(round))

+ target := int64(13 + round)

+ startQ := quantity()

+ var f, u answer

+ var wg sync.WaitGroup

+ wg.Add(2)

+ go func() { defer wg.Done(); f = raw("POST", itemPath, fenceForm, key) }()

+ go func() { defer wg.Done(); u = raw("POST", itemPath, updateForm(target, at), key) }()

+ wg.Wait()

+ refusedOrBusy := func(a answer) bool { return mismatch(a) || a.status == 409 }

+ endQ := quantity()

+ switch {

+ case f.status == 200 && refusedOrBusy(u) && endQ == startQ:

+ fenceWon++

+ case u.status == 200 && refusedOrBusy(f) && endQ == target:

+ updateWon++

+ default:

+ t.Fatalf("round %d: fence answered %s, update answered %s, quantity %d -> %d: both took the key, or neither did", round, errOf(f), errOf(u), startQ, endQ)

+ }

+ // Whatever a 409 refused must still be refused or

+ // replayed afterwards, never run.

+ if u.status == 409 {

+ if a := raw("POST", itemPath, updateForm(target, at), key); !mismatch(a) {

+ t.Fatalf("round %d: the update, retried after its 409 against a fence that won: %s, want 400 idempotency_error", round, errOf(a))

+ }

+ if q := quantity(); q != startQ {

+ t.Fatalf("round %d: the update ran on its retry: quantity %d", round, q)

+ }

+ t.Logf("VERIFIED over 6 races: the fence took the key %d times, the update %d times, never both", fenceWon, updateWon)

+ if fenceWon == 0 || updateWon == 0 {

+ t.Logf("NOT OBSERVED: one side never won a race in this run, so only one ordering was exercised")

+ }

+ })

+ t.Run("6 a canceled subscription", func(t *testing.T) {

+ before := quantity()

+ if a := raw("DELETE", "/v1/subscriptions/"+subID, nil, ""); a.status != 200 {

+ t.Fatalf("cancel: %s", errOf(a))

+ }

+ canceled = true

+ u := raw("POST", itemPath, updateForm(before+1, at), k("six-update"))

+ if u.status < 400 || u.status >= 500 {

+ t.Fatalf("a quantity update on a canceled subscription's item: %s, want a definite refusal", errOf(u))

+ }

+ t.Logf("OBSERVED: a quantity update on a canceled subscription's item is answered %s", errOf(u))

+ f := raw("POST", itemPath, fenceForm, k("six-fence"))

+ t.Logf("OBSERVED: a fence request on a canceled subscription's item is answered %s", errOf(f))

+ kind, ferr := c.FenceQuantityKey(ctx, item, 6, k("six-fence-2"))

+ t.Logf("OBSERVED: FenceQuantityKey on it returns kind=%q err=%v", kind, ferr)

+ // Nothing here is a pass or a fail of the fence. It is the

+ // fact the worker's ended-subscription path needs: if the

+ // fence cannot be taken there, every canceled team

+ // subscription with an unconfirmed operation files a review

+ // for a person, and whether that is acceptable is decided

+ // from this log before the worker is switched on.

+ if f.status == 200 && kind != FenceTaken {

+ t.Fatalf("the raw fence was accepted and FenceQuantityKey says %q", kind)

+ }

+ })

+ }

- t.Log("NOT VERIFIABLE HERE: behaviour after the provider has forgotten a key (documented: at least 24 hours, then a reused key runs as a new request). The fence is bounded by that, by design.")

+ t.Log("NOT VERIFIABLE HERE: behaviour after the provider has forgotten a key (documented: at least 24 hours, then a reused key runs as a new request), and the provider's later reconciliation of a request it answered with a 500. The fence is bounded by the first, and an operation whose key is held by its own request is never retired without a person because of the second.")

}

diff --git a/internal/console/billingquantity.go b/internal/console/billingquantity.go

index 4a8cbeb19..09f3f2f91 100644

--- a/internal/console/billingquantity.go

+++ b/internal/console/billingquantity.go

@@ -2,7 +2,9 @@ package console

import (

"context"

+ "crypto/rand"

"database/sql"

+ "encoding/hex"

"errors"

"fmt"

"net/http"

@@ -85,15 +87,26 @@ const (

// Filed when additions from a period that has since renewed are closed

// without a charge: a person should know seats went unbilled.

billingQuantityUnchargedReason = "billing_quantity_uncharged_period"

- // Filed when the provider refuses the request that fences an

- // operation's key. The operation cannot be acknowledged without the

- // fence, so the account's quantity work waits on a person.

- billingQuantityFenceReason = "billing_quantity_fence_refused"

+ // fenceKindReviewed: a person resolved the review of an operation the

+ // worker could not settle. It is recorded on the operation as its

+ // fence, and lets the operation be retired.

+ fenceKindReviewed = "reviewed"

// Filed when the provider's quantity is observed to have gone down

// inside a period. The console never asks for that.

billingQuantityLoweredReason = "billing_quantity_lowered"

)

+// billingQuantityUnsettledReason is the review of an operation the worker

+// cannot settle: its key could not be fenced (the provider refused the

+// fence request), or the key is held by the operation's own request and

+// the provider shows that request's effect has not landed, which is what

+// an indeterminate provider error looks like. The operation stays

+// outstanding and nothing replaces it. Resolving the review is the way out:

+// the worker then retires the operation.

+func billingQuantityUnsettledReason(created int64) string {

+ return "billing_quantity_operation_unsettled:" + strconv.FormatInt(created, 10)

// prorationBehavior maps the configured policy to the provider's behaviour

// and to the outcome recorded on the events. ok is false when no policy is

// configured.

@@ -229,9 +242,9 @@ func (s *Store) closeEndedQuantityAdditions(accountID int64, now time.Time) (ove

if over {

// Not while a sender holds its lease: it is acknowledged on a later

// pass, once no request for it can still be on its way.

- // Nor before its key is fenced, unless the provider confirmed it:

- // processQuantityAdditions fences it, and the next pass lands here.

- if _, err := tx.Exec(`UPDATE billing_quantity_operations SET acked_at=? WHERE account_id=? AND acked_at=0 AND send_lease_until<=? AND `+quantityOperationSettled, max(now.Unix(), 1), accountID, now.Unix()); err != nil {

+ // Nor before it may be retired (quantityOperationRetirable):

+ // processQuantityAdditions settles it, and a later pass lands here.

+ if _, err := tx.Exec(`UPDATE billing_quantity_operations SET acked_at=? WHERE account_id=? AND acked_at=0 AND send_lease_until<=? AND `+quantityOperationRetirable, max(now.Unix(), 1), accountID, now.Unix()); err != nil {

return false, err

}

@@ -378,6 +391,13 @@ type quantityOperation struct {

// quantity from now on (fenceQuantityOperation). An operation nobody

// can show ran is acknowledged only when it is set.

FencedAt int64

+ // FenceKind says what holds the key: the fence request

+ // (billingstripe.FenceTaken), the operation's own request

+ // (FenceHeldByRequest), or a person's review (fenceKindReviewed).

+ FenceKind string

+ // Serial counts the account's operations. It is part of the key, so no

+ // two operations of an account ever share one.

+ Serial int64

// Revision is billing_accounts.revision as read when the target was

// computed: any change to the billing row since (plan, band, terms,

// status, period, quantity) is a different world.

@@ -419,14 +439,23 @@ func (s *Store) migrateBillingQuantityOperations() error {

if err := s.addColumnIfMissing("billing_quantity_operations", "send_lease_until", `INTEGER NOT NULL DEFAULT 0`); err != nil {

return err

}

- return s.addColumnIfMissing("billing_quantity_operations", "fenced_at", `INTEGER NOT NULL DEFAULT 0`)

+ if err := s.addColumnIfMissing("billing_quantity_operations", "fenced_at", `INTEGER NOT NULL DEFAULT 0`); err != nil {

+ return err

+ }

+ if err := s.addColumnIfMissing("billing_quantity_operations", "fence_kind", `TEXT NOT NULL DEFAULT ''`); err != nil {

+ return err

+ }

+ if err := s.addColumnIfMissing("billing_quantity_operations", "send_lease_token", `TEXT NOT NULL DEFAULT ''`); err != nil {

+ return err

+ }

+ return s.addColumnIfMissing("billing_quantity_operations", "serial", `INTEGER NOT NULL DEFAULT 0`)

}

func (s *Store) quantityOperationFor(accountID int64) (quantityOperation, bool, error) {

var op quantityOperation

- err := s.db.QueryRow(`SELECT account_id,subscription,period_start,period_end,item,from_quantity,target,generation,behavior,idempotency_key,created_at,acked_at,revision,confirmed_at,send_lease_until,fenced_at

+ err := s.db.QueryRow(`SELECT account_id,subscription,period_start,period_end,item,from_quantity,target,generation,behavior,idempotency_key,created_at,acked_at,revision,confirmed_at,send_lease_until,fenced_at,fence_kind,serial

FROM billing_quantity_operations WHERE account_id=?`, accountID).

- Scan(&op.AccountID, &op.Subscription, &op.PeriodStart, &op.PeriodEnd, &op.Item, &op.FromQuantity, &op.Target, &op.Generation, &op.Behavior, &op.Key, &op.CreatedAt, &op.AckedAt, &op.Revision, &op.ConfirmedAt, &op.SendLeaseUntil, &op.FencedAt)

+ Scan(&op.AccountID, &op.Subscription, &op.PeriodStart, &op.PeriodEnd, &op.Item, &op.FromQuantity, &op.Target, &op.Generation, &op.Behavior, &op.Key, &op.CreatedAt, &op.AckedAt, &op.Revision, &op.ConfirmedAt, &op.SendLeaseUntil, &op.FencedAt, &op.FenceKind, &op.Serial)

if errors.Is(err, sql.ErrNoRows) {

return op, false, nil

}

@@ -437,8 +466,7 @@ func (s *Store) quantityOperationFor(accountID int64) (quantityOperation, bool,

// if and only if everything op was computed from is still true. In one

// transaction, under the membership write lock, it requires that:

-// - no operation is outstanding for the account, and op's key is not

-// the key of the account's previous operation;

+// - no operation is outstanding for the account;

// - the billing row is the very revision op was read from (which covers

// every column at once: plan, band, terms, customer), and is still the

// subscription, period, billed quantity and active status op names;

@@ -448,11 +476,14 @@ func (s *Store) quantityOperationFor(accountID int64) (quantityOperation, bool,

// op.Target, with op.Generation the newest pending addition;

// - op.Target is above the billed quantity (an operation only raises).

+// When it records the operation it completes op's key with the account's

+// operation serial and sets both on op.

+//

// started is false when any of these fails. The caller then has nothing to

// send, and the next pass recomputes from the present. This is what stops a

// process that read an older state from recording, and then sending, a

// lower target after another process has finished a newer one.

-func (s *Store) beginQuantityOperation(op quantityOperation) (started bool, err error) {

+func (s *Store) beginQuantityOperation(op *quantityOperation) (started bool, err error) {

tx, err := s.db.Begin()

if err != nil {

return false, err

@@ -468,18 +499,20 @@ func (s *Store) beginQuantityOperation(op quantityOperation) (started bool, err

if outstanding {

return false, nil

}

- // A key is never used for a second operation. The account's previous

- // operation may have been fenced, and then the provider holds its key

- // for a request that is not this one. A key repeats only when the same

- // target is recomputed in the same second; the next pass is a later

- // second and a different key.

- var reused bool

- if err := tx.QueryRow(`SELECT EXISTS(SELECT 1 FROM billing_quantity_operations WHERE idempotency_key=?)`, op.Key).Scan(&reused); err != nil {

+ // A key is never used for a second operation: an earlier one may have

+ // been fenced, and the provider then holds its key for a request that

+ // is not this one. Everything else in the key can recur (the same

+ // target, recomputed in a second a clock has returned to), so the key

+ // ends in the account's operation serial, which only goes up and is

+ // kept on the row across every operation the account ever has.

+ var serial int64

+ switch err := tx.QueryRow(`SELECT serial FROM billing_quantity_operations WHERE account_id=?`, op.AccountID).Scan(&serial); {

+ case errors.Is(err, sql.ErrNoRows):

+ case err != nil:

return false, err

}

- if reused {

- return false, nil

- }

+ serial++

+ key := op.Key + "-" + strconv.FormatInt(serial, 10)

state, err := scanBilling(tx.QueryRow(`SELECT `+billingColumns+` FROM billing_accounts WHERE account_id=?`, op.AccountID), op.AccountID)

if err != nil {

return false, err

@@ -512,15 +545,20 @@ func (s *Store) beginQuantityOperation(op quantityOperation) (started bool, err

if err != nil || target != op.Target || pending.Newest != op.Generation {

return false, nil

}

- if _, err := tx.Exec(`INSERT INTO billing_quantity_operations(account_id,subscription,period_start,period_end,item,from_quantity,target,generation,behavior,idempotency_key,created_at,acked_at,revision)

- VALUES(?,?,?,?,?,?,?,?,?,?,?,0,?)

+ if _, err := tx.Exec(`INSERT INTO billing_quantity_operations(account_id,subscription,period_start,period_end,item,from_quantity,target,generation,behavior,idempotency_key,created_at,acked_at,revision,serial)

+ VALUES(?,?,?,?,?,?,?,?,?,?,?,0,?,?)

ON CONFLICT(account_id) DO UPDATE SET subscription=excluded.subscription,period_start=excluded.period_start,period_end=excluded.period_end,item=excluded.item,

from_quantity=excluded.from_quantity,target=excluded.target,generation=excluded.generation,behavior=excluded.behavior,

- idempotency_key=excluded.idempotency_key,created_at=excluded.created_at,acked_at=0,revision=excluded.revision,confirmed_at=0,send_lease_until=0,fenced_at=0`,

- op.AccountID, op.Subscription, op.PeriodStart, op.PeriodEnd, op.Item, op.FromQuantity, op.Target, op.Generation, op.Behavior, op.Key, op.CreatedAt, op.Revision); err != nil {

+ idempotency_key=excluded.idempotency_key,created_at=excluded.created_at,acked_at=0,revision=excluded.revision,confirmed_at=0,

+ send_lease_until=0,send_lease_token='',fenced_at=0,fence_kind='',serial=excluded.serial`,

+ op.AccountID, op.Subscription, op.PeriodStart, op.PeriodEnd, op.Item, op.FromQuantity, op.Target, op.Generation, op.Behavior, key, op.CreatedAt, op.Revision, serial); err != nil {

return false, err

}

- return true, tx.Commit()

+ if err := tx.Commit(); err != nil {

+ return false, err

+ }

+ op.Key, op.Serial = key, serial

+ return true, nil

}

// confirmQuantityOperation records that the provider answered the

@@ -550,13 +588,16 @@ func (s *Store) confirmQuantityOperation(accountID int64, key string, now time.T

// cannot disagree about whether the lease has run out.

const quantitySendLease = 2 * billingClockTolerance

-// claimQuantitySend takes (or extends) the send lease on the account's

-// outstanding operation key. ok is false when that operation is no longer

-// outstanding (it was retired or completed), or its key has been fenced at

-// the provider: it must not be sent.

-func (s *Store) claimQuantitySend(accountID int64, key string, now time.Time) (until time.Time, ok bool, err error) {

+// claimQuantitySend takes the send lease on the account's outstanding

+// operation key for one invocation, named by token. The lease is one

+// sender's: it is not taken while another's is live, and only the token

+// that took it gives it back. ok is false when another sender holds it,

+// when the operation is no longer outstanding (it was retired or

+// completed), or when its key has been fenced: it must not be sent.

+func (s *Store) claimQuantitySend(accountID int64, key, token string, now time.Time) (until time.Time, ok bool, err error) {

until = now.Add(quantitySendLease)

- res, err := s.db.Exec(`UPDATE billing_quantity_operations SET send_lease_until=? WHERE account_id=? AND idempotency_key=? AND acked_at=0 AND fenced_at=0`, until.Unix(), accountID, key)

+ res, err := s.db.Exec(`UPDATE billing_quantity_operations SET send_lease_until=?, send_lease_token=?

+ WHERE account_id=? AND idempotency_key=? AND acked_at=0 AND fenced_at=0 AND send_lease_until<=?`, until.Unix(), token, accountID, key, now.Unix())

if err != nil {

return time.Time{}, false, err

}

@@ -565,18 +606,28 @@ func (s *Store) claimQuantitySend(accountID int64, key string, now time.Time) (u

}

// releaseQuantitySend gives the lease up once the request has returned,

-// whatever it returned.

-func (s *Store) releaseQuantitySend(accountID int64, key string) error {

- _, err := s.db.Exec(`UPDATE billing_quantity_operations SET send_lease_until=0 WHERE account_id=? AND idempotency_key=?`, accountID, key)

+// whatever it returned. It releases only the lease token took: a sender

+// whose lease ran out and was taken by another does not release the

+// other's.

+func (s *Store) releaseQuantitySend(accountID int64, key, token string) error {

+ _, err := s.db.Exec(`UPDATE billing_quantity_operations SET send_lease_until=0, send_lease_token='' WHERE account_id=? AND idempotency_key=? AND send_lease_token=?`, accountID, key, token)

return err

}

+func newQuantitySendToken() (string, error) {

+ var raw [12]byte

+ if _, err := rand.Read(raw[:]); err != nil {

+ return "", err

+ }

+ return hex.EncodeToString(raw[:]), nil

// retireQuantityOperation acknowledges an operation that did not run and is

// no longer wanted, unless a send lease on it is live. retired false means

// some process may be sending it: it is left outstanding and looked at

// again next pass.

func (s *Store) retireQuantityOperation(accountID int64, key string, now time.Time) (retired bool, err error) {

- res, err := s.db.Exec(`UPDATE billing_quantity_operations SET acked_at=? WHERE account_id=? AND idempotency_key=? AND acked_at=0 AND send_lease_until<=? AND `+quantityOperationSettled, max(now.Unix(), 1), accountID, key, now.Unix())

+ res, err := s.db.Exec(`UPDATE billing_quantity_operations SET acked_at=? WHERE account_id=? AND idempotency_key=? AND acked_at=0 AND send_lease_until<=? AND `+quantityOperationRetirable, max(now.Unix(), 1), accountID, key, now.Unix())

if err != nil {

return false, err

}

@@ -632,14 +683,67 @@ func (s *Store) retireQuantityOperation(accountID int64, key string, now time.Ti

// internal/billingstripe, which runs only with a test-mode key and has not

// been run. Until it has passed, the worker must stay off: it starts only

// when CONSOLE_BILLING_QUANTITY_FENCE says so (BillingConfig.quantityWorker).

-const quantityOperationSettled = `(confirmed_at<>0 OR fenced_at<>0)`

+//

+// Two conditions, because "the key is held" is two different facts.

+//

+// quantityOperationSettled admits recording an operation as having LANDED

+// (the provider shows its target): confirmed, or fenced in any way. Held by

+// the fence request or by the operation's own request, no further request

+// under the key can change the quantity, so what the provider shows is what

+// is recorded.

+//

+// quantityOperationRetirable admits RETIRING an operation as not having

+// run, after which a replacement may be issued. That needs more. A key held

+// by the fence request shows the operation's own request never ran. A key

+// held by the operation's own request shows the opposite: it began

+// executing, and if the provider does not show its effect, its outcome is

+// not known (the provider calls a 500 indeterminate and may reconcile it

+// later). Such an operation is not retired on that evidence. It is held

+// with a review, and retired only once a person has resolved the review,

+// which is recorded on the operation as its fence.

+const (

+ quantityOperationSettled = `(confirmed_at<>0 OR fenced_at<>0)`

+ quantityOperationRetirable = `(confirmed_at<>0 OR (fenced_at<>0 AND fence_kind IN ('fence','reviewed')))`

-// markQuantityFenced records that the provider holds the operation's key.

-func (s *Store) markQuantityFenced(accountID int64, key string, now time.Time) error {

- _, err := s.db.Exec(`UPDATE billing_quantity_operations SET fenced_at=? WHERE account_id=? AND idempotency_key=? AND acked_at=0 AND fenced_at=0`, max(now.Unix(), 1), accountID, key)

+// markQuantityFenced records what holds the operation's key.

+func (s *Store) markQuantityFenced(accountID int64, key, kind string, now time.Time) error {

+ _, err := s.db.Exec(`UPDATE billing_quantity_operations SET fenced_at=?, fence_kind=? WHERE account_id=? AND idempotency_key=? AND acked_at=0 AND fenced_at=0`, max(now.Unix(), 1), kind, accountID, key)

return err

}

+// markQuantityReviewed records that a person resolved the review of an

+// operation the worker could not settle. It is taken only against a

+// resolved review of that very operation, read in the same transaction.

+func (s *Store) markQuantityReviewed(op quantityOperation, now time.Time) (bool, error) {

+ tx, err := s.db.Begin()

+ if err != nil {

+ return false, err

+ }

+ defer tx.Rollback()

+ var resolved int64

+ switch err := tx.QueryRow(`SELECT resolved_at FROM billing_reviews WHERE account_id=? AND subscription=? AND reason=?`,

+ op.AccountID, op.Subscription, billingQuantityUnsettledReason(op.CreatedAt)).Scan(&resolved); {

+ case errors.Is(err, sql.ErrNoRows):

+ return false, nil

+ case err != nil:

+ return false, err

+ }

+ if resolved == 0 {

+ return false, nil

+ }

+ res, err := tx.Exec(`UPDATE billing_quantity_operations SET fenced_at=CASE WHEN fenced_at=0 THEN ? ELSE fenced_at END, fence_kind=?

+ WHERE account_id=? AND idempotency_key=? AND acked_at=0 AND confirmed_at=0`, max(now.Unix(), 1), fenceKindReviewed, op.AccountID, op.Key)

+ if err != nil {

+ return false, err

+ }

+ n, err := res.RowsAffected()

+ if err != nil || n != 1 {

+ return false, err

+ }

+ return true, tx.Commit()

// fileQuantityDateReviewTx files the dating hold for an operation, unless

// it is already open.

func fileQuantityDateReviewTx(tx *sql.Tx, accountID int64, subscription string, created int64, now time.Time) error {

@@ -757,13 +861,15 @@ func (s *Server) processQuantityAdditions(ctx context.Context, accountID int64,

return err

}

if over {

- // An operation it could not acknowledge is one nobody can show

- // ran. Its key is fenced now; the next pass acknowledges it.

+ // An operation it could not acknowledge is one that may not be

+ // retired yet. It is settled now (fenced, or held for a person);

+ // a later pass acknowledges it.

op, ok, err := s.store.quantityOperationFor(accountID)

if err != nil || !ok || op.AckedAt != 0 {

return err

}

- return s.fenceQuantityOperation(ctx, op, now)

+ _, err = s.settleQuantityOperation(ctx, op, now)

+ return err

}

step, err := s.raiseQuantity(ctx, acct, behavior, now)

if err != nil || step.done {

@@ -872,7 +978,7 @@ func (s *Server) raiseQuantity(ctx context.Context, acct *Account, behavior stri

op := quantityOperation{AccountID: acct.ID, Subscription: state.Subscription, PeriodStart: state.PeriodStart, PeriodEnd: state.PeriodEnd, Item: item.ID,

FromQuantity: state.Quantity, Target: target, Generation: pending.Newest, Behavior: behavior, CreatedAt: created, Revision: state.Revision,

Key: fmt.Sprintf("carloku-qty-%s-%s-%d-%d-%d-%s-%d", acct.Sqid, state.Subscription, state.PeriodStart, pending.Newest, target, behavior, created)}

- started, err := s.store.beginQuantityOperation(op)

+ started, err := s.store.beginQuantityOperation(&op)

if err != nil {

return quantityStep{}, err

}

@@ -925,12 +1031,18 @@ func (s *Server) sendQuantityOperation(ctx context.Context, state billingState,

// write that succeeds only while the operation is still outstanding, and

// while it is live nobody can retire or replace the operation. The

// request's deadline falls before the lease ends.

+ token, err := newQuantitySendToken()

+ if err != nil {

+ return quantityStep{}, err

+ }

claimed := quantityClock()

- until, ok, err := s.store.claimQuantitySend(op.AccountID, op.Key, claimed)

+ until, ok, err := s.store.claimQuantitySend(op.AccountID, op.Key, token, claimed)

if err != nil {

return quantityStep{}, err

}

if !ok {

+ // Another sender holds the lease, or the operation is no longer

+ // to be sent. Either way this invocation sends nothing.

return quantityStep{done: true}, nil

}

sendCtx, cancel := context.WithTimeout(ctx, until.Add(-billingClockTolerance).Sub(claimed))

@@ -938,10 +1050,26 @@ func (s *Server) sendQuantityOperation(ctx context.Context, state billingState,

cancel()

if err != nil {

// The request has returned: this process is no longer sending.

- if rerr := s.store.releaseQuantitySend(op.AccountID, op.Key); rerr != nil {

+ if rerr := s.store.releaseQuantitySend(op.AccountID, op.Key, token); rerr != nil {

return quantityStep{}, errors.Join(err, rerr)

}

var refused *billingstripe.Error

+ if errors.As(err, &refused) && refused.Replayed && refused.HTTPStatus >= 500 {

+ // The provider answered from memory with a server error: this

+ // operation's request began executing earlier and ended in a

+ // result the provider itself calls indeterminate. Sending it

+ // again will be answered the same way for as long as the key

+ // is remembered, so it is not sent again. The key is recorded

+ // as held by the operation's own request, and a review tells a

+ // person now, not when the replay limit runs out.

+ if merr := s.store.markQuantityFenced(op.AccountID, op.Key, billingstripe.FenceHeldByRequest, quantityClock()); merr != nil {

+ return quantityStep{}, errors.Join(err, merr)

+ }

+ if rerr := s.store.fileBillingReviewOnce(op.AccountID, op.Subscription, billingQuantityUnsettledReason(op.CreatedAt), quantityClock()); rerr != nil {

+ return quantityStep{}, errors.Join(err, rerr)

+ }

+ return quantityStep{}, err

+ }

if errors.As(err, &refused) && refused.RefusedParam("proration_date") {

// The provider refused the request outright over its date: it

// did not execute, and the same date will be refused on every

@@ -967,7 +1095,7 @@ func (s *Server) sendQuantityOperation(ctx context.Context, state billingState,

if err := s.store.confirmQuantityOperation(op.AccountID, op.Key, quantityClock()); err != nil {

return quantityStep{}, err

}

- if err := s.store.releaseQuantitySend(op.AccountID, op.Key); err != nil {

+ if err := s.store.releaseQuantitySend(op.AccountID, op.Key, token); err != nil {

return quantityStep{}, err

}

if hook := b.quantityAfterSend; hook != nil {

@@ -991,10 +1119,15 @@ func (s *Server) resolveQuantityOperation(ctx context.Context, acct *Account, st

// and decided next pass, from a fresh read. While some process

// holds the right to send it, neither happens. The additions stay

// pending and are computed afresh once it is retired.

- if !op.settled() {

- return quantityStep{done: true}, s.fenceQuantityOperation(ctx, op, now)

+ //

+ // And it is retired only when the evidence is that it did not run

+ // (settleQuantityOperation). A key held by its own request is not

+ // that evidence.

+ retirable, err := s.settleQuantityOperation(ctx, op, now)

+ if err != nil || !retirable {

+ return quantityStep{done: true}, err

}

- _, err := s.store.retireQuantityOperation(acct.ID, op.Key, quantityClock())

+ _, err = s.store.retireQuantityOperation(acct.ID, op.Key, quantityClock())

return quantityStep{done: true}, err

}

if op.Subscription != state.Subscription {

@@ -1079,36 +1212,82 @@ func (s *Server) resolveQuantityOperation(ctx context.Context, acct *Account, st

return s.sendQuantityOperation(ctx, state, op, read)

}

-// settled reports whether the operation may be acknowledged: the provider

-// confirmed its request, or holds its key fenced.

+// settled reports whether the operation may be recorded as landed: the

+// provider confirmed its request, or something holds its key.

func (op quantityOperation) settled() bool { return op.ConfirmedAt != 0 || op.FencedAt != 0 }

+// retirable reports whether the operation may be retired as not having

+// run (quantityOperationRetirable).

+func (op quantityOperation) retirable() bool {

+ return op.ConfirmedAt != 0 || (op.FencedAt != 0 && (op.FenceKind == billingstripe.FenceTaken || op.FenceKind == fenceKindReviewed))

+// settleQuantityOperation moves an operation that is no longer wanted

+// toward being retirable, and reports whether it is. It never retires.

+//

+// - Confirmed, fenced by the fence request, or reviewed: retirable.

+// - A person has resolved its review: recorded, and retirable from the

+// next pass (the decision is made on a fresh provider read).

+// - Not fenced: the fence is attempted (fenceQuantityOperation).

+// - Its key is held by its own request: that request began executing and

+// the provider does not show its effect. Its outcome is not known, and

+// the provider may still reconcile it. A review is filed, once, and the

+// operation waits for a person.

+func (s *Server) settleQuantityOperation(ctx context.Context, op quantityOperation, now time.Time) (retirable bool, err error) {

+ if op.retirable() {

+ return true, nil

+ }

+ if op.SendLeaseUntil > quantityClock().Unix() {

+ return false, nil

+ }

+ if reviewed, err := s.store.markQuantityReviewed(op, quantityClock()); err != nil || reviewed {

+ return false, err

+ }

+ if op.FencedAt == 0 {

+ return false, s.fenceQuantityOperation(ctx, op, now)

+ }

+ return false, s.store.fileBillingReviewOnce(op.AccountID, op.Subscription, billingQuantityUnsettledReason(op.CreatedAt), now)

// fenceQuantityOperation consumes op's idempotency key at the provider (see

-// quantityOperationSettled) and records that it did. It does nothing while

-// a sender holds the lease: that sender is running, and will finish or let

-// the lease run out. A refusal the provider will repeat files a review,

-// once, because the operation cannot be acknowledged without the fence.

+// "The fence") and records what holds it. It does nothing while a sender

+// holds the lease: that sender is running, and will finish or let the lease

+// run out. It does nothing while the operation's review is open: the

+// provider has already refused, the answer will not change by asking every

+// pass, and a person has been told. A refusal the provider will repeat

+// files that review, once.

func (s *Server) fenceQuantityOperation(ctx context.Context, op quantityOperation, now time.Time) error {

if op.settled() || op.SendLeaseUntil > quantityClock().Unix() {

return nil

}

+ reason := billingQuantityUnsettledReason(op.CreatedAt)

+ switch rev, err := s.store.billingReview(op.AccountID, op.Subscription, reason); {

+ case errors.Is(err, sql.ErrNoRows):

+ case err != nil:

+ return err

+ case rev.ResolvedAt == 0:

+ return nil

+ default:

+ _, err := s.store.markQuantityReviewed(op, quantityClock())

+ return err

+ }

if hook := s.billing.quantityBeforeFence; hook != nil {

hook()

}

- fenced, err := s.billing.client.FenceQuantityKey(ctx, op.Item, op.Generation, op.Key)

+ kind, err := s.billing.client.FenceQuantityKey(ctx, op.Item, op.Generation, op.Key)

if err != nil {

var refused *billingstripe.Error

if errors.As(err, &refused) && refused.Code == billingstripe.CodeAPIError && refused.HTTPStatus >= 400 && refused.HTTPStatus < 500 && refused.HTTPStatus != http.StatusTooManyRequests {

- if rerr := s.store.fileBillingReviewOnce(op.AccountID, op.Subscription, billingQuantityFenceReason, now); rerr != nil {

+ if rerr := s.store.fileBillingReviewOnce(op.AccountID, op.Subscription, reason, now); rerr != nil {

return errors.Join(err, rerr)

}

return err

}

- if !fenced {

+ if kind == "" {

return nil

}

- return s.store.markQuantityFenced(op.AccountID, op.Key, quantityClock())

+ return s.store.markQuantityFenced(op.AccountID, op.Key, kind, quantityClock())

}

// quantityMismatch files the review for a provider quantity that is

diff --git a/internal/console/billingquantity_sim_test.go b/internal/console/billingquantity_sim_test.go

index 71aa5738c..2cb6bf1dd 100644

--- a/internal/console/billingquantity_sim_test.go

+++ b/internal/console/billingquantity_sim_test.go

@@ -243,14 +243,17 @@ func simulateQuantity(t *testing.T, seed int64) (trace []string, problem string)

// Passes are periodic: each step is a later second.

elapsed += time.Second

q.mu.Lock()

- q.failPost, q.failAfter = false, false

- switch rng.Intn(6) {

+ q.failPost, q.failAfter, q.failIndeterminate = false, false, false

+ switch rng.Intn(8) {

case 0:

q.failPost = true

logf(" next request fails before it runs")

case 1:

q.failAfter = true

- logf(" next request runs and its answer is lost")

+ logf(" next request runs and is answered 500, which the provider remembers")

+ case 2:

+ q.failIndeterminate = true

+ logf(" next request is answered 500, remembered, with no effect yet")

}

q.mu.Unlock()

switch rng.Intn(4) {

@@ -280,7 +283,7 @@ func simulateQuantity(t *testing.T, seed int64) (trace []string, problem string)

}

// Settle: no failures, nothing interleaved.

q.mu.Lock()

- q.failPost, q.failAfter = false, false

+ q.failPost, q.failAfter, q.failIndeterminate = false, false, false

q.mu.Unlock()

for _, rt := range []*billingRuntime{fx.srv.billing, procB.billing} {

rt.quantityBeforeBegin, rt.quantityBeforeSend, rt.quantityAfterSend = nil, nil, nil

diff --git a/internal/console/billingquantity_test.go b/internal/console/billingquantity_test.go

index 3f5c9ef8b..23512c6e0 100644

--- a/internal/console/billingquantity_test.go

+++ b/internal/console/billingquantity_test.go

@@ -29,7 +29,12 @@ type quantityStripe struct {

posts []map[string]string // quantity, proration_behavior, proration_date, key

failPost bool

start time.Time

- failAfter bool // accept the POST, then answer 500: a lost response

+ failAfter bool // run the POST, then answer 500, and remember the 500 for the key

+ // failIndeterminate answers 500 and remembers it for the key WITHOUT

+ // changing the quantity: the provider's indeterminate error, whose

+ // effect may arrive later (a test sets the quantity when it does).

+ failIndeterminate bool

+ saved500 map[string]bool

// byKey is the provider's idempotency memory: a key it has seen is

// answered with what it answered then, and changes nothing.

byKey map[string]int64

@@ -103,6 +108,13 @@ func (q *quantityStripe) serve(w http.ResponseWriter, r *http.Request) {

return

}

q.replays++

+ // The provider answers a remembered key with what it answered

+ // then, a 500 included.

+ w.Header().Set("Idempotent-Replayed", "true")

+ if q.saved500[key] {

+ w.WriteHeader(http.StatusInternalServerError)

+ return

+ }

_ = json.NewEncoder(w).Encode(map[string]any{"id": "si_team", "price": map[string]string{"id": "price_community_A"}, "quantity": prev})

return

}

@@ -122,6 +134,15 @@ func (q *quantityStripe) serve(w http.ResponseWriter, r *http.Request) {

_ = json.NewEncoder(w).Encode(item())

return

}

+ if q.saved500 == nil {

+ q.saved500 = map[string]bool{}

+ }

+ if q.failIndeterminate {

+ q.failIndeterminate = false

+ q.byKey[key], q.params[key], q.saved500[key] = n, r.PostForm.Encode(), true

+ w.WriteHeader(http.StatusInternalServerError)

+ return

+ }

q.byKey[key], q.params[key] = n, r.PostForm.Encode()

q.posts = append(q.posts, map[string]string{"quantity": r.PostForm.Get("quantity"), "proration_behavior": r.PostForm.Get("proration_behavior"), "proration_date": r.PostForm.Get("proration_date"), "key": key})

if n < q.quantity {

@@ -130,6 +151,7 @@ func (q *quantityStripe) serve(w http.ResponseWriter, r *http.Request) {

q.quantity = n

if q.failAfter {

q.failAfter = false

+ q.saved500[key] = true

w.WriteHeader(http.StatusInternalServerError)

return

}

@@ -784,7 +806,7 @@ func TestBeginQuantityOperationIsACompareAndSet(t *testing.T) {

op := good

op.Key = "k-" + name

mutate(&op)

- if started, err := fx.st.beginQuantityOperation(op); err != nil || started {

+ if started, err := fx.st.beginQuantityOperation(&op); err != nil || started {

t.Fatalf("%s: started=%v err=%v, want refused", name, started, err)

}

@@ -825,21 +847,23 @@ func TestBeginQuantityOperationIsACompareAndSet(t *testing.T) {

rebind("sub_other")

other := good

other.Subscription, other.Key = "sub_other", "k-other-subscription"

- if started, err := fx.st.beginQuantityOperation(other); err != nil || started {

+ if started, err := fx.st.beginQuantityOperation(&other); err != nil || started {

t.Fatalf("a proposal for a subscription the account does not bill: started=%v err=%v, want refused", started, err)

}

rebind("sub_team")

- if started, err := fx.st.beginQuantityOperation(good); err != nil || !started {

+ if started, err := fx.st.beginQuantityOperation(&good); err != nil || !started {

t.Fatalf("the true proposal: started=%v err=%v", started, err)

}

// One outstanding operation at a time.

again := good

again.Key = "k-again"

- if started, err := fx.st.beginQuantityOperation(again); err != nil || started {

+ if started, err := fx.st.beginQuantityOperation(&again); err != nil || started {

t.Fatalf("a second operation over an outstanding one: started=%v err=%v", started, err)

}

- if op, _, _ := fx.st.quantityOperationFor(OpsAccountID); op.Key != "k-good" {

- t.Fatalf("the outstanding operation was replaced: %+v", op)

+ // The recorded key is the proposed one completed with the account's

+ // operation serial.

+ if op, _, _ := fx.st.quantityOperationFor(OpsAccountID); op.Key != "k-good-1" || op.Serial != 1 || good.Key != "k-good-1" {

+ t.Fatalf("the outstanding operation was replaced, or its key carries no serial: %+v (proposer saw %q)", op, good.Key)

}

@@ -1203,7 +1227,7 @@ func TestQuantityReconcileWaitsWhileASenderHoldsTheLease(t *testing.T) {

q.mu.Lock()

q.quantity = 11

q.mu.Unlock()

- if _, ok, err := fx.st.claimQuantitySend(OpsAccountID, op.Key, time.Now()); err != nil || !ok {

+ if _, ok, err := fx.st.claimQuantitySend(OpsAccountID, op.Key, "another-sender", time.Now()); err != nil || !ok {

t.Fatalf("claim: %v %v", ok, err)

}

if err := fx.srv.reconcileBillingSubscription(ctx, "sub_team", "evt_quantity_in_flight", time.Now().UTC()); !errors.Is(err, errBillingPending) {

@@ -1221,7 +1245,7 @@ func TestQuantityReconcileWaitsWhileASenderHoldsTheLease(t *testing.T) {

if len(q.fences)+len(q.mismatched) != 0 {

t.Fatalf("the key was fenced under a live lease: %v %v", q.fences, q.mismatched)

}

- if err := fx.st.releaseQuantitySend(OpsAccountID, op.Key); err != nil {

+ if err := fx.st.releaseQuantitySend(OpsAccountID, op.Key, "another-sender"); err != nil {

t.Fatal(err)

}

fencePass(t, fx, q)

@@ -1915,13 +1939,13 @@ func TestQuantityOperationIsNeverAcknowledgedUnfenced(t *testing.T) {

if cur, _, _ := fx.st.quantityOperationFor(OpsAccountID); cur.AckedAt != 0 {

t.Fatalf("acknowledged with neither a confirmation nor a fence: %+v", cur)

}

- if _, ok, err := fx.st.claimQuantitySend(OpsAccountID, op.Key, now.Add(-time.Hour)); err != nil || !ok {

+ if _, ok, err := fx.st.claimQuantitySend(OpsAccountID, op.Key, "t1", now.Add(-time.Hour)); err != nil || !ok {

t.Fatalf("an unfenced operation could not be claimed: %v %v", ok, err)

}

- if err := fx.st.markQuantityFenced(OpsAccountID, op.Key, now); err != nil {

+ if err := fx.st.markQuantityFenced(OpsAccountID, op.Key, billingstripe.FenceTaken, now); err != nil {

t.Fatal(err)

}

- if _, ok, err := fx.st.claimQuantitySend(OpsAccountID, op.Key, now); err != nil || ok {

+ if _, ok, err := fx.st.claimQuantitySend(OpsAccountID, op.Key, "t2", now); err != nil || ok {

t.Fatalf("a fenced operation was claimed for sending: %v %v", ok, err)

}

op, _, _ = fx.st.quantityOperationFor(OpsAccountID)

@@ -1990,32 +2014,179 @@ func TestQuantityRequestThatRanJustBeforeTheFenceIsRecordedNotRetired(t *testing

// A fence the provider refuses in a way it will repeat cannot be waited

// out. The operation stays outstanding (it is never acknowledged unfenced)

-// and a review is filed, once, so a person sees why the account's quantity

-// work has stopped.

-func TestQuantityRefusedFenceFilesAReviewAndAcknowledgesNothing(t *testing.T) {

+// and a review is filed, once. While the review is open the provider is not

+// asked again every pass. Resolving the review is the way out: a person

+// has looked, the operation is retired, and the addition it covered is

+// charged by a fresh operation under a key of its own.

+func TestQuantityRefusedFenceHoldsForAPersonAndIsRetiredOnceReviewed(t *testing.T) {

fx, q, op := agedQuantityFixture(t)

ctx := context.Background()

if _, err := fx.st.db.Exec(`UPDATE billing_quantity_operations SET created_at=? WHERE account_id=?`, time.Now().Add(-quantityOperationReplayLimit-time.Minute).Unix(), OpsAccountID); err != nil {

t.Fatal(err)

}

+ op, _, _ = fx.st.quantityOperationFor(OpsAccountID)

q.mu.Lock()

q.refuseFence = true

q.mu.Unlock()

+ fences := 0

+ q.duringFence = func() { fences++ }

+ if err := fx.srv.billingQuantityPass(ctx, time.Now().UTC()); err == nil {

+ t.Fatal("a refused fence must surface")

+ }

for i := 0; i < 3; i++ {

- if err := fx.srv.billingQuantityPass(ctx, time.Now().UTC()); err == nil {

- t.Fatal("a refused fence must surface")

+ if err := fx.srv.billingQuantityPass(ctx, time.Now().UTC()); err != nil {

+ t.Fatalf("pass %d with the review open: %v", i, err)

}

+ if fences != 1 {

+ t.Fatalf("the provider was asked to fence %d times while the review stood open, want once", fences)

+ }

if cur, _, _ := fx.st.quantityOperationFor(OpsAccountID); cur.Key != op.Key || cur.AckedAt != 0 || cur.FencedAt != 0 {

t.Fatalf("acknowledged or marked fenced over a refused fence: %+v", cur)

}

- rev, err := fx.st.billingReview(OpsAccountID, "sub_team", billingQuantityFenceReason)

+ reason := billingQuantityUnsettledReason(op.CreatedAt)

+ rev, err := fx.st.billingReview(OpsAccountID, "sub_team", reason)

if err != nil || rev.ResolvedAt != 0 || rev.Occurrences != 1 {

t.Fatalf("review for the refused fence: %+v %v", rev, err)

}

if len(q.posts) != 0 {

t.Fatalf("sent: %+v", q.posts)

}

+ // A person resolves it.

+ if err := fx.st.SetOperator(reviewOperator, true); err != nil {

+ t.Fatal(err)

+ }

+ if err := fx.st.resolveBillingReview(billingReviewResolution{AccountID: OpsAccountID, Subscription: "sub_team", Reason: reason, Occurrence: 1, Actor: reviewOperator, Note: "checked at the provider"}, time.Now()); err != nil {

+ t.Fatal(err)

+ }

+ q.mu.Lock()

+ q.refuseFence = false

+ q.mu.Unlock()

+ for i := 0; i < 3; i++ {

+ if err := fx.srv.billingQuantityPass(ctx, time.Now().UTC()); err != nil {

+ t.Fatalf("pass %d after the review was resolved: %v", i, err)

+ }

+ if rev, _ := fx.st.billingReview(OpsAccountID, "sub_team", reason); rev.ResolvedAt == 0 || rev.Occurrences != 1 {

+ t.Fatalf("the resolved review was filed again: %+v", rev)

+ }

+ if len(q.posts) != 1 || q.posts[0]["key"] == op.Key || q.posts[0]["quantity"] != "11" {

+ t.Fatalf("posts %+v: want the addition charged by a fresh operation under its own key", q.posts)

+ }

+ if state, _ := fx.st.billingAccount(OpsAccountID); state.Quantity != 11 || len(pendingAdditions(t, fx)) != 0 {

+ t.Fatalf("not charged after the review: quantity %d", state.Quantity)

+ }

+// The provider answers the operation's request with a 500 and remembers it.

+// Its own documentation calls that result indeterminate: the request began,

+// its effect may or may not have been applied, and the provider may still

+// reconcile it. The fence then finds the key held by that request. That is

+// not evidence the operation did not run. It is not retired and nothing

+// replaces it: it is held with a review. When the provider's reconciliation

+// lands the quantity, it is recorded as landed, with the hold that says its

+// date is unproven.

+func TestQuantityIndeterminateRequestIsHeldNotRetired(t *testing.T) {

+ fx, q, now := newQuantityFixture(t, billingProrationImmediate)

+ ctx := context.Background()

+ if err := fx.st.AddMember(OpsAccountID, "eleventh@example.test", RoleMember, now.Add(time.Minute)); err != nil {

+ t.Fatal(err)

+ }

+ q.mu.Lock()

+ q.failIndeterminate = true

+ q.mu.Unlock()

+ if err := fx.srv.billingQuantityPass(ctx, time.Now().UTC()); err == nil {

+ t.Fatal("the provider's 500 must surface")

+ }

+ // Long after: past the replay limit, the worker would retire an

+ // operation the provider shows did not run.

+ if _, err := fx.st.db.Exec(`UPDATE billing_quantity_operations SET created_at=? WHERE account_id=?`, time.Now().Add(-quantityOperationReplayLimit-time.Minute).Unix(), OpsAccountID); err != nil {

+ t.Fatal(err)

+ }

+ op, _, _ := fx.st.quantityOperationFor(OpsAccountID)

+ for i := 0; i < 4; i++ {

+ if err := fx.srv.billingQuantityPass(ctx, time.Now().UTC()); err != nil {

+ t.Fatalf("pass %d: %v", i, err)

+ }

+ cur, _, _ := fx.st.quantityOperationFor(OpsAccountID)

+ if cur.Key != op.Key || cur.AckedAt != 0 || cur.FenceKind != billingstripe.FenceHeldByRequest {

+ t.Fatalf("an operation whose request ended indeterminately was retired or replaced: %+v", cur)

+ }

+ if len(q.posts) != 0 || q.quantity != 10 {

+ t.Fatalf("something was sent in its place: posts %+v quantity %d", q.posts, q.quantity)

+ }

+ rev, err := fx.st.billingReview(OpsAccountID, "sub_team", billingQuantityUnsettledReason(op.CreatedAt))

+ if err != nil || rev.ResolvedAt != 0 || rev.Occurrences != 1 {

+ t.Fatalf("no single open review for the indeterminate operation: %+v %v", rev, err)

+ }

+ if len(pendingAdditions(t, fx)) != 1 || !quantityHeld(t, fx) {

+ t.Fatal("the addition is no longer owed and held")

+ }

+ // The provider reconciles the request: the quantity lands.

+ q.mu.Lock()

+ q.quantity = 11

+ q.mu.Unlock()

+ for i := 0; i < 2; i++ {

+ if err := fx.srv.billingQuantityPass(ctx, time.Now().UTC()); err != nil {

+ t.Fatalf("pass %d after the reconciliation: %v", i, err)

+ }

+ assertLastAddition(t, fx, quantityOutcomeUnverified, "si_team:quantity=11:always_invoice")

+ if cur, _, _ := fx.st.quantityOperationFor(OpsAccountID); cur.AckedAt == 0 {

+ t.Fatalf("the landed operation is still outstanding: %+v", cur)

+ }

+ if rev, err := fx.st.billingReview(OpsAccountID, "sub_team", billingQuantityDateReviewReason(op.CreatedAt)); err != nil || rev.ResolvedAt != 0 {

+ t.Fatalf("no open hold for the charge whose date is unproven: %+v %v", rev, err)

+ }

+ if len(q.posts) != 0 {

+ t.Fatalf("a second request was sent: %+v", q.posts)

+ }

+// The send lease belongs to one sender. A second process that reads the

+// same outstanding operation does not take it while the first holds it,

+// sends nothing, and cannot give it back: releasing is by the token that

+// took it. So nothing is concluded under the first sender, and its

+// confirmation is recorded when its answer arrives.

+func TestQuantitySendLeaseBelongsToOneSender(t *testing.T) {

+ fx, q, op := agedQuantityFixture(t)

+ ctx := context.Background()

+ now := time.Now()

+ if _, ok, err := fx.st.claimQuantitySend(OpsAccountID, op.Key, "first", now); err != nil || !ok {

+ t.Fatalf("the first sender's claim: %v %v", ok, err)

+ }

+ if _, ok, err := fx.st.claimQuantitySend(OpsAccountID, op.Key, "second", now.Add(time.Second)); err != nil || ok {

+ t.Fatalf("a second sender took a live lease: %v %v", ok, err)

+ }

+ if err := fx.st.releaseQuantitySend(OpsAccountID, op.Key, "second"); err != nil {

+ t.Fatal(err)

+ }

+ if cur, _, _ := fx.st.quantityOperationFor(OpsAccountID); cur.SendLeaseUntil <= now.Unix() {

+ t.Fatalf("a sender that never held the lease released it: %+v", cur)

+ }

+ // A whole pass in another process, while the first sender holds it:

+ // it sends nothing and concludes nothing.

+ procB := &Server{cfg: fx.srv.cfg, store: fx.srv.store, billing: &billingRuntime{locks: newStoreWalkLocks(), client: fx.srv.billing.client, cfg: fx.srv.billing.cfg}}

+ if err := procB.billingQuantityPass(ctx, time.Now().UTC()); err != nil {

+ t.Fatal(err)

+ }

+ if len(q.posts) != 0 || len(q.fences) != 0 {

+ t.Fatalf("a second process acted under the first sender's lease: posts %+v fences %v", q.posts, q.fences)

+ }

+ if cur, _, _ := fx.st.quantityOperationFor(OpsAccountID); cur.SendLeaseUntil <= now.Unix() || cur.AckedAt != 0 {

+ t.Fatalf("the first sender's lease was cleared or its operation acknowledged: %+v", cur)

+ }

+ // The holder gives it back, and after it runs out anyone may take it.

+ if err := fx.st.releaseQuantitySend(OpsAccountID, op.Key, "first"); err != nil {

+ t.Fatal(err)

+ }

+ if _, ok, err := fx.st.claimQuantitySend(OpsAccountID, op.Key, "second", now.Add(2*time.Second)); err != nil || !ok {

+ t.Fatalf("after the release: %v %v", ok, err)

+ }

+ if _, ok, err := fx.st.claimQuantitySend(OpsAccountID, op.Key, "third", now.Add(quantitySendLease+3*time.Second)); err != nil || !ok {

+ t.Fatalf("after a lease ran out without a release: %v %v", ok, err)

+ }

}

// A fenced operation the provider shows did not run is retired by the next

@@ -2030,7 +2201,7 @@ func TestQuantityFencedOperationIsRetiredAndReplacedNeverSent(t *testing.T) {

t.Fatal(err)

}

op.Key = "carloku-qty-aged"

- if err := fx.st.markQuantityFenced(OpsAccountID, op.Key, time.Now()); err != nil {

+ if err := fx.st.markQuantityFenced(OpsAccountID, op.Key, billingstripe.FenceTaken, time.Now()); err != nil {

t.Fatal(err)

}

if err := fx.srv.billingQuantityPass(ctx, time.Now().UTC()); err != nil {

@@ -2056,26 +2227,38 @@ func TestQuantityFencedOperationIsRetiredAndReplacedNeverSent(t *testing.T) {

}

-// A key is never used for a second operation: the previous one may have

-// been fenced, and the provider then holds that key for another request.

-func TestBeginQuantityOperationNeverReusesAKey(t *testing.T) {

+// A key is never used for a second operation, however the rest of the key

+// recurs. Three operations for the same target are recorded in the same

+// second (a clock that returned to it): every part of the key but the

+// serial is the same each time, and all three keys differ. The account's

+// row keeps only its latest operation, so the serial is what remembers.

+func TestQuantityOperationKeysNeverRecur(t *testing.T) {

fx, _, op := agedQuantityFixture(t)

- if err := fx.st.markQuantityFenced(OpsAccountID, op.Key, time.Now()); err != nil {

- t.Fatal(err)

- }

- if retired, err := fx.st.retireQuantityOperation(OpsAccountID, op.Key, time.Now()); err != nil || !retired {

- t.Fatalf("retire: %v %v", retired, err)

- }

- state, _ := fx.st.billingAccount(OpsAccountID)

- again := op

- again.AckedAt, again.FencedAt, again.Revision = 0, 0, state.Revision

- again.CreatedAt = time.Now().Unix()

- if started, err := fx.st.beginQuantityOperation(again); err != nil || started {

- t.Fatalf("an operation was recorded under its predecessor's key: started=%v err=%v", started, err)

- }

- again.Key = op.Key + "-next"

- if started, err := fx.st.beginQuantityOperation(again); err != nil || !started {

- t.Fatalf("a fresh key was refused: started=%v err=%v", started, err)

+ base := strings.TrimSuffix(op.Key, "-"+strconv.FormatInt(op.Serial, 10))

+ if op.Serial != 1 || base == op.Key {

+ t.Fatalf("fixture: the first operation's key carries no serial: %+v", op)

+ }

+ seen := map[string]bool{op.Key: true}

+ prev := op

+ for round := 2; round <= 4; round++ {

+ if err := fx.st.markQuantityFenced(OpsAccountID, prev.Key, billingstripe.FenceTaken, time.Now()); err != nil {

+ t.Fatal(err)

+ }

+ if retired, err := fx.st.retireQuantityOperation(OpsAccountID, prev.Key, time.Now()); err != nil || !retired {

+ t.Fatalf("round %d: retire: %v %v", round, retired, err)

+ }

+ state, _ := fx.st.billingAccount(OpsAccountID)

+ next := op

+ next.Key, next.Serial, next.AckedAt, next.FencedAt, next.FenceKind, next.Revision = base, 0, 0, 0, "", state.Revision

+ next.CreatedAt = time.Now().Unix()

+ if started, err := fx.st.beginQuantityOperation(&next); err != nil || !started {

+ t.Fatalf("round %d: begin: started=%v err=%v", round, started, err)

+ }

+ if seen[next.Key] || next.Serial != int64(round) {

+ t.Fatalf("round %d: key %q (serial %d) was used before: %v", round, next.Key, next.Serial, seen)

+ }

+ seen[next.Key] = true

+ prev = next

}

@@ -2117,3 +2300,49 @@ func TestQuantityLoweredAtTheProviderFilesAReview(t *testing.T) {

t.Fatalf("a quantity lowered inside the period was taken with no review, or filed twice: %+v %v", rev, err)

}

+// The provider answers an operation's request with a 500 and remembers it.

+// The next pass sends the request again and is answered the same 500 from

+// memory. That is the provider saying the result is indeterminate and will

+// not change by asking. The worker stops asking at once: the operation is

+// held with a review after that one replay, not replayed every pass until

+// the replay limit runs out a day later.

+func TestQuantityReplayedServerErrorHoldsAtOnce(t *testing.T) {

+ fx, q, now := newQuantityFixture(t, billingProrationImmediate)

+ ctx := context.Background()

+ if err := fx.st.AddMember(OpsAccountID, "eleventh@example.test", RoleMember, now.Add(time.Minute)); err != nil {

+ t.Fatal(err)

+ }

+ q.mu.Lock()

+ q.failIndeterminate = true

+ q.mu.Unlock()

+ if err := fx.srv.billingQuantityPass(ctx, time.Now().UTC()); err == nil {

+ t.Fatal("the provider's 500 must surface")

+ }

+ op, _, _ := fx.st.quantityOperationFor(OpsAccountID)

+ if op.FencedAt != 0 {

+ t.Fatalf("a first 500, not answered from memory, was taken as remembered: %+v", op)

+ }

+ if err := fx.srv.billingQuantityPass(ctx, time.Now().UTC()); err == nil {

+ t.Fatal("the replayed 500 must surface")

+ }

+ if q.replays != 1 {

+ t.Fatalf("replays = %d, want the one that showed the 500 is remembered", q.replays)

+ }

+ for i := 0; i < 4; i++ {

+ if err := fx.srv.billingQuantityPass(ctx, time.Now().UTC()); err != nil {

+ t.Fatalf("pass %d with the operation held: %v", i, err)

+ }

+ if q.replays != 1 || len(q.posts) != 0 {

+ t.Fatalf("the held operation was sent again: replays %d posts %+v", q.replays, q.posts)

+ }

+ cur, _, _ := fx.st.quantityOperationFor(OpsAccountID)

+ if cur.Key != op.Key || cur.AckedAt != 0 || cur.FenceKind != billingstripe.FenceHeldByRequest {

+ t.Fatalf("the operation was retired or replaced: %+v", cur)

+ }

+ rev, err := fx.st.billingReview(OpsAccountID, "sub_team", billingQuantityUnsettledReason(op.CreatedAt))

+ if err != nil || rev.ResolvedAt != 0 || rev.Occurrences != 1 {

+ t.Fatalf("no single open review: %+v %v", rev, err)

+ }

diff --git a/internal/objstore/aws.go b/internal/objstore/aws.go

index 4e25bdccf..9fc3ac75c 100644

--- a/internal/objstore/aws.go

+++ b/internal/objstore/aws.go

@@ -10,6 +10,7 @@ import (

"encoding/json"

"errors"

"fmt"

+ "github.com/carlosframework/platform/internal/sigfence"

"github.com/aws/aws-sdk-go-v2/aws"

awsconfig "github.com/aws/aws-sdk-go-v2/config"

@@ -78,7 +79,10 @@ var _ Provisioner = (*awsProvisioner)(nil)

// twin, so a non-AWS backend (endpoint set, "aws" backend) still means

// IAM-against-real-AWS in v1 — there is nothing else for it to point at.

func NewAWSProvisioner(ctx context.Context, region, endpoint, boundary string) (Provisioner, error) {

- s3Client, err := s3ClientFor(ctx, region, endpoint, func(o *awss3.Options) { o.HTTPSignerV4 = newFencedSigner() })

+ s3Client, err := s3ClientFor(ctx, region, endpoint, func(o *awss3.Options) {

+ o.HTTPSignerV4 = sigfence.NewS3Signer()

+ o.APIOptions = append(o.APIOptions, sigfence.WatchAttempts)

+ })

if err != nil {

return nil, err

}

@@ -88,8 +92,13 @@ func NewAWSProvisioner(ctx context.Context, region, endpoint, boundary string) (

}

// Both clients sign through the fence (fence.go): a request made inside

// a walk carries the walk's lease instant as its signing time, so AWS

- // itself refuses it once that lease is too old.

- iamClient := iam.NewFromConfig(cfg, func(o *iam.Options) { o.HTTPSignerV4 = newFencedSigner() })

+ // itself refuses it once that lease is too old. Both also record on the

+ // walk when a request needed a second attempt: the first may still

+ // arrive, so the walk does not give its lease back early.

+ iamClient := iam.NewFromConfig(cfg, func(o *iam.Options) {

+ o.HTTPSignerV4 = sigfence.NewSigner()

+ o.APIOptions = append(o.APIOptions, sigfence.WatchAttempts)

+ })

return &awsProvisioner{s3: s3Client, iam: iamClient, region: region, boundary: boundary}, nil

}

diff --git a/internal/objstore/fence.go b/internal/objstore/fence.go

index 3edb48641..a9b115042 100644

--- a/internal/objstore/fence.go

+++ b/internal/objstore/fence.go

@@ -3,11 +3,9 @@ package objstore

import (

"context"

"errors"

- "net/http"

"time"

- "github.com/aws/aws-sdk-go-v2/aws"

- v4 "github.com/aws/aws-sdk-go-v2/aws/signer/v4"

+ "github.com/carlosframework/platform/internal/sigfence"

)

// The fence: what makes a walk's lease mean something at the BACKEND.

@@ -25,8 +23,11 @@ import (

// - AWS signs each request with a timestamp and rejects one whose

// timestamp is more than fifteen minutes from its own clock. The

// signer normally stamps "now"; under a fence it stamps the lease's

-// instant instead (fencedSigner). A request sent after that window is

-// rejected by AWS whenever the process happens to send it.

+// instant instead (internal/sigfence). A request sent after that

+// window is rejected by AWS whenever the process happens to send it.

+// The signer is installed on every client a walk reaches: this

+// package's S3 and IAM clients, and the deployment bucket's own client

+// (internal/release), which holds the store records and the ledgers.

// - The in-memory provisioner refuses a call whose fence is older than

// the same window.

@@ -34,43 +35,26 @@ import (

// plus fenceSkew), and a lease that was not given back cleanly cannot be

// taken over until it has run out. So by the time another walk may take

// the name, every request the earlier walk could ever send is one the

-// backend refuses. fenceSkew is the allowance for this host's clock

-// running ahead of the backend's.

+// backend refuses. fenceSkew is how much longer than the window the lease

+// lasts.

+//

+// 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

+// against the backend's. With the margin as long as the window no check is

+// needed. A taker whose clock runs ahead by less than the window takes

+// over no earlier than the end of the previous holder's window. A taker

+// whose clock runs ahead by more than that stamps its own requests further

+// in the future than the backend accepts, and can do nothing.

const (

- fenceWindow = 15 * time.Minute

- fenceSkew = 5 * time.Minute

+ fenceWindow = sigfence.Window

+ fenceSkew = sigfence.Window

)

// ErrFenced: the backend refused a request whose walk's lease is too old.

var ErrFenced = errors.New("objstore: request refused: its walk's lease has run out")

-type fenceKey struct{}

-// withFence marks every provider call made with ctx as part of a walk

-// whose lease was taken at `at`.

-func withFence(ctx context.Context, at time.Time) context.Context {

- return context.WithValue(ctx, fenceKey{}, at.UTC())

-func fenceFrom(ctx context.Context) (time.Time, bool) {

- at, ok := ctx.Value(fenceKey{}).(time.Time)

- return at, ok

-// fencedSigner signs with the walk's lease instant when the request's

-// context carries one, and with the time it is given otherwise. It is

-// installed on the provisioner's S3 and IAM clients.

-type fencedSigner struct {

- inner interface {

- SignHTTP(ctx context.Context, credentials aws.Credentials, r *http.Request, payloadHash string, service string, region string, signingTime time.Time, optFns ...func(*v4.SignerOptions)) error

- }

-func newFencedSigner() fencedSigner { return fencedSigner{inner: v4.NewSigner()} }

+// withFence marks every provider and store call made with ctx as part of a

+// walk whose lease was taken at `at`.

+func withFence(ctx context.Context, at time.Time) context.Context { return sigfence.With(ctx, at) }

-func (f fencedSigner) SignHTTP(ctx context.Context, credentials aws.Credentials, r *http.Request, payloadHash string, service string, region string, signingTime time.Time, optFns ...func(*v4.SignerOptions)) error {

- if at, ok := fenceFrom(ctx); ok {

- signingTime = at

- }

- return f.inner.SignHTTP(ctx, credentials, r, payloadHash, service, region, signingTime, optFns...)

+func fenceFrom(ctx context.Context) (time.Time, bool) { return sigfence.From(ctx) }

diff --git a/internal/objstore/fence_aws_test.go b/internal/objstore/fence_aws_test.go

index 022cbb2da..f53da8df9 100644

--- a/internal/objstore/fence_aws_test.go

+++ b/internal/objstore/fence_aws_test.go

@@ -13,6 +13,8 @@ import (

"github.com/aws/aws-sdk-go-v2/service/iam"

awss3 "github.com/aws/aws-sdk-go-v2/service/s3"

"github.com/aws/smithy-go"

+ "github.com/carlosframework/platform/internal/sigfence"

)

// TestFenceAgainstAWS checks the fence's premise against the real backend,

@@ -20,8 +22,18 @@ import (

// than its window, whenever it arrives, and accepts one inside it.

// It runs only when CARLOS_FENCE_AWS_BUCKET names a bucket the caller's

-// credentials may list. It lists one key of that bucket and asks IAM for a

-// user that does not exist. It changes nothing.

+// credentials may list. It lists one key of that bucket, asks for one

+// object that does not exist, and asks IAM for a user that does not exist.

+// It changes nothing.

+//

+// What it establishes: S3 and IAM apply the signing-time window to requests

+// signed by this package's signer, the SDK's retries do not get round it

+// (each attempt is signed again with the same stamp), and the signer signs

+// an S3 key with characters that need escaping the way S3 verifies it.

+//

+// What it does not: it sends no delete and no write. The window is applied

+// to the signature before the operation is looked at, so there is no reason

+// a delete would differ, but that is an inference and not an observation.

func TestFenceAgainstAWS(t *testing.T) {

bucket := os.Getenv("CARLOS_FENCE_AWS_BUCKET")

if bucket == "" {

@@ -32,31 +44,36 @@ func TestFenceAgainstAWS(t *testing.T) {

region = "eu-west-1"

}

ctx := context.Background()

- s3c, err := s3ClientFor(ctx, region, "", func(o *awss3.Options) {

- o.HTTPSignerV4 = newFencedSigner()

- o.RetryMaxAttempts = 1

- })

- if err != nil {

- t.Fatal(err)

- }

- cfg, err := awsconfig.LoadDefaultConfig(ctx, awsconfig.WithRegion(region))

- if err != nil {

- t.Fatal(err)

+ clients := func(attempts int) (*awss3.Client, *iam.Client) {

+ s3c, err := s3ClientFor(ctx, region, "", func(o *awss3.Options) {

+ o.HTTPSignerV4 = sigfence.NewS3Signer()

+ o.APIOptions = append(o.APIOptions, sigfence.WatchAttempts)

+ o.RetryMaxAttempts = attempts

+ })

+ if err != nil {

+ t.Fatal(err)

+ }

+ cfg, err := awsconfig.LoadDefaultConfig(ctx, awsconfig.WithRegion(region))

+ if err != nil {

+ t.Fatal(err)

+ }

+ return s3c, iam.NewFromConfig(cfg, func(o *iam.Options) {

+ o.HTTPSignerV4 = sigfence.NewSigner()

+ o.APIOptions = append(o.APIOptions, sigfence.WatchAttempts)

+ o.RetryMaxAttempts = attempts

+ })

}

- iamc := iam.NewFromConfig(cfg, func(o *iam.Options) {

- o.HTTPSignerV4 = newFencedSigner()

- o.RetryMaxAttempts = 1

- })

code := func(err error) string {

var api smithy.APIError

if errors.As(err, &api) {

- return api.ErrorCode() + ": " + api.ErrorMessage()

+ return api.ErrorCode()

}

if err != nil {

- return err.Error()

+ return "error: " + err.Error()

}

return "ok"

}

+ s3c, iamc := clients(1)

list := func(age time.Duration) string {

_, err := s3c.ListObjectsV2(withFence(ctx, time.Now().Add(-age)), &awss3.ListObjectsV2Input{Bucket: aws.String(bucket), MaxKeys: aws.Int32(1)})

return code(err)

@@ -75,17 +92,42 @@ func TestFenceAgainstAWS(t *testing.T) {

{fenceWindow + fenceSkew, true},

} {

s3Answer, iamAnswer := list(c.age), getUser(c.age)

- t.Logf("signed %v ago: S3 says %q; IAM says %q", c.age, s3Answer, iamAnswer)

- s3Refused := strings.Contains(s3Answer, "RequestTimeTooSkewed")

- iamRefused := strings.Contains(iamAnswer, "Signature expired") || strings.Contains(iamAnswer, "SignatureDoesNotMatch") || strings.Contains(iamAnswer, "RequestExpired")

- if s3Refused != c.refused {

- t.Errorf("S3, signed %v ago: refused=%v, want %v (%s)", c.age, s3Refused, c.refused, s3Answer)

+ t.Logf("signed %v ago: S3 answers %q; IAM answers %q", c.age, s3Answer, iamAnswer)

+ // Accepted means the operation's own answer came back: the list

+ // succeeded, and IAM looked for the user and did not find it.

+ // Refused means the signature's age was the answer.

+ wantS3, wantIAM := "ok", "NoSuchEntity"

+ if c.refused {

+ wantS3, wantIAM = "RequestTimeTooSkewed", "SignatureDoesNotMatch"

}

- if c.refused && !iamRefused {

- t.Errorf("IAM, signed %v ago: accepted (%s); the fence does not hold for IAM", c.age, iamAnswer)

+ if s3Answer != wantS3 {

+ t.Errorf("S3, signed %v ago: %q, want %q", c.age, s3Answer, wantS3)

}

- if !c.refused && iamRefused {

- t.Logf("IAM refuses a request signed only %v ago: its window is shorter than S3's, which is the safe direction", c.age)

+ if iamAnswer != wantIAM {

+ t.Errorf("IAM, signed %v ago: %q, want %q", c.age, iamAnswer, wantIAM)

}

+ // A key with characters that need escaping. A signer that escaped it

+ // twice would be told its signature does not match. Any other answer

+ // (the object is not there, or these credentials may not read it) means

+ // the signature was accepted.

+ _, err := s3c.GetObject(withFence(ctx, time.Now().Add(-time.Minute)), &awss3.GetObjectInput{Bucket: aws.String(bucket), Key: aws.String("carlos fence probe/a+b=c~d (1).json")})

+ answer := code(err)

+ t.Logf("a key that needs escaping, signed a minute ago: S3 answers %q", answer)

+ if answer == "SignatureDoesNotMatch" || strings.HasPrefix(answer, "error:") || answer == "ok" {

+ t.Errorf("a key that needs escaping: %q, want the operation's own refusal (NoSuchKey or AccessDenied)", answer)

+ }

+ // With the SDK's retries on, as the walks run. The SDK treats a skewed

+ // request as retryable and signs each attempt again. Every attempt

+ // must carry the same old stamp and be refused, and the walk's doubt

+ // must be raised by the retry.

+ retrying, _ := clients(3)

+ fenced, doubt := sigfence.Track(withFence(ctx, time.Now().Add(-fenceWindow-time.Minute)))

+ _, err = retrying.ListObjectsV2(fenced, &awss3.ListObjectsV2Input{Bucket: aws.String(bucket), MaxKeys: aws.Int32(1)})

+ t.Logf("signed %v ago, SDK retries on: S3 answers %q; a second attempt was made: %v", fenceWindow+time.Minute, code(err), doubt.Raised())

+ if code(err) != "RequestTimeTooSkewed" {

+ t.Errorf("with retries on, a request past the window: %q, want it refused on every attempt", code(err))

+ }

}

diff --git a/internal/objstore/ownership.go b/internal/objstore/ownership.go

index b30a2c4b8..7633bfde7 100644

--- a/internal/objstore/ownership.go

+++ b/internal/objstore/ownership.go

@@ -508,6 +508,31 @@ func beginBucketTeardown(ctx context.Context, s release.Store, bucket, acct, app

return destroy, scannerOwed, other, foreign, err

}

+// markBucketDestroyed records on the ledger that acct/app's teardown has

+// deleted the bucket, at the moment it has. The marker must still be that

+// teardown's: a ledger that names nobody, or somebody else, as destroying

+// the bucket is not one this teardown may speak for. It is recorded at the

+// deletion and not only when the teardown closes its interval, because the

+// teardown may never get that far: its own app can take the store back,

+// which clears the marker, and stores that left this bucket to another

+// account would then never learn that it went.

+func markBucketDestroyed(ctx context.Context, s release.Store, bucket, acct, app string, at time.Time) error {

+ at = at.UTC()

+ return writeBucketOwnership(ctx, s, bucket, func(b *BucketOwnership) (*BucketOwnership, error) {

+ if b == nil {

+ return nil, errOwnershipUnchanged

+ }

+ if d := b.Destroying; d == nil || d.Account != acct || d.App != app {

+ return nil, fmt.Errorf("objstore: ownership of %s changed during the teardown of %s/%s", bucket, acct, app)

+ }

+ if b.DestroyedAt != nil && !b.DestroyedAt.Before(at) {

+ return nil, errOwnershipUnchanged

+ }

+ b.DestroyedAt = &at

+ return b, nil

+ })

// stillDestroying re-reads the ledger and reports whether acct/app's

// decision to destroy the bucket still stands: its marker is there and no

// other app holds the bucket. A teardown asks immediately before each

diff --git a/internal/objstore/ownership_test.go b/internal/objstore/ownership_test.go

index 6a2e2c5a5..13d2b72b4 100644

--- a/internal/objstore/ownership_test.go

+++ b/internal/objstore/ownership_test.go

@@ -5,6 +5,7 @@ import (

"encoding/json"

"errors"

"github.com/aws/aws-sdk-go-v2/aws"

+ "github.com/carlosframework/platform/internal/sigfence"

"net/http"

"testing"

"time"

@@ -1439,7 +1440,7 @@ func TestPausedWalkCannotActAfterItsLeaseIsTaken(t *testing.T) {

// the walk's lease instant, not with the time it happens to be sent, so

// AWS's own check on the age of a signature is a check on the lease.

func TestFencedSignerStampsTheLeaseInstant(t *testing.T) {

- signer := newFencedSigner()

+ signer := sigfence.NewSigner()

creds := aws.Credentials{AccessKeyID: "AKIDEXAMPLE", SecretAccessKey: "secret"}

lease := time.Date(2026, 9, 3, 10, 0, 0, 0, time.UTC)

sentAt := lease.Add(40 * time.Minute) // the process woke much later

@@ -1630,3 +1631,475 @@ func TestTeardownAfterAFailedTakeBackDeletesTheBucketTheGrantRecreated(t *testin

}

+// hookStore runs a hook at one store call, once, so a test can let another

+// walk run exactly there.

+type hookStore struct {

+ release.Store

+ at string // "get:<key>" or "delete:<key>"

+ hook func()

+ done bool

+func (h *hookStore) fire(what string) {

+ if !h.done && h.hook != nil && what == h.at {

+ h.done = true

+ h.hook()

+ }

+func (h *hookStore) Get(ctx context.Context, key string) ([]byte, string, error) {

+ h.fire("get:" + key)

+ return h.Store.Get(ctx, key)

+func (h *hookStore) DeleteIfMatch(ctx context.Context, key, etag string) error {

+ h.fire("delete:" + key)

+ return release.DeleteIfMatch(ctx, h.Store, key, etag)

+// A teardown of a store that was only declared deletes its record without

+// taking a lease: there is no bucket name to lease. Between its last read

+// and its delete, another process grants the store, creating a bucket, a

+// user and credentials. The delete names the generation the teardown read,

+// so the backend refuses it, and the granted store keeps its record.

+func TestDeclarationOnlyTeardownCannotEraseAStoreGrantedMeanwhile(t *testing.T) {

+ ctx := context.Background()

+ dir := release.NewDirStore(t.TempDir())

+ _, pub := testSealKeypair(t)

+ p := NewMemProvisioner("eu-west-1")

+ leaseClock(t)

+ declare(t, dir, "bab", "titogo", "TITO_S3")

+ s := &hookStore{Store: dir, at: "delete:" + Key("bab", "titogo")}

+ s.hook = func() {

+ if _, err := Grant(ctx, dir, p, testDeployment(), pub, "bab", "titogo", "op", GrantSourceOperator, "", t0); err != nil {

+ t.Errorf("the grant that lands in the gap: %v", err)

+ }

+ err := Teardown(ctx, s, p, nil, "bab", "titogo")

+ if !s.done {

+ t.Fatal("fixture: the teardown never reached its delete")

+ }

+ if !errors.Is(err, release.ErrCASConflict) {

+ t.Fatalf("teardown: %v, want the backend's refusal of a delete of a generation that is gone", err)

+ }

+ rec, _, gerr := Get(ctx, dir, "bab", "titogo")

+ if gerr != nil || rec == nil || rec.Provisioned == nil {

+ t.Fatalf("the granted store's record was erased: %+v %v", rec, gerr)

+ }

+ if _, ok := p.Users[UserName("bab", "titogo")]; !ok {

+ t.Fatal("fixture: the grant did not create the user")

+ }

+// A teardown reads its record, which names bucket A, and is held before it

+// takes A's lease. Another teardown finishes, the app is declared again and

+// granted bucket B, and data lands in B. The first teardown then takes A's

+// lease and reads the record again. It names B now. The lease it holds is

+// not B's, so it stops before writing or calling anything, and B is whole.

+func TestTeardownStopsWhenItsRecordNamesAnotherBucketThanItsLease(t *testing.T) {

+ ctx := context.Background()

+ dir := release.NewDirStore(t.TempDir())

+ _, pub := testSealKeypair(t)

+ p := NewMemProvisioner("eu-west-1")

+ advance := leaseClock(t)

+ const bucketB = "carlos-store-adopted-b"

+ declare(t, dir, "bab", "titogo", "TITO_S3")

+ if _, err := Grant(ctx, dir, p, testDeployment(), pub, "bab", "titogo", "op", GrantSourceOperator, "", t0); err != nil {

+ t.Fatal(err)

+ }

+ s := &hookStore{Store: dir, at: "get:" + walkLockKey(ledgerBucket)}

+ s.hook = func() {

+ if err := Teardown(ctx, dir, p, nil, "bab", "titogo"); err != nil {

+ t.Errorf("the teardown that finishes first: %v", err)

+ }

+ advance(time.Hour)

+ declare(t, dir, "bab", "titogo", "TITO_S3")

+ if _, err := Grant(ctx, dir, p, testDeployment(), pub, "bab", "titogo", "op", GrantSourceOperator, bucketB, t0.Add(time.Hour)); err != nil {

+ t.Errorf("the grant of bucket B: %v", err)

+ }

+ p.Buckets[bucketB]["invoice.pdf"] = []byte("data in B")

+ }

+ err := Teardown(ctx, s, p, nil, "bab", "titogo")

+ if !s.done {

+ t.Fatal("fixture: the teardown never reached its lease")

+ }

+ if !errors.Is(err, release.ErrCASConflict) {

+ t.Fatalf("teardown holding A's lease over a record that names B: %v, want it to stop", err)

+ }

+ if got := string(p.Buckets[bucketB]["invoice.pdf"]); got != "data in B" {

+ t.Fatalf("bucket B was emptied or deleted under another bucket's lease (object now %q)", got)

+ }

+ rec, _, gerr := Get(ctx, dir, "bab", "titogo")

+ if gerr != nil || rec == nil || rec.Provisioned == nil || rec.Provisioned.Bucket != bucketB || rec.Teardown != nil {

+ t.Fatalf("the store granted B was written to by the stopped teardown: %+v %v", rec, gerr)

+ }

+ if _, ok := p.Users[UserName("bab", "titogo")]; !ok {

+ t.Fatal("the stopped teardown deleted the user")

+ }

+// doubtProvisioner reports, as the SDK's retry would, that one call needed

+// a second attempt. The call itself succeeds.

+type doubtProvisioner struct {

+ *MemProvisioner

+ at string

+func (p *doubtProvisioner) DeleteUser(ctx context.Context, user string) error {

+ if p.at == "DeleteUser" {

+ sigfence.Raise(ctx)

+ }

+ return p.MemProvisioner.DeleteUser(ctx, user)

+// A request of a walk fails ambiguously, the SDK tries again, the second

+// attempt succeeds, and the walk returns success. The first attempt was

+// never shown not to have run and can still arrive inside its signature's

+// window. So the walk does not free the name: it stays reserved until the

+// lease has run out, exactly as after a walk that failed.

+func TestWalkThatNeededASecondAttemptKeepsItsLease(t *testing.T) {

+ ctx := context.Background()

+ s := release.NewDirStore(t.TempDir())

+ _, pub := testSealKeypair(t)

+ mem := NewMemProvisioner("eu-west-1")

+ advance := leaseClock(t)

+ declare(t, s, "bab", "titogo", "TITO_S3")

+ if _, err := Grant(ctx, s, mem, testDeployment(), pub, "bab", "titogo", "op", GrantSourceOperator, "", t0); err != nil {

+ t.Fatal(err)

+ }

+ // Ordinarily a successful walk frees the name at once.

+ if w, err := acquireBucketWalk(ctx, s, ledgerBucket, "bac", "other", "grant"); err != nil {

+ t.Fatalf("after a clean grant the name is still reserved: %v", err)

+ } else {

+ w.release(true)

+ }

+ if err := Teardown(ctx, s, &doubtProvisioner{MemProvisioner: mem, at: "DeleteUser"}, nil, "bab", "titogo"); err != nil {

+ t.Fatalf("the teardown whose DeleteUser needed a second attempt: %v", err)

+ }

+ if _, err := acquireBucketWalk(ctx, s, ledgerBucket, "bac", "other", "grant"); !errors.Is(err, ErrBucketBusy) {

+ t.Fatalf("the name was freed with an attempt of the walk unaccounted for: %v", err)

+ }

+ advance(walkLease - time.Second)

+ if _, err := acquireBucketWalk(ctx, s, ledgerBucket, "bac", "other", "grant"); !errors.Is(err, ErrBucketBusy) {

+ t.Fatalf("the name was freed before the lease ran out: %v", err)

+ }

+ advance(2 * time.Second)

+ if w, err := acquireBucketWalk(ctx, s, ledgerBucket, "bac", "other", "grant"); err != nil {

+ t.Fatalf("after the lease ran out: %v", err)

+ } else {

+ w.release(true)

+ }

+// A store that held its minted name and left is not a holder. If a store

+// provisioned before ledgers has since taken the name (a record, no

+// interval), a new minting grant by the first store is refused like anyone

+// else's: by the bucket that is already there, and, where the bucket cannot

+// be seen, by the other store's record.

+func TestMintingGrantByAStoreThatLeftIsRefusedForALegacyAdopter(t *testing.T) {

+ for _, bucketVisible := range []bool{true, false} {

+ ctx := context.Background()

+ s := release.NewDirStore(t.TempDir())

+ _, pub := testSealKeypair(t)

+ p := NewMemProvisioner("eu-west-1")

+ advance := leaseClock(t)

+ declare(t, s, "bab", "titogo", "TITO_S3")

+ if _, err := Grant(ctx, s, p, testDeployment(), pub, "bab", "titogo", "op", GrantSourceOperator, "", t0); err != nil {

+ t.Fatal(err)

+ }

+ if err := Teardown(ctx, s, p, nil, "bab", "titogo"); err != nil {

+ t.Fatal(err)

+ }

+ advance(time.Hour)

+ // The legacy adopter: a record that names the bucket, a bucket

+ // with its data, and no interval on the ledger.

+ legacy := &Record{Declared: &Declared{EnvPrefix: "OTHER_S3", At: t0, By: "member@example.com"},

+ Grant: &GrantBlock{Backend: "aws", Bucket: ledgerBucket, Source: GrantSourceOperator, At: t0, By: "op"},

+ Provisioned: &Provisioned{Bucket: ledgerBucket, IAMUser: UserName("bac", "other"), KeyID: "LEGACYKEY"}}

+ if _, err := Put(ctx, s, "bac", "other", legacy, ""); err != nil {

+ t.Fatal(err)

+ }

+ if bucketVisible {

+ p.Buckets[ledgerBucket] = map[string][]byte{"invoice.pdf": []byte("the adopter's data")}

+ }

+ declare(t, s, "bab", "titogo", "TITO_S3")

+ _, err := Grant(ctx, s, p, testDeployment(), pub, "bab", "titogo", "op", GrantSourceOperator, "", t0.Add(2*time.Hour))

+ if !errors.Is(err, ErrBucketHeld) {

+ t.Fatalf("bucket visible=%v: a store that left was granted its old name over a legacy adopter: %v", bucketVisible, err)

+ }

+ if _, ok := p.Users[UserName("bab", "titogo")]; ok {

+ t.Fatalf("bucket visible=%v: the refused grant created credentials", bucketVisible)

+ }

+ if rec, _, _ := Get(ctx, s, "bab", "titogo"); rec != nil && rec.Grant != nil {

+ t.Fatalf("bucket visible=%v: the refused grant left a grant block: %+v", bucketVisible, rec)

+ }

+// An adopting grant reads a record whose grant block names bucket A, which

+// a minting grant of the same app wrote a moment ago. That minting grant is

+// then refused (another account's store holds A) and takes its block back.

+// The adopting grant takes A's lease and reads the record again: no block.

+// By its own arguments it is for bucket B, not A. It must not write A as

+// its own adopted bucket, which would skip every holder check and hand this

+// app credentials to the other account's data.

+func TestGrantDoesNotAdoptABucketItOnlySawInAStaleRecord(t *testing.T) {

+ ctx := context.Background()

+ dir := release.NewDirStore(t.TempDir())

+ _, pub := testSealKeypair(t)

+ p := NewMemProvisioner("eu-west-1")

+ leaseClock(t)

+ const bucketB = "carlos-store-adopted-b"

+ // Another account's store holds the name the app would mint.

+ declare(t, dir, "bac", "other", "OTHER_S3")

+ if _, err := Grant(ctx, dir, p, testDeployment(), pub, "bac", "other", "op", GrantSourceOperator, ledgerBucket, t0); err != nil {

+ t.Fatal(err)

+ }

+ p.Buckets[ledgerBucket]["invoice.pdf"] = []byte("the other account's data")

+ // The record as the adopting grant first reads it: a grant block for

+ // the minted name, written by a minting grant still in progress.

+ rec := &Record{Declared: &Declared{EnvPrefix: "TITO_S3", At: t0, By: "member@example.com"},

+ Grant: &GrantBlock{Backend: "aws", Bucket: ledgerBucket, Source: GrantSourceOperator, At: t0, By: "op", Minted: true}}

+ if _, err := Put(ctx, dir, "bab", "titogo", rec, ""); err != nil {

+ t.Fatal(err)

+ }

+ s := &hookStore{Store: dir, at: "get:" + walkLockKey(ledgerBucket)}

+ s.hook = func() {

+ // The minting grant is refused and takes its block back.

+ cur, etag, err := Get(ctx, dir, "bab", "titogo")

+ if err != nil || cur == nil {

+ t.Error(err)

+ return

+ }

+ cur.Grant = nil

+ if _, err := Put(ctx, dir, "bab", "titogo", cur, etag); err != nil {

+ t.Error(err)

+ }

+ _, err := Grant(ctx, s, p, testDeployment(), pub, "bab", "titogo", "op", GrantSourceOperator, bucketB, t0.Add(time.Hour))

+ if !s.done {

+ t.Fatal("fixture: the grant never reached its lease")

+ }

+ if !errors.Is(err, release.ErrCASConflict) {

+ t.Fatalf("the adopting grant over a record that changed under it: %v, want it to stop", err)

+ }

+ if _, ok := p.Users[UserName("bab", "titogo")]; ok {

+ t.Fatal("the grant created credentials")

+ }

+ if cur, _, _ := Get(ctx, dir, "bab", "titogo"); cur == nil || cur.Grant != nil {

+ t.Fatalf("the grant wrote a block for a bucket it only saw in a stale record: %+v", cur)

+ }

+ // Run again, it is for bucket B, as it was asked.

+ got, err := Grant(ctx, dir, p, testDeployment(), pub, "bab", "titogo", "op", GrantSourceOperator, bucketB, t0.Add(2*time.Hour))

+ if err != nil || got.Provisioned == nil || got.Provisioned.Bucket != bucketB {

+ t.Fatalf("the grant run again: %+v %v, want bucket B", got, err)

+ }

+// A rotation reads its record and stalls. The store is torn down and

+// granted again: the user's name is the same, and the new grant delivers a

+// new key. The rotation wakes. It runs under the bucket's lease like any

+// walk, reads the record again there, finds it is not the store it was

+// asked to rotate, and stops. The new store's key is not reaped.

+func TestRotationStopsWhenTheStoreWasReplacedWhileItWaited(t *testing.T) {

+ for _, verb := range []string{"rotate", "finish"} {

+ 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")

+ if _, err := Grant(ctx, dir, p, testDeployment(), pub, "bab", "titogo", "op", GrantSourceOperator, "", t0); err != nil {

+ t.Fatal(err)

+ }

+ if verb == "finish" {

+ if _, err := Rotate(ctx, dir, p, pub, "bab", "titogo", t0.Add(time.Minute)); err != nil {

+ t.Fatal(err)

+ }

+ var regranted *Record

+ s := &hookStore{Store: dir, at: "get:" + walkLockKey(ledgerBucket)}

+ s.hook = func() {

+ if err := Teardown(ctx, dir, p, nil, "bab", "titogo"); err != nil {

+ t.Errorf("teardown: %v", err)

+ }

+ advance(time.Hour)

+ declare(t, dir, "bab", "titogo", "TITO_S3")

+ var err error

+ if regranted, err = Grant(ctx, dir, p, testDeployment(), pub, "bab", "titogo", "op", GrantSourceOperator, "", t0.Add(time.Hour)); err != nil {

+ t.Errorf("re-grant: %v", err)

+ }

+ var err error

+ if verb == "rotate" {

+ _, err = Rotate(ctx, s, p, pub, "bab", "titogo", t0.Add(2*time.Hour))

+ } else {

+ _, err = RotateFinish(ctx, s, p, "bab", "titogo")

+ }

+ if !s.done || regranted == nil {

+ t.Fatalf("%s: fixture: the rotation never reached its lease", verb)

+ }

+ if !errors.Is(err, release.ErrCASConflict) {

+ t.Fatalf("%s over a store replaced while it waited: %v, want it to stop", verb, err)

+ }

+ u, ok := p.Users[UserName("bab", "titogo")]

+ if !ok || u.Keys[regranted.Provisioned.KeyID] == "" || len(u.Keys) != 1 {

+ t.Fatalf("%s: the new store's key was reaped or another was minted: %+v", verb, u)

+ }

+ // And a rotation is a walk: nothing else runs on the name with it.

+ w, err := acquireBucketWalk(ctx, dir, ledgerBucket, "bab", "titogo", "rotate")

+ if err != nil {

+ t.Fatalf("%s: the stopped rotation did not give its lease back: %v", verb, err)

+ }

+ if _, err := Grant(ctx, dir, p, testDeployment(), pub, "bab", "titogo", "op", GrantSourceOperator, "", t0.Add(3*time.Hour)); !errors.Is(err, ErrBucketBusy) {

+ t.Fatalf("%s: a grant ran while a rotation held the name: %v", verb, err)

+ }

+ w.release(true)

+ }

+// A store's teardown deletes the bucket, fails before it finishes, and the

+// same store is then granted again, which clears the teardown's marker.

+// The ledger already says the bucket was destroyed, written when it was.

+// So a store of another account that had left that bucket, and was told its

+// objects were still within reach, can finish: they went with the bucket.

+func TestTakeBackAfterDeletingTheBucketStillRecordsItsDestruction(t *testing.T) {

+ ctx := context.Background()

+ s := release.NewDirStore(t.TempDir())

+ _, pub := testSealKeypair(t)

+ p := NewMemProvisioner("eu-west-1")

+ advance := leaseClock(t)

+ const shared = "carlos-store-shared-archive"

+ at := t0

+ clock := ownershipNow

+ ownershipNow = func() time.Time { return at }

+ defer func() { ownershipNow = clock }()

+ step := func() { at = at.Add(time.Hour); advance(time.Hour) }

+ for _, acct := range []string{"bac", "bad"} {

+ declare(t, s, acct, "one", "ARCHIVE_S3")

+ if _, err := Grant(ctx, s, p, testDeployment(), pub, acct, "one", "op", GrantSourceOperator, shared, at); err != nil {

+ t.Fatal(err)

+ }

+ step()

+ if err := Teardown(ctx, s, p, nil, "bac", "one"); !errors.Is(err, ErrBucketRetained) {

+ t.Fatalf("the store that leaves: %v, want ErrBucketRetained", err)

+ }

+ // The last holder deletes the bucket and fails at its user.

+ step()

+ p.FailNext = map[string]error{"DeleteUser": errors.New("injected DeleteUser")}

+ if err := Teardown(ctx, s, p, nil, "bad", "one"); err == nil {

+ t.Fatal("fixture: the last holder's teardown was meant to fail after deleting the bucket")

+ }

+ p.FailNext = nil

+ if _, exists := p.Buckets[shared]; exists {

+ t.Fatal("fixture: the bucket was not deleted")

+ }

+ ledger, _, err := ReadBucketOwnership(ctx, s, shared)

+ if err != nil || ledger == nil || ledger.DestroyedAt == nil {

+ t.Fatalf("the ledger does not say the bucket was destroyed at the time it was: %+v %v", ledger, err)

+ }

+ // It takes the store back.

+ step()

+ if _, err := Grant(ctx, s, p, testDeployment(), pub, "bad", "one", "op", GrantSourceOperator, shared, at); err != nil {

+ t.Fatalf("the take-back: %v", err)

+ }

+ // The store that left can now finish, though another account holds a

+ // bucket of that name: it is a new bucket, and holds none of its objects.

+ step()

+ if err := Teardown(ctx, s, p, nil, "bac", "one"); err != nil {

+ t.Fatalf("the retained teardown after the bucket it left was destroyed: %v", err)

+ }

+ if rec, _, _ := Get(ctx, s, "bac", "one"); rec != nil {

+ t.Fatalf("the retained store's record still stands: %+v", rec)

+ }

+ if _, exists := p.Buckets[shared]; !exists {

+ t.Fatal("finishing the retained teardown touched the new bucket")

+ }

+// Whoever takes a lease over judges "run out" by its own clock, and that

+// clock is never checked against the backend's. So the lease outlasts the

+// backend's window by a whole second window. A taker whose clock runs

+// ahead by anything less than the window cannot take the name while the

+// backend would still accept the previous holder's requests. (One whose

+// clock runs ahead by more stamps its own requests further in the future

+// than the backend accepts, and can do nothing with the lease it took.)

+func TestLeaseIsNotTakenOverEarlyByAClockThatRunsAhead(t *testing.T) {

+ ctx := context.Background()

+ s := release.NewDirStore(t.TempDir())

+ trueTime := time.Now()

+ var ahead time.Duration

+ prev := walkClock

+ walkClock = func() time.Time { return trueTime.Add(ahead) }

+ t.Cleanup(func() { walkClock = prev })

+ holder, err := acquireBucketWalk(ctx, s, ledgerBucket, "bab", "titogo", "teardown")

+ if err != nil {

+ t.Fatal(err)

+ }

+ stale := withFence(ctx, holder.lock.SignedAt)

+ // The holder's walk failed: the name stays reserved.

+ holder.release(false)

+ // The last instant the backend still accepts the holder.

+ trueTime = trueTime.Add(fenceWindow - time.Second)

+ if err := memFenced(stale); err != nil {

+ t.Fatalf("fixture: the backend already refuses the holder: %v", err)

+ }

+ for _, skew := range []time.Duration{0, time.Minute, 6 * time.Minute, fenceWindow - 2*time.Second} {

+ ahead = skew

+ if _, err := acquireBucketWalk(ctx, s, ledgerBucket, "bac", "other", "grant"); !errors.Is(err, ErrBucketBusy) {

+ t.Fatalf("a taker whose clock runs %v ahead took the name while the backend still accepts the previous holder: %v", skew, err)

+ }

+ // Once the backend refuses the holder, an accurate clock still waits

+ // out the margin, and then takes it.

+ ahead = 0

+ trueTime = trueTime.Add(fenceWindow + 2*time.Second)

+ if _, err := acquireBucketWalk(ctx, s, ledgerBucket, "bac", "other", "grant"); err != nil {

+ t.Fatalf("after the lease ran out: %v", err)

+ }

+// The scan sidecar is deleted as the generation the teardown read. One

+// written since (by a store granted again under the name, whose sweep has

+// run) is not removed by a delete that was decided before it existed.

+func TestTeardownDoesNotDeleteASidecarWrittenAfterItLooked(t *testing.T) {

+ ctx := context.Background()

+ dir := release.NewDirStore(t.TempDir())

+ _, pub := testSealKeypair(t)

+ p := NewMemProvisioner("eu-west-1")

+ leaseClock(t)

+ declare(t, dir, "bab", "titogo", "TITO_S3")

+ if _, err := Grant(ctx, dir, p, testDeployment(), pub, "bab", "titogo", "op", GrantSourceOperator, "", t0); err != nil {

+ t.Fatal(err)

+ }

+ if _, err := dir.Put(ctx, ScanKey("bab", "titogo"), []byte(`{"v":1,"old":true}`), ""); err != nil {

+ t.Fatal(err)

+ }

+ s := &hookStore{Store: dir, at: "delete:" + ScanKey("bab", "titogo")}

+ s.hook = func() {

+ _, etag, err := dir.Get(ctx, ScanKey("bab", "titogo"))

+ if err != nil {

+ t.Error(err)

+ return

+ }

+ if _, err := dir.Put(ctx, ScanKey("bab", "titogo"), []byte(`{"v":1,"new":true}`), etag); err != nil {

+ t.Error(err)

+ }

+ err := Teardown(ctx, s, p, nil, "bab", "titogo")

+ if !s.done {

+ t.Fatal("fixture: the teardown never reached the sidecar")

+ }

+ if err == nil {

+ t.Fatal("the teardown finished over a sidecar that changed under it")

+ }

+ raw, _, gerr := dir.Get(ctx, ScanKey("bab", "titogo"))

+ if gerr != nil || string(raw) != `{"v":1,"new":true}` {

+ t.Fatalf("the newer sidecar was deleted or altered: %q %v", raw, gerr)

+ }

+ if rec, _, _ := Get(ctx, dir, "bab", "titogo"); rec == nil {

+ t.Fatal("the record was deleted by a teardown that did not finish")

+ }

diff --git a/internal/objstore/walk.go b/internal/objstore/walk.go

index 0c9631d13..c07195ae5 100644

--- a/internal/objstore/walk.go

+++ b/internal/objstore/walk.go

@@ -10,6 +10,7 @@ import (

"errors"

"fmt"

"log"

+ "os"

"regexp"

"strings"

"time"

@@ -169,13 +170,20 @@ func Grant(ctx context.Context, s release.Store, p Provisioner, d Deployment,

if rec.Provisioned != nil {

startKeyID = rec.Provisioned.KeyID

}

- bucket := adoptBucket

- if rec.Grant != nil && rec.Grant.Bucket != "" {

- bucket = rec.Grant.Bucket // a re-run never re-chooses the bucket

- }

- if bucket == "" {

- bucket = MintedBucket(d, acct, app)

+ // Which bucket this grant is for, as a rule of the record and the

+ // arguments: applied here to choose the name to lease, and again to the

+ // record read under the lease.

+ bucketFor := func(r *Record) string {

+ b := adoptBucket

+ if r.Grant != nil && r.Grant.Bucket != "" {

+ b = r.Grant.Bucket // a re-run never re-chooses the bucket

+ }

+ if b == "" {

+ b = MintedBucket(d, acct, app)

+ }

+ return b

}

+ bucket := bucketFor(rec)

// Before anything is created or recorded: the name is about to become the

// IAM policy's resource ARN, so it is checked whether it was minted from a

// deployment prefix or handed over by an operator adopting a bucket.

@@ -199,8 +207,13 @@ func Grant(ctx context.Context, s release.Store, p Provisioner, d Deployment,

if rec == nil || rec.Declared == nil {

return nil, ErrNotDeclared

}

- if held := rec.Grant; held != nil && held.Bucket != "" && held.Bucket != bucket {

- return nil, fmt.Errorf("objstore: store record for %s/%s changed while its walk was starting — re-run: %w", acct, app, release.ErrCASConflict)

+ // The lease is on the name chosen from the first read. The record read

+ // under the lease must lead to the same name by the same rule. It does

+ // not when a grant block appeared, changed, or was removed in between

+ // (a refused grant removes its own): the name leased was then somebody

+ // else's choice, and this grant must not go on to write it as its own.

+ if now := bucketFor(rec); now != bucket {

+ return nil, fmt.Errorf("objstore: store record for %s/%s changed while its walk was starting (it led to %s and now leads to %s); run it again: %w", acct, app, bucket, now, release.ErrCASConflict)

}

rebuilt = rec.Teardown != nil

startKeyID = ""

@@ -289,17 +302,22 @@ func Grant(ctx context.Context, s release.Store, p Provisioner, d Deployment,

if lerr != nil {

return nil, fmt.Errorf("objstore: read ownership of %s: %w", bucket, lerr)

}

+ // Holding means an interval that is still open: a grant of this

+ // store that crashed after it opened one. A closed interval says

+ // the store once held the name and left. What has happened to the

+ // name since is exactly what the checks below are for, so a store

+ // that left is treated like one that never held it.

held := false

if ledger != nil {

for _, iv := range ledger.Intervals {

- if iv.Account == acct && iv.App == app {

+ if iv.Account == acct && iv.App == app && iv.open() {

held = true

}

if !held {

if canProbe && perr == nil && exists {

- return nil, refused(fmt.Errorf("%w: %s already exists and %s/%s has never held it", ErrBucketHeld, bucket, acct, app))

+ return nil, refused(fmt.Errorf("%w: %s already exists and %s/%s does not hold it", ErrBucketHeld, bucket, acct, app))

}

// And never while another store's RECORD names the bucket. A

// store provisioned before ledgers existed holds its bucket

@@ -493,7 +511,7 @@ func Grant(ctx context.Context, s release.Store, p Provisioner, d Deployment,

// between. A blind retry would clobber a concurrent rotate and strand ITS

// delivered key, which is the exact failure the retry exists to prevent.

func Rotate(ctx context.Context, s release.Store, p Provisioner, sealPub *ecdh.PublicKey,

- acct, app string, now time.Time) (*Record, error) {

+ acct, app string, now time.Time) (rotated *Record, walkErr error) {

rec, etag, err := Get(ctx, s, acct, app)

if err != nil {

return nil, err

@@ -504,6 +522,18 @@ func Rotate(ctx context.Context, s release.Store, p Provisioner, sealPub *ecdh.P

if rec.Declared == nil {

return nil, ErrNotDeclared // no prefix to deliver into

}

+ // A rotation deletes and mints keys of a user whose name is the same

+ // for every store this app will ever have. It is a walk like a grant

+ // or a teardown: it runs under the bucket's lease and inside its fence,

+ // on the record as read under the lease.

+ ctx, rec, etag, done, err := leaseForRotation(ctx, s, acct, app, rec)

+ if err != nil {

+ return nil, err

+ }

+ defer func() { done(walkErr) }()

+ if rec.Declared == nil {

+ return nil, ErrNotDeclared

+ }

pr := rec.Provisioned

startKeyID := pr.KeyID // the key the app is holding, and the retry guard

// 1. finish the pending rotation late. The reap below would collect this

@@ -567,7 +597,7 @@ func Rotate(ctx context.Context, s release.Store, p Provisioner, sealPub *ecdh.P

// using, which is why it is a separate verb a member asks for rather than a

// timer: only the member knows every host, worker and hibernating tenant has

// picked the new pair up.

-func RotateFinish(ctx context.Context, s release.Store, p Provisioner, acct, app string) (*Record, error) {

+func RotateFinish(ctx context.Context, s release.Store, p Provisioner, acct, app string) (finished *Record, walkErr error) {

rec, etag, err := Get(ctx, s, acct, app)

if err != nil {

return nil, err

@@ -575,6 +605,11 @@ func RotateFinish(ctx context.Context, s release.Store, p Provisioner, acct, app

if rec == nil || rec.Provisioned == nil {

return nil, ErrNotProvisioned

}

+ ctx, rec, etag, done, err := leaseForRotation(ctx, s, acct, app, rec)

+ if err != nil {

+ return nil, err

+ }

+ defer func() { done(walkErr) }()

pr := rec.Provisioned

if pr.PrevKeyID == "" {

return nil, ErrNoPendingFinish

@@ -592,6 +627,50 @@ func RotateFinish(ctx context.Context, s release.Store, p Provisioner, acct, app

return rec, nil

}

+// leaseForRotation takes the walk lease on the provisioned bucket's name

+// for a rotation, and returns the context every call of the rotation must

+// use, the record as it stands under the lease, and what to call when the

+// rotation returns. It refuses when the record read under the lease is no

+// longer the provisioned store the rotation was asked for: another walk

+// finished in between, and the keys this rotation would touch belong to

+// whatever stands there now.

+func leaseForRotation(ctx context.Context, s release.Store, acct, app string, first *Record) (context.Context, *Record, string, func(error), error) {

+ bucket, keyID := first.Provisioned.Bucket, first.Provisioned.KeyID

+ if !ValidBucketName(bucket) {

+ // A name no grant of this build would have written has no lease to

+ // take. The rotation runs as it always did.

+ rec, etag, err := Get(ctx, s, acct, app)

+ if err == nil && (rec == nil || rec.Provisioned == nil) {

+ err = ErrNotProvisioned

+ }

+ return ctx, rec, etag, func(error) {}, err

+ }

+ walk, err := acquireBucketWalk(ctx, s, bucket, acct, app, "rotate")

+ if err != nil {

+ return ctx, nil, "", nil, err

+ }

+ bound, cancel := walk.bound(ctx)

+ done := func(walkErr error) {

+ cancel()

+ walk.release(walkEndedClean(walkErr))

+ }

+ rec, etag, err := Get(bound, s, acct, app)

+ switch {

+ case err != nil:

+ case rec == nil || rec.Provisioned == nil:

+ err = ErrNotProvisioned

+ case rec.Provisioned.Bucket != bucket || rec.Provisioned.KeyID != keyID || rec.Teardown != nil:

+ err = fmt.Errorf("objstore: store record for %s/%s changed while its rotation was starting; run it again: %w", acct, app, release.ErrCASConflict)

+ }

+ if err != nil {

+ // Nothing was called under the lease: it is given back clean.

+ cancel()

+ walk.release(true)

+ return ctx, nil, "", nil, err

+ }

+ return bound, rec, etag, done, nil

// Teardown dismantles a store: bucket contents, bucket, IAM principal (keys

// and inline policy with it), the delivered config vars, and finally the

// record. It is the shared engine under the console's "Delete now" purge and

@@ -681,6 +760,21 @@ func Teardown(ctx context.Context, s release.Store, p Provisioner, sc Scanner, a

if rec.Grant != nil {

startBucket = rec.Grant.Bucket

}

+ // The lease is on the bucket the record named when it was first

+ // read. If another teardown finished in between and the app was

+ // granted again, the record may name a different bucket now, and

+ // this walk holds no lease on that one. It stops here, before it

+ // has written or called anything; a re-run leases the right name.

+ now := ""

+ switch {

+ case rec.Provisioned != nil:

+ now = rec.Provisioned.Bucket

+ case rec.Grant != nil:

+ now = rec.Grant.Bucket

+ }

+ if now != "" && now != leased {

+ return fmt.Errorf("objstore: store record for %s/%s changed while its teardown was starting (it named %s and now names %s); run it again: %w", acct, app, leased, now, release.ErrCASConflict)

+ }

if rec.Grant == nil && rec.Provisioned == nil {

return deleteRecordIfUnchanged(ctx, s, acct, app, startKeyID, startBucket)

}

@@ -827,6 +921,14 @@ func Teardown(ctx context.Context, s release.Store, p Provisioner, sc Scanner, a

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)

}

+ // And on the ledger, now: stores that left this bucket to another

+ // account are waiting to learn that it is gone, and this teardown

+ // may never reach its close (its app can take the store back).

+ if ValidBucketName(bucket) {

+ if err := markBucketDestroyed(ctx, s, bucket, acct, app, rec.Teardown.BucketDeletedAt); err != nil {

+ return fmt.Errorf("objstore: record the destruction of %s: %w", bucket, err)

+ }

}

// DeleteUser reaps the keys and the inline policy, so the credential dies

// with the estate it was scoped to — no orphan principal keeps a key that

@@ -937,7 +1039,10 @@ func Teardown(ctx context.Context, s release.Store, p Provisioner, sc Scanner, a

// still holds any key, and the box-side auto-purge never reaches it

// because the tombstone goes last. Delete is idempotent on an absent key,

// so a store that was never swept costs one no-op.

- if err := s.Delete(ctx, ScanKey(acct, app)); err != nil {

+ // It is deleted as the generation this walk read, like the record

+ // below: a delete that arrives late must not remove a sidecar written

+ // since by a store granted again under this name.

+ if err := deleteGeneration(ctx, s, ScanKey(acct, app)); err != nil {

return fmt.Errorf("objstore: delete scan sidecar for %s/%s: %w", acct, app, err)

}

// Last (§9). Everything it named is gone.

@@ -951,7 +1056,7 @@ func Teardown(ctx context.Context, s release.Store, p Provisioner, sc Scanner, a

// out here and can still run there, so the lease is not freed.

func walkEndedClean(err error) bool {

return err == nil || errors.Is(err, ErrBucketRetained) || errors.Is(err, ErrBucketHeld) ||

- errors.Is(err, ErrBucketBeingDestroyed) || errors.Is(err, ErrNotDeclared)

+ errors.Is(err, ErrBucketBeingDestroyed) || errors.Is(err, ErrNotDeclared) || errors.Is(err, ErrNoPendingFinish)

}

// grantElapsed is how long a grant has been running. A seam, so a test can

@@ -1038,7 +1143,7 @@ func otherStoreHolders(ctx context.Context, s release.Store, bucket, acct, app s

// resolving, the sentence above is a claim about nothing and this window is a

// whole walk wide again.

func deleteRecordIfUnchanged(ctx context.Context, s release.Store, acct, app, startKeyID, startBucket string) error {

- fresh, _, err := Get(ctx, s, acct, app)

+ fresh, etag, err := Get(ctx, s, acct, app)

if err != nil {

return fmt.Errorf("objstore: re-read store record for %s/%s: %w", acct, app, err)

}

@@ -1055,12 +1160,34 @@ func deleteRecordIfUnchanged(ctx context.Context, s release.Store, acct, app, st

if keyID != startKeyID || bucket != startBucket {

return fmt.Errorf("objstore: store record for %s/%s changed during teardown — re-run: %w", acct, app, release.ErrCASConflict)

}

- if err := s.Delete(ctx, Key(acct, app)); err != nil {

+ // The delete names the generation just read. A re-read followed by an

+ // unconditional delete leaves a gap a grant can land in, and a delete

+ // that reaches the backend late (its response lost, the walk long

+ // over) would remove whatever record stands there by then. Conditional

+ // on the generation, neither can remove a record this walk did not

+ // read: the backend refuses, and the teardown is run again.

+ if err := release.DeleteIfMatch(ctx, s, Key(acct, app), etag); err != nil {

+ if errors.Is(err, release.ErrCASConflict) {

+ return fmt.Errorf("objstore: store record for %s/%s changed during teardown — re-run: %w", acct, app, err)

+ }

return fmt.Errorf("objstore: delete store record for %s/%s: %w", acct, app, err)

}

return nil

}

+// deleteGeneration deletes the object at key as the generation it reads

+// now, or does nothing if there is none. It never deletes unconditionally.

+func deleteGeneration(ctx context.Context, s release.Store, key string) error {

+ _, etag, err := s.Get(ctx, key)

+ if errors.Is(err, os.ErrNotExist) {

+ return nil

+ }

+ if err != nil {

+ return err

+ }

+ return release.DeleteIfMatch(ctx, s, key, etag)

// writeEnv delivers the whole coordinate set in ONE appconfig.Apply — one CAS

// write, so an app never reads a bucket name paired with the previous key.

// Both key halves are sealed for the deployment's secrets key; the plaintext

diff --git a/internal/objstore/walk_sim_test.go b/internal/objstore/walk_sim_test.go

index 5ee5ec522..e4c584af7 100644

--- a/internal/objstore/walk_sim_test.go

+++ b/internal/objstore/walk_sim_test.go

@@ -35,10 +35,25 @@ import (

// Quiet. A teardown that reports success has not left its store's

// objects where another account's credentials can read them.

-// The walks are not only run one after another. With some probability a

-// second walk (another app's grant or teardown) runs in the MIDDLE of the

-// first, at the moment the first calls the provider to create, empty or

-// delete a bucket or a user: the interleaving of two console processes.

+// What the schedules contain, and what they do not.

+//

+// A second walk is started in the MIDDLE of the first, at the moment the

+// first calls the provider to create, empty or delete a bucket or a user.

+// With the lease in place that second walk is refused as busy before it

+// does anything, which is the lease working, and tests little else. So in

+// some of those moments the first walk is instead FROZEN for longer than

+// its lease, as a stopped process would be. The second walk (the same

+// app's or another's) then takes the name over and runs. The first walk

+// wakes on its stale lease, and everything it does from there, to the

+// provider and to the store, must be refused by the backend's fence.

+//

+// Failures come in two kinds: before the effect (the call did nothing) and

+// after it (the call ran and its answer was lost), for the provider and

+// for the store's writes.

+//

+// Outside the model: a second bucket name, rotations, re-declaration under

+// a different prefix, stores with no ledger interval, independent clocks,

+// and the SDK's retries. Those have their own tests, not random schedules.

// Settles. With no more failures, re-running every unfinished teardown

// reaches: no bucket, no user, no record, no scanner plan, no

@@ -58,26 +73,101 @@ func (a *simApp) name() string { return a.acct + "/" + a.app }

type interleavingProvisioner struct {

*MemProvisioner

between func(method string)

+ // lost names a call that runs and then answers an error: the effect

+ // happened and the walk does not know it.

+ lost string

+func (p *interleavingProvisioner) answer(method string, err error) error {

+ if err == nil && p.lost == method {

+ p.lost = ""

+ return errors.New("injected " + method + ", after it ran")

+ }

+ return err

}

func (p *interleavingProvisioner) EnsureBucket(ctx context.Context, name string) (string, error) {

p.between("EnsureBucket")

- return p.MemProvisioner.EnsureBucket(ctx, name)

+ region, err := p.MemProvisioner.EnsureBucket(ctx, name)

+ return region, p.answer("EnsureBucket", err)

}

func (p *interleavingProvisioner) EmptyBucket(ctx context.Context, name string) error {

p.between("EmptyBucket")

- return p.MemProvisioner.EmptyBucket(ctx, name)

+ return p.answer("EmptyBucket", p.MemProvisioner.EmptyBucket(ctx, name))

}

func (p *interleavingProvisioner) DeleteBucket(ctx context.Context, name string) error {

p.between("DeleteBucket")

- return p.MemProvisioner.DeleteBucket(ctx, name)

+ return p.answer("DeleteBucket", p.MemProvisioner.DeleteBucket(ctx, name))

}

func (p *interleavingProvisioner) DeleteUser(ctx context.Context, user string) error {

p.between("DeleteUser")

- return p.MemProvisioner.DeleteUser(ctx, user)

+ return p.answer("DeleteUser", p.MemProvisioner.DeleteUser(ctx, user))

+// simStore is the deployment bucket as a walk reaches it: it refuses a

+// request whose walk's lease is too old, as AWS does for the store's own

+// client (the fenced signer is installed there too), and it can fail a

+// write before or after the write happened.

+type simStore struct {

+ release.Store

+ failPut string // "", "before" or "after"

+func (s *simStore) Get(ctx context.Context, key string) ([]byte, string, error) {

+ if err := memFenced(ctx); err != nil {

+ return nil, "", err

+ }

+ return s.Store.Get(ctx, key)

+func (s *simStore) Put(ctx context.Context, key string, body []byte, ifMatch string) (string, error) {

+ if err := memFenced(ctx); err != nil {

+ return "", err

+ }

+ // Only a walk's own writes fail: the lease object is taken and given

+ // back outside the fence, and a failure there is a refusal to start.

+ _, fenced := fenceFrom(ctx)

+ if fenced && s.failPut == "before" {

+ s.failPut = ""

+ return "", errors.New("injected store write, before it was written")

+ }

+ etag, err := s.Store.Put(ctx, key, body, ifMatch)

+ if fenced && err == nil && s.failPut == "after" {

+ s.failPut = ""

+ return "", errors.New("injected store write, after it was written")

+ }

+ return etag, err

+func (s *simStore) List(ctx context.Context, prefix string) ([]string, error) {

+ if err := memFenced(ctx); err != nil {

+ return nil, err

+ }

+ return s.Store.List(ctx, prefix)

+func (s *simStore) ListDirs(ctx context.Context, prefix string) ([]string, error) {

+ if err := memFenced(ctx); err != nil {

+ return nil, err

+ }

+ return s.Store.ListDirs(ctx, prefix)

+func (s *simStore) Delete(ctx context.Context, key string) error {

+ if err := memFenced(ctx); err != nil {

+ return err

+ }

+ return s.Store.Delete(ctx, key)

+func (s *simStore) DeleteIfMatch(ctx context.Context, key, etag string) error {

+ if err := memFenced(ctx); err != nil {

+ return err

+ }

+ return release.DeleteIfMatch(ctx, s.Store, key, etag)

}

func TestBucketNameInvariantsUnderRandomWalks(t *testing.T) {

@@ -100,11 +190,32 @@ func TestBucketNameInvariantsUnderRandomWalks(t *testing.T) {

}

+// simTransientFault makes the simulation break an invariant for the length

+// of one mid-walk check and repair it. Only the oracle's own test sets it.

+var simTransientFault bool

+// The mid-walk check is real: a violation that exists only between a nested

+// walk's return and the outer walk's next step is reported, not repaired

+// out of sight.

+func TestSimulationOracleSeesATransientViolation(t *testing.T) {

+ simTransientFault = true

+ defer func() { simTransientFault = false }()

+ for seed := int64(0); seed < 200; seed++ {

+ if _, problem := simulateBucketName(t, seed, 40); strings.Contains(problem, "in the middle of") && strings.Contains(problem, "Data:") {

+ return

+ }

+ t.Fatal("200 seeds with a transient fault injected and the oracle reported none of them")

func simulateBucketName(t *testing.T, seed int64, steps int) (trace []string, problem string) {

t.Helper()

ctx := context.Background()

rng := rand.New(rand.NewSource(seed))

+ // s is what the simulation itself reads and writes. walked is the same

+ // store as a walk reaches it.

s := release.NewDirStore(t.TempDir())

+ walked := &simStore{Store: s}

_, pub := testSealKeypair(t)

mem := NewMemProvisioner("eu-west-1")

ip := &interleavingProvisioner{MemProvisioner: mem, between: func(string) {}}

@@ -125,8 +236,12 @@ func simulateBucketName(t *testing.T, seed int64, steps int) (trace []string, pr

declare(t, s, a.acct, a.app, "S3")

}

was := a.state

- _, err := Grant(ctx, s, p, testDeployment(), pub, a.acct, a.app, "op", GrantSourceOperator, a.adopt, at)

+ _, err := Grant(ctx, walked, p, testDeployment(), pub, a.acct, a.app, "op", GrantSourceOperator, a.adopt, at)

logf("grant %s: %v", a.name(), err)

+ if errors.Is(err, ErrBucketBusy) {

+ // Refused before it began. The store is what it was.

+ return

+ }

if err != nil {

// A refused or failed grant does not un-report a retained

// bucket, and does not take a live store's data away.

@@ -170,7 +285,7 @@ func simulateBucketName(t *testing.T, seed int64, steps int) (trace []string, pr

was := a.state

a.state = "broken"

estate := rec.Grant != nil || rec.Provisioned != nil

- err := Teardown(ctx, s, p, sc, a.acct, a.app)

+ err := Teardown(ctx, walked, p, sc, a.acct, a.app)

logf("teardown %s: %v", a.name(), err)

if errors.Is(err, ErrBucketBusy) {

// Refused before it began: another walk holds the name. The

@@ -208,12 +323,19 @@ func simulateBucketName(t *testing.T, seed int64, steps int) (trace []string, pr

}

return nil

}

+ // midWalk is the app whose walk is in progress when the invariants are

+ // checked in the middle of it. Its own state is in transit and is not

+ // judged; everyone else's is.

+ var midWalk *simApp

check := func() string {

if mintLeak != "" {

return mintLeak

}

bucket, exists := mem.Buckets[ledgerBucket]

for _, a := range apps {

+ if a == midWalk {

+ continue

+ }

switch a.state {

case "live":

if !exists {

@@ -298,11 +420,18 @@ func simulateBucketName(t *testing.T, seed int64, steps int) (trace []string, pr

snapshot()

at = at.Add(30 * time.Minute)

a := apps[rng.Intn(len(apps))]

- mem.FailNext = nil

- if rng.Intn(3) == 0 {

+ mem.FailNext, ip.lost, walked.failPut = nil, "", ""

+ switch rng.Intn(9) {

+ case 0, 1, 2:

f := faults[rng.Intn(len(faults))]

mem.FailNext = map[string]error{f: errors.New("injected " + f)}

logf(" next %s fails", f)

+ case 3:

+ ip.lost = []string{"EnsureBucket", "EmptyBucket", "DeleteBucket", "DeleteUser"}[rng.Intn(4)]

+ logf(" next %s runs and its answer is lost", ip.lost)

+ case 4:

+ walked.failPut = []string{"before", "after"}[rng.Intn(2)]

+ logf(" the walk's next store write fails %s it is written", walked.failPut)

}

// Sometimes another app's walk runs in the middle of this one, at

// one of its provider calls. Only one level deep, and the data

@@ -315,17 +444,47 @@ func simulateBucketName(t *testing.T, seed int64, steps int) (trace []string, pr

return

}

inner = true

+ // One time in three the outer walk is frozen here for longer

+ // than its lease. What runs next takes the name over, and may

+ // be the same app's walk, as a second process retrying would.

+ frozen := rng.Intn(3) == 0

b := apps[rng.Intn(len(apps))]

- if b == a {

+ if b == a && !frozen {

inner = false

return

}

+ if frozen {

+ at = at.Add(walkLease + time.Minute)

+ logf(" -- %s's walk is frozen at %s until its lease has run out", a.name(), method)

+ }

logf(" -- during %s of %s:", method, a.name())

if rng.Intn(2) == 0 {

grant(b, at)

} else {

_ = teardown(b)

}

+ // The invariants hold the moment the nested walk returns, for

+ // every app but the one whose walk it ran inside. Checked here

+ // and kept: the outer walk could otherwise repair what the

+ // nested one broke before anything looked.

+ midWalk = a

+ if simTransientFault {

+ // The oracle's own test: break another live app's data

+ // for the length of this check and put it back, as a

+ // faulty nested walk repaired by the outer one would.

+ for _, o := range apps {

+ if o != a && o.state == "live" && mem.Buckets[ledgerBucket] != nil {

+ saved := mem.Buckets[ledgerBucket][object(o)]

+ delete(mem.Buckets[ledgerBucket], object(o))

+ defer func() { mem.Buckets[ledgerBucket][object(o)] = saved }()

+ break

+ }

+ if problem := check(); problem != "" && midProblem == "" {

+ midProblem = fmt.Sprintf("in the middle of %s's walk, after %s's: %s", a.name(), b.name(), problem)

+ }

+ midWalk = nil

logf(" -- back to %s", a.name())

inner = false

}

@@ -363,7 +522,7 @@ func simulateBucketName(t *testing.T, seed int64, steps int) (trace []string, pr

}

// No more failures. Everything unfinished is re-run until nothing moves.

- mem.FailNext = nil

+ mem.FailNext, ip.lost, walked.failPut = nil, "", ""

ip.between = func(string) {}

logf("-- settle --")

for round := 0; round < 8; round++ {

diff --git a/internal/objstore/walklock.go b/internal/objstore/walklock.go

index 0eea6e2fe..ef8ed523d 100644

--- a/internal/objstore/walklock.go

+++ b/internal/objstore/walklock.go

@@ -11,6 +11,7 @@ import (

"time"

"github.com/carlosframework/platform/internal/release"

+ "github.com/carlosframework/platform/internal/sigfence"

)

// The walk lease: one walk at a time on a bucket name, across every

@@ -96,6 +97,10 @@ type bucketWalk struct {

lock walkLock

etag string

closed bool

+ // doubt is raised when a request of the walk needed more than one

+ // attempt. The earlier attempt may still arrive, so the walk does not

+ // free the name early however it ended.

+ doubt *sigfence.Doubt

}

// acquireBucketWalk takes the bucket's lease for one invocation, or

@@ -155,7 +160,8 @@ func (w *bucketWalk) bound(ctx context.Context) (context.Context, context.Cancel

// The time left is measured on the lease's clock and counted down on

// the context's own.

left := w.lock.SignedAt.Add(fenceWindow - walkMargin).Sub(walkClock())

- return context.WithTimeout(withFence(ctx, w.lock.SignedAt), left)

+ ctx, w.doubt = sigfence.Track(withFence(ctx, w.lock.SignedAt))

+ return context.WithTimeout(ctx, left)

}

// held reports whether the lease is still this invocation's: the object

@@ -175,9 +181,9 @@ func (w *bucketWalk) held(ctx context.Context) (bool, error) {

return cur.Token == w.lock.Token, nil

}

-// release gives the lease up. clean says the walk returned without error:

-// every call it made was answered, so nothing of it can still be on its

-// way, and the name is freed (unless an attempt it took over from has not

+// release gives the lease up. clean says the walk returned without error

+// and every request it made was answered on its first attempt, so nothing

+// of it can still be on its way, and the name is freed (unless an attempt it took over from has not

// run out). Otherwise the lease is marked ended and stands until it runs

// out.

@@ -189,6 +195,9 @@ func (w *bucketWalk) release(clean bool) {

return

}

w.closed = true

+ // A walk that returned success after a request was attempted twice has

+ // not had every attempt answered. It ends like a walk that failed.

+ clean = clean && !w.doubt.Raised()

done := w.lock

done.Ended = true

if clean {

diff --git a/internal/release/store.go b/internal/release/store.go

index c296106f7..cd45453c5 100644

--- a/internal/release/store.go

+++ b/internal/release/store.go

@@ -11,6 +11,7 @@ import (

"encoding/hex"

"errors"

"fmt"

+ "github.com/carlosframework/platform/internal/sigfence"

"io"

"io/fs"

"net/http"

@@ -543,6 +544,16 @@ func NewS3Store(ctx context.Context, bucket string) (*S3Store, error) {

// deployment that cannot be tested from a compatible store.

o.RequestChecksumCalculation = aws.RequestChecksumCalculationWhenRequired

o.ResponseChecksumValidation = aws.ResponseChecksumValidationWhenRequired

+ // A bucket-name walk (internal/objstore) writes its store record,

+ // its ownership ledger and its delivered variables through this

+ // client. Those writes are fenced like the walk's own provider

+ // calls: under a context that carries a walk's fence the request is

+ // stamped with the walk's lease instant, and a second attempt is

+ // recorded on the walk. Under any other context the signer signs as

+ // the SDK's own S3 signer does and nothing is recorded. The retryer

+ // is not touched.

+ o.HTTPSignerV4 = sigfence.NewS3Signer()

+ o.APIOptions = append(o.APIOptions, sigfence.WatchAttempts)

})

return &S3Store{Client: client, Bucket: bucket}, nil

}

diff --git a/internal/sigfence/sigfence.go b/internal/sigfence/sigfence.go

new file mode 100644

index 000000000..77a401fe3

--- /dev/null

+++ b/internal/sigfence/sigfence.go

@@ -0,0 +1,155 @@

+// Package sigfence makes a lease mean something at the backend.

+//

+// A walk (internal/objstore) holds a lease on a bucket name and makes many

+// AWS calls under it. A lease the walk checks before a call is not enough:

+// the process can be paused after the check, and wake after the lease has

+// passed to another walk. Only the backend can refuse the request it then

+// sends.

+//

+// AWS signs every request with a timestamp and refuses one whose timestamp

+// is more than fifteen minutes from its own clock. The signer here stamps

+// the instant the walk's lease was taken, in place of "now", on every

+// request made with a context that carries a fence (With). A request sent

+// after that window is refused by AWS whenever it is sent.

+//

+// It has to cover every remote mutation a walk makes, on every client the

+// walk reaches: the object-store provisioner's S3 and IAM clients, and the

+// deployment bucket's own S3 client (internal/release), which holds the

+// store records and the ownership ledgers. A client without this signer is

+// a way round the fence.

+//

+// The second thing it carries is doubt. The SDK retries a request whose

+// first attempt failed ambiguously, and reports success if the retry

+// succeeds. The first attempt was never shown not to have run: it can

+// still arrive, inside its signature's window. A walk in which any request

+// needed more than one attempt therefore does not give its lease back

+// early. WatchAttempts records that on the walk's context (Track).

+package sigfence

+import (

+ "context"

+ "net/http"

+ "sync/atomic"

+ "time"

+ "github.com/aws/aws-sdk-go-v2/aws"

+ v4 "github.com/aws/aws-sdk-go-v2/aws/signer/v4"

+ "github.com/aws/smithy-go/middleware"

+// Window is how far a request's signing time may be from AWS's clock

+// before AWS refuses it.

+const Window = 15 * time.Minute

+type fenceKey struct{}

+// With marks every request made with ctx as part of a walk whose lease

+// was taken at `at`.

+func With(ctx context.Context, at time.Time) context.Context {

+ return context.WithValue(ctx, fenceKey{}, at.UTC())

+// From returns the fence ctx carries.

+func From(ctx context.Context) (time.Time, bool) {

+ at, ok := ctx.Value(fenceKey{}).(time.Time)

+ return at, ok

+// Signer signs with the walk's lease instant when the request's context

+// carries a fence, and with the time it is given otherwise. With no fence

+// it is the SDK's own signer.

+type Signer struct {

+ inner interface {

+ SignHTTP(ctx context.Context, credentials aws.Credentials, r *http.Request, payloadHash string, service string, region string, signingTime time.Time, optFns ...func(*v4.SignerOptions)) error

+ }

+// NewSigner returns the signer to install as a client's HTTPSignerV4, for

+// every service but S3.

+func NewSigner() Signer { return Signer{inner: v4.NewSigner()} }

+// NewS3Signer is NewSigner for an S3 client. S3 signs the object key as it

+// is written, without escaping it a second time, and the SDK builds its

+// own S3 signer that way. A replacement that did not would sign any key

+// with a character that needs escaping wrongly.

+func NewS3Signer() Signer {

+ return Signer{inner: v4.NewSigner(func(o *v4.SignerOptions) { o.DisableURIPathEscaping = true })}

+func (f Signer) SignHTTP(ctx context.Context, credentials aws.Credentials, r *http.Request, payloadHash string, service string, region string, signingTime time.Time, optFns ...func(*v4.SignerOptions)) error {

+ if at, ok := From(ctx); ok {

+ signingTime = at

+ }

+ return f.inner.SignHTTP(ctx, credentials, r, payloadHash, service, region, signingTime, optFns...)

+// Doubt records that some request made with a context needed more than

+// one attempt, or was otherwise left with an attempt that may still

+// arrive.

+type Doubt struct{ raised atomic.Bool }

+// Raise records the doubt.

+func (d *Doubt) Raise() {

+ if d != nil {

+ d.raised.Store(true)

+ }

+// Raised reports whether any was recorded.

+func (d *Doubt) Raised() bool { return d != nil && d.raised.Load() }

+type doubtKey struct{}

+// Track returns ctx carrying a Doubt that the retryer raises.

+func Track(ctx context.Context) (context.Context, *Doubt) {

+ d := &Doubt{}

+ return context.WithValue(ctx, doubtKey{}, d), d

+// Raise records doubt on the walk ctx belongs to, if it belongs to one.

+func Raise(ctx context.Context) {

+ if d, ok := ctx.Value(doubtKey{}).(*Doubt); ok {

+ d.Raise()

+ }

+// WatchAttempts is installed on a fenced client's APIOptions. It raises

+// the walk's doubt whenever a request is attempted more than once, whatever

+// retryer the client uses and without changing it: one middleware runs once

+// for the operation and starts a count, one runs for every attempt and

+// raises the doubt at the second. A retry after a definite refusal (a

+// throttle) raises it too. That is more cautious than it needs to be and

+// costs only time: the walk's name stays reserved until its lease runs out.

+func WatchAttempts(stack *middleware.Stack) error {

+ // An operation built without the SDK's retry step (a presigned

+ // request) is attempted once by construction. There is nothing to

+ // watch, and failing here would fail the operation.

+ if _, ok := stack.Finalize.Get("Retry"); !ok {

+ return nil

+ }

+ if err := stack.Finalize.Insert(operationStart{}, "Retry", middleware.Before); err != nil {

+ return err

+ }

+ return stack.Finalize.Insert(everyAttempt{}, "Retry", middleware.After)

+type attemptsKey struct{}

+type operationStart struct{}

+func (operationStart) ID() string { return "sigfence.OperationStart" }

+func (operationStart) HandleFinalize(ctx context.Context, in middleware.FinalizeInput, next middleware.FinalizeHandler) (middleware.FinalizeOutput, middleware.Metadata, error) {

+ return next.HandleFinalize(context.WithValue(ctx, attemptsKey{}, new(atomic.Int32)), in)

+type everyAttempt struct{}

+func (everyAttempt) ID() string { return "sigfence.EveryAttempt" }

+func (everyAttempt) HandleFinalize(ctx context.Context, in middleware.FinalizeInput, next middleware.FinalizeHandler) (middleware.FinalizeOutput, middleware.Metadata, error) {

+ if n, ok := ctx.Value(attemptsKey{}).(*atomic.Int32); ok && n.Add(1) > 1 {

+ Raise(ctx)

+ }

+ return next.HandleFinalize(ctx, in)

diff --git a/internal/sigfence/sigfence_test.go b/internal/sigfence/sigfence_test.go

new file mode 100644

index 000000000..a39db682b

--- /dev/null

+++ b/internal/sigfence/sigfence_test.go

@@ -0,0 +1,123 @@

+package sigfence

+import (

+ "context"

+ "net/http"

+ "net/http/httptest"

+ "strings"

+ "testing"

+ "time"

+ "github.com/aws/aws-sdk-go-v2/aws"

+ "github.com/aws/aws-sdk-go-v2/credentials"

+ awss3 "github.com/aws/aws-sdk-go-v2/service/s3"

+ "github.com/aws/smithy-go/middleware"

+// A request made inside a walk is signed with the walk's lease instant,

+// not with the time it happens to be sent.

+func TestSignerStampsTheFence(t *testing.T) {

+ leased := time.Date(2026, 10, 1, 3, 0, 0, 0, time.UTC)

+ sent := leased.Add(40 * time.Minute)

+ creds := aws.Credentials{AccessKeyID: "AKIDEXAMPLE", SecretAccessKey: "secret"}

+ sign := func(ctx context.Context) string {

+ r, _ := http.NewRequest("DELETE", "https://bucket.s3.eu-west-1.amazonaws.com/", nil)

+ if err := NewSigner().SignHTTP(ctx, creds, r, "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", "s3", "eu-west-1", sent); err != nil {

+ t.Fatal(err)

+ }

+ return r.Header.Get("X-Amz-Date")

+ }

+ if got := sign(With(context.Background(), leased)); got != "20261001T030000Z" {

+ t.Fatalf("fenced request stamped %s, want the lease instant", got)

+ }

+ if got := sign(context.Background()); got != "20261001T034000Z" {

+ t.Fatalf("unfenced request stamped %s, want the time it was sent", got)

+ }

+ // The credential scope's date follows the stamp, so a request held

+ // across midnight is still stamped whole.

+ r, _ := http.NewRequest("GET", "https://iam.amazonaws.com/", nil)

+ night := time.Date(2026, 10, 1, 23, 59, 0, 0, time.UTC)

+ if err := NewSigner().SignHTTP(With(context.Background(), night), creds, r, "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", "iam", "us-east-1", night.Add(10*time.Minute)); err != nil {

+ t.Fatal(err)

+ }

+ if !strings.Contains(r.Header.Get("Authorization"), "/20261001/us-east-1/iam/") {

+ t.Fatalf("credential scope does not follow the stamp: %s", r.Header.Get("Authorization"))

+ }

+// Against a real S3 client: the first attempt of a request fails in a way

+// the SDK retries, and the second succeeds. The caller sees success. The

+// walk's doubt is raised all the same, because the first attempt was never

+// shown not to have run. And both attempts carry the lease's stamp: a retry

+// is signed again, and must not be signed with the time of the retry.

+func TestRetriedRequestRaisesDoubtAndKeepsItsStamp(t *testing.T) {

+ var stamps []string

+ srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

+ stamps = append(stamps, r.Header.Get("X-Amz-Date"))

+ if len(stamps) == 1 {

+ w.WriteHeader(http.StatusInternalServerError)

+ return

+ }

+ w.WriteHeader(http.StatusNoContent)

+ }))

+ defer srv.Close()

+ client := awss3.New(awss3.Options{

+ Region: "eu-west-1", BaseEndpoint: aws.String(srv.URL), UsePathStyle: true,

+ Credentials: credentials.NewStaticCredentialsProvider("AKIDEXAMPLE", "secret", ""),

+ HTTPSignerV4: NewS3Signer(),

+ APIOptions: []func(*middleware.Stack) error{WatchAttempts},

+ })

+ leased := time.Now().Add(-10 * time.Minute).UTC().Truncate(time.Second)

+ ctx, doubt := Track(With(context.Background(), leased))

+ if _, err := client.DeleteBucket(ctx, &awss3.DeleteBucketInput{Bucket: aws.String("b")}); err != nil {

+ t.Fatalf("the retried request: %v", err)

+ }

+ if len(stamps) != 2 {

+ t.Fatalf("%d attempts, want 2", len(stamps))

+ }

+ want := leased.Format("20060102T150405Z")

+ if stamps[0] != want || stamps[1] != want {

+ t.Fatalf("attempts stamped %v, want both %s", stamps, want)

+ }

+ if !doubt.Raised() {

+ t.Fatal("a request that needed a second attempt left no doubt on its walk")

+ }

+ // One attempt, answered: no doubt. And a context that belongs to no

+ // walk is signed with the time it is sent and records nothing.

+ ctx, doubt = Track(With(context.Background(), leased))

+ if _, err := client.DeleteBucket(ctx, &awss3.DeleteBucketInput{Bucket: aws.String("b")}); err != nil {

+ t.Fatal(err)

+ }

+ if doubt.Raised() {

+ t.Fatal("a request answered on its first attempt raised doubt")

+ }

+ if _, err := client.DeleteBucket(context.Background(), &awss3.DeleteBucketInput{Bucket: aws.String("b")}); err != nil {

+ t.Fatal(err)

+ }

+ if stamps[len(stamps)-1] == want {

+ t.Fatal("a request outside any walk was stamped with a lease instant")

+ }

+ var none *Doubt

+ none.Raise()

+ if none.Raised() {

+ t.Fatal("a nil doubt reports raised")

+ }

+// An S3 key with characters that need escaping is signed the way S3

+// verifies it: escaped once. The canonical request, and so the signature,

+// differs if the signer escapes it again.

+func TestS3SignerDoesNotEscapeTheKeyTwice(t *testing.T) {

+ creds := aws.Credentials{AccessKeyID: "AKIDEXAMPLE", SecretAccessKey: "secret"}

+ at := time.Date(2026, 10, 1, 3, 0, 0, 0, time.UTC)

+ sign := func(s Signer) string {

+ r, _ := http.NewRequest("GET", "https://bucket.s3.eu-west-1.amazonaws.com/apps/a%20b/c%2Bd.json", nil)

+ if err := s.SignHTTP(context.Background(), creds, r, "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", "s3", "eu-west-1", at); err != nil {

+ t.Fatal(err)

+ }

+ return r.Header.Get("Authorization")

+ }

+ if sign(NewS3Signer()) == sign(NewSigner()) {

+ t.Fatal("the S3 signer signs an escaped key exactly as the general signer does: it escapes it twice")

+ }