Skip to content

fix(cloud-apps): withdraw amount + app reference miss planner-nested options.parameters — full-balance withdrawal bug - #11810

Merged
lalalune merged 1 commit into
developfrom
fix/apps-withdraw-nested-amount
Jul 3, 2026
Merged

fix(cloud-apps): withdraw amount + app reference miss planner-nested options.parameters — full-balance withdrawal bug#11810
lalalune merged 1 commit into
developfrom
fix/apps-withdraw-nested-amount

Conversation

@NubsCarson

Copy link
Copy Markdown
Member

What

Two nested-planner-args misses in plugin-cloud-apps, one of them a HIGH money bug on the WITHDRAW_APP_EARNINGS path.

1. HIGH (money): parseWithdrawAmount missed the planner's validated amount → staged the FULL balance

parseWithdrawAmount (src/actions/withdraw-app-earnings.ts) read AMOUNT_OPTION_KEYS (amount/usd/value) on top-level options only. But the real planner path nests validated args under options.parameterspackages/core/src/runtime/execute-planned-tool-call.ts sets handlerOptions.parameters = validation.args.

Failure: "withdraw fifty dollars from Acme" → planner extracts amount: 50 into options.parametersparseWithdrawAmount misses it, the prose has no digits → requested = null → the handler stages the full withdrawable balance and the confirm prompt asks about the wrong number. A user confirming withdraws everything instead of $50.

Fix: read the nested options.parameters object first, then top-level, then the text regexes — exactly mirroring readStructuredConfirmation (safety.ts) and actionParams (domain-intent.ts), via a new shared plannerOptionSources helper in client.ts.

Also tightened the bare-number text fallback: the number must now be a standalone whitespace-bounded token not followed by an alphanumeric, so a digit glued into an app name ("withdraw my Acme2 earnings") can never read as a $2.00 amount. $50 / 50 dollars / 50 usd / standalone withdraw 50 all still parse.

2. Shared: extractAppReference had the same top-level-only miss

extractAppReference (src/client.ts) read REFERENCE_OPTION_KEYS on top-level only, so on the real planner path a validated appName was ignored and resolution fell back to fuzzy-matching the raw message text. Now nested-first via the same helper — this fixes app-by-reference resolution for every action that uses it (withdraw, delete, update, monetization, deploy, backup, key rotation, frontend, ad slots, domains).

Why the existing tests missed it

Every existing test passed options top-level ({ confirm: true }, { appName: ... }) — the direct-call shape — never the planner's nested { parameters: { ... } } shape. New tests exercise the nested shape end-to-end.

Tests (all real — SDK boundary faked only, per plugin convention)

  • MONEY REGRESSION (handler, two-phase): first ask with { parameters: { appName: "Acme Bot", amount: 50 } } + digit-free prose stages $50.00 (not the $100 balance); nested { parameters: { confirm: true } } then withdraws exactly 50 via the idempotent endpoint. Fails on the old code (staged $100, withdrew 100).
  • Digit-in-name regression (handler): "withdraw my Acme2 earnings" stages the full balance, never $2.00 (on old code this hit a wrong below_threshold refusal off the phantom $2).
  • parseWithdrawAmount unit coverage: nested-first precedence, top-level fallback, string/$1,250.50 coercion, non-positive rejection, standalone-token text parsing, Acme2/App2000/a1b2/50k never parse as amounts.
  • extractAppReference unit coverage: nested planner shape (appName/app/appId), nested-over-top precedence, top-level fallback, text fallback.

Verification

  • bun test plugins/plugin-cloud-apps275 pass / 0 fail (28 files)
  • tsgo --noEmit (plugin typecheck) → clean
  • biome check plugins/plugin-cloud-apps → clean, no fixes

Evidence notes

  • Real-LLM trajectory: N/A — no prompt/action-description change; the fix is in pure option-parsing helpers on the handler path, proven by the full two-phase handler regression tests above (the planner shape asserted is the exact one execute-planned-tool-call.ts:270 produces).
  • UI proof: N/A — no user-visible surface changed; connector reply text paths unchanged.

[cloud-money] review requested — money-out path. cc lane charter #11157.

Signed: [cloud-security]

…amount + app reference

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.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your trial has ended. Reactivate Greptile to resume code reviews.

@coderabbitai

coderabbitai Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c4ad260b-9ef1-4ce2-85c4-b15ba0506c07

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/apps-withdraw-nested-amount

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@lalalune
lalalune merged commit 5393692 into develop Jul 3, 2026
33 of 37 checks passed
@lalalune
lalalune deleted the fix/apps-withdraw-nested-amount branch July 3, 2026 04:05
@lalalune

lalalune commented Jul 3, 2026

Copy link
Copy Markdown
Member

Post-merge validation pass from a clean PR worktree rebased on current develop:

  • bun test plugins/plugin-cloud-apps -> 275 tests passed
  • bun run --cwd plugins/plugin-cloud-apps typecheck -> passed after rebuilding worktree-local @elizaos/cloud-sdk declarations and fixing the worktree node_modules symlink resolution to avoid the stale main-checkout SDK dist
  • bunx @biomejs/biome check plugins/plugin-cloud-apps -> passed
  • git diff --check origin/develop..HEAD -> passed

Reviewed the money-flow invariant: the first ask now reads nested planner options.parameters.amount, freezes that parsed amount in pending confirmation metadata, and the confirm turn uses the frozen pending state instead of reparsing follow-up prose. App reference resolution now prefers nested planner parameters with top-level fallback, covered by tests.

NubsCarson added a commit that referenced this pull request Jul 3, 2026
…nblock ALL deploys (#11847)

Every develop deploy since ~03:46 UTC failed at the Deploy API Worker step:
  No matching export in "src/stubs/elizaos-core.ts" for import
  "runWithTrajectoryPurpose"  (shared/src/email-classification/email-classifier.ts:20)

email-classifier (@elizaos/shared, pulled into the Worker bundle transitively)
imports runWithTrajectoryPurpose from @elizaos/core, which the Worker aliases
to this stub — but the stub only had runWithTrajectoryContext, not the newer
runWithTrajectoryPurpose (added to core in trajectory-context.ts). So the
esbuild bundle failed and NO deploy could ship — blocking the develop→main
promote that carries the merged money fixes (#11810 withdraw, #11817
book-influencer, #11686 sweep) to prod.

Fix: add the Worker-safe stub mirroring runWithTrajectoryContext — no trajectory
context manager exists in the Worker bundle (it lives on the agent sidecar and
this path is never invoked on a Worker route), so just run the fn.

Verified: `wrangler deploy --env production --dry-run` now builds clean
(15.8 MB bundle, no missing-export error).
@claude

claude Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Claude encountered an error —— View job


I'll analyze this and get back to you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants