Record review round three, its dispositions, and what the fixes changed
The Astra and Opus round 3 reviews are committed unedited. The dispositions file lists each finding, its fix and the reproduction that fails without it, the tests the reviewers showed were weak and what changed, and three things found while fixing and not fixed: a retried teardown can delete a bucket another app has since been granted, the provider's refusal of an out-of-period proration date is unconfirmed in test mode, and a host test that had been passing by luck. The plans are corrected against the code: partial-month billing is built (a month with no paid day is still refused quietly), the attestation plan no longer claims the fold unions ledgers, the collector plan records that credits and discounts hold the month and that no ledger on AWS can claim completeness without a principal inventory. The handoff, the short handoff and the requirements audit are refreshed, and the handoff records a gate environment that keeps build artifacts out of the shared /tmp.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
10 files changed,
+302
−27
.claude/HANDOFF.md+6 −5docs/superpowers/plans/2026-10-01-bucket-ownership-attestation.md+1 −1docs/superpowers/plans/2026-10-01-durability-attestation-path.md+1 −1docs/superpowers/plans/2026-10-01-partial-month-billing.md+13 −4docs/superpowers/plans/2026-10-01-store-transfer-cur-collector.md+3 −0docs/superpowers/specs/CODEX-DISPOSITIONS-carloku-paid-launch-overnight.md+40 −0docs/superpowers/specs/CODEX-FINDINGS-carloku-paid-launch-overnight-round3.md+95 −0docs/superpowers/specs/OPUS-REVIEW-carloku-paid-launch-overnight-round3.md+125 −0implementation/carloku-paid-launch/AGENT_HANDOFF.md+13 −11implementation/carloku-paid-launch/requirements-audit.json+5 −5
CI cancelled — superseded by a newer push — run details
diff --git a/.claude/HANDOFF.md b/.claude/HANDOFF.md| index a1c0e57d3..cac9fbc46 100644 |
| --- a/.claude/HANDOFF.md |
| +++ b/.claude/HANDOFF.md |
| @@ -7,16 +7,17 @@ |
| Get Carloku's paid plans live with Stripe Tax and the complete metered model; have the pricing page and signup flow reviewable in the morning. |
| **STATUS:** |
| -Code tip 8e7499ae (later commits are documents and the regenerated preview only). On that commit: `go build`, `go vet`, gofmt and `go test ./...` plus the vendored Litestream package passed outside the sandbox (60 packages ok, log `logs/gate-review-fixes.txt` in the run directory) and all five cross builds passed. Amadan CI for the tip must still be read. Independent review: round 1 (GPT-6 Astra) rejected with ten findings, all accepted and fixed with reproductions (`docs/superpowers/specs/CODEX-DISPOSITIONS-carloku-paid-launch-overnight.md`); round 2 by Astra and the AGENTS-required Opus production review were started against 8e7499ae and their results are not yet in. Landed tonight: console GET billing page, signup and billing demo harness, durability epoch race fix, unmetered-fallback escapes, AWS Cost and Usage Report transfer collector with bucket ownership ledger, statement folds for both new meters, operator doors for review holds and the house marker, team quantity worker behind an unset policy switch, Stripe provisioning plan and print-only script. Review servers running on docs-t1: pricing http://docs-t1.tail5fd3ba.ts.net:38009 (fallback http://100.119.180.57:38009), billing page http://docs-t1.tail5fd3ba.ts.net:45033 (fallback http://100.119.180.57:45033), interactive demo http://100.119.180.57:41881. Billing is OFF; live Stripe untouched; no AWS or production change. Copy on both pages is a fresh draft: the 2026-09-22 approvals were lost and are not claimed. |
| +Code tip c3913dd9 (later commits are documents only). On that commit: `go build`, `go vet`, gofmt and `make test` passed outside the sandbox (60 packages ok, log `logs/gate-round3-fixes.txt` in the run directory). Cross builds and Amadan CI for the tip: see the Amadan branch handoff, which is updated when they are read. Independent review: GPT-6 Astra rejected rounds 1, 2 and 3 (ten, eight, five findings); the AGENTS-required Opus production review accepted with fixes in rounds 2 and 3 (eight findings each). Every finding was accepted and fixed with a reproduction (`docs/superpowers/specs/CODEX-DISPOSITIONS-carloku-paid-launch-overnight.md`). Round four of both runs against the tip; nothing lands until it passes. Landed in this run: console GET billing page, signup and billing demo harness, durability epoch race fix, unmetered-fallback escapes, AWS Cost and Usage Report transfer collector with bucket ownership ledger, statement folds for both new meters, partial-month billing on whole paid days, operator doors for review holds and the house marker, team quantity worker behind an unset policy switch, Stripe provisioning plan and print-only script. Review servers running on docs-t1, each answering 200 on its IP form at 02:00 on 2026-10-01: pricing http://docs-t1.tail5fd3ba.ts.net:38009 (fallback http://100.119.180.57:38009), console billing page http://docs-t1.tail5fd3ba.ts.net:45033 (fallback http://100.119.180.57:45033), signup and billing demo http://100.119.180.57:41881. The tailnet hostname was not verified from this box. Billing is OFF. No AWS, Stripe or production change was made. |
| **NEEDED:** |
| - Paul: approve or edit both copy reviews; confirm the HALF code word; decide the proration policy (`CONSOLE_BILLING_PRORATION` stays unset until then). |
| -- Scoped approvals, prepared as plans only: the rk_live restricted key and Stripe catalog (`implementation/carloku-paid-launch/stripe-provisioning.md`), the Cost and Usage Report export (`docs/superpowers/plans/2026-10-01-store-transfer-cur-collector.md`). |
| -- Decisions: how existing and adopted store buckets get an attested ownership history; which attestation path for replication writes (`docs/superpowers/plans/2026-10-01-durability-attestation-path.md`); whether a partial first month is waived. |
| +- Scoped approvals, prepared as plans only: the rk_live restricted key and Stripe catalog (`implementation/carloku-paid-launch/stripe-provisioning.md`); the Cost and Usage Report export (`docs/superpowers/plans/2026-10-01-store-transfer-cur-collector.md`); Stripe test mode, to confirm the provider refuses a proration date outside the current period. |
| +- A read of the AWS billing console: does account 271376211898 carry credits or discounts on S3 transfer? If so the transfer meter never completes as built. |
| +- A choice of attestation path for replication writes (`docs/superpowers/plans/2026-10-01-durability-attestation-path.md`). |
| **RECOMMENDED:** |
| -- Read the round 2 Astra findings and the Opus review, fix what they find, and re-review the exact tip before anything lands. |
| -- Then: quantity reduction at renewal, a `carlos` verb and docs for the operator doors, Free-tier enforcement, dormancy and pool accounting (none started). |
| +- Read the round-four reviews, fix what they find, re-review the exact tip. |
| +- Then: the AWS principal inventory and the bucket ownership attestation door, quantity reduction at renewal, a `carlos` verb and docs for the operator doors, Free-tier enforcement, dormancy and pool accounting (none started). |
| **SESSION:** |
| Not safe to close: three servers are serving on this box (pids and stop file under `/home/paulca/.local/share/carloku-paid-launch-20260930/logs`), and two review processes may still be running. |
diff --git a/docs/superpowers/plans/2026-10-01-bucket-ownership-attestation.md b/docs/superpowers/plans/2026-10-01-bucket-ownership-attestation.md| index 087096d91..e64f0b429 100644 |
| --- a/docs/superpowers/plans/2026-10-01-bucket-ownership-attestation.md |
| +++ b/docs/superpowers/plans/2026-10-01-bucket-ownership-attestation.md |
| @@ -4,7 +4,7 @@ Date: 2026-10-01. Status: proposal with the evidence it would rest on; not built |
| ## The problem |
| -Object-store transfer is attributed by `usage/buckets/<bucket>.json`. A ledger resolves an owner only when it can vouch that it names everyone the platform ever gave access to the bucket. Today that is true only for a bucket whose ledger was created by the grant that minted it. Two kinds of bucket cannot get there on their own: |
| +Object-store transfer is attributed by `usage/buckets/<bucket>.json`. A ledger resolves an owner only when it can vouch that it names everyone the platform ever gave access to the bucket. Today that is true only for a bucket whose ledger was created by the grant that minted it, when that grant could show the bucket did not exist and that no principal's policy named it (`objstore.GranteeProber`). The AWS provisioner cannot list principals yet, so on AWS no ledger claims completeness at all, new stores included. The IAM inventory read in step 2 below is therefore needed twice: by the grant, for every new store, and by this door. Beyond that, two kinds of bucket cannot get there on their own: |
| - the stores provisioned before ledgers existed (four on the flagship); |
| - any bucket an operator adopted, which existed and may have been reachable before the platform knew it. |
diff --git a/docs/superpowers/plans/2026-10-01-durability-attestation-path.md b/docs/superpowers/plans/2026-10-01-durability-attestation-path.md| index b360df866..0bdab3adb 100644 |
| --- a/docs/superpowers/plans/2026-10-01-durability-attestation-path.md |
| +++ b/docs/superpowers/plans/2026-10-01-durability-attestation-path.md |
| @@ -25,7 +25,7 @@ The external Litestream binary is the one writer that makes the attestation fals |
| **A. Scope the attestation (smaller, recommended first).** The ledger already supports attested spans with an exclusion set (`DurabilityAttestation{Attested, Unmetered}`). The agent knows which accounts own unit-backed routes (the registry). On open, and whenever the route set changes, it calls `SetAttestation` naming every account that owns a unit-backed route on the box. Accounts with only exec-backed routes are then attested; the others are excluded. Two things are needed before this is honest: the console fold must read completeness per account (today `SourceDays` is one number for every account, so an exclusion set holds everyone), and the agent must prove the set is complete at every instant, which means reading it under the same registry witness the ownership proof uses. Tight durability is a Business feature, and Business tenants on unit-backed routes would stay unbilled for writes until B. |
| -**B. Meter the root replicator (complete).** Replace the external binary in `carlos-litestream.service` with `carlos ops replicate`, running the vendored Litestream already in this tree with the request-meter middleware and its own ledger directory, attributing each database to its route's owner from the registry and the box's own databases to the platform purpose. Both ledgers publish under the box; the fold already unions every ledger directory it finds. After that every S3 replication client on the box is instrumented and `AllClientsInstrumented` can be set from a startup check that the external unit is absent. This changes a root unit on every box and needs a rollout plan and a restore drill before and after. |
| +**B. Meter the root replicator (complete).** Replace the external binary in `carlos-litestream.service` with `carlos ops replicate`, running the vendored Litestream already in this tree with the request-meter middleware and its own ledger directory, attributing each database to its route's owner from the registry and the box's own databases to the platform purpose. The fold reads one ledger per box (`usage/durability/<box>/days/<date>.json`, keyed by box in `durabilityfold.go`), so a second ledger on the same box cannot simply be published beside the first: it would overwrite it. Either the root replicator writes into the same ledger as the tenant replicator, or the path and the fold gain a ledger label under the box and the fold requires every expected label to be present for the day. That choice is part of building B and is not made here. After that every S3 replication client on the box is instrumented and `AllClientsInstrumented` can be set from a startup check that the external unit is absent. This changes a root unit on every box and needs a rollout plan and a restore drill before and after. |
| ## What would flip the flag, and the test that must exist first |
diff --git a/docs/superpowers/plans/2026-10-01-partial-month-billing.md b/docs/superpowers/plans/2026-10-01-partial-month-billing.md| index 76c7272ab..2b4ce2c7c 100644 |
| --- a/docs/superpowers/plans/2026-10-01-partial-month-billing.md |
| +++ b/docs/superpowers/plans/2026-10-01-partial-month-billing.md |
| @@ -1,10 +1,10 @@ |
| # Billing a month the paid plan covers only partly |
| -Date: 2026-10-01. Status: design; not built. Required work, not a waiver: the complete metered model must bill a first month that starts mid-month, a month a subscription ends in, and a free-to-paid change, correctly. |
| +Date: 2026-10-01. Status: built on `feat/carloku-paid-launch-20260916`, awaiting independent review. Required work, not a waiver: the complete metered model must bill a first month that starts mid-month, a month a subscription ends in, and a free-to-paid change, correctly. |
| -## What happens today |
| +## What happened before this was built |
| -`plans.AllowanceForMonth` refuses (`ErrAssignmentHistory`) any month not covered from its first instant to its last by active, paid assignments. The refusal is quiet: no review is filed, the invoice pass retries the month forever, and the month is never billed. Every subscriber's first month is in this state. |
| +`plans.AllowanceForMonth` refused (`ErrAssignmentHistory`) any month not covered from its first instant to its last by active, paid assignments. The refusal is quiet: no review is filed, the invoice pass retries the month forever, and the month is never billed. Every subscriber's first month is in this state. |
| It refuses for a real reason. A statement holds one total per meter for the month. With usage as a single monthly number, there is no way to say how much of it fell inside the paid part, and charging overage against usage from the free part would bill people for what Free allowed. |
| @@ -25,6 +25,15 @@ What this does not do: bill Free usage. Free is bounded by enforcement, which is |
| `internal/usage/statement.go` (the `Days` field and its invariant), `internal/console/economicspass.go` (accumulate per day in the sweep and both folds), `internal/plans/month.go` (intervals instead of refusal for an uncovered start or end), `internal/console/billingcalculation.go` (count whole paid days when the month is partial; record which days were counted in the frozen calculation so the invoice can be reproduced), and the tests that pin today's refusals, which change from "holds" to "bills the paid days". |
| +## As built |
| + |
| +- `plans.AllowanceForMonth` returns `Partial` and the `Covered` paid spans. It refuses only a history it cannot read, a month with no paid span, and paid spans that disagree on the terms version or the house marker. |
| +- `usage.Statement.Days` holds each meter by UTC day. `TestStatementDaysSumToTheMonthlyMeters` pins that the days sum to the monthly totals for every source. |
| +- `billingUsageOnPaidDays` counts a day only when its 24 hours lie inside one paid span. The frozen calculation records the counted days and names its arithmetic `base_units_floor_whole_paid_days_v1`. A statement with no daily meters for a partial month is refused with `billing_statement_days_unusable` and held, never billed on the monthly total. |
| +- `TestInvoiceEngineBillsAPartialFirstMonthOnItsPaidDays` bills a subscription that starts mid-month end to end; the renewal tests cover a partial month frozen behind a review. |
| + |
| +Not built: point 6. A month with no paid day is still refused by `AllowanceForMonth` (no paid span) and retried quietly; it is never billed, but it is not yet recorded as waived. |
| + |
| ## Order |
| -After the review findings on the branch are closed, because it changes the frozen calculation that the invoice engine and its recovery tests rest on, and it wants its own independent review. |
| +Built after the round-two findings were closed. It changes the frozen calculation that the invoice engine and its recovery tests rest on, so it is in the scope of review round four. |
diff --git a/docs/superpowers/plans/2026-10-01-store-transfer-cur-collector.md b/docs/superpowers/plans/2026-10-01-store-transfer-cur-collector.md| index 9f3e397cd..a9c2e0318 100644 |
| --- a/docs/superpowers/plans/2026-10-01-store-transfer-cur-collector.md |
| +++ b/docs/superpowers/plans/2026-10-01-store-transfer-cur-collector.md |
| @@ -37,6 +37,9 @@ Verification after apply: within 24 hours `billing/cur/carloku-transfer/metadata |
| - Scoped approval for the four changes above, then a first real delivery read end to end. |
| - Existing stores: the four stores already provisioned on the flagship have no ownership ledger, and a re-grant writes one that does not claim completeness, because credentials existed before the ledger did. Their charged transfer holds the month until an operator attests each bucket's history (who has ever been granted it). That door does not exist yet and must be designed: it grants billing authority. |
| - Confirm against the first real report that usage types, resource ids and the invoice id behave as the fixtures assume, in particular how the 100 GB free allowance is written across buckets. Anything unexpected arrives as `store_egress_unclassified_charge`, never as a bill. |
| +- **Credits and discounts hold the month.** AWS writes `Credit`, `Refund`, `Fee`, `EdpDiscount`, `PrivateRateDiscount` and `BundledDiscount` lines on the same transfer usage types as the usage they adjust. Any such line with a non-zero cost holds store transfer for the month for every account (`transfer_adjustment`), because AWS then charged something other than what the usage lines say, and nothing lets an operator release that hold. Before the export is approved, check whether account 271376211898 carries promotional credits, an enterprise discount or a private rate that touches S3 transfer. If it does, the meter will never complete as built, and the classifier needs a ruling on how a discounted byte is charged on before the export is worth creating. This is a read of the account's billing console, not a change. |
| +- **Existing stores on AWS never complete yet.** A ledger claims completeness only when the provisioner can list every principal whose policy names the bucket and finds none. The AWS provisioner cannot list principals today (the console role has no IAM list permission), so on AWS no ledger claims completeness, new stores included, and store transfer holds. The principal inventory is the first step of the ownership attestation plan (`2026-10-01-bucket-ownership-attestation.md`). |
| +- The statements catch-up revisits every month in a 36-month window that is open, was never written, or is missing an account's statement. It costs one read per account per metered month in a console's first pass and nothing after a statement has been seen closed. |
| - The per-month rebuild door for a statement closed before this existed is still missing (requirements audit step 1). |
| ## Scoped approval request |
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 ba0a1c966..e3d0645c5 100644 |
| --- a/docs/superpowers/specs/CODEX-DISPOSITIONS-carloku-paid-launch-overnight.md |
| +++ b/docs/superpowers/specs/CODEX-DISPOSITIONS-carloku-paid-launch-overnight.md |
| @@ -44,3 +44,43 @@ Reviewed commit `8e7499ae`. Astra round 2 (`CODEX-FINDINGS-carloku-paid-launch-o |
| | Opus N8, minor | Accepted: the idempotency key carries the behaviour and the period; a mismatch review is filed once while open; the no-op house change is not audited; the unavailable line is shown for Personal only; the stale comment is corrected. Not changed: after a crash between reconcile and mark the outcome is recorded as `included` with reference `quantity_already_billed`, and `charged` is recorded when the prorated invoice is created, not when it is paid. Both are stated here rather than hidden. | | | |
| Operational consequence both reviews drew, and it stands: do not set `CONSOLE_STORE_EGRESS_REPORT_PREFIX` in production until existing store buckets have an attested ownership history, and do not set `CONSOLE_BILLING_PRORATION` until Paul has decided the policy and renewal reduction exists. |
| + |
| +# Dispositions: round 3 (Astra and Opus) |
| + |
| +Reviewed commit `e07f6475`. Astra round 3 (`CODEX-FINDINGS-carloku-paid-launch-overnight-round3.md`, unedited): reject, four P1 and one P2. Opus round 3 (`OPUS-REVIEW-carloku-paid-launch-overnight-round3.md`, unedited): accept with fixes, one P1, one P2, six P3. Every finding was checked against the code. All were accepted. Fixes are in `4abe94cb` (quantity worker) and `c3913dd9` (everything else, with partial-month billing). |
| + |
| +| Finding | Disposition | Where | Reproduction | |
| +| --- | --- | --- | --- | |
| +| Astra R1, Opus 1: a stale proposal replaces a newer acknowledged operation and lowers the quantity | Accepted. Recording an operation is a compare-and-set in one transaction under the membership write lock. It re-reads the billing row and recomputes the target, and records nothing unless the subscription, period, period end, billed quantity, target and newest pending addition are what the proposal was computed from and no operation is outstanding. | `billingquantity.go` `beginQuantityOperation` | `TestQuantityWorkerRefusesAStaleProposal` (pauses before the operation is recorded), `TestBeginQuantityOperationIsACompareAndSet` (each condition alone). Both fail with the conditions removed. | |
| +| Astra R2, Opus 3: a renewal between the provider read and the update charges an old period's seat into the new one | Accepted. The request carries `proration_date`, the instant the operation was computed at, which the provider refuses outside the subscription's current period. The operation persists its period end and is never sent at or past it. | `billingquantity.go` `sendQuantityOperation`, `billingstripe/client.go` `UpdateQuantityWith` | `TestQuantityWorkerCannotChargeAcrossARenewal`, `TestQuantityWorkerDatesTheProrationInsideThePeriod`. Both fail when the date is not sent. | |
| +| Astra R3: an absent bucket can keep an unrecorded legacy grantee | Accepted. Completeness also needs an inventory of principals whose policy names the bucket (`GranteeProber`), and it must be empty. Teardown records a legacy store, incomplete, before destroying anything. | `objstore/walk.go`, `objstore/provisioner.go` | `TestGrantClaimsNothingWhileAnotherPrincipalNamesTheBucket`, `TestFailedLegacyTeardownLeavesARecordTheNextGrantInherits` (both start with a provisioned store and no ledger; both fail without the pre-teardown record), `TestGrantClaimsNothingWithoutAPrincipalInventory`. | |
| +| Astra R4, Opus 7: a gap in platform statements hides earlier held months; read errors read as absence; a missing tenant statement reads as complete | Accepted. The walk reads the whole 36-month window, takes the oldest month with a platform statement as where metering began, and builds gaps inside that range. An unreadable statement is logged and the month swept again. An account that existed in the month and has no statement is open. Statements seen closed are remembered for the life of the process, which removes the per-pass read cost Opus noted. Deleted accounts are documented as not retried. | `console/economicspass.go` `statementMonthState` | `TestStatementCatchUpReachesAcrossGapsAndPastSixMonths`, `TestStatementCatchUpClosesAHeldMonthBehindAGap`, `TestStatementCatchUpWritesAMissingAccountStatement`, `TestStatementCatchUpWalksPastAnUnreadableMonth`. Each fails with its condition reverted, including the old six-month bound. | |
| +| Astra R5: a crash after the mark strands the operation | Accepted. The mark and the acknowledgement commit in one transaction. Account discovery includes unacknowledged operations. An ended subscription's operation is closed with its additions and no provider call. | `billingquantity.go` | `TestQuantityWorkerFindsAnOperationWithNothingPending`, `TestQuantityWorkerClosesAnOperationOfAnEndedSubscription`. | |
| +| Opus 2: an operation that landed is relabelled uncharged after a renewal | Accepted. On a period move, a provider quantity equal to the operation's target marks the operation's additions as charged with its reference. | `billingquantity.go` `markOperationLanded` | `TestQuantityWorkerRecordsALandedOperationAfterARenewal`. | |
| +| Opus 4: the durability span check holds a new box's first day for good | Accepted. Spans are compared to the hour. The comment now says what the check catches: a registration that was not backdated. | `console/durabilityfold.go` | `TestDurabilityFoldHoldsBeforeABoxRegisteredItsLedger`, extended with a race of seconds and a gap of an hour. | |
| +| Opus 5: completeness dated from the grant block, not the probe | Accepted. `CompleteSince` is the observation instant. | `objstore/ownership.go` | `TestCompletenessDatesFromTheObservation`. | |
| +| Opus 6: credits and discounts will hold store transfer every month | Accepted as an operational check. The collector plan now requires reading the account's credits and discounts before the export is approved. No code change. | `docs/superpowers/plans/2026-10-01-store-transfer-cur-collector.md` | none | |
| +| Opus 8: the attestation plan relies on a union the fold does not do | Accepted. The plan now says the fold reads one ledger per box and names the two ways a second ledger could be carried. | `docs/superpowers/plans/2026-10-01-durability-attestation-path.md` | none | |
| + |
| +Weak tests the reviewers named, and what changed: |
| + |
| +- The send window was only tested with a synthetic start. It is now measured from the provider read, and `TestQuantityWorkerMeasuresThePauseFromTheProviderRead` stalls the production path. |
| +- `TestBillingOperatorHouseNoOpLeavesNoAuditTrail` now asserts the general audit trail and fails without the handler's fix. |
| +- The recovery tests assert the recovered addition's outcome and reference. |
| +- The classifier table asserts that every case named as a hold holds the month. Fee and discount lines were added, and `TestIsHold` lists both transfer hold reasons. |
| +- The evidence boundary test now moves the boundary before the month and expects no change. |
| +- A resolved mismatch review that is still a mismatch is filed again. That is intended and `TestQuantityMismatchReviewIsFiledOncePerResolution` pins it. |
| + |
| +Not changed, with reasons: |
| + |
| +- The unavailable-plan text guard in `billing.html` has no test. Only Personal can be unavailable through the handler, so the guard has no reachable state to test. |
| +- `charged` is recorded when the provider accepts an `always_invoice` update. It does not establish that the prorated invoice was paid. Payment state belongs to the invoice webhooks, not to this worker. |
| +- `TestPartialBackfillOfASharedBucketResolvesNoOwner` already passed at round 2, as Astra says. The cases it was meant to pin are covered by the tests named above. |
| + |
| +Found while fixing, not fixed: |
| + |
| +- A teardown retried after a failed `DeleteUser` empties and deletes the bucket named on its record. If another app was granted that name in between, the retry destroys the new owner's bucket. It needs an operator to have adopted a name another app mints, then a failed revocation, then a reuse. It predates this branch and is a data-loss path, not a billing one. It should be filed as its own discussion. |
| +- Stripe's refusal of a `proration_date` outside the current period is assumed from its documentation and enforced by the fake. Test mode is not authorized in this run. Confirm it there before `CONSOLE_BILLING_PRORATION` is set. |
| +- `TestDurabilityHostPublishRegistersIncompleteSource` raced its temp dir removal against the ledger's late writer and failed about once in fifteen runs. The test helper now waits for the writer. Earlier green gates on this branch passed by luck on that test. |
| + |
| +Operational consequences, unchanged: do not set `CONSOLE_STORE_EGRESS_REPORT_PREFIX` in production until store buckets have an attested ownership history, and do not set `CONSOLE_BILLING_PRORATION` until Paul has decided the policy and renewal reduction exists. New: on AWS no ownership ledger claims completeness until the provisioner can list principals. |
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-carloku-paid-launch-overnight-round3.md b/docs/superpowers/specs/CODEX-FINDINGS-carloku-paid-launch-overnight-round3.md| new file mode 100644 |
| index 000000000..b9c3ea051 |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-FINDINGS-carloku-paid-launch-overnight-round3.md |
| @@ -0,0 +1,95 @@ |
| +## 1. Disposition table |
| + |
| +Reviewed `aed42815..e07f6475`, including `8e7499ae..e07f6475`. References below name lines at `e07f6475`; files changed during the review were read from that commit. No files were modified, network commands issued, or Stripe/AWS CLIs invoked. Repository tests were inspected, not run. Two memory-only SQLite reproductions confirmed the operation-conflict and account-discovery failures below. Other statements about tests without their fixes are based on source inspection. |
| + |
| +| Finding | Disposition and implementation | Reproducing tests; would they pass without the fix? | |
| +|---|---|---| |
| +| **Astra 1 / Opus N2 — House marker races checkout** | **Closed.** `internal/console/billingstore.go:232` compares the transactional house value with the requested coupon. `internal/console/billingoperator.go:283` also takes the process-local billing lock. | `TestCheckoutIntentRefusesAStaleHouseSnapshot`, `internal/console/billingoperator_test.go:217`, checks both mismatches and verifies that no intent was inserted. **No** without the transactional check. It does not test the handler lock independently. | |
| +| **Astra 2 — Competing processes lower quantity** | **Partly closed.** `internal/console/billingquantity.go:294` persists an operation; `:518` resumes it. However, an acknowledged newer operation can be replaced by a stale proposal. See finding R1. | `TestQuantityWorkerCannotLowerAQuantityAcrossProcesses`, `billingquantity_test.go:445`, exercises a pause **after** operation insertion. **No** without operation serialization, but it misses the pre-insertion race. `TestQuantityWorkerDoesNotSendAfterALongPause`, `:494`, would fail without the elapsed-time guard; it directly supplies an old `began` value. | |
| +| **Astra 3 / Opus N1 — Expired additions raise the renewed subscription** | **Partly closed.** `internal/console/billingquantity.go:129` separates periods; `:217` closes stale additions without charging; `:486` checks the provider’s period when reading. Renewal between that read and the update remains unsafe. See R2. | `TestQuantityWorkerNeverChargesAnExpiredPeriodsAddition`, `billingquantity_test.go:352`: **No** without period filtering. `TestQuantityWorkerSeatsAMemberWhoOutlivedTheirPeriod`, `:389`, covers a surviving member. Both renew **before** the worker starts; neither renews between GET and POST. | |
| +| **Astra 4 / Opus N6 — Minted name does not prove exclusive ownership** | **Partly closed.** `internal/objstore/walk.go:170` persists `Minted`; `:197` requires an absent bucket and no provisioned block. Bucket absence still does not prove absence of surviving credentials. See R3. | `TestGrantClaimsNothingForABucketThatAlreadyExists`, `ownership_test.go:504`, and `TestGrantClaimsNothingWhenTheProbeFails`, `:526`: **No** without the probe protection. `TestReRunOfAnAdoptingGrantIsStillAdopted`, `:545`: **No** without persisted provenance. `TestPartialBackfillOfASharedBucketResolvesNoOwner`, `:625`, **would already pass at round 2**: its re-granted record is provisioned and its adopted name differs from its minted name. | |
| +| **Astra 5 — Ownership closes before credential revocation** | **Closed for the reported, ledgered-store scenario.** `internal/objstore/walk.go:571` requires successful `DeleteUser` before closing ownership at `:585`. | `TestTeardownKeepsOwnershipOpenUntilCredentialsAreRevoked`, `internal/objstore/ownership_test.go:577`, exercises failed revocation, adoption by another app, and retry. **No** with the former ordering. It begins with an existing ledger; R3 covers the missing-ledger case. | |
| +| **Astra 6 / Opus N7 — Transfer fees, refunds and credits disappear** | **Closed in code.** `internal/awsbilling/classify.go:120` returns `transfer_adjustment` for nonzero transfer adjustments; `:56` makes that reason a hold. | Credit/refund cases in `TestChargedS3TransferTable`, `classify_test.go:71` and `:76`: **No** without the classification change. **Yes** if only the new `IsHold` entry is removed: those cases never assert that an uncounted reason holds, and `TestIsHold` at `:153` omits `ReasonTransferAdjustment`. No positive-fee reproduction was added. | |
| +| **Astra 7 / Opus N4 — Catch-up abandons held statements** | **Partly closed.** `internal/console/economicspass.go:1320` checks other accounts, and `:1291` extends discovery to 36 months. A missing platform statement still stops discovery, and missing tenant statements are ignored. See R4. The hard horizon remains. | `TestStatementCatchUpRetriesAnAccountLeftOpen`, `meterfolds_test.go:497`: **No** without the per-account check. **Yes** with the old six-month limit retained: September closes in November, is reopened once, then closes again in December. Advancing through later months does not test recovery after six months. | |
| +| **Astra 8 — Evidence omits completeness boundary** | **Closed.** `internal/usage/evidence_month.go:106` includes the month-clipped `complete_from` timestamp. | `TestBucketOwnershipEvidenceKeepsTheCompletenessBoundary`, `internal/usage/billable_test.go:114`: **No** without the boundary. It verifies a September 1 → September 20 mutation. Despite its comment, it does not test a harmless boundary change before September. | |
| +| **Opus N3 — Durability registration hides earlier unmetered hours** | **Closed.** `internal/console/durabilityfold.go:68` requires the durability window to cover each independently expected metrics/edge window. | `TestDurabilityFoldHoldsBeforeABoxRegisteredItsLedger`, `internal/console/meterfolds_test.go:465`: **No** without the span comparison. It covers registration ten hours into an otherwise covered day. | |
| +| **Opus N5 — Ended-subscription additions remain pending** | **Closed for the additions.** `internal/console/billingquantity.go:152` closes additions belonging to ended or replaced subscriptions without contacting the provider. Outstanding operations are a separate remaining problem; see R5. | `TestQuantityWorkerClosesAdditionsOfAnEndedSubscription`, `billingquantity_test.go:416`: **No** without the cleanup. It verifies cancellation, event outcome and zero provider calls; it does not exercise a replacement subscription or an outstanding operation. | |
| +| **Opus N8 — Quantity-worker and operator minor issues** | **Partly closed.** Behavior and period enter the key at `billingquantity.go:459`; open-review repetition is suppressed at `:574`; no-op general auditing is conditional at `billingoperator.go:291`; Personal-only text is guarded at `templates/billing.html:64`; the catch-up comment is corrected at `economicspass.go:1216`. Persisted operations preserve their behavior during recovery. However, `charged` still does not establish payment, and resolving a persistent mismatch still permits its review to reopen through `billingquantity.go:580` and `billingstore.go:497`. | No policy-change or repeated/resolved-mismatch regression exists in `billingquantity_test.go`. `TestBillingOperatorHouseNoOpLeavesNoAuditTrail`, `billingoperator_test.go:255`, **passes without the handler’s audit fix**: it counts only `billing_house_changes`, which already omitted no-ops. `TestBillingPageDoesNotOfferAPlanCheckoutWouldRefuse`, `billingpage_test.go:348`, **passes without the new Personal-only text guard**. Recovery tests at `billingquantity_test.go:266` and `:510` do not assert the recovered event’s outcome. | |
| + |
| +## 2. New findings |
| + |
| +### R1. P1 — A stale proposal can replace a newer acknowledged quantity operation |
| + |
| +**Where:** [billingquantity.go:294](/home/paulca/amadan.net/carlos/platform-paid-launch/internal/console/billingquantity.go:294), [billingquantity.go:460](/home/paulca/amadan.net/carlos/platform-paid-launch/internal/console/billingquantity.go:460), [billingquantity.go:205](/home/paulca/amadan.net/carlos/platform-paid-launch/internal/console/billingquantity.go:205). |
| + |
| +**Scenario:** |
| + |
| +1. Provider and database quantity are 10. Process A reads generation 1, target 11, and provider quantity 10. It pauses immediately before `beginQuantityOperation`. |
| +2. Another member joins. Process B reads generation 2, creates target 12, sends it, reconciles quantity 12, marks both additions processed and acknowledges its operation. |
| +3. A resumes. The conflict clause checks only `acked_at<>0`, so it overwrites B’s completed row with generation 1, target 11. |
| +4. A sends its previously unused key and lowers the provider to 11. Reconciliation records 11. The marking transaction finds no remaining additions and returns success before checking the billed quantity. |
| + |
| +Twelve members now have eleven billed seats, and no pending addition remains to repair this. The entire sequence fits inside the production worker’s two-minute timeout. |
| + |
| +Executing the checked-in conflict SQL against an in-memory database confirmed that the acknowledged `(target=12, generation=2)` row accepts replacement by `(target=11, generation=1)`. |
| + |
| +**Smallest acceptable fix:** Make operation creation conditional on the exact predecessor and billing revision observed while calculating it. In the same transaction, revalidate subscription, period, quantity and pending generation. A changed predecessor must force recomputation. Add a regression that pauses before insertion, rather than only before sending. |
| + |
| +### R2. P1 — Renewal between the provider read and update charges an old-period addition in the new period |
| + |
| +**Where:** [billingquantity.go:486](/home/paulca/amadan.net/carlos/platform-paid-launch/internal/console/billingquantity.go:486), [billingquantity.go:495](/home/paulca/amadan.net/carlos/platform-paid-launch/internal/console/billingquantity.go:495), [client.go:636](/home/paulca/amadan.net/carlos/platform-paid-launch/internal/billingstripe/client.go:636). |
| + |
| +**Scenario:** An annual subscription has ten current members and an unprocessed eleventh-seat addition from someone who joined and left during the ending period. Seconds before renewal, the worker reads the old period, chooses target 11 and records its operation. The subscription renews at 10 before the POST executes. |
| + |
| +`sendQuantityOperation` checks only elapsed send time. The request carries quantity and proration behavior; the period embedded in its idempotency key imposes no execution condition. The same subscription item is therefore raised to 11 in the renewed period, although that member left before renewal. |
| + |
| +The later period check in `markQuantityAdditionsProcessed` can refuse bookkeeping, but it cannot undo the provider update or its proration. Both configured policies are affected: the charge is either invoiced immediately or deferred. |
| + |
| +**Smallest acceptable fix:** Persist the operation’s original period end and enforce that boundary on first sends and retries. The provider operation must establish that its proration belongs to the recorded period; otherwise hold the boundary-crossing attempt for reconciliation. Add a regression that renews after `quantityProviderItem` returns and before the POST. A period check performed only after charging is insufficient. |
| + |
| +### R3. P1 — An absent bucket can retain an unrecorded legacy grantee |
| + |
| +**Where:** [walk.go:197](/home/paulca/amadan.net/carlos/platform-paid-launch/internal/objstore/walk.go:197), [walk.go:564](/home/paulca/amadan.net/carlos/platform-paid-launch/internal/objstore/walk.go:564), [ownership.go:260](/home/paulca/amadan.net/carlos/platform-paid-launch/internal/objstore/ownership.go:260). |
| + |
| +**Scenario:** |
| + |
| +1. Before ownership ledgers existed, app B adopted the deterministic bucket name belonging to app A. B still has its bucket-scoped credentials, and no ownership ledger exists. |
| +2. B’s teardown deletes the bucket, then fails at `DeleteUser`. The current teardown does not establish an incomplete ledger before this failure. |
| +3. A is declared or re-declared and receives its first grant. Its new grant has `Minted=true`, no provisioned block, and a successful probe reporting that the bucket is absent. |
| +4. The grant creates a **complete** ledger naming only A and recreates the bucket. B’s surviving credentials can access it, but B has no interval. |
| + |
| +Transfer by B now resolves to A and can be billed to A. Keeping an existing interval open fixes the prior teardown scenario; it does not protect a store that never had an interval. |
| + |
| +**Smallest acceptable fix:** Do not treat bucket absence as proof that no credential can reach that name. Establish an incomplete ownership record before tearing down a legacy estate, and require evidence excluding unrecorded surviving grants before claiming completeness for a reused name. If that evidence is unavailable, hold. Add the failed-teardown/name-reuse regression starting with a provisioned store and **no ledger**. |
| + |
| +### R4. P1 — A gap in platform statements permanently hides earlier held months |
| + |
| +**Where:** [economicspass.go:1267](/home/paulca/amadan.net/carlos/platform-paid-launch/internal/console/economicspass.go:1267), [economicspass.go:1302](/home/paulca/amadan.net/carlos/platform-paid-launch/internal/console/economicspass.go:1302), [economicspass.go:1324](/home/paulca/amadan.net/carlos/platform-paid-launch/internal/console/economicspass.go:1324). |
| + |
| +**Scenario:** September has an open statement awaiting transfer evidence. The statements worker is unavailable during October and November. It resumes in December after September’s evidence is repaired. |
| + |
| +The normal pass creates December and November statements. Catch-up starts with October, finds no platform statement and breaks. September is never reached. Every later pass encounters that same October gap, so September remains open permanently despite being repairable and within the 36-month window. |
| + |
| +The helper also treats read/parse errors as absence. Separately, once the platform statement is closed, a missing tenant statement is treated as complete because the check requires `found && !closed`; an initial tenant write failure can therefore remain unrepaired. |
| + |
| +**Smallest acceptable fix:** Do not infer the start of metering from one absent platform object. Continue across gaps within the search window or use an explicit metering-start record. Preserve read errors as errors, and retry missing statements for accounts known to belong to that month. Add tests with an intervening missing platform month and a missing expected tenant statement. |
| + |
| +### R5. P2 — A crash after marking additions leaves an operation undiscoverable |
| + |
| +**Where:** [billingquantity.go:91](/home/paulca/amadan.net/carlos/platform-paid-launch/internal/console/billingquantity.go:91), [billingquantity.go:376](/home/paulca/amadan.net/carlos/platform-paid-launch/internal/console/billingquantity.go:376), [billingquantity.go:359](/home/paulca/amadan.net/carlos/platform-paid-launch/internal/console/billingquantity.go:359). |
| + |
| +**Scenario:** The provider update and reconciliation succeed. `markQuantityAdditionsProcessed` commits, then the process crashes before `ackQuantityOperation`. |
| + |
| +`pendingQuantityAccounts` selects only unprocessed addition events. It never reads `billing_quantity_operations`, despite the pass comment promising to process outstanding operations. With no further membership changes, the operation remains unacknowledged forever. An ended subscription produces the same orphan when its additions are closed and processing returns before resolving its operation. |
| + |
| +The in-memory reproduction of the checked-in discovery query returned no account for precisely this state. This does not itself double-charge, but the promised recovery step never runs. |
| + |
| +**Smallest acceptable fix:** Include unacknowledged operations in account discovery and finish terminal-subscription operations without provider calls. Prefer committing the successful event mark and matching acknowledgement atomically. Add a crash regression between mark and acknowledgement, plus cancellation with an outstanding operation. |
| + |
| +## 3. Handoff claims the code contradicts |
| + |
| +- [AGENT_HANDOFF.md:35](/home/paulca/amadan.net/carlos/platform-paid-launch/implementation/carloku-paid-launch/AGENT_HANDOFF.md:35) describes the completeness condition as establishing “no delivered credentials.” The code checks `Provisioned == nil` on the requesting app and bucket absence; neither excludes another legacy app’s surviving credentials. R3 demonstrates false attribution. |
| +- [AGENT_HANDOFF.md:38](/home/paulca/amadan.net/carlos/platform-paid-launch/implementation/carloku-paid-launch/AGENT_HANDOFF.md:38) says one persisted operation prevents conflicting targets. R1 bypasses that protection through the acknowledged-row conflict clause. The same paragraph’s current-period guarantee fails when renewal occurs between the provider read and update, as in R2. |
| +- [AGENT_HANDOFF.md:75](/home/paulca/amadan.net/carlos/platform-paid-launch/implementation/carloku-paid-launch/AGENT_HANDOFF.md:75) says all findings were fixed with reproductions. Quantity serialization, ownership completeness and catch-up remain partly closed. Several claimed reproductions also pass with the relevant protection removed, as detailed in the table. |
| + |
| +**Verdict: reject.** |
| \ No newline at end of file |
diff --git a/docs/superpowers/specs/OPUS-REVIEW-carloku-paid-launch-overnight-round3.md b/docs/superpowers/specs/OPUS-REVIEW-carloku-paid-launch-overnight-round3.md| new file mode 100644 |
| index 000000000..eb40942cd |
| --- /dev/null |
| +++ b/docs/superpowers/specs/OPUS-REVIEW-carloku-paid-launch-overnight-round3.md |
| @@ -0,0 +1,125 @@ |
| +# Opus production and authority review, round 3: Carloku paid launch, `aed42815..e07f6475` |
| + |
| +I did this review read-only. I read the code statically, modified nothing, and ran no tests and no network commands. Where I say a test "would fail without the fix", that comes from reading the code, not from reverting anything. The changes reviewed are `8e7499ae..e07f6475`. |
| + |
| +## 1. Disposition table |
| + |
| +| Finding | Status | Fix | Reproducing test | Would the test pass without the fix? | |
| +|---|---|---|---|---| |
| +| Astra 1 / N2: house marker changes between checkout's read and the intent | **Closed** | `billingstore.go:228-234` re-reads the marker inside the intent transaction. `billingoperator.go:283-285` takes the billing lock. SQLite serialises the two writers. | `billingoperator_test.go:217` `TestCheckoutIntentRefusesAStaleHouseSnapshot` | No | |
| +| Astra 2: two processes can lower a quantity | **Partly closed** | `billingquantity.go:249-307` persists one operation per account and uses a deterministic key. That covers a pause *after* the row is written. A pause between the provider read and `beginQuantityOperation` still lowers the quantity (new finding 1). | `billingquantity_test.go` `TestQuantityWorkerCannotLowerAQuantityAcrossProcesses`. Its pause is at `quantityBeforeSend`, after `begin`, so it does not cover the remaining case. | No for the case it tests. The remaining case is untested. | |
| +| Astra 3 / N1: an expired period's addition raises the renewed subscription | **Closed** | `:129-139` splits pending additions by `period_start`. `:213-225` closes stale ones as uncharged and files a review. `:486` checks the provider's period. A narrow renewal race remains (new finding 3). | `TestQuantityWorkerNeverChargesAnExpiredPeriodsAddition`, `TestQuantityWorkerSeatsAMemberWhoOutlivedTheirPeriod` | No | |
| +| Astra 4 / N6: `minted` does not prove this grant created the bucket | **Closed**, small residual (new finding 5) | `objstore.go:58-63` (`GrantBlock.Minted`), `walk.go:197-203` (recorded mint, no `Provisioned` block, and a probe showing the bucket does not exist), `aws.go:261-275` | `TestGrantClaimsNothingForABucketThatAlreadyExists`, `TestGrantClaimsNothingWhenTheProbeFails`, `TestReRunOfAnAdoptingGrantIsStillAdopted`, `TestPartialBackfillOfASharedBucketResolvesNoOwner` | No | |
| +| Astra 5: teardown closes ownership while credentials survive | **Closed** | `walk.go:571-588`: the interval closes only after `DeleteUser` succeeds | `TestTeardownKeepsOwnershipOpenUntilCredentialsAreRevoked` | No | |
| +| Astra 6 / N7: charged transfer on a non-usage line disappears | **Closed** | `classify.go:121-125`, `IsHold` `:56` | classifier table in `classify_test.go` | No. See new finding 6 for the operational consequence. | |
| +| Astra 7 / N4: the catch-up watches only the platform statement and stops at six months | **Closed**, residuals (new finding 7) | `economicspass.go:1287-1305` (36 months, warning after 6), `:1329-1360` (`statementMonthState` checks every account) | `meterfolds_test.go` `TestStatementCatchUpRetriesAnAccountLeftOpen` | No | |
| +| Astra 8: evidence drops the completeness timestamp | **Closed** | `evidence_month.go:102-119` (`complete_from`, clipped to the month). The ledger is new on this branch, so no evidence pinned in the old format exists anywhere. | `billable_test.go` `TestBucketOwnershipEvidenceKeepsTheCompletenessBoundary` | No | |
| +| N3: the fold skips a box's days before its ledger registered | **Closed** in the fold (`durabilityfold.go:57-72`) | | `TestDurabilityFoldHoldsBeforeABoxRegisteredItsLedger` | No. See new finding 4: `boxEarliestEvidence` backdates durability's first-seen, so this branch rarely fires, and when it does it can hold a day for good. | |
| +| N5: additions of an ended subscription hold forever | **Closed** | `billingquantity.go:152-180` | `TestQuantityWorkerClosesAdditionsOfAnEndedSubscription` | No | |
| +| N8: minor | **Mostly closed** | The key carries the behaviour (`:457-459`). The mismatch review is filed once while open (`:561-584`). A no-op house set leaves no audit entry (`billingoperator.go:288-297`). The Personal-only line is fixed (`billing.html:64`). The stale comment is fixed (`economicspass.go:1216`). The two outcome labels are unchanged; the dispositions say so openly. | `TestBillingOperatorHouseNoOpLeavesNoAuditTrail` | No | |
| + |
| +## 2. New findings, most severe first |
| + |
| +### 1. P1: A stale process can still lower a quantity and mark the additions covered |
| + |
| +**Where:** `internal/console/billingquantity.go:399-460` and the conflict clause at `:300`. |
| + |
| +`beginQuantityOperation` only refuses while another operation is unacknowledged (`WHERE acked_at<>0`). It does not check that the state the caller read is still current. The send window does not help here: both callers pass `time.Now()` as `began` (`:468`, `:554`), so it measures only the test hook. |
| + |
| +**Scenario:** both consoles run the hourly pass. Quantity is 10 on both sides, and member X joins (gen 5, desired 11). |
| + |
| +1. Process A reads the state, pending (`Newest=5`), members (11) and the provider (10). It then stalls before `begin`, on a SQLite busy wait, CPU steal or a slow `GetSubscription` response. |
| +2. Y joins (gen 6). Process B does a full cycle: it begins K(gen 6, target 12), sends, reconciles 12, marks additions up to gen 6 and acknowledges. |
| +3. A resumes. The row is acknowledged, so its `begin` succeeds with K(gen 5, target 11, from 10). Stripe has never seen that key, so it executes and lowers the quantity from 12 to 11. With `always_invoice` this issues a credit. |
| +4. A reconciles to 11. Its mark for additions up to gen 5 finds nothing pending and succeeds, then A acknowledges. |
| + |
| +The account now has 12 members, is billed for 11, has no pending additions, and drops out of every later pass. This is Astra 2's outcome by a different interleaving. |
| + |
| +**Smallest acceptable fix:** make `beginQuantityOperation` a compare-and-set on what the caller read, in one transaction: |
| +- refuse when the stored row's `generation >= op.Generation` (or `target >= op.Target`) for the same subscription and period; |
| +- refuse when `billing_accounts.quantity` and `period_start` differ from `op.FromQuantity` and `op.PeriodStart`; |
| +- refuse when `op.Target` is not above the billed quantity. |
| + |
| +Add a test that pauses between `quantityProviderItem` and `begin`. Either delete the send window or make it measure from the provider read. As written it is dead code. |
| + |
| +### 2. P2: An operation that landed is relabelled "uncharged" after a renewal, with a review saying the seats went unbilled |
| + |
| +**Where:** `billingquantity.go:540-548` together with `:216-224`. |
| + |
| +**Scenario:** |
| +1. An operation lands at Stripe and the customer is charged, by `always_invoice`'s prorated invoice or by `create_prorations` on the renewal invoice. |
| +2. The response is lost, or the reconcile fails, and the subscription renews before the next pass. |
| +3. On the next pass `op.PeriodStart != state.PeriodStart`. The landed case is skipped because it requires the same period, so the operation is abandoned. |
| +4. The now-stale additions are closed as `period_renewed_uncharged`, and `billing_quantity_uncharged_period` tells an operator the seats went unbilled. |
| + |
| +An operator acting on that review would bill the seat a second time. |
| + |
| +**Smallest acceptable fix:** before abandoning on a period move, compare the provider quantity with `op.Target`. If it matches, mark that operation's generation range with the operation's reference and outcome, not the uncharged outcome. Alternatively, record the operation's key on the stale events and keep the uncharged review only for events no operation covered. |
| + |
| +### 3. P3: A renewal between the provider read and the update charges an old-period seat across the new year |
| + |
| +**Where:** `billingquantity.go:477-503`. |
| + |
| +The period check (`:486`) runs on the `GetSubscription` response. If Stripe renews before `UpdateQuantityWith` lands, the raise is prorated across the new year, for a member who may have left before the renewal. The window is a single round trip at the renewal instant. |
| + |
| +**Smallest acceptable fix:** send `proration_date` set to the instant of the read. Stripe refuses a `proration_date` outside the current period, which turns the check into a precondition the provider enforces. |
| + |
| +### 4. P3: The durability span check can hold a new box's first day for good, and seldom catches the case it was written for |
| + |
| +**Where:** `durabilityfold.go:57-72`, with `host/durability.go:161`, `host/usage_loop.go:289-314` and `usage/lifecycle.go:372`. |
| + |
| +Durability's first-seen is `boxEarliestEvidence`: the earliest of edge evidence, metrics evidence and the ledger's own creation time. So a ledger that opens late is normally backdated, and the N3 hold actually comes from missing day files, not from the span check. |
| + |
| +Conversely, on a fresh box the durability publisher registers on its first tick, before metrics has published anything. Its first-seen is then the ledger's creation time. Metrics registers with its spool's creation time, which can be seconds earlier. First-seen is write-once, so `dFrom.After(from)` holds that box's first day permanently once attestation is on. |
| + |
| +The effect is in the customer's favour: the replication meter for that month cannot complete. It is still a permanent hold with no repair. |
| + |
| +**Smallest acceptable fix:** compare spans at hour granularity, flooring both `from` values. Or make durability registration wait for the box's metrics or edge first-seen. |
| + |
| +### 5. P3: A complete ledger vouches from the grant block's time, not from the probe |
| + |
| +**Where:** `walk.go:204`, `ownership.go:262-265`. |
| + |
| +On a re-run after a crash between the grant-block `Put` and `OpenBucketOwner`, `CompleteSince` is the block's `At`, but the probe ran later. Suppose a ledger-less, pre-ledger store held that name in between and was torn down; teardown writes no ledger for it. Its transfer in that window is then attributed to the new app. This is contrived, since it needs a pre-ledger adoption of a minted name. |
| + |
| +**Smallest acceptable fix:** set `CompleteSince` to the probe instant, the `now` of the run that probed, not to `grantOwnershipFrom`. |
| + |
| +### 6. P3, operational: promotional credits and discount lines will hold store transfer every month |
| + |
| +**Where:** `classify.go:121-125`. |
| + |
| +AWS writes `Credit`, `EdpDiscount`, `PrivateRateDiscount` and `BundledDiscount` lines on the same transfer usage type as the usage they discount. Holding them follows ruling 2. But if Team Tito's account carries any credit or discount, `store_egress_gb` never completes, and nothing lets an operator release the hold. |
| + |
| +**Smallest acceptable fix:** none needed in code. Before the export is approved, check whether the account has credits or discounts, and record that in the collector plan. |
| + |
| +### 7. P3: The catch-up has a few edges |
| + |
| +**Where:** `economicspass.go:1287-1360`. |
| + |
| +- A platform statement that cannot be read or parsed counts as "not found" (`:1336-1343`) and `break`s the walk. A corrupt month therefore hides every older open month permanently. |
| +- `Accounts()` excludes deleted accounts (`store.go:993`), so a deleted account's open month is never retried. That is in the customer's favour, and it is undocumented. |
| +- Once a month is closed, every pass reads one statement per account per month, up to 36 times the number of accounts in S3 GETs per hour. |
| + |
| +**Smallest acceptable fix:** separate "not found" from a read or parse error, and `continue` on the error with a log line. Note the deleted-account behaviour in the doc comment. Optionally, persist a month-closed marker once every statement in the month is closed. |
| + |
| +### 8. P3: The attestation plan relies on a capability the fold does not have |
| + |
| +**Where:** `docs/superpowers/plans/2026-10-01-durability-attestation-path.md`, option B. |
| + |
| +The plan says "the fold already unions every ledger directory it finds". The fold keys ledgers by box (`durabilityfold.go:53-56`), and the path is `usage/durability/<box>/days/<date>.json`. A second ledger on the same box would collide with the first, or need a label that is not a box. The code is not wrong today; the plan needs correcting before B is built. |
| + |
| +## 3. Handoff claims the code contradicts |
| + |
| +- `AGENT_HANDOFF.md:38`, "One persisted operation per account keeps two console processes from sending conflicting targets": a process that read the state before another process's whole cycle still sends a lower target under a fresh key (finding 1). |
| +- The disposition for Astra 2, and the comment at `billingquantity.go:246-248` ("a process that was paused past quantityOperationSendWindow does not send at all"): both call sites pass `time.Now()`, so the window never trips in production. `TestQuantityWorkerDoesNotSendAfterALongPause` passes a synthetic `began` that production never supplies, so it asserts less than its name says. |
| +- `AGENT_HANDOFF.md:33`, that the fold holds a day for a box whose ledger "registered later": registration is backdated by `boxEarliestEvidence`, so that hold comes from missing day files. The span check mostly catches a benign registration race instead (finding 4). |
| + |
| +I confirmed, unchanged since round 2: |
| +- With `CONSOLE_BILLING_PRORATION` unset, the quantity pass makes no provider call (`billingquantity.go:322-325`). |
| +- No production code sets attestation true. The only callers of `SetAllClientsInstrumented` and `SetAttestation` are in `usage` itself and in tests. |
| +- No Stripe identifier, postal code or secret reaches a template or a log line. |
| +- The idempotency key carries only the sqid and the Stripe subscription id, and it goes only to Stripe. |
| + |
| +## 4. Verdict |
| + |
| +**Accept with fixes.** Finding 1 must be fixed, with a test that pauses before `begin`, before `CONSOLE_BILLING_PRORATION` is ever set. Finding 2 should be fixed at the same time. Neither the house door nor anything in objstore blocks. |
diff --git a/implementation/carloku-paid-launch/AGENT_HANDOFF.md b/implementation/carloku-paid-launch/AGENT_HANDOFF.md| index 8a64add27..2ae73fd70 100644 |
| --- a/implementation/carloku-paid-launch/AGENT_HANDOFF.md |
| +++ b/implementation/carloku-paid-launch/AGENT_HANDOFF.md |
| @@ -30,40 +30,42 @@ When a review is approved, apply each string byte for byte: the site by replacin |
| 1. Plans and calculations, Stripe boundary, entitlements and membership, quantity history, invoice engine: as described in the September handoff (kept in git history at `aed42815`). Unchanged in this run. |
| 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 holds any day on which a box that metrics or edge expected is not expected by the durability source for at least the same span (no ledger at all, or one registered later). Attestation is still false everywhere in production. |
| +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 holds a day when a box that metrics or edge expected has no ledger directory, has no day file, or is expected by the durability source for a shorter span, compared to the hour. The host backdates a late ledger's first-seen, so a late ledger is mostly caught by its missing day files. Attestation is still false everywhere in production. |
| 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) with a cost on a transfer usage type. |
| -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, which needs a grant block recorded as minting, no delivered credentials and a probe showing the bucket did not exist yet. Adopted buckets, pre-existing stores and anything the probe cannot answer for hold. Teardown closes the interval only after the credentials are revoked. |
| -6. Statement folds: `console/durabilityfold.go` (`durability_writes_1k`, policy in `usage.BillableReplicationWrites`) and `console/storeegressfold.go` (`store_egress_gb`). `usage.MeterSource` now names a source for both meters. Month evidence pins ownership ledgers by their month projection and knows the durability box directory. |
| +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. That 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`). The AWS provisioner cannot list principals yet, so on AWS no ledger claims completeness, new stores included, and store transfer holds. Completeness dates from the observation. Teardown records a legacy store, incomplete, before destroying anything, and closes the interval only after the credentials are revoked. |
| +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 each meter by UTC day (`Statement.Days`), summing to the monthly totals. With a transfer report configured, the statements catch-up revisits every month in a 36-month window that is open, was never written, or is missing an account's statement. |
| 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 `CONSOLE_BILLING_PRORATION` (`immediate` or `next_invoice`). Unset, which is how it must stay until Paul decides, it makes no provider call. One persisted operation per account keeps two console processes from sending conflicting targets. Only additions of the subscription's current period can raise it; older ones are closed uncharged with a review; additions of an ended subscription are closed. It does not yet lower the quantity at renewal. |
| +8. Team quantity worker (`billingquantity.go`) behind `CONSOLE_BILLING_PRORATION` (`immediate` or `next_invoice`). Unset, which is how it must stay until Paul decides, it makes no provider call. An operation is recorded by a compare-and-set on everything its target was computed from, so a process that read an older state records and sends nothing. The request dates its proration inside the period it was computed for (`proration_date`), which the provider refuses after a renewal; that refusal is assumed from Stripe's documentation and has not been confirmed in test mode. 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. Plans: `docs/superpowers/plans/2026-10-01-store-transfer-cur-collector.md` and `2026-10-01-durability-attestation-path.md`. |
| +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. A month with no paid day is still refused quietly and never billed. |
| +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). |
| ## Why no overage can be invoiced yet, precisely |
| The monthly freeze requires all five billed meters complete. Two cannot be, in production, today: |
| -- `store_egress_gb`: no Cost and Usage Report export exists, so the source is unconfigured. It needs the scoped infrastructure approval in the collector plan, a first real delivery, and ownership ledgers for the stores that already exist. |
| +- `store_egress_gb`: no Cost and Usage Report export exists, so the source is unconfigured. It needs the scoped infrastructure approval in the collector plan, a first real delivery, a principal inventory in the AWS provisioner, and attested ownership for the stores that already exist. Check first whether the AWS account carries credits or discounts on S3 transfer: any such line holds the month. |
| - `durability_writes_1k`: no box attests, because the external `carlos-litestream` binary replicates unit-backed routes outside the ledger. The attestation plan gives the two ways out. Do not set the flag to pass a test. |
| ## Still missing from the full requirements audit |
| - Quantity at renewal: the worker raises a subscription for added members but nothing lowers it at renewal for members who left, so a renewal bills the old quantity. Must be built before team plans are sold. The proration policy itself is Paul's decision and the switch is unset. |
| -- Partial months: a first month that starts mid-month, a month a subscription ends in and a free-to-paid change all hold permanently and quietly today (`plans.ErrAssignmentHistory`). This is required engineering, not a waiver: the design is `docs/superpowers/plans/2026-10-01-partial-month-billing.md` (daily meters on the statement, whole paid days counted against a prorated allowance). Not built. |
| +- Partial months: built (item 10). Left over: a month with no paid day is refused quietly and retried, where the design records it as waived. |
| - Months spanning two tax codes hold with a review; per-line tax codes are not implemented. |
| - The operator doors have no `carlos` verb and no documentation page, and have not had their own independent review. |
| -- Ownership history for existing and adopted store buckets: their ledgers cannot vouch for who had access, so their charged transfer holds. The evidence-backed way out is designed in `docs/superpowers/plans/2026-10-01-bucket-ownership-attestation.md` (match store records against an IAM inventory, claim completeness from that instant, never earlier). Not built. Transfer before a bucket's attestation can never be attributed, so attest before the first billable month. |
| +- Ownership history for store buckets: on AWS no ledger can vouch for who had access (no principal inventory), and existing and adopted buckets never could, so charged transfer holds. The evidence-backed way out is designed in `docs/superpowers/plans/2026-10-01-bucket-ownership-attestation.md` (match store records against an IAM inventory, claim completeness from that instant, never earlier). Not built. Transfer before a bucket's attestation can never be attributed, so attest before the first billable month. |
| - Free tier: no Free entitlement, no app cap, no storage refusal, no per-plan hibernation idle, no flush-only replication class, nothing at runtime reads entitlements. |
| - Dormancy archive and warnings, recognised-revenue pool accounting at 15 percent, the books site: not started. |
| - Statement rebuild door for a closed month: missing. |
| - Move and checkpoint metering tests in `internal/host/movespass_test.go`: missing. |
| - An operator verb to backfill bucket ownership for existing stores: missing. |
| +- A teardown retried after a failed revocation deletes the bucket named on its record, even if another app has since been granted that name. Predates this branch; data loss, not billing; not fixed. See the round-three dispositions. |
| ## Verification in this run |
| Recorded in the run directory's `logs/`. Targeted: BillingPage tests; usage Durability under `-race`; host and activator under `-race` outside the sandbox; awsbilling; objstore; the new fold and evidence tests, with a mutation check that the hold conditions are pinned. The full gate result and the Amadan CI result for the tip are in `.claude/HANDOFF.md`, which is rewritten at each checkpoint; do not trust a gate claim that is not there with a commit. |
| -Environment that works on docs-t1: inside the sandbox `GOCACHE=$TMPDIR/gocache GOFLAGS='-mod=mod -buildvcs=false' GOPROXY=off`; socket-binding packages (host, activator, edge) and the full gate outside the sandbox with `TMPDIR=/tmp/k GOTMPDIR=/tmp/k` (a real, short, 0700 directory) and the default build cache. Without `-buildvcs=false` eight `TestControlPlane*` tests fail in a linked worktree with "error obtaining VCS status"; that is the worktree, not the code, and it reproduces on `aed42815`. |
| +Environment that works on docs-t1: inside the sandbox `GOCACHE=$TMPDIR/gocache GOFLAGS='-mod=mod -buildvcs=false' GOPROXY=off`; socket-binding packages (host, activator, edge) and the full gate outside the sandbox with the default build cache, the go command's `GOTMPDIR` on disk, and the test processes alone given `TMPDIR=/tmp/k GOTMPDIR=/tmp/k` through `-exec` (a wrapper script that exports both and execs its arguments). The test temp root must be short (unix socket paths cap at 107 bytes), a real directory (the isolated-exec tests refuse symlinked paths) and outside `$HOME` (the CLI config walk finds `~/.carlos/config` from anywhere under it), which leaves `/tmp/<one letter>`. `/tmp` is a shared tmpfs: keep build artifacts out of it and empty the directory afterwards. Without `-buildvcs=false` eight `TestControlPlane*` tests fail in a linked worktree with "error obtaining VCS status"; that is the worktree, not the code, and it reproduces on `aed42815`. |
| ## Stripe and secrets |
| @@ -72,7 +74,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 round 1 (ten findings) and round 2 (eight); the Opus production review accepted with fixes (eight). All were accepted and fixed with reproductions (`docs/superpowers/specs/CODEX-DISPOSITIONS-carloku-paid-launch-overnight.md`). A further round of both must pass on the exact tip before anything lands; do not read "fixed" as "accepted". |
| +2. Independent review: Astra rejected rounds 1, 2 and 3 (ten, eight and five findings); Opus accepted with fixes twice (eight and eight). All were accepted and fixed with reproductions (`docs/superpowers/specs/CODEX-DISPOSITIONS-carloku-paid-launch-overnight.md`). Round four of both must pass on the exact tip before anything lands; do not read "fixed" as "accepted". Partial-month billing has had no independent review yet and is in round four's scope. |
| 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 partial-month billing and the bucket ownership attestation (both designed in `docs/superpowers/plans/`), quantity reduction at renewal, a `carlos` verb and documentation for the operator doors, then Free-tier enforcement. |
| +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. Choose and build the attestation path. Until then replication writes hold every month. |
diff --git a/implementation/carloku-paid-launch/requirements-audit.json b/implementation/carloku-paid-launch/requirements-audit.json| index 59b9c7c64..de601b5a7 100644 |
| --- a/implementation/carloku-paid-launch/requirements-audit.json |
| +++ b/implementation/carloku-paid-launch/requirements-audit.json |
| @@ -7,7 +7,7 @@ |
| { |
| "step": 1, |
| "requirement": "statement closure", |
| - "observed_status": "implemented locally; historical adjustment/rebuild door missing", |
| + "observed_status": "implemented locally; with a transfer report configured the catch-up revisits every month in a 36-month window that is open, never written or missing an account's statement; historical adjustment/rebuild door missing", |
| "inspection_anchor": "internal/usage/statement.go; internal/console/economicspass.go", |
| "complete": false |
| }, |
| @@ -28,7 +28,7 @@ |
| { |
| "step": 4, |
| "requirement": "object-store transfer", |
| - "observed_status": "collector implemented locally on the AWS Cost and Usage Report: charged-line classifier, newest-delivery reader with digests, append-only bucket ownership ledger opened at grant and closed at teardown, monthly fold that holds on no delivery, uninvoiced month, unclassified charge or unowned bucket, and holds the close until provable. Not complete: no Data Export or bucket policy exists (scoped approval pending, plan in docs/superpowers/plans/2026-10-01-store-transfer-cur-collector.md), existing stores have no ownership ledger until re-granted, and no real delivery has been read", |
| + "observed_status": "collector implemented locally on the AWS Cost and Usage Report: charged-line classifier, newest-delivery reader with digests, append-only bucket ownership ledger opened at grant and closed at teardown, monthly fold that holds on no delivery, uninvoiced month, unclassified charge, adjustment line or unowned bucket, and holds the close until provable. Not complete: no Data Export or bucket policy exists (scoped approval pending, plan in docs/superpowers/plans/2026-10-01-store-transfer-cur-collector.md); on AWS no ownership ledger can claim completeness until the provisioner can list principals, and existing and adopted buckets need the attestation door (docs/superpowers/plans/2026-10-01-bucket-ownership-attestation.md); credits or discounts on S3 transfer in the AWS account would hold every month; no real delivery has been read", |
| "inspection_anchor": "internal/awsbilling; internal/objstore/ownership.go; internal/console/storeegressfold.go", |
| "complete": false |
| }, |
| @@ -49,7 +49,7 @@ |
| { |
| "step": 7, |
| "requirement": "durable billing and effective terms", |
| - "observed_status": "history exists; invoice journal/engine implemented locally and recovery race tests pass; partial-month handling incomplete", |
| + "observed_status": "history exists; invoice journal/engine implemented locally and recovery race tests pass; a partly covered month (first month, ending month, lapse) bills the UTC days wholly inside a paid span against the allowance weighted over those spans, with the counted days frozen in the calculation; a month with no paid day is still refused quietly; team quantity worker implemented behind an unset policy switch and does not lower quantity at renewal; partial-month billing awaits independent review", |
| "inspection_anchor": "internal/console/billingstore.go; internal/console/billinginvoicestore.go", |
| "complete": false |
| }, |
| @@ -70,7 +70,7 @@ |
| { |
| "step": 10, |
| "requirement": "house marker and console door", |
| - "observed_status": "billing state supports house; operator door not verified complete", |
| + "observed_status": "billing state supports house; operator API doors for review holds and the house marker implemented with audit rows and tests; no carlos verb or documentation page", |
| "inspection_anchor": "internal/console/billingstore.go", |
| "complete": false |
| }, |
| @@ -98,7 +98,7 @@ |
| { |
| "step": 14, |
| "requirement": "monthly overage pass", |
| - "observed_status": "local invoice engine recovery tests pass; P5/P6 evidence deliberately refuses charges", |
| + "observed_status": "local invoice engine recovery tests pass, including a partial first month end to end; in production the transfer and replication meters cannot read complete, so every month holds", |
| "inspection_anchor": "internal/console/billinginvoice.go; internal/console/billingcalculation.go", |
| "complete": false |
| }, |