From 8545f6cd543a7aecd41e1d416ccfee8b4e5c859d Mon Sep 17 00:00:00 2001 From: NubsCarson Date: Fri, 3 Jul 2026 03:55:57 +0000 Subject: [PATCH] fix(cloud-apps): read planner-nested options.parameters for withdraw amount + app reference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MONEY (HIGH): parseWithdrawAmount read AMOUNT_OPTION_KEYS on top-level options only, but the real planner path nests validated args under options.parameters (execute-planned-tool-call.ts sets handlerOptions.parameters = validation.args). 'withdraw fifty dollars from Acme' therefore parsed no amount and staged the FULL withdrawable balance, which a confirm then withdrew. Amounts are now read from the nested parameters object first, then top-level, then text — mirroring readStructuredConfirmation (safety.ts) and actionParams (domain-intent.ts) via a new shared plannerOptionSources helper in client.ts. Also tightens the bare-number text fallback so a digit glued into an app name ('withdraw my Acme2 earnings') can never read as a $2.00 amount: the number must be a standalone whitespace-bounded token not followed by an alphanumeric. Shared: extractAppReference had the same top-level-only miss; every action that resolves an app by reference now sees the nested planner shape first. Tests: nested {parameters:{amount:50}} / {parameters:{appName:...}} planner-shape coverage (unit + full two-phase handler regression), the Acme2 digit-in-name regression, and standalone-token amount parsing. bun test plugins/plugin-cloud-apps: 275 pass / 0 fail; tsgo + biome clean. --- .../__tests__/reference-resolution.test.ts | 52 +++++++- .../__tests__/withdraw-app-earnings.test.ts | 121 +++++++++++++++++- .../src/actions/withdraw-app-earnings.ts | 25 +++- plugins/plugin-cloud-apps/src/client.ts | 33 ++++- 4 files changed, 215 insertions(+), 16 deletions(-) diff --git a/plugins/plugin-cloud-apps/__tests__/reference-resolution.test.ts b/plugins/plugin-cloud-apps/__tests__/reference-resolution.test.ts index 3fdce8c8bebb2..a9588b9c9e343 100644 --- a/plugins/plugin-cloud-apps/__tests__/reference-resolution.test.ts +++ b/plugins/plugin-cloud-apps/__tests__/reference-resolution.test.ts @@ -1,6 +1,10 @@ import { describe, expect, it } from "bun:test"; -import { findAppByReference, matchAppByReference } from "../src/client.ts"; -import { makeApp } from "./helpers"; +import { + extractAppReference, + findAppByReference, + matchAppByReference, +} from "../src/client.ts"; +import { makeApp, makeMessage } from "./helpers"; const app = (name: string, id?: string) => makeApp({ @@ -68,3 +72,47 @@ describe("matchAppByReference / findAppByReference — ambiguity-safe resolution expect(findAppByReference([app("Acme")], " ")).toBeNull(); }); }); + +describe("extractAppReference — planner options (nested `parameters` first)", () => { + const msg = makeMessage("do something with my app"); + + it("REGRESSION: reads the real planner shape — args nested under options.parameters", () => { + // execute-planned-tool-call.ts puts validated args at options.parameters; + // the old top-level-only read missed them and fell back to the raw text. + expect( + extractAppReference(msg, { parameters: { appName: "Acme Bot" } }), + ).toBe("Acme Bot"); + expect(extractAppReference(msg, { parameters: { app: "Acme Bot" } })).toBe( + "Acme Bot", + ); + expect(extractAppReference(msg, { parameters: { appId: "id-acme" } })).toBe( + "id-acme", + ); + }); + + it("nested planner args win over top-level keys", () => { + expect( + extractAppReference(msg, { + appName: "Top Level", + parameters: { appName: "Nested" }, + }), + ).toBe("Nested"); + }); + + it("still reads top-level options (direct handler calls)", () => { + expect(extractAppReference(msg, { appName: "Acme Bot" })).toBe("Acme Bot"); + }); + + it("falls back to top-level when parameters carries no reference", () => { + expect( + extractAppReference(msg, { appName: "Acme Bot", parameters: {} }), + ).toBe("Acme Bot"); + }); + + it("falls back to the message text when options carry no reference", () => { + expect(extractAppReference(msg, { parameters: {} })).toBe( + "do something with my app", + ); + expect(extractAppReference(msg)).toBe("do something with my app"); + }); +}); diff --git a/plugins/plugin-cloud-apps/__tests__/withdraw-app-earnings.test.ts b/plugins/plugin-cloud-apps/__tests__/withdraw-app-earnings.test.ts index a6f345fb3bd0f..278c6fb51f600 100644 --- a/plugins/plugin-cloud-apps/__tests__/withdraw-app-earnings.test.ts +++ b/plugins/plugin-cloud-apps/__tests__/withdraw-app-earnings.test.ts @@ -18,7 +18,7 @@ mock.module("@elizaos/cloud-sdk", () => ({ ElizaCloudClient: FakeElizaCloudClient, })); -const { withdrawAppEarningsAction } = await import( +const { parseWithdrawAmount, withdrawAppEarningsAction } = await import( "../src/actions/withdraw-app-earnings.ts" ); @@ -186,6 +186,64 @@ describe("WITHDRAW_APP_EARNINGS", () => { expect(result?.success).toBe(true); }); + it("MONEY REGRESSION: planner-nested amount stages $50, NOT the full balance", async () => { + // Real planner path (execute-planned-tool-call.ts): validated args arrive + // under options.parameters and the text carries no digits. The old + // top-level-only read missed the amount → requested=null → the FULL $100 + // balance was staged and a confirm withdrew all of it. + const withdrawals = trackWithdrawals(); + const runtime = keyedRuntime(); + const cb = captureCallback(); + const first = await withdrawAppEarningsAction.handler( + runtime, + makeMessage("withdraw fifty dollars from my Acme Bot earnings"), + undefined, + { parameters: { appName: "Acme Bot", amount: 50 } }, + cb.fn, + ); + expect((first?.data as { amount: number }).amount).toBe(50); + expect(cb.calls[0]?.text).toContain("$50.00"); + expect(cb.calls[0]?.text).not.toContain("$100.00"); + expect(withdrawals.calls).toHaveLength(0); + + const result = await withdrawAppEarningsAction.handler( + runtime, + makeMessage("confirmo"), + undefined, + { parameters: { confirm: true } }, + captureCallback().fn, + ); + expect(result?.success).toBe(true); + expect(withdrawals.calls).toHaveLength(1); + expect(withdrawals.calls[0]?.request.amount).toBe(50); // NOT the full 100 + }); + + it("REGRESSION: a digit inside the app name never reads as an amount", async () => { + // "withdraw my Acme2 earnings" — the old bare-number regex captured the + // "2" of "Acme2" as $2.00 (here: a below-threshold refusal; elsewhere a + // wrong staged amount). It must stage the full balance instead. + const acme2 = makeApp({ + id: "id-acme2", + name: "Acme2", + slug: "acme2", + monetization_enabled: true, + }); + setListApps(() => Promise.resolve({ success: true, apps: [acme2] })); + const withdrawals = trackWithdrawals(); + const cb = captureCallback(); + const result = await withdrawAppEarningsAction.handler( + keyedRuntime(), + makeMessage("withdraw my Acme2 earnings"), + undefined, + undefined, + cb.fn, + ); + expect(result?.success).toBe(true); + expect((result?.data as { amount: number }).amount).toBe(100); + expect(cb.calls[0]?.text).toContain("$100.00"); + expect(withdrawals.calls).toHaveLength(0); + }); + it("structured confirm without a pending prompt does NOT withdraw", async () => { const withdrawals = trackWithdrawals(); const result = await withdrawAppEarningsAction.handler( @@ -330,3 +388,64 @@ describe("WITHDRAW_APP_EARNINGS", () => { expect((result?.data as { reason: string }).reason).toBe("error"); }); }); + +describe("parseWithdrawAmount — planner options (nested `parameters` first) + text", () => { + it("MONEY REGRESSION: reads the amount nested under options.parameters — the real planner shape", () => { + // Missing this returned null → the handler staged the FULL withdrawable + // balance for a "withdraw fifty dollars from Acme" ask. + expect( + parseWithdrawAmount("withdraw fifty dollars from Acme", { + parameters: { amount: 50 }, + }), + ).toBe(50); + expect( + parseWithdrawAmount("", { parameters: { amount: "$1,250.50" } }), + ).toBe(1250.5); + expect(parseWithdrawAmount("", { parameters: { usd: 25 } })).toBe(25); + }); + + it("nested planner args win over top-level keys", () => { + expect( + parseWithdrawAmount("", { amount: 10, parameters: { amount: 50 } }), + ).toBe(50); + }); + + it("still reads top-level options (direct handler calls)", () => { + expect(parseWithdrawAmount("", { amount: 50 })).toBe(50); + expect(parseWithdrawAmount("", { value: "75" })).toBe(75); + }); + + it("ignores non-positive / non-numeric option values", () => { + expect(parseWithdrawAmount("", { parameters: { amount: 0 } })).toBeNull(); + expect(parseWithdrawAmount("", { parameters: { amount: -5 } })).toBeNull(); + expect( + parseWithdrawAmount("", { parameters: { amount: "fifty" } }), + ).toBeNull(); + }); + + it("parses explicit currency and standalone amounts from text", () => { + expect(parseWithdrawAmount("withdraw $50 from Acme")).toBe(50); + expect(parseWithdrawAmount("withdraw 50 dollars from Acme")).toBe(50); + expect(parseWithdrawAmount("cash out 12.5 usd")).toBe(12.5); + expect(parseWithdrawAmount("withdraw 50 from Acme")).toBe(50); + expect(parseWithdrawAmount("payout 25")).toBe(25); + }); + + it("REGRESSION: a digit glued into an app name is NOT an amount", () => { + // The old bare-number regex captured the "2" of "Acme2" → $2.00. + expect(parseWithdrawAmount("withdraw my Acme2 earnings")).toBeNull(); + expect(parseWithdrawAmount("cash out App2000")).toBeNull(); + expect(parseWithdrawAmount("payout for my a1b2 app")).toBeNull(); + }); + + it("rejects a number glued to trailing letters (not a standalone token)", () => { + expect(parseWithdrawAmount("withdraw 50k from Acme")).toBeNull(); + }); + + it("returns null (= full balance) when no amount appears anywhere", () => { + expect(parseWithdrawAmount("withdraw my Acme earnings")).toBeNull(); + expect(parseWithdrawAmount("withdraw fifty dollars")).toBeNull(); + expect(parseWithdrawAmount("")).toBeNull(); + expect(parseWithdrawAmount("", undefined)).toBeNull(); + }); +}); diff --git a/plugins/plugin-cloud-apps/src/actions/withdraw-app-earnings.ts b/plugins/plugin-cloud-apps/src/actions/withdraw-app-earnings.ts index f467b4f08bc7a..b031605a1a0d5 100644 --- a/plugins/plugin-cloud-apps/src/actions/withdraw-app-earnings.ts +++ b/plugins/plugin-cloud-apps/src/actions/withdraw-app-earnings.ts @@ -40,6 +40,7 @@ import { logger } from "@elizaos/core"; import { extractAppReference, getCloudClient, + plannerOptionSources, resolveApp, resolveCloudApiKey, resolveCloudSiteBaseUrl, @@ -77,15 +78,22 @@ function newIdempotencyKey(): string { const AMOUNT_OPTION_KEYS = ["amount", "usd", "value"] as const; -/** Parse a withdrawal amount (USD) from planner options or text; null = "all". */ +/** + * Parse a withdrawal amount (USD) from planner options or text; null = "all". + * + * MONEY-CRITICAL: on the real planner path the validated `amount` arrives + * NESTED under `options.parameters` (execute-planned-tool-call.ts), so the + * nested object is read before the top level ({@link plannerOptionSources}). + * Missing it here silently upgraded "withdraw $50" to the FULL withdrawable + * balance at the confirm stage. + */ export function parseWithdrawAmount( text: string, options?: unknown, ): number | null { - if (options && typeof options === "object") { - const opts = options as Record; + for (const source of plannerOptionSources(options)) { for (const key of AMOUNT_OPTION_KEYS) { - const v = opts[key]; + const v = source[key]; if (typeof v === "number" && Number.isFinite(v) && v > 0) return v; if (typeof v === "string") { const n = Number(v.replace(/[$,]/g, "").trim()); @@ -94,11 +102,14 @@ export function parseWithdrawAmount( } } const body = text ?? ""; - // Prefer an explicit currency amount: "$50", "50 dollars", "50 usd". + // Prefer an explicit currency amount: "$50", "50 dollars", "50 usd". The + // bare-number fallback requires a standalone whitespace-bounded token not + // glued to letters, so a digit inside an app name ("Acme2") never reads as + // a dollar amount. const m = /\$\s*(\d+(?:\.\d+)?)/.exec(body) ?? - /(\d+(?:\.\d+)?)\s*(?:dollars?|usd)\b/i.exec(body) ?? - /\b(?:withdraw(?:al)?|cash\s*out|pay\s*out|payout)\b[^$\d]*?(\d+(?:\.\d+)?)\b/i.exec( + /(?:^|\s)(\d+(?:\.\d+)?)\s*(?:dollars?|usd)\b/i.exec(body) ?? + /\b(?:withdraw(?:al)?|cash\s*out|pay\s*out|payout)\b[^$\d]*?(?:^|\s)(\d+(?:\.\d+)?)(?![A-Za-z0-9])/i.exec( body, ); if (m) { diff --git a/plugins/plugin-cloud-apps/src/client.ts b/plugins/plugin-cloud-apps/src/client.ts index 4d92f68db0611..c56a4b4c5a4be 100644 --- a/plugins/plugin-cloud-apps/src/client.ts +++ b/plugins/plugin-cloud-apps/src/client.ts @@ -288,18 +288,39 @@ const REFERENCE_OPTION_KEYS = [ ] as const; /** - * Pull an app reference from planner-supplied options or, failing that, the raw - * message text. Mirrors the read-core's `resolveReference` so the mutating - * actions resolve apps identically. + * The candidate objects that may carry planner-validated action args, most + * authoritative first. On the real planner path the validated args arrive + * NESTED under `options.parameters` (execute-planned-tool-call.ts sets + * `handlerOptions.parameters = validation.args`); only direct handler calls / + * scenario `action`-kind turns place them at the top level. Mirrors + * `readStructuredConfirmation` (safety.ts) and `actionParams` + * (domain-intent.ts) so every option read sees both shapes, nested first. + */ +export function plannerOptionSources( + options?: unknown, +): ReadonlyArray> { + if (!options || typeof options !== "object") return []; + const top = options as Record; + const nested = + top.parameters && typeof top.parameters === "object" + ? (top.parameters as Record) + : undefined; + return nested ? [nested, top] : [top]; +} + +/** + * Pull an app reference from planner-supplied options (nested + * `options.parameters` first — the real planner path — then top-level) or, + * failing that, the raw message text. Mirrors the read-core's + * `resolveReference` so the mutating actions resolve apps identically. */ export function extractAppReference( message: Memory, options?: unknown, ): string { - if (options && typeof options === "object") { - const opts = options as Record; + for (const source of plannerOptionSources(options)) { for (const key of REFERENCE_OPTION_KEYS) { - const value = opts[key]; + const value = source[key]; if (typeof value === "string" && value.trim().length > 0) { return value.trim(); }