amadan
WIP: void a finalized invoice with a wrong total; measure the provider's idle limit in the sandbox
On top of the connection-bound transport: - The invoice engine voids an open invoice whose subtotal is not the frozen total in the visit that finds it, so it is not left payable from the hosted page while it waits for a person. - The sandbox test holds a request on an established connection for six minutes and requires that the provider did not process it. - The plan records what is built and the one premise that is unmeasured. Not gated, not reviewed. Both paths stay disabled.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
5 files changed,
+197
−4
docs/superpowers/plans/2026-10-01-late-request-fencing.md+11 −0internal/billingstripe/invoices.go+13 −0internal/billingstripe/sandbox_fence_test.go+46 −0internal/console/billinginvoice.go+46 −4internal/console/billinginvoice_test.go+81 −0
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 d79b299c8..0d122db5e 100644 |
| --- a/docs/superpowers/plans/2026-10-01-late-request-fencing.md |
| +++ b/docs/superpowers/plans/2026-10-01-late-request-fencing.md |
| @@ -62,6 +62,17 @@ If `T_idle` turns out shorter than the lease margin (five minutes), the key fenc |
| - The invoice engine voids an invoice whose finalized total fails verification, in the pass that finds it. With the mechanism above that state should be unreachable; the void is there so that if it is ever reached, the invoice is not left payable. |
| - The simulation's fake provider forgets keys after 24 hours, so a schedule that relied on a permanent key fails. |
| +## Built so far on the branch (2026-10-01, not gated, not reviewed) |
| + |
| +- The transport: one connection per request, established before the last check, no reuse, no keepalive, no retry, HTTP/1.1, no proxy. The last check reads the caller's send deadline and the context's deadline on the wall clock, so a suspended sender wakes to a closed window without any caller asking for it. Tests against a server that closes idle connections: a held request is never processed and never re-sent; a request past its deadline is never written; every request has its own connection (`internal/billingstripe/transport.go`, `transport_test.go`). |
| +- The quantity sender passes its lease's margin as the send deadline. |
| +- The invoice engine voids an open invoice whose subtotal is not the frozen total, in the visit that finds it (`finalizedOrVoided`), and records `billing_invoice_voided_wrong_total`. Test: a duplicate line lands between the line check and the finalize; the invoice ends void, never paid. |
| +- The sandbox test measures the idle limit: a request held six minutes (configurable) on an established connection must not be processed. |
| + |
| +Not built: the fake provider forgetting keys after 24 hours in the quantity simulation, with holds longer than that; a review of this branch. |
| + |
| +What this does not yet prove, stated plainly: the provider's idle limit is unmeasured. Until the sandbox run measures it, the argument above is a design with one untested premise, and both paths stay disabled. The void narrows the invoice exposure to the seconds between a wrong finalize and the void in the same visit; it does not remove it by itself, which is why the transport is the fix and the void is the net. |
| + |
| ## What stays true |
| - A consumed key is still not a permanent token. The argument above does not need it to be. |
diff --git a/internal/billingstripe/invoices.go b/internal/billingstripe/invoices.go| index 5fbf0bd6e..1b745757d 100644 |
| --- a/internal/billingstripe/invoices.go |
| +++ b/internal/billingstripe/invoices.go |
| @@ -288,6 +288,19 @@ func (c *Client) FinalizeInvoice(ctx context.Context, id, key string) (*Invoice, |
| return &out, nil |
| } |
| +// VoidInvoice voids a finalized, unpaid invoice. A void invoice cannot be |
| +// paid, by the engine or by the customer from the hosted page. |
| +func (c *Client) VoidInvoice(ctx context.Context, id, key string) (*Invoice, error) { |
| + if !validStripeObject(id, "in_") { |
| + return nil, &Error{Code: CodeInvalidID} |
| + } |
| + var out Invoice |
| + if err := c.do(ctx, http.MethodPost, "/v1/invoices/"+id+"/void", url.Values{}, key, &out); err != nil { |
| + return nil, err |
| + } |
| + return &out, nil |
| +} |
| + |
| func (c *Client) PayInvoice(ctx context.Context, id, key string) (*Invoice, error) { |
| if !validStripeObject(id, "in_") { |
| return nil, &Error{Code: CodeInvalidID} |
diff --git a/internal/billingstripe/sandbox_fence_test.go b/internal/billingstripe/sandbox_fence_test.go| index c298b605f..ae4a92fee 100644 |
| --- a/internal/billingstripe/sandbox_fence_test.go |
| +++ b/internal/billingstripe/sandbox_fence_test.go |
| @@ -443,5 +443,51 @@ func TestSandboxQuantityFence(t *testing.T) { |
| }) |
| } |
| + // The connection bound (transport.go). A request held after its last |
| + // check, on an established connection, for longer than the provider's |
| + // idle limit must never be processed. This measures that the limit is |
| + // no longer than the wait: the request is written after the wait, and |
| + // the provider must not have run it. The late-request design needs the |
| + // limit to be under about a day; the lease's own margin is five |
| + // minutes, and the default wait is a little over that. |
| + t.Run("idle: a request held on an established connection past the provider's idle limit is not processed", func(t *testing.T) { |
| + idle := 6 * time.Minute |
| + if d, err := time.ParseDuration(os.Getenv("CARLOS_STRIPE_SANDBOX_IDLE")); err == nil && d > 0 { |
| + idle = d |
| + } |
| + probe := raw("POST", "/v1/customers", url.Values{"name": {"carloku idle probe " + run}}, "carloku-fence-check-"+run+"-idle-customer") |
| + probeID := id(probe, "customer") |
| + t.Cleanup(func() { raw("DELETE", "/v1/customers/"+probeID, nil, "") }) |
| + held := newOneShotTransport() |
| + held.afterConnect = func() { |
| + t.Logf("connection established; holding the request for %v", idle) |
| + time.Sleep(idle) |
| + } |
| + form := url.Values{"metadata[carloku_idle_probe]": {run}} |
| + req, err := http.NewRequestWithContext(ctx, "POST", defaultBaseURL+"/v1/customers/"+probeID, strings.NewReader(form.Encode())) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + req.Header.Set("Authorization", "Bearer "+key) |
| + req.Header.Set("Stripe-Version", apiVersion) |
| + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") |
| + req.Header.Set("Idempotency-Key", "carloku-fence-check-"+run+"-idle") |
| + resp, err := held.RoundTrip(req) |
| + answered := "no answer (the connection was closed)" |
| + if err == nil { |
| + answered = "HTTP " + strconv.Itoa(resp.StatusCode) |
| + resp.Body.Close() |
| + } |
| + after := raw("GET", "/v1/customers/"+probeID, nil, "") |
| + md, _ := after.body["metadata"].(map[string]any) |
| + if md["carloku_idle_probe"] == run { |
| + t.Fatalf("a request held for %v on an established connection was processed (%s): the provider's idle limit is longer than that. Raise CARLOS_STRIPE_SANDBOX_IDLE to find it; the design needs it under a day and assumes nothing until it is measured", idle, answered) |
| + } |
| + if err == nil { |
| + t.Fatalf("the held request was answered %s and yet changed nothing: not the refusal this check is looking for", answered) |
| + } |
| + t.Logf("VERIFIED: a request held %v after its connection was established was not processed (%s)", idle, answered) |
| + }) |
| + |
| t.Log("NOT VERIFIABLE HERE: behaviour after the provider has forgotten a key (documented: at least 24 hours, then a reused key runs as a new request), and the provider's later reconciliation of a request it answered with a 500. The fence is bounded by the first, and an operation whose key is held by its own request is never retired without a person because of the second.") |
| } |
diff --git a/internal/console/billinginvoice.go b/internal/console/billinginvoice.go| index bc94209de..0e76b47ca 100644 |
| --- a/internal/console/billinginvoice.go |
| +++ b/internal/console/billinginvoice.go |
| @@ -989,7 +989,7 @@ func (r *billingInvoiceRun) stepDraft(ctx context.Context) error { |
| if err != nil { |
| return "", "", err |
| } |
| - return inv.ID, inv.Status, r.verifyFinalized(inv) |
| + return r.finalizedOrVoided(ctx, inv) |
| }, |
| recover: r.recoverFinalize, |
| ack: r.ackFinalized, |
| @@ -1001,7 +1001,8 @@ func (r *billingInvoiceRun) stepDraft(ctx context.Context) error { |
| return "", "", "", "", err |
| } |
| if inv.Status != "draft" { |
| - return inv.ID, inv.Status, "", "", r.verifyInvoice(inv, "open", "paid") |
| + id, status, verr := r.finalizedOrVoided(ctx, inv) |
| + return id, status, "", "", verr |
| } |
| return "", "", "", "billing_invoice_refused", nil |
| }, |
| @@ -1092,7 +1093,44 @@ func (r *billingInvoiceRun) recoverFinalize(ctx context.Context, op billingOpera |
| } |
| return "", "", nil |
| } |
| - return inv.ID, inv.Status, r.verifyFinalized(inv) |
| + return r.finalizedOrVoided(ctx, inv) |
| +} |
| + |
| +// finalizedOrVoided verifies an invoice the provider reports as finalized. |
| +// One failure gets more than a review: an OPEN invoice, the one this month |
| +// is bound to, whose subtotal is not the frozen total. Finalized, it is |
| +// payable by the customer from the provider's hosted page whatever this |
| +// engine decides, and its amount is wrong. It is voided here, in the pass |
| +// that found it, so it is not left payable while it waits for a person. |
| +// |
| +// The lines are checked against the frozen calculation immediately before |
| +// the finalize, so this state needs a line to land on the draft between |
| +// that check and the finalize. The transport is built so that cannot |
| +// happen (billingstripe, transport.go). This is the net under that. |
| +// |
| +// A void that fails in a way that may succeed later leaves the finalize |
| +// unacknowledged, and the next visit comes back here. A void the provider |
| +// refuses outright is reported as the open invoice it still is, and held. |
| +func (r *billingInvoiceRun) finalizedOrVoided(ctx context.Context, inv *billingstripe.Invoice) (string, string, error) { |
| + verr := r.verifyFinalized(inv) |
| + if verr == nil { |
| + return inv.ID, inv.Status, nil |
| + } |
| + if inv == nil { |
| + return "", "", verr |
| + } |
| + if inv.Status != "open" || inv.ID != r.rec.InvoiceID || inv.Customer != r.rec.Customer || inv.Subtotal == r.calc.Invoice.TotalCents { |
| + return inv.ID, inv.Status, verr |
| + } |
| + voided, err := r.b.client.VoidInvoice(ctx, inv.ID, r.operationKey("void", 1)) |
| + if err != nil { |
| + var stripeErr *billingstripe.Error |
| + if errors.As(err, &stripeErr) && definitiveRefusal(stripeErr) { |
| + return inv.ID, inv.Status, verr |
| + } |
| + return "", "", err |
| + } |
| + return voided.ID, voided.Status, verr |
| } |
| func (r *billingInvoiceRun) ackFinalized(_, outcome string) (string, string, string) { |
| @@ -1101,6 +1139,10 @@ func (r *billingInvoiceRun) ackFinalized(_, outcome string) (string, string, str |
| return "open", r.rec.InvoiceID, "" |
| case "paid": |
| return "complete", r.rec.InvoiceID, "" |
| + case "void": |
| + // Voided by finalizedOrVoided: the month stays where it is, bound |
| + // to an invoice nobody can pay, until a person has looked. |
| + return "", "", "billing_invoice_voided_wrong_total" |
| } |
| return "", "", "billing_invoice_mismatch" |
| } |
| @@ -1120,7 +1162,7 @@ func (r *billingInvoiceRun) stepFinalizing(ctx context.Context) error { |
| if err != nil { |
| return "", "", err |
| } |
| - return inv.ID, inv.Status, r.verifyFinalized(inv) |
| + return r.finalizedOrVoided(ctx, inv) |
| }, |
| recover: r.recoverFinalize, |
| ack: r.ackFinalized, |
diff --git a/internal/console/billinginvoice_test.go b/internal/console/billinginvoice_test.go| index 390d7fe72..d026ab869 100644 |
| --- a/internal/console/billinginvoice_test.go |
| +++ b/internal/console/billinginvoice_test.go |
| @@ -153,6 +153,8 @@ func (f *fakeInvoiceStripe) serve(w http.ResponseWriter, r *http.Request) { |
| kind = "finalize" |
| case strings.HasSuffix(path, "/pay"): |
| kind = "pay" |
| + case strings.HasSuffix(path, "/void"): |
| + kind = "void" |
| } |
| f.arrivals[kind]++ |
| if prior, ok := f.keyed[key]; ok { |
| @@ -200,6 +202,18 @@ func (f *fakeInvoiceStripe) serve(w http.ResponseWriter, r *http.Request) { |
| return |
| } |
| f.fault(kind, w, func() any { inv.status = "open"; return f.invoiceJSON(inv, false) }, key) |
| + case strings.HasSuffix(path, "/void"): |
| + inv := f.invoices[strings.TrimSuffix(strings.TrimPrefix(path, "/v1/invoices/"), "/void")] |
| + if inv == nil { |
| + w.WriteHeader(http.StatusNotFound) |
| + return |
| + } |
| + if inv.status != "open" { |
| + w.WriteHeader(http.StatusBadRequest) |
| + w.Write([]byte(`{"error":{"type":"invalid_request_error","code":"invoice_not_open"}}`)) |
| + return |
| + } |
| + f.fault(kind, w, func() any { inv.status = "void"; return f.invoiceJSON(inv, false) }, key) |
| case strings.HasSuffix(path, "/pay"): |
| inv := f.invoices[strings.TrimSuffix(strings.TrimPrefix(path, "/v1/invoices/"), "/pay")] |
| if inv == nil { |
| @@ -1558,3 +1572,70 @@ func TestInvoiceEngineBillsAPartialFirstMonthOnItsPaidDays(t *testing.T) { |
| t.Fatal("a billable partial month sent nothing to the provider") |
| } |
| } |
| + |
| +// The exposure this guards against: a finalized invoice is open at the |
| +// provider, and an open invoice can be paid by the customer from the hosted |
| +// page whatever the engine decides. Here a duplicate line lands on the |
| +// draft between the engine's line check and its finalize (a stale copy of a |
| +// line request, arriving late). The invoice is finalized with a total that |
| +// is not the frozen one. The engine voids it in the same visit, records |
| +// that it did, holds the month for a person, and never charges it. |
| +func TestInvoiceEngineVoidsAFinalizedInvoiceWithAWrongTotal(t *testing.T) { |
| + ix := newInvoiceFixture(t) |
| + landed := false |
| + ix.srv.billing.invoice.hooks = &billingInvoiceHooks{beforePost: func(kind string) error { |
| + if kind != "finalize" || landed { |
| + return nil |
| + } |
| + landed = true |
| + ix.fake.mu.Lock() |
| + defer ix.fake.mu.Unlock() |
| + for _, inv := range ix.fake.invoices { |
| + if inv.status == "draft" && len(inv.lines) > 0 { |
| + dup := inv.lines[0] |
| + dup.id, dup.item = dup.id+"_late", dup.item+"_late" |
| + inv.lines = append(inv.lines, dup) |
| + } |
| + } |
| + return nil |
| + }} |
| + err := ix.run(t) |
| + ix.srv.billing.invoice.hooks = nil |
| + if !landed { |
| + t.Fatal("fixture: the engine never reached its finalize") |
| + } |
| + if err == nil { |
| + t.Fatal("the visit that finalized a wrong total reported success") |
| + } |
| + ix.fake.mu.Lock() |
| + var statuses []string |
| + for _, inv := range ix.fake.invoices { |
| + statuses = append(statuses, inv.status) |
| + } |
| + ix.fake.mu.Unlock() |
| + if len(statuses) != 1 || statuses[0] != "void" { |
| + t.Fatalf("invoices at the provider are %v: the wrong-total invoice was left payable (visit: %v)", statuses, err) |
| + } |
| + _, mutations, _ := ix.fake.counts() |
| + if mutations["void"] != 1 || mutations["pay"] != 0 { |
| + t.Fatalf("mutations %v: want one void and no payment", mutations) |
| + } |
| + found := false |
| + for _, reason := range ix.reviews(t) { |
| + if strings.HasPrefix(reason, "billing_invoice_voided_wrong_total") { |
| + found = true |
| + } |
| + } |
| + if !found { |
| + t.Fatalf("no review says the invoice was voided: %v", ix.reviews(t)) |
| + } |
| + // Later visits charge nothing and do not finalize anything else. |
| + for i := 0; i < 3; i++ { |
| + ix.now = ix.now.Add(time.Hour) |
| + _ = ix.run(t) |
| + } |
| + _, mutations, _ = ix.fake.counts() |
| + if mutations["pay"] != 0 || mutations["finalize"] != 1 || mutations["void"] != 1 { |
| + t.Fatalf("after later visits, mutations %v", mutations) |
| + } |
| +} |