fix(cli): resume pending settlement safely

[h1-hunt]

Summary

• retry payment_settlement_pending responses in-process with the byte-identical PAYMENT-SIGNATURE, idempotency key, method, path, body, and provider path
• handle reconciled settlements and server-issued replacement payment challenges without allowing an unbounded signing loop
• reapply --max-usd before signing a replacement challenge and extend money-sensitive guidance to pending, reconciled, and authorization-conflict responses
• document the in-process recovery behavior and the deliberate absence of persisted payment signatures

Safety properties

• a pending idempotency key never receives a freshly generated authorization
• replacement payment signing is allowed only after a pending-settlement response, with a distinct server-issued x-h402-replacement-idempotency-key and a valid replacement challenge
• replacement signing is bounded and reuses the original request contract
• payment signatures are not persisted to disk

Verification

• npm run test -w @h402/cli -- tests/docs.test.ts tests/api.test.ts tests/call-settlement-retry.test.ts tests/call-max-usd.test.ts
• git diff --check
• npm run typecheck
• npm run lint
• npm test
• npm run build

Closes #43

[clawd800]

GPT review — changes requested

Reviewed head 0d71a9c1e9d872ab5423dceb36bd0f14803963ec.

1. [P1] Preserve settlement-risk guidance after a pending retry fails. Once the loop observes payment_settlement_pending, that response is consumed internally. If the next exact-signature retry fails at the network layer or returns a generic 5xx/malformed response, the final error only appends the idempotency key and omits the warning that settlement may already have occurred. Because the caller never sees the pending response, it can switch to a new key and pay again. Preserve the pending-settlement state and the “do not sign/pay with a new key” guidance across every non-definitive exit after pending is observed. Please add network-error and generic-5xx regressions.

2. [P2] Require the previous-key binding before signing a replacement. replacementPaymentFromResponse accepts a missing x-h402-previous-idempotency-key; it rejects only a present mismatch. The server contract emits both replacement headers, so absence means the response cannot be safely correlated to the pending authorization. Require previousIdempotencyKey === currentIdempotencyKey (including rejecting a missing value) before invoking the signer, with a regression for the missing-header case.

The normal retry path otherwise preserves the exact signature/request, bounds retries and replacement signing, and reapplies --max-usd. CI, CodeQL, build, lint, typecheck, and the existing test suites pass.

[sebayaki]

Review follow-up

Addressed both GPT findings in commit 78e034f.

1. Settlement-risk guidance now survives every non-definitive exit after a pending settlement. callCommand tracks that a payment_settlement_pending response was observed; any later failure — network drop, generic 5xx, or a refusal raised by the replacement path — gets the do-not-repay guidance appended (deduplicated when the backend body already carried it). Regressions added for a post-pending network error and a post-pending generic 500.

2. Replacement signing now requires the previous-key binding. A replacement 402 whose x-h402-previous-idempotency-key header is missing is refused before any signing, matching the server contract that always emits both headers. Regression added for the missing-header case.

Verification: npm test (169 cli + 14 core), npm run typecheck, npm run lint all pass.

[h1-hunt]

Final review

Verdict: REJECT — blocking money-safety gap

Reviewed head b6f046451e7f19db51ab498c315593cd3e178eba against current main 2d335d93a0c487da00db26b8c1a0fdc1be318d84.

Blocker

packages/cli/src/commands.ts:504-505, 553-559, 588-589 — an ambiguous failure of the first signed request still loses the do-not-repay guidance.

settlementMayHaveOccurred remains false until a 409 payment_settlement_pending response has been received. The first await sendSignedRequest() therefore has no risk state yet. If that signed request reaches the server but the response is lost (ECONNRESET, timeout, etc.), the catch path adds only the idempotency key and omits the warning not to sign/pay under a new key. A generic non-settlement-coded 5xx on that first signed request follows the same risk classification.

I added a disposable regression with this sequence:

1. Initial unsigned request → valid payable 402
2. CLI signs once
3. First signed request → ECONNRESET

It failed as follows:

• Actual message: Request ... failed: ECONNRESET (idempotency-key: idem-pending-43)
• Missing: do NOT sign or pay with a new idempotency key
• Signer calls: 1; HTTP calls: 2

A signed request is settlement-risk-bearing as soon as it is sent, not only after the backend manages to return the explicit pending code. Please preserve the risk guidance across every non-definitive exit after any signed send; optionally retry the byte-identical signed request/key in-process on bounded network failures as well. Add regressions for the first signed request returning a network error and a generic 5xx.

Verification performed

• Fresh merge into current main: clean
• Existing focused suites (docs, api, settlement retry, max USD): 40/40 passed
• npm run typecheck: passed
• npm run lint: passed
• npm run build: passed
• Added first-signed-request network regression: failed and reproduced missing safety guidance
• GitHub CI and CodeQL: green

No paid, wallet-signing, production, or provider call was made; signing and HTTP were mocked.

[h1-hunt]

Review follow-up

Addressed the final first-signed-request money-safety blocker in commit a4c839e.

• Settlement risk is now recorded immediately before the first signed request is sent, not only after an explicit payment_settlement_pending response.
• If that first signed request loses its response at the network layer or returns a generic 5xx, the final error preserves both the active idempotency key and the do not sign/pay with a new key guidance.
• Added first-signed-request regressions for ECONNRESET and a generic 500. Each proves one signature, two HTTP calls, and the retained key/guidance.

Verification:

• focused settlement retry suite: 12/12
• full suite: 186/186 (172 CLI + 14 core)
• typecheck, lint, and diff check
• build, pack verification, and clean tarball install smoke
• independent money-safety review: pass

No real wallet signing, provider call, payment, or production request was made; HTTP and signing remained mocked.

[h1-hunt]

GPT Reviewer

Verdict: REJECT

Reviewed head a4c839e3964fade35a833658c4451ccba68e030d against current base 2d335d93a0c487da00db26b8c1a0fdc1be318d84 using a bounded, tool-free review bundle.

An initial replacement-marked 402 bypasses the pending-settlement gate and can cause a fresh authorization to be signed under the original idempotency key. All shared deterministic checks passed.

Findings

1. BLOCKER — Initial replacement challenge bypasses the pending-settlement signing guard (packages/cli/src/commands.ts:546)
   • Evidence: The initial 402 is converted directly to paymentRequired without inspecting x-h402-replacement-idempotency-key, then passed to signPayment(paymentRequired, first.headers, idempotencyKey). The sawPendingSettlement guard is only applied to later responses from signed requests. Consequently, an initial unsigned request—such as a later invocation using a pending key—can receive a valid replacement-marked 402 and cause the CLI to create a fresh authorization before observing pending settlement, pairing it with the original key rather than the server-issued replacement key. This violates the stated safety property and can expose the user to a second charge.
   • Suggested fix: Before resolving the wallet or signing the initial challenge, detect and reject replacement metadata because this process has not observed a pending-settlement response. Alternatively, route every 402 through one state machine that permits replacement signing only after recorded pending state and always uses the validated replacement key. Add a regression where the first unsigned response carries a valid challenge and both replacement headers, asserting one HTTP request and zero signer calls.

Shared exact-snapshot validation

• diff-check: PASS (0.0s)
• npm-ci: PASS (1.833s)
• typecheck: PASS (3.912s)
• lint: PASS (3.671s)
• test: PASS (2.978s)
• build: PASS (2.278s)
• No paid h402/provider calls were made.

[h1-hunt]

Opus Reviewer

Verdict: PASS

Reviewed head a4c839e3964fade35a833658c4451ccba68e030d against current base 2d335d93a0c487da00db26b8c1a0fdc1be318d84 using a bounded, tool-free review bundle.

This PR makes the CLI resume a payment_settlement_pending response by resending the byte-identical PAYMENT-SIGNATURE + idempotency key (never re-signing the pending key), optionally signing one server-issued replacement challenge under strict binding, and preserving do-not-repay guidance across every non-definitive exit after a signed send.

I traced the full callCommand state machine and all supporting helpers, and verified the four money-safety properties from the PR description hold:

• Pending key never gets a fresh authorization: the pending loop reuses signedPayment.paymentSignature/key byte-identically (single paidHeaders, hoisted requestBody); re-signing only occurs for a replacement whose key is proven distinct (replacementIdempotencyKey === currentIdempotencyKey rejected).
• Replacement signing is guarded (402 + replacement header + previousIdempotencyKey === currentKey with missing rejected + distinct key + valid challenge + preceding sawPendingSettlement), re-checks --max-usd, and is bounded to MAX_REPLACEMENT_PAYMENTS = 1, guaranteeing loop termination (≤2 iterations).
• Settlement-risk guidance survives first-signed-send failures (ECONNRESET/5xx) because settlementMayHaveOccurred is set before the first signed send; withSettlementRiskGuidance de-dupes via the shared IDEMPOTENCY_MONEY_GUIDANCE constant.
• No signature persistence anywhere in the diff.

stdout/stderr contract preserved (only final success to stdout; all failures thrown to the error envelope). The api.ts money-sensitive code-set expansion is mirrored in api.test.ts; docs edits match implementation and the new docs.test assertions. The refactor does not worsen the pre-existing (unchanged) absence of the auth header on the paid retry. Shared validation passes: typecheck, lint, build, and 172 CLI + 14 core tests.

Items considered and dismissed as non-actionable: the deliberate double assertUnderMaxUsd/selectBaseUsdcRequirement fail-fast-before-wallet check (documented by comment), the conservative over-warning that names the server-issued activeIdempotencyKey on a max-usd replacement refusal (a tested, intentional safe-direction choice), and the inherent trust that a replacement challenge implies a genuinely failed original (bounded by per-payment --max-usd and a single replacement). None are defects. Zero actionable findings.

Findings

None.

Shared exact-snapshot validation

• diff-check: PASS (0.0s)
• npm-ci: PASS (1.833s)
• typecheck: PASS (3.912s)
• lint: PASS (3.671s)
• test: PASS (2.978s)
• build: PASS (2.278s)
• No paid h402/provider calls were made.