The void's key changes with the hour, so a remembered failure cannot keep a wrong invoice payable for a day
From the first review of this branch (Opus P2). A provider that began a void and failed replays that failure under the key for 24 hours. With one fixed key the wrong-total invoice stayed open that long. Voiding is safe to ask for again, so the key now carries the hour. The test fake remembers a failure under its key, which it could not do before. The void is remediation. It does not close the late-request defects. Billing is off.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
4 files changed,
+82
−4
docs/superpowers/plans/2026-10-01-late-request-fencing.md+1 −1docs/superpowers/specs/DISPOSITIONS-late-request-fencing.md+2 −2internal/console/billinginvoice.go+8 −1internal/console/billinginvoice_test.go+71 −0
CI passed — run details
diff --git a/docs/superpowers/plans/2026-10-01-late-request-fencing.md b/docs/superpowers/plans/2026-10-01-late-request-fencing.md| index 331e49b88..ed71e8887 100644 |
| --- a/docs/superpowers/plans/2026-10-01-late-request-fencing.md |
| +++ b/docs/superpowers/plans/2026-10-01-late-request-fencing.md |
| @@ -85,7 +85,7 @@ A wrong-total invoice that has been finalized is open at the provider from that |
| | The process dies between the finalize and the void, and the next visit is within the replay window (22 hours) | until that visit | voids then | `TestInvoiceWrongTotalIsPayableUntilTheNextVisitWhenTheProcessDiesBeforeTheVoid` | |
| | The customer pays in that window | it was paid | cannot void; records `billing_invoice_paid_wrong_total`; the month is not marked complete; a person refunds | `TestInvoiceWrongTotalPaidByTheCustomerBeforeTheVoidIsHeldNotCompleted` | |
| | The next visit comes after the replay window | until a person voids it | the read-only lookup records `open_wrong_total` (or `paid_wrong_total`) and files a review; it does not void | `TestInvoiceWrongTotalFoundPastTheReplayWindowIsRecordedAndReviewed` | |
| -| The void request fails or times out | until a later visit's void lands; if the provider remembers a 5xx for the void's key, that is up to 24 hours, because the key is fixed (open) | retries the void on the next visit | `TestInvoiceWrongTotalWhenTheVoidFails`, first two cases | |
| +| The void request fails or times out | until a later visit's void lands; if the provider remembers a failure for the void's key, that is the rest of the hour, because the key changes with the hour | retries the void on the next visit | `TestInvoiceWrongTotalWhenTheVoidFails`, first two cases | |
| | The provider refuses the void | until a person acts | records a review, never charges, month not complete | `TestInvoiceWrongTotalWhenTheVoidFails`, third case | |
| So after the void there is still customer-payable exposure in five of six cases, and a customer who pays is owed a refund. A void is sent only for the invoice this month is bound to in every checked field, with a subtotal the provider actually sent. Only prevention closes the defect: no line may land on the draft between the line check and the finalize. As designed here that is not achieved: see the withdrawn sentence above. |
diff --git a/docs/superpowers/specs/DISPOSITIONS-late-request-fencing.md b/docs/superpowers/specs/DISPOSITIONS-late-request-fencing.md| index c79613c08..f00fa494c 100644 |
| --- a/docs/superpowers/specs/DISPOSITIONS-late-request-fencing.md |
| +++ b/docs/superpowers/specs/DISPOSITIONS-late-request-fencing.md |
| @@ -16,7 +16,7 @@ Both found the transport itself sound in what it does: no redirect, retry, conne |
| | Astra P1: the plan's argument for the invoice path assumes the key was first used after this sender's check; a retry at hour 21 leaves about two hours, not the provider's whole idle limit | Accepted, not fixed. The bound must be counted from the key's first possible use. Open. | plan document | none | |
| | Both, P2: a void made on the recovery path past 22 hours is an unjournaled, unauthorized write, and its outcome is returned as an error before it is acknowledged, so it is neither recorded nor reviewed, and the visit repeats | Fixed in the commit after `a64517ba`. The lookup past the replay window is read-only again: it does not void. What it finds (open or paid with a wrong total) is acknowledged on the operation and filed as a review. An invoice found open that way stays payable until a person voids it. | `console/billinginvoice.go` `perform`, `recoverFinalize`, `finalizedOrVoided` | `TestInvoiceWrongTotalFoundPastTheReplayWindowIsRecordedAndReviewed` | |
| | Astra P2: an invoice response with a missing subtotal, or failing on mode, currency or metadata, can authorize voiding a correct invoice | Fixed in the same commit. A void needs the invoice to pass every identity check `verifyInvoice` makes and to carry a subtotal the provider actually sent. | `console/billinginvoice.go` `finalizedOrVoided`; `billingstripe/invoices.go` `HasSubtotal` | `TestInvoiceWithNoSubtotalInTheAnswerIsNotVoided` | |
| -| Opus P2: the void uses a fixed key, so a remembered 5xx replays for 24 hours | Accepted, not fixed. Open. | `console/billinginvoice.go` | none | |
| +| Opus P2: the void uses a fixed key, so a remembered 5xx replays for 24 hours | Fixed. The void's key changes with the hour, so a remembered failure blocks it for the rest of that hour at most; a void asked again for an invoice already void is refused outright and read. The fake now remembers a failure under its key. | `console/billinginvoice.go` `finalizedOrVoided` | `TestInvoiceVoidIsNotBlockedForADayByARememberedFailure`, `TestInvoiceVoidAskedAgainInALaterHourLearnsItIsAlreadyVoid` | |
| | Both, P2: the sandbox idle probe can log VERIFIED without having held a connection, and has no positive control | Accepted, not fixed. Open. It has never been run. | `billingstripe/sandbox_fence_test.go` | none | |
| | Astra P3, Opus P3: transport tests that do not exercise what their names say (the hold placed before the check; the proxy never exercised; a response-timeout test that can pass on a dial timeout) | Accepted, not fixed. Open. | `billingstripe/transport_test.go` | none | |
| | Both, claims: the plan says both workers pass their lease ends; that the void ensures the invoice is not left payable; that a mutation arrives within the idle limit; `client.go` says status 0 means the request never reached Stripe | Corrected. The plan's delivery bound is marked withdrawn and its argument kept only as a record. | plan document, `client.go` | none | |
| @@ -32,4 +32,4 @@ The plan rests on one sentence: a request held past the provider's idle limit is |
| - It does not cover bytes already handed to the kernel. A stopped process does not stop the kernel sending or retransmitting them, and a suspended machine sends them when it resumes. |
| - Several pauses with progress between them are not bounded by one idle interval. |
| -So the wall-clock check plus the provider's idle limit bounds when a request can BEGIN to be written. It does not bound when a complete request is received. Until there is a supported provider-side bound on that (or a design that does not need one), items 7 and 9 stay open and both paths stay disabled, whatever the sandbox probe later measures. Nobody is asked to accept that risk. |
| +So the wall-clock check plus the provider's idle limit bounds when a request can BEGIN to be written. It does not bound when a complete request is received. Until there is a supported provider-side bound on that (or a design that does not need one), items 7 and 9 stay open and both paths stay disabled, whatever the sandbox probe later measures. Nobody is asked to accept that risk. The void's key fixed again. |
diff --git a/internal/console/billinginvoice.go b/internal/console/billinginvoice.go| index 565d9a850..5bb4a6d91 100644 |
| --- a/internal/console/billinginvoice.go |
| +++ b/internal/console/billinginvoice.go |
| @@ -1183,7 +1183,14 @@ func (r *billingInvoiceRun) finalizedOrVoided(ctx context.Context, inv *billings |
| default: |
| return inv.ID, inv.Status, verr |
| } |
| - voided, err := r.b.client.VoidInvoice(ctx, inv.ID, r.operationKey("void", 1)) |
| + // The void's key changes with the hour. A provider that began a |
| + // request and failed remembers that failure under the key and replays |
| + // it for a day; with one fixed key the wrong invoice would stay |
| + // payable for that day. Voiding is safe to ask for again: an invoice |
| + // already void refuses the second request outright, and that refusal |
| + // is read below. So the longest a remembered failure can block the |
| + // void is the rest of the hour it happened in. |
| + voided, err := r.b.client.VoidInvoice(ctx, inv.ID, r.operationKey("void", r.now.UTC().Unix()/3600)) |
| if err == nil { |
| return voided.ID, voided.Status, verr |
| } |
diff --git a/internal/console/billinginvoice_test.go b/internal/console/billinginvoice_test.go| index b7505b780..281b65f65 100644 |
| --- a/internal/console/billinginvoice_test.go |
| +++ b/internal/console/billinginvoice_test.go |
| @@ -42,6 +42,7 @@ type fakeInvoiceStripe struct { |
| requests int |
| loseResponse map[string]bool // apply, then answer 502 once |
| dropRequest map[string]bool // do not apply, answer 502 once |
| + rememberFailure map[string]bool // do not apply, answer 500 and replay it for the key |
| refuse map[string]int // definitive HTTP status once |
| extraLine bool |
| omitSubtotal bool // finalized invoices are answered with no subtotal field |
| @@ -126,6 +127,16 @@ func (f *fakeInvoiceStripe) fault(kind string, w http.ResponseWriter, apply func |
| w.WriteHeader(http.StatusBadGateway) |
| return |
| } |
| + if f.rememberFailure[kind] { |
| + // What the provider does with a request that began and failed: it |
| + // remembers the failure under the key and replays it. |
| + delete(f.rememberFailure, kind) |
| + body := []byte(`{"error":{"type":"api_error"}}`) |
| + f.keyed[key] = fakeKeyed{status: http.StatusInternalServerError, body: body} |
| + w.WriteHeader(http.StatusInternalServerError) |
| + w.Write(body) |
| + return |
| + } |
| f.mutations[kind]++ |
| body, _ := json.Marshal(apply()) |
| f.keyed[key] = fakeKeyed{status: 200, body: body} |
| @@ -163,6 +174,7 @@ func (f *fakeInvoiceStripe) serve(w http.ResponseWriter, r *http.Request) { |
| } |
| f.arrivals[kind]++ |
| if prior, ok := f.keyed[key]; ok { |
| + w.Header().Set("Idempotent-Replayed", "true") |
| w.WriteHeader(prior.status) |
| w.Write(prior.body) |
| return |
| @@ -1942,3 +1954,62 @@ func TestInvoiceWithNoSubtotalInTheAnswerIsNotVoided(t *testing.T) { |
| t.Fatalf("no review: %v", ix.reviews(t)) |
| } |
| } |
| + |
| +// The provider began a void, failed, and remembers the failure under the |
| +// void's key: every request with that key is answered with the same |
| +// failure. The void's key changes with the hour, so the wrong invoice is |
| +// voided on the first visit of the next hour, not a day later. Within the |
| +// hour it stays open, and payable, and this test says so. |
| +func TestInvoiceVoidIsNotBlockedForADayByARememberedFailure(t *testing.T) { |
| + ix := wrongTotalFixture(t) |
| + ix.fake.mu.Lock() |
| + if ix.fake.rememberFailure == nil { |
| + ix.fake.rememberFailure = map[string]bool{} |
| + } |
| + ix.fake.rememberFailure["void"] = true |
| + ix.fake.mu.Unlock() |
| + if err := ix.run(t); err == nil { |
| + t.Fatal("the visit whose void failed reported success") |
| + } |
| + if got := ix.invoiceStatus(t); got != "open" { |
| + t.Fatalf("after the failed void the invoice is %q; this test pins that it is still open", got) |
| + } |
| + // The same hour: the same key, the same remembered failure. |
| + ix.now = ix.now.Add(time.Minute) |
| + _ = ix.run(t) |
| + if got := ix.invoiceStatus(t); got != "open" { |
| + t.Fatalf("a second visit in the same hour: the invoice is %q; the fake was meant to replay the failure", got) |
| + } |
| + // The next hour: a new key. |
| + ix.now = ix.now.Add(time.Hour) |
| + _ = ix.run(t) |
| + if got := ix.invoiceStatus(t); got != "void" { |
| + t.Fatalf("the first visit of the next hour left the wrong-total invoice %q", got) |
| + } |
| + if !ix.hasReview(t, "billing_invoice_voided_wrong_total") { |
| + t.Fatalf("no review says the invoice was voided: %v", ix.reviews(t)) |
| + } |
| +} |
| + |
| +// The void ran and its answer was lost, and the engine comes back in a |
| +// later hour, with another key. The invoice is already void, the provider |
| +// refuses the second void outright, and the engine reads the invoice and |
| +// records it as voided. It is voided once. |
| +func TestInvoiceVoidAskedAgainInALaterHourLearnsItIsAlreadyVoid(t *testing.T) { |
| + ix := wrongTotalFixture(t) |
| + ix.fake.loseResponse["void"] = true |
| + if err := ix.run(t); err == nil { |
| + t.Fatal("the visit whose void answer was lost reported success") |
| + } |
| + ix.now = ix.now.Add(2 * time.Hour) |
| + _ = ix.run(t) |
| + if got := ix.invoiceStatus(t); got != "void" { |
| + t.Fatalf("the invoice is %q", got) |
| + } |
| + if _, mutations, _ := ix.fake.counts(); mutations["void"] != 1 || mutations["pay"] != 0 { |
| + t.Fatalf("mutations %v: want one void and no payment", mutations) |
| + } |
| + if !ix.hasReview(t, "billing_invoice_voided_wrong_total") { |
| + t.Fatalf("no review says the invoice was voided: %v", ix.reviews(t)) |
| + } |
| +} |