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