fix(payments): transparent lifecycle durability — no double charges, no lost paid results

.changeset/transparent-payment-durability.md

@@ -0,0 +1,17 @@
+---
+'@contextvm/sdk': patch
+---
+
+Transparent payment robustness (CEP-8): prevent double charges and lost paid results.
+
+- **No double charge on redelivery:** the pending-payment entry is now retained until TTL once an invoice has been issued, instead of being deleted on every failure. Previously a `verifyPayment` timeout (or any post-invoice failure) disarmed the dedup, so a spec-blessed client retry of the same request event minted a second invoice that the client auto-paid — violating CEP-8's "MUST NOT charge more than once for the same transparent request event". Pre-invoice failures still delete the entry so retries are free.
+- **No paid-but-undelivered:** a failed `payment_accepted` publish (relay error, or the idle paying client's session LRU-evicted mid-payment) no longer aborts the forward. The acceptance notification is a SHOULD; the capability result is the point.
+- **Pending capacity fails closed:** at `maxPendingPayments` capacity the middleware purges expired entries and then refuses new priced requests (best-effort `payment_rejected`, no invoice minted) instead of silently evicting a live payment's dedup entry. `maxPendingPayments: 0` now refuses all priced requests.
+- **Route survival for paid requests:** the correlation route (and session) is snapshotted when an invoice is issued; the response router falls back to that snapshot on route miss, so paid results still reach the client after duplicate-delivery cleanup popped the route or the paying client's session was evicted. The dead "recreate session if active routes" guard in the eviction handler was removed (snapshots make it unnecessary, and re-inserting into the session LRU from inside its own eviction callback would corrupt capacity accounting).
+- **No leaked open-stream writers on dropped requests:** a request dropped by middleware (payment gating, policy) or failed by a throwing middleware chain never produces the normal-path response that would reap its writer, so its `OpenStreamWriter` reservation leaked until transport teardown. Drop cleanup now releases the writer alongside the correlation route.
+- **Non-positive TTLs fall back to the default:** `paymentTtlMs: 0` (both lifecycles) and invoice `ttl: 0` (explicit gating) previously birth-expired the pending/grant entry while the invoice stayed payable, disarming the redelivery dedup. They now fall back to the default window, matching the guard `getVerificationTimeoutMs` already applied.
+- **Double registration refuses:** calling `withServerPayments` twice on the same transport used to silently register a second middleware pair with its own dedup closures, minting two invoices and double-charging every priced request. It now throws.
+- **Client handler failures resolve the request:** a throwing in-band payment handler previously surfaced only on `onerror` — the pending MCP request hung until the server TTL, while policy/handler _declines_ resolved it with a synthesized error. Handler crashes now synthesize the same decline error (and still surface on `onerror`).
+- **Verify-task crash cleanup:** the detached explicit-gating verification task's outer catch now clears the pending identity (belt-and-suspenders; idempotent), so a store failure inside the inner catch cannot pin the identity as pending for the whole TTL.
+- **Gating errors carry the client's request id:** `-32042`/`-32043` responses previously leaked the server's internal routing key (the Nostr event id) into the JSON-RPC `id` field, because the targeted-response exit path skipped the id restore the normal response path performs. The restore now runs on both exit paths, so the wire conforms to JSON-RPC 2.0 and CEP-8's examples. SDK clients are unaffected either way (the client transport restores ids independently).
+- **Client: double-wrap guard, retry floor, bounded retry counters, type-aware cache keys** (reported by the rs-sdk maintainers): `withClientPayments` now refuses a second wrap of the same transport (a chained double-wrap double-paid every offer through two independent pipelines with separate dedup sets); `-32043` retries are floored at `minRetryDelayMs` (default 1 s — `retry_after: 0` could otherwise re-send a byte-identical Nostr event, which relays and servers swallow as a duplicate, silently losing the retry); retry counters are bounded by the same LRU capacity as the raw-request cache; and the raw-request cache plus retry counters key by `JSON.stringify(id)` so numeric `5` and string `"5"` no longer collide (a collision retried the wrong request's payload).

src/__mocks__/mock-relay-server.ts

@@ -45,6 +45,7 @@ type ConnectionInstance = {
   cleanup: () => void;
   cleanupWithoutClosingSocket: () => void;
   closeSocket: (code: number, reason: string) => void;
+  terminateSocket: () => void;
   handle: (message: string) => void;
   send: (message: NostrRelayMessage) => void;
 };
@@ -102,6 +103,19 @@ export function startMockRelay(
       }
     }
 
+    terminateSocket(): void {
+      // Simulate the relay process dying: hard reset with no close frame, so
+      // clients observe an abnormal closure (1006 / wasClean=false) — the
+      // signal reconnect logic keys on. A frame-based close(1011) reads as
+      // wasClean=true on bun >= 1.4.0, which stopped clients from
+      // reconnecting after a simulated relay outage.
+      try {
+        this.socket.terminate();
+      } catch {
+        this.closeSocket(1011, 'Relay paused');
+      }
+    }
+
     cleanup(): void {
       // Used by stop()/pause() to actively take the relay offline.
       // Close the socket first, then drop all subscriptions.
@@ -298,9 +312,13 @@ export function startMockRelay(
     relayUrl: `ws://127.0.0.1:${port}`,
     httpUrl: `http://127.0.0.1:${port}`,
     stop: () => {
-      // Close any existing WebSocket connections before stopping the server.
+      // Drop connections *uncleanly* (hard reset) before stopping the server:
+      // tests use stop()+restart-on-same-port to simulate outages/partitions,
+      // and clients only reconnect on abnormal closures. A graceful 1001 reads
+      // as a clean close on bun >= 1.4.0 (no remap), which stopped pools from
+      // reconnecting after recovery.
       for (const instance of state.connections.values()) {
-        instance.closeSocket(1001, 'Relay stopping');
+        instance.terminateSocket();
         instance.cleanupWithoutClosingSocket();
       }
       state.connections.clear();
@@ -310,8 +328,9 @@ export function startMockRelay(
     pause: () => {
       runtime.acceptingWs = false;
       for (const instance of state.connections.values()) {
-        // Close *uncleanly* so clients reconnect (applesauce-relay only retries on !wasClean).
-        instance.closeSocket(1011, 'Relay paused');
+        // Drop *uncleanly* so clients reconnect (applesauce-relay only retries
+        // on !wasClean).
+        instance.terminateSocket();
         instance.cleanupWithoutClosingSocket();
       }
       state.connections.clear();

src/payments/authorization-store.ts

@@ -17,6 +17,11 @@ interface PaidAuthorization {
  * meaning it is strictly single-process. For multi-process horizontal scaling,
  * implementers should use a distributed lock (e.g. Redis Redlock) keyed by
  * the canonical invocation identity to prevent duplicate payments.
+ *
+ * NOTE: Atomic reserve-then-dispatch (`claim()` here, `trySetPending()` in the
+ * gating middlewares) relies on JavaScript run-to-completion: there is no
+ * interleaving point between the two calls. Porters to await-capable runtimes
+ * must compose them into a single critical section.
  */
 export class AuthorizationStore {
   private readonly authorizations: LruCache<PaidAuthorization>;

src/payments/client-payments.test.ts

@@ -1,21 +1,13 @@
 import { describe, expect, test } from 'bun:test';
-import type { Transport } from '@contextvm/mcp-sdk/shared/transport';
 import type { JSONRPCMessage } from '@contextvm/mcp-sdk/types.js';
 import { withClientPayments } from './client-payments.js';
 import type { PaymentHandlerRequest } from './types.js';
 import { NostrClientTransport } from '../transport/nostr-client-transport.js';
+import type { TransportWithContext } from '../transport/nostr-client-transport.js';
 import { PrivateKeySigner } from '../signer/private-key-signer.js';
 import { EncryptionMode } from '../core/interfaces.js';
 import { MockRelayHub } from '../__mocks__/mock-relay-handler.js';
 
-/** Minimal fake transport that exposes onmessageWithContext for unit tests. */
-type TransportWithContext = Transport & {
-  onmessageWithContext?: (
-    message: JSONRPCMessage,
-    ctx: { eventId: string; correlatedEventId?: string },
-  ) => void;
-};
-
 const createMockNostrTransport = (): NostrClientTransport => {
   const hub = new MockRelayHub();
   return new NostrClientTransport({
@@ -267,6 +259,74 @@ describe('withClientPayments()', () => {
     expect(handleCalls).toBe(0);
   });
 
+  test('synthesizes JSON-RPC error when the payment handler throws, instead of hanging the request', async () => {
+    const transport = createMockNostrTransport();
+
+    const observed: JSONRPCMessage[] = [];
+    const errors: Error[] = [];
+
+    const paid = withClientPayments(transport, {
+      handlers: [
+        {
+          pmi: 'fake',
+          async handle(): Promise<void> {
+            throw new Error('wallet exploded');
+          },
+        },
+      ],
+    });
+
+    paid.onmessage = (msg) => observed.push(msg);
+    paid.onerror = (err) => errors.push(err);
+
+    await paid.start();
+
+    (
+      transport as unknown as {
+        correlationStore: {
+          registerRequest: (eventId: string, req: unknown) => void;
+        };
+      }
+    ).correlationStore.registerRequest('req-event-id', {
+      originalRequestId: 42,
+      isInitialize: false,
+      progressToken: undefined,
+      originalRequestContext: { method: 'tools/call', capability: 'tool:add' },
+    });
+
+    const paymentRequired: JSONRPCMessage = {
+      jsonrpc: '2.0',
+      method: 'notifications/payment_required',
+      params: { amount: 1, pay_req: 'x', pmi: 'fake' },
+    };
+
+    (transport as unknown as TransportWithContext).onmessageWithContext?.(
+      paymentRequired,
+      {
+        eventId: 'evt',
+        correlatedEventId: 'req-event-id',
+      },
+    );
+
+    await new Promise((r) => setTimeout(r, 0));
+
+    const errResp = observed.find(
+      (
+        m,
+      ): m is {
+        jsonrpc: '2.0';
+        id: number;
+        error: { code: number; message: string; data?: unknown };
+      } => 'id' in m && m.id === 42 && 'error' in m,
+    );
+    expect(errResp?.error?.code).toBe(-32000);
+    expect(errResp?.error?.message).toBe(
+      'Payment handler failed: wallet exploded',
+    );
+    // The crash still surfaces on onerror.
+    expect(errors[0]?.message).toMatch(/wallet exploded/);
+  });
+
   test('synthesizes JSON-RPC error when canHandle declines and correlation exists', async () => {
     const transport = createMockNostrTransport();
 
@@ -923,6 +983,7 @@ describe('withClientPayments()', () => {
     const paid = withClientPayments(transport, {
       handlers: [{ pmi: 'fake', async handle(): Promise<void> {} }],
       paymentInteraction: 'explicit_gating',
+      minRetryDelayMs: 1,
     });
     paid.onmessage = (msg) => observed.push(msg);
     await paid.start();
@@ -1058,6 +1119,7 @@ describe('withClientPayments()', () => {
       handlers: [{ pmi: 'fake', async handle(): Promise<void> {} }],
       paymentInteraction: 'explicit_gating',
       maxPendingRetries: 2,
+      minRetryDelayMs: 1,
     });
     paid.onmessage = (msg) => observed.push(msg);
     await paid.start();
@@ -1107,4 +1169,130 @@ describe('withClientPayments()', () => {
 
     await paid.close();
   });
+
+  test('refuses wrapping a transport that already has client payments', () => {
+    const transport = createMockNostrTransport();
+    const paid = withClientPayments(transport, { handlers: [] });
+
+    // A chained second wrap double-pays every offer; a sibling wrap silently
+    // kills the first wrapper's trampolines. Both must fail fast.
+    expect(() => withClientPayments(paid, { handlers: [] })).toThrow(
+      /already called on this transport/,
+    );
+    expect(() => withClientPayments(transport, { handlers: [] })).toThrow(
+      /already called on this transport/,
+    );
+  });
+
+  test('floors -32043 retry delay so retry_after: 0 cannot re-send within the same second', async () => {
+    const transport = createMockNostrTransport();
+    transport
+      .getInternalStateForTesting()
+      .correlationStore.registerRequest('req-event-id-floor', {
+        originalRequestId: 77,
+        isInitialize: false,
+        originalRequestContext: { method: 'tools/call' },
+      });
+
+    let sentMessage: JSONRPCMessage | undefined;
+    transport.send = async (msg) => {
+      sentMessage = msg;
+    };
+
+    const paid = withClientPayments(transport, {
+      handlers: [{ pmi: 'fake', async handle(): Promise<void> {} }],
+      minRetryDelayMs: 50,
+    });
+    paid.onmessage = (): void => {};
+    await paid.start();
+
+    await paid.send({
+      jsonrpc: '2.0',
+      id: 77,
+      method: 'tools/call',
+      params: { name: 'floored' },
+    });
+    sentMessage = undefined; // Reset so only the retry is observed
+    (transport as unknown as TransportWithContext).onmessageWithContext?.(
+      {
+        jsonrpc: '2.0',
+        id: 77,
+        error: {
+          code: -32043,
+          message: 'Payment Pending',
+          data: { retry_after: 0 },
+        },
+      },
+      { eventId: 'evt-floor', correlatedEventId: 'req-event-id-floor' },
+    );
+
+    // Before the floor elapses: no retry.
+    await new Promise((r) => setTimeout(r, 10));
+    expect(sentMessage).toBeUndefined();
+
+    // After the floor: the original request is retried.
+    await new Promise((r) => setTimeout(r, 120));
+    expect(sentMessage as unknown).toEqual({
+      jsonrpc: '2.0',
+      id: 77,
+      method: 'tools/call',
+      params: { name: 'floored' },
+    });
+
+    await paid.close();
+  });
+
+  test('keeps numeric and string request ids with the same text form distinct', async () => {
+    const sent: JSONRPCMessage[] = [];
+    const baseTransport: TransportWithContext = {
+      onmessage: undefined,
+      onmessageWithContext: undefined,
+      onerror: undefined,
+      onclose: undefined,
+      async start(): Promise<void> {},
+      async send(message: JSONRPCMessage): Promise<void> {
+        sent.push(message);
+      },
+      async close(): Promise<void> {},
+    };
+
+    const paid = withClientPayments(baseTransport, { minRetryDelayMs: 1 });
+    await paid.start();
+
+    await paid.send({
+      jsonrpc: '2.0',
+      id: 5,
+      method: 'tools/call',
+      params: { name: 'numeric' },
+    });
+    await paid.send({
+      jsonrpc: '2.0',
+      id: '5',
+      method: 'tools/call',
+      params: { name: 'string' },
+    });
+    sent.length = 0;
+
+    // -32043 answering the NUMERIC id 5 must retry the numeric request —
+    // with String(id) keys the later string write overwrote it.
+    baseTransport.onmessageWithContext?.(
+      {
+        jsonrpc: '2.0',
+        id: 5,
+        error: {
+          code: -32043,
+          message: 'Payment Pending',
+          data: { retry_after: 0 },
+        },
+      },
+      { eventId: 'evt', correlatedEventId: 'req' },
+    );
+
+    await new Promise((r) => setTimeout(r, 30));
+
+    expect(sent).toHaveLength(1);
+    expect((sent[0] as { params?: { name?: string } }).params?.name).toBe(
+      'numeric',
+    );
+  });
 });