OPL-5278: wait out x402 202 pending answers and never pay a challenge twice

[mboyd1]

Problem

The BananaBlocks server (OPL-5253, bananablocks 4ced125, not yet deployed) now answers an x402 payment proof that the network has not accepted yet with 202 Accepted, Retry-After: 10 and {"status":"pending","error":"..."}. A 202 grants nothing. The client should resubmit the same proof until it gets 200, and never pay the challenge again.

bb treated any 2xx as settled, failed with upgrade response contained no settlement details, and kept no proof. A rerun then built and paid a second transaction for the same challenge, and only one payment is credited. x402 is live on production, so this has to ship before or with the server deploy.

Root cause

• submitProof made one POST and decoded any 2xx as an UpgradeResult.
• Nothing was kept between runs, so every run started from a fresh challenge and built a new payment.
• The server re-serves only an unexpired open challenge (store.CreateOrReuseX402Challenge: consumed_at IS NULL AND expires_at > NOW()). The default TTL is 600s. So a proof still pending past expiry gets a new challenge id on rerun, and matching saved proofs by challenge id alone would still pay twice.

Fix

• Pending loop. settleProof resubmits the identical X402-Proof after each Retry-After until it gets 200 or a terminal answer.
  • Retry-After is read as integer seconds. Missing or invalid values wait 10s, and the result is clamped to 1-60s.
  • A new --wait flag (default 10m) caps the total wait. It is separate from --timeout, which still bounds each request through the client's http.Client.Timeout.
  • A 429 from the upgrade route's own limiter is waited out the same way.
  • Each pending answer prints one progress line to stderr, and Ctrl-C stops the wait at once.
  • The rate-limit auto-upgrade path (payChallenge) uses the same loop. Its submits are no longer cut off by the old whole-flow flagTimeout context; only the payment build keeps that timeout.
• Never pay twice.
  • Before the first submit, the proof is saved to os.UserConfigDir()/bb/pending-upgrades.json. The write is atomic (temp file + rename), the file is 0600 and its directory 0700.
  • Entries are keyed by a sha256 fingerprint of host + API key (the key itself is not stored) and the challenge id. Each entry holds the challenge id, tier, amount, txid, proof header, pay URL and creation time.
  • If the save fails, nothing is submitted. bb never broadcasts, so nothing is paid.
  • On a later run, any saved entry for the key is resumed instead of building a payment, whatever challenge the server serves now. That covers the spec's "same challenge id re-served" case and the expired case above. ensureNotExpired does not apply to a resume. --dry-run only reports the saved entry.
  • The rate-limit inline offer refuses to pay while an entry is saved and points to bb key upgrade.
• Deleting entries. An entry is removed on 200, and on answers after which the same proof can never settle: 400, 402, 404, 409 (other than consumed), 410 and 422.
  • When --wait runs out, on network or 5xx errors and on Ctrl-C, the entry is kept. The error names the txid and the file and says rerun \bb key upgrade` to resume it — do not pay again`.
• Lost 200. On 409 challenge already consumed, bb reads GET /api/v1/key/usage.
  • If the key's tier is at or above the target (equal, or ranked by free < pro < enterprise), bb reports success and deletes the entry.
  • Otherwise it returns an error and keeps the entry. The server's settle clears only the local node's resolver cache ("other LB nodes converge within the cache TTL"), so another node can report the old tier for a while, and a rerun re-checks.
• Malformed store file. It fails closed: bb returns an error and never reads the file as empty or overwrites it, because it may record a payment that is still settling.
• 409 message. It no longer says "the payment tx was not broadcast by this attempt". The server raises payment txid already used, and a settle-time downgrade, after broadcasting (settleAndGrant), and this path now also deletes the saved proof.
• Docs. README and bb key upgrade --help cover the pending wait, --wait and resume.

Tests

All tests run against httptest fakes of the server contract, with an injected clock and sleep, so nothing really sleeps. No payment is made and no real key is used.

• TestUpgradePendingThenSettles: 202 (Retry-After 7), 202 (no header), 200 → success. Exactly 3 submits, all with the identical proof. Waits are [7s, 10s], one payment is built, the proof is on disk before every submit, and the entry is gone afterwards.
• TestUpgradePendingBudgetThenResume: always 202 with --wait 25s → submits at 0s, 10s and 20s, then an error that names the txid and says to resume, not pay again. The entry is kept. The rerun comes after the server has switched to a new challenge id; it resubmits the saved proof, builds no second payment and fetches no challenge.
• TestUpgradeResumesSavedProofForSameChallenge: saved entry plus the same, already expired challenge id re-served → the saved proof is resubmitted. BuildPayment is never called (the seam fails the test if it is).
• TestUpgradeConsumedChecksTier: 409 consumed with tier pro or enterprise → success and entry deleted. With tier free or empty → error and entry kept.
• TestUpgradeTerminalAndTransientAnswers: 422, 409 txid-used and 410 → error and entry deleted. 500, and a 200 with no settlement → error and entry kept. None of these is resubmitted.
• TestUpgradeRetriesAfter429, TestUpgradeCancelWhilePending, TestSleepCtxCancels, TestParseRetryAfter (missing, non-numeric, HTTP-date, 0, negative, 61, 86400, int64 overflow).
• TestPayChallengeRefusesWhileSaved, TestPayChallengeInlineWaitsForPending (auto-upgrade path).
• internal/x402/pending_test.go: file 0600 and dir 0700, a loose-permission file tightened to 0600, per-key filtering, replace-on-put, delete, file removed when empty, missing and empty files read as no entries, a malformed file fails closed and is left unmodified, and the fingerprint differs by host and key without containing the key.

Red without the fix (mutation battery): each source file was copied into mktemp -d, one mutant applied at a time (match count checked, go vet gate), the targeted tests run, and the file copied back (verified byte-identical). All 24/24 mutants were killed. They include:

• 202 treated as settled
• resume disabled
• save-before-submit skipped
• tier check always true
• consumed not recognised
• tier rank ignored
• terminal answers keep the entry
• every error drops the entry
• each Retry-After clamp or default changed
• wait budget removed (test times out)
• sleepCtx ignores the context
• 429 not retried
• the inline offer ignores the saved entry
• a 200 deletes nothing
• a malformed file read as empty
• 0644 file or 0755 dir
• duplicate put
• wrong delete filter
• fingerprint ignores host
• the 409 "not broadcast" claim restored

Gates

• make build ✅
• go vet ./... ✅
• go test ./... ✅ (also -race -count=3)
• golangci-lint run (v2.12.2) ✅ 0 issues

Linear: https://linear.app/openprotocollabs/issue/OPL-5278

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

[linear]

OPL-5278

[mboyd1]

Review round 1: what happened to each finding (head b68378f)

1. A bare 404 deleted the saved proof (key.go proofCannotSettle): fixed.
proofCannotSettle now takes the *api.Error. A 404 counts as terminal only when the message contains challenge not found, which is the body the server sends at bananablocks internal/server/x402.go:162 @4ced125. Any other 404 goes to the default arm: the entry is kept and the error carries the resume hint. Test TestUpgrade404OnlyChallengeNotFoundDropsProof covers two cases. A text/plain 404 page not found keeps the entry and names the resume. {"error":"challenge not found"} deletes it. The mutants return true and return false in the 404 arm are both caught.

2. A consumed challenge with the key on a lower tier wedged the key forever: fixed.
The entry now records last_submit_at just before each submit. When the server answers challenge already consumed and the key's tier is lower, the entry is kept only if the previous submit was less than an hour ago (consumedTierGrace). An older entry is removed with an error explaining that it can never settle, and the next run fetches a challenge and pays as normal.
I measured from the last submit rather than from CreatedAt, as the finding suggested, because of one case. Suppose a proof stays pending for more than an hour, then settles, the 200 is lost and the tier read lags. With a CreatedAt bound, that entry would be dropped and the user told to buy again.
Tests:

• TestUpgradeConsumedStaleEntryIsDropped (31 days old): the entry is removed, and a second run fetches once and builds once.
• TestUpgradeConsumedYoungEntryIsKept (59 minutes old).
• TestUpgradeConsumedAfterRecentSubmitIsKept: saved 3 hours ago, the previous submit 10 seconds ago, so it is kept.
• The existing TestUpgradeConsumedChecksTier still passes.

All of these mutants are caught: grace set to 0, grace set to 1000 days, the age check disabled, the age measured from CreatedAt, the submit mark removed, and the mark taken before prevSubmit is read.

3. A saved proof was never matched by challenge id: fixed.

• New PendingStore.ForChallenge(id) searches every fingerprint.
• bb key upgrade: after fetchChallenge and before ensureNotExpired, the wallet and buildPayment, it resumes any saved entry for the fetched challenge id. --dry-run only reports the match.
• payChallenge refuses when either ForKey or ForChallenge(ch.ChallengeID) has an entry.
• KeyFingerprint now normalises its inputs: scheme and host are lowercased, the default port and a trailing slash are dropped, and the API key is TrimSpaced. IPv6 literals keep their brackets.

Tests:

• TestUpgradeResumesSavedProofByChallengeID: saved under 127.0.0.1, run against localhost, zero builds.
• TestPayChallengeRefusesSavedChallengeID.
• TestKeyFingerprintCanonical.
• TestPendingStoreForChallenge.

4. No test covered --wait versus --timeout on the inline path: fixed.
TestPayChallengeSettleNotBoundByTimeout returns 202, 202, then 200 and records whether any pending wait runs under a context deadline. The mutation from the finding (ctx, cancel := context.WithTimeout(ctx, flagTimeout); buildCtx := ctx) now fails this test, and only this test. The rest of the suite still passes under it.

5. The CI lint job is red because of the environment: not changed. It needs someone to approve a change outside this ticket's scope.
The failure is real. setup-go go-version: stable resolves to go1.27.1, and golangci-lint v2.12.2 cannot typecheck the 1.27 standard library. The fix belongs in .github/workflows/ci.yml. That file is outside this ticket's files in scope, and this is not a security finding, so I did not edit it. Candidate fixes: use go-version-file: go.mod in the lint job, or bump golangci-lint-action to a release built with go1.27.
Locally, golangci-lint v2.12.2 (go1.25) reports 0 issues on this head, and make build, go vet ./..., go test ./... and go test -race -count=3 on the changed packages all pass.

Non-blocking notes

• The server exempts /api/v1/key/upgrade from the per-key limiter (bananablocks server.go:1678 @4ced125). A proof resubmit therefore cannot get a rate-limit 402 carrying a new challenge, so keeping 402 in proofCannotSettle is safe.
• resumeSaved still resumes only the oldest entry per run. The others are listed and resumed on later runs.

[mboyd1]

Review round 2 dispositions (commit 56bd42b):

1. Consumed answer extended the one-hour grace (key.go settleProof): fixed. The server returns "challenge already consumed" without settling anything: the ConsumedAt pre-check in VerifyAndApply, or the atomic settle refusing the request in settleAndGrant. So a submit that gets that answer cannot be the one that settled. The consumed arm now restores the previous LastSubmitAt (new unmarkSubmit) before confirmConsumed, which also covers the keep path when the usage read fails. TestUpgradeConsumedAfterRecentSubmitIsKept now expects the 202 submit's time, 10s earlier. New test TestUpgradeConsumedRerunDoesNotExtendGrace saves an entry 50m ago with tier free and always-409-consumed replies: the first run keeps it with LastSubmitAt still zero, then after +30m the rerun drops it with "can never settle again". Mutant (unmarkSubmit call replaced with _ = prevLast): KILLED by both tests.
2. No --dry-run test with a saved entry: fixed. runUpgradeMode adds a dry-run switch. New test TestUpgradeDryRunDoesNotResumeSaved covers two cases: the same fingerprint, and the localhostURL spelling with the same challenge id. It checks that no proof is submitted, no payment is built, the entry is still saved, and stdout decodes as the challenge JSON. Mutants: ForKey guard removed, KILLED. ForChallenge guard changed to if true, KILLED.
3. The 400/402 arms of proofCannotSettle were untested: fixed. Added rows 400 malformed and 402 insufficient to TestUpgradeTerminalAndTransientAnswers. The fake server returns these replies for a POST that carries a proof. Mutants: 400 dropped from the case, KILLED. 402 dropped, KILLED.

README: added a note that a submit answered "consumed" does not restart the hour.
Gates: make build, go vet ./..., go test ./... (and -race on cmd/x402) and golangci-lint run (0 issues) all pass.

[mboyd1]

Review round 3: dispositions (066bf08)

1. A stale at-tier entry was reported as this run's settlement (key.go confirmConsumed): fixed.
confirmConsumed now computes the age from the last earlier submit once. When that submit is more than consumedTierGrace (1h) old, the entry is removed and the run exits non-zero, whatever the tier. At or above the tier the error says the saved payment "settled earlier", names the key's tier and expiry, and says "this run bought nothing". Nothing is written to stdout and no "upgrade settled" line is printed.
I chose to fail rather than fall through into a purchase. Otherwise a user following the resume hint more than an hour after a lost 200 would be charged a second time under --yes.
The young at-tier case (the real lost-200 rerun) still reports success (TestUpgradeConsumedChecksTier).
New test: TestUpgradeConsumedStaleAtTierIsNotThisRunsSettlement. It sets up a 25-day-old entry, usage "pro" and a 409 consumed. It asserts err != nil, empty stdout, entry removed, and 1 submit / 0 challenge fetches / 0 builds. It then checks that the next run fetches 1 challenge and builds 1 payment.

2. settleProof gave up on the first 5xx or transport failure: fixed.

• submitProof now wraps failures in transit in *transientSubmitError: the c.Do error when it is not an *api.Error, and a body-read failure.
• settleProof resubmits the same proof after a wait in two cases: that error while ctx.Err() == nil, and any *api.Error with status >= 500. The wait is Retry-After when present, otherwise 10s, and each attempt goes through the existing --wait deadline check.
• Ctrl-C during a failure in transit still returns at once with the resume hint.
• 409-consumed, 429 and proofCannotSettle are unchanged.
• The "500" row now expects 7 identical submits in a 1m budget with the entry kept. New rows cover a 502 from the LB (HTML body) and a 503 with Retry-After: 30 (3 submits).
• New tests:
  • TestUpgradeRetriesAfterDroppedConnection: the connection is hijacked and closed before the answer, or a 200 is aborted mid-body, then a 200. It asserts 2 identical submits, one 10s wait, 1 build and the entry deleted.
  • TestUpgradeTransientErrorAfterCancelStops: a cancel while the connection drops gives 1 submit, no "resubmitting" line and the entry kept.
• README and --help now describe the new retry behaviour.

3. No test showed that only the consumed arm un-records a submit: fixed (test added).
New test: TestUpgradeLostResponseSubmitStaysRecorded, with subtests for a dropped connection and a 500. It seeds an entry created 3h ago with usage "free". The first submit's response is lost and the resubmit gets 409 consumed, all in one run. It asserts the entry is kept with the rerun hint and that LastSubmitAt equals the time of the lost submit.

Mutation battery (scratch copy of key.go, restored with diff -q; each mutant was checked to match exactly once and to pass go vet). All 6 were killed:

mutant	killed by
drop the stale at-tier guard	TestUpgradeConsumedStaleAtTierIsNotThisRunsSettlement
5xx not retried	Terminal table 500/502/503, LostResponse/500
transport failures not retried	RetriesAfterDroppedConnection (both), LostResponse/dropped_connection
transport arm ignores ctx	TransientErrorAfterCancelStops
unmarkSubmit(errw, store, e, prevLast) at the top of if err != nil (the round-3 mutant)	LostResponseSubmitStaysRecorded (both subtests)
body-read failure not transient	RetriesAfterDroppedConnection/mid-body

Gates: make build, go vet ./..., go test ./... (internal/cmd also 3x with -race) and golangci-lint run (0 issues) all pass.

[mboyd1]

Review round 4 — dispositions (0b92895)

Fixed — 429 arm of settleProof did not un-record the submit (key.go:475).
Verified against bananablocks 4ced125 internal/server/x402.go:54-60: upgradeLimiter.acquire answers 429 before DecodeProofHeader/VerifyAndApply, and that is the only 429 in the upgrade path, so a 429 settled nothing. The 429 arm now calls unmarkSubmit(errw, store, e, prevLast) before waiting, the same as the consumed arm. A following "challenge already consumed" on a long-settled entry is judged by its real last submit and hits the stale at-tier guard.

Regression test TestUpgradeRateLimitedResubmitDoesNotRefreshStaleEntry: a 25-day-old seeded entry, usage pro, replies 429 (Retry-After 1) then 409 consumed. It expects the "settled earlier" / "bought nothing" error, empty stdout, no "upgrade settled" on stderr, the entry deleted, 2 resubmits, 0 fetches, 0 builds and one 1s wait. Mutation check: with the new unmarkSubmit line deleted (file copied to a temp dir and restored afterwards; go vet clean), the test fails with "challenge ch-old was already settled ... upgrade settled".

The transient and 5xx arms still keep the submit recorded, because a lost response or an app-side 5xx may be the submit that settled. TestUpgradeLostResponseSubmitStaysRecorded still passes.

Gates: make build, go vet ./..., go test ./..., golangci-lint run (0 issues).

[mboyd1]

Review round 1 (resumed run) dispositions, head e00f68a:

1. CI lint red (ci.yml): fixed. The lint job's setup-go now uses go-version-file: go.mod. The job log shows setup-go resolving spec 1.25.0 and the go.mod toolchain line then running go1.25.12. On this head, lint, build-test and govulncheck all pass (runs 36283322441 and 36283324429). build-test and govulncheck stay on stable, as the file's comment intends.
2. Resume past expires_at via the ForChallenge path is untested: fixed. TestUpgradeResumesSavedProofByChallengeID now uses a challenge that expired a minute ago by the real clock. I also rewrote the overclaiming comment on TestUpgradeResumesSavedProofForSameChallenge: that test resumes by fingerprint, before any challenge is fetched. Mutant (ensureNotExpired inserted before store.ForChallenge) is now KILLED.
3. --wait default and negative rejection are untested: fixed. New TestUpgradeWaitFlagDefault asserts DefValue 10m0s. New TestUpgradeRejectsNegativeWait expects the error, with 0 server requests and 0 builds. Mutants time.Minute default and if false && upgradeWait < 0 are both KILLED.

I ran the mutants against a scratch copy of key.go, restored it and confirmed the restore with diff -q. Locally, make build, go vet, go test and golangci-lint v2.12.2 all pass (0 issues).