From 5b10862a72593770e93efd4dfeb279fab20f7bf3 Mon Sep 17 00:00:00 2001 From: Gentech Labs Date: Tue, 25 Aug 2026 17:10:41 +0000 Subject: [PATCH] fix(wallet): hold reservation on ambiguous x402 settlement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit x402 is fire-and-forget per request. If the shared 30s budget (or parent abort signal) expires mid-handshake after the signed, paid request is dispatched, the server may already have settled the payment on-chain — even though we never saw the response. Previously the caller's finally block released the reservation unconditionally on this ambiguous-failure path, so totalReserved() under-counted and the next hold() saw headroom that didn't exist. Now postWithPayment tracks whether it dispatched the signed request and surfaces settlementAmbiguous on abort. Each Modal paid handler (create, exec, status, terminate) keeps the reservation held and invalidates the balance cache when the outcome is ambiguous — erring tight, never loose. A genuinely-absent spend self-heals on the next balance refetch. Ref #128 --- src/tools/modal.ts | 69 +++++++++++++++++++++++++++++++++++++++++----- test/local.mjs | 35 +++++++++++++++++++++++ 2 files changed, 97 insertions(+), 7 deletions(-) diff --git a/src/tools/modal.ts b/src/tools/modal.ts index 5010f40c..d7d99854 100644 --- a/src/tools/modal.ts +++ b/src/tools/modal.ts @@ -181,7 +181,19 @@ async function postWithPayment( resourceDescription: string, abortSignal: AbortSignal, timeoutMs: number, -): Promise<{ ok: boolean; status: number; body: Record; raw: string }> { +): Promise<{ + ok: boolean; + status: number; + body: Record; + raw: string; + /** + * True when the paid x402 request was dispatched but its outcome is unknown + * (the shared 30s budget expired mid-handshake). x402 is fire-and-forget, so + * an aborted paid request may already have settled on-chain. Callers should + * err toward "spent" and keep the reservation held in this case. + */ + settlementAmbiguous: boolean; +}> { const chain = loadChain(); const headers: Record = { 'Content-Type': 'application/json', @@ -192,6 +204,9 @@ async function postWithPayment( abortSignal.addEventListener('abort', onParentAbort, { once: true }); const timer = setTimeout(() => ctrl.abort(), timeoutMs); + // Did we dispatch the signed (paid) request? Only then is an abort ambiguous. + let paidRequestDispatched = false; + try { const payload = JSON.stringify(body); let response = await fetch(endpoint, { method: 'POST', signal: ctrl.signal, headers, body: payload }); @@ -199,8 +214,9 @@ async function postWithPayment( if (response.status === 402) { const paymentHeaders = await signPayment(response, chain, endpoint, resourceDescription); if (!paymentHeaders) { - return { ok: false, status: 402, body: { error: 'payment signing failed' }, raw: '' }; + return { ok: false, status: 402, body: { error: 'payment signing failed' }, raw: '', settlementAmbiguous: false }; } + paidRequestDispatched = true; response = await fetch(endpoint, { method: 'POST', signal: ctrl.signal, @@ -212,7 +228,19 @@ async function postWithPayment( const raw = await response.text().catch(() => ''); let parsed: Record = {}; try { parsed = raw ? JSON.parse(raw) : {}; } catch { /* leave as {} */ } - return { ok: response.ok, status: response.status, body: parsed, raw }; + return { ok: response.ok, status: response.status, body: parsed, raw, settlementAmbiguous: false }; + } catch (err) { + // Only reachable via abort (parent signal or the 30s budget) or a network + // error. If the signed payment request had been dispatched, the settlement + // is ambiguous — the server may already have settled it even though we + // never saw the response. Surface that so the reservation stays held. + return { + ok: false, + status: 0, + body: {}, + raw: '', + settlementAmbiguous: paidRequestDispatched, + }; } finally { clearTimeout(timer); abortSignal.removeEventListener('abort', onParentAbort); @@ -380,6 +408,7 @@ export const modalCreateCapability: CapabilityHandler = { // Wallet reservation — block over-spend if other in-flight calls hold balance. let reservation: ReservationToken | null = null; + let settlementAmbiguous = false; try { reservation = await walletReservation.hold(price); if (!reservation) { @@ -407,6 +436,7 @@ export const modalCreateCapability: CapabilityHandler = { ctx.abortSignal, 90_000, // 90s — sandbox cold-start can be slow on fresh GPU pulls ); + settlementAmbiguous = res.settlementAmbiguous; const latencyMs = Date.now() - callStartedAt; if (!res.ok) { @@ -458,7 +488,14 @@ export const modalCreateCapability: CapabilityHandler = { `Next: ModalExec({ sandbox_id: "${sandboxId}", command: ["python","-c","print(1)"] })`, }; } finally { - walletReservation.release(reservation); + // If the paid request may have settled (aborted mid-handshake), keep the + // reservation held — err tight, never loose. It self-heals at the next + // session/ledger reset or a fresh balance refetch on the next hold. + if (settlementAmbiguous) { + walletReservation.invalidateBalance(); + } else { + walletReservation.release(reservation); + } } }, }; @@ -521,6 +558,7 @@ export const modalExecCapability: CapabilityHandler = { } let reservation: ReservationToken | null = null; + let settlementAmbiguous = false; try { reservation = await walletReservation.hold(EXEC_PRICE_USD); // For micro-cost calls don't hard-block on insufficient — just proceed. @@ -556,6 +594,7 @@ export const modalExecCapability: CapabilityHandler = { ctx.abortSignal, Math.max(30_000, ((coercedTimeout ?? 300) + 30) * 1000), ); + settlementAmbiguous = res.settlementAmbiguous; const latencyMs = Date.now() - callStartedAt; if (!res.ok) { @@ -605,7 +644,11 @@ export const modalExecCapability: CapabilityHandler = { const isError = rawExit !== null ? rawExit !== 0 : !hasAnyOutput; return { output: sections.join('\n\n'), isError }; } finally { - walletReservation.release(reservation); + if (settlementAmbiguous) { + walletReservation.invalidateBalance(); + } else { + walletReservation.release(reservation); + } } }, }; @@ -633,6 +676,7 @@ export const modalStatusCapability: CapabilityHandler = { if (!sandbox_id) return { output: 'Error: sandbox_id is required', isError: true }; let reservation: ReservationToken | null = null; + let settlementAmbiguous = false; try { reservation = await walletReservation.hold(STATUS_PRICE_USD); } catch { /* ignore */ } try { @@ -644,6 +688,7 @@ export const modalStatusCapability: CapabilityHandler = { ctx.abortSignal, 30_000, ); + settlementAmbiguous = res.settlementAmbiguous; const latencyMs = Date.now() - callStartedAt; if (!res.ok) { @@ -657,7 +702,11 @@ export const modalStatusCapability: CapabilityHandler = { const extra = JSON.stringify(res.body, null, 2); return { output: `Sandbox \`${sandbox_id}\` status: **${status}**\n\n${extra}` }; } finally { - walletReservation.release(reservation); + if (settlementAmbiguous) { + walletReservation.invalidateBalance(); + } else { + walletReservation.release(reservation); + } } }, }; @@ -687,6 +736,7 @@ export const modalTerminateCapability: CapabilityHandler = { if (!sandbox_id) return { output: 'Error: sandbox_id is required', isError: true }; let reservation: ReservationToken | null = null; + let settlementAmbiguous = false; try { reservation = await walletReservation.hold(TERMINATE_PRICE_USD); } catch { /* ignore */ } try { @@ -698,6 +748,7 @@ export const modalTerminateCapability: CapabilityHandler = { ctx.abortSignal, 30_000, ); + settlementAmbiguous = res.settlementAmbiguous; const latencyMs = Date.now() - callStartedAt; // Always remove from tracker — even on failure, retrying is wasteful. @@ -717,7 +768,11 @@ export const modalTerminateCapability: CapabilityHandler = { return { output: `Sandbox \`${sandbox_id}\` terminated.` }; } finally { - walletReservation.release(reservation); + if (settlementAmbiguous) { + walletReservation.invalidateBalance(); + } else { + walletReservation.release(reservation); + } } }, }; diff --git a/test/local.mjs b/test/local.mjs index 118ff0ce..188059b2 100644 --- a/test/local.mjs +++ b/test/local.mjs @@ -3378,6 +3378,41 @@ test('paid media and Modal tools pass measured latency to recordUsage', async () 'ModalTerminate recordUsage must receive latencyMs'); }); +// ─── ambiguous-settlement reservation guard (issue #128) ───────────────── +// +// Verified 2026-08-25: x402 is fire-and-forget per request. If the 30s budget +// (or parent signal) aborts the signed, paid request mid-handshake, the server +// may already have settled it on-chain. Releasing the reservation in that case +// under-counts totalReserved() and lets the next hold() see headroom that +// doesn't exist. The fix holds the reservation (errs tight) on the ambiguous +// path and refetches the true on-chain balance. The regression asserts both the +// ambiguity flag is surfaced from the payment helper and that each Modal paid +// handler routes ambiguous settlements to a hold (invalidateBalance) instead of +// a release. +test('postWithPayment surfaces settlement ambiguity and Modal handlers hold on ambiguous settlement', async () => { + const fs = await import('node:fs'); + const path = await import('node:path'); + const readDist = (file) => fs.readFileSync( + path.join(process.cwd(), 'dist', 'tools', file), + 'utf-8', + ); + + const modal = readDist('modal.js'); + // The payment helper must track whether it dispatched the signed request and + // surface that as an ambiguity flag on the aborted/error path. + assert.match(modal, /settlementAmbiguous:/, 'postWithPayment must surface settlement ambiguity'); + assert.match(modal, /paidRequestDispatched/, 'postWithPayment must track whether the paid request was dispatched'); + // Every Modal paid handler must route an ambiguous settlement to a balance + // invalidation (err-tight, keep the reservation held) rather than a release. + assert.equal((modal.match(/if \(settlementAmbiguous\)/g) ?? []).length, 4, + 'all four Modal paid handlers must guard the ambiguous-settlement path'); + assert.match(modal, /settlementAmbiguous\)\s*\{\s*walletReservation\.invalidateBalance\(\)/, + 'ambiguous settlement must invalidate the balance cache (keep reservation held)'); + // Sanity: the normal path must still release. + assert.match(modal, /walletReservation\.release\(reservation\)/, + 'definitive outcomes must still release the reservation'); +}); + // ─── stripLargeImageData: prevent multi-MB session jsonl files ───── // // Verified 2026-05-05: a 5-turn session with .png reads grew to 12 MB