wallet: two tools an agent shouldn't have, and two that were one - #1448
Merged
Conversation
The wallet surface was five tools. It is now one, plus `pay`. wallet_transfer moved credits to another account by username, irreversibly, in a single call with no confirmation step. The same agent holds mail_inbox, news_read, web_fetch and db_list — four ways to read text a stranger wrote — and nothing downstream could tell "the user asked" from "the agent read it in an email". The web form's CSRF token does not apply to a caller holding a bearer token, so the caps were the only thing between an injected instruction and £100 a day. Transferring credits is something a person does a handful of times, deliberately, and /wallet/transfer already does it well. It is not a capability an agent should be granted, so it is no longer a tool. The page is untouched. wallet_topup returned card tiers to a caller that cannot complete a card purchase. Its only real output was "tell your human where to go", which belongs in the message you get when a call fails for want of credits — so those messages now name /wallet/topup, the page that actually sells credits, rather than /wallet. wallet and wallet_balance both answered "what is in my wallet" with different halves of the answer, which left the planner flipping a coin. Credits are what calls are charged in and USDC is how you top them up: two fields of one thing. They are merged into wallet_balance, with `wallet` kept as an alias so "my wallet" still resolves, and it is no longer path-backed — pointing a tool at /wallet is what made it scrape the wallet web page for data in the first place. A test now fails if any tool moves money out of an account. `pay` is named as the deliberate exception rather than pattern-matched: settling a call to another server against your own wallet is buying something, not giving it away, and the spend limit bounds it. Verified against a running binary: 56 tools listed, wallet_transfer and wallet_topup answer "Tool not found", the `wallet` alias still resolves, and /wallet, /wallet/transfer and /wallet/topup all still serve. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KdcPjN9ndJwMGKQSRrAGPE
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 789e3480b5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - apps_run: Run JavaScript code and return the result (args: {"code":"return 2+2"}) | ||
| - wallet_balance: Check your wallet credit balance (no args) | ||
| - wallet: Get your Base wallet address and USDC balance (no args). Sending USDC there tops up credits. | ||
| - wallet_balance: Check your balance — credits, plus your Base address and USDC for topping up (no args). To add credits, point the user at /wallet/topup; there is no tool for it. |
There was a problem hiding this comment.
After retiring wallet_topup, the home-page ToolsDropdownHTML still lists “💳 Topup” under “Available Tools” (agent/agent.go:691-692), and the MCP catalog in README.md:73 still advertises wallet_topup. Users following either catalog will call a tool that now returns “Tool not found”; update these public tool lists as part of the removal.
Useful? React with 👍 / 👎.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The wallet surface was five tools. It is now one, plus
pay.wallet_transfer moved credits to another account by username, irreversibly, in a single call with no confirmation step. The same agent holds mail_inbox, news_read, web_fetch and db_list — four ways to read text a stranger wrote — and nothing downstream could tell "the user asked" from "the agent read it in an email". The web form's CSRF token does not apply to a caller holding a bearer token, so the caps were the only thing between an injected instruction and £100 a day. Transferring credits is something a person does a handful of times, deliberately, and /wallet/transfer already does it well. It is not a capability an agent should be granted, so it is no longer a tool. The page is untouched.
wallet_topup returned card tiers to a caller that cannot complete a card purchase. Its only real output was "tell your human where to go", which belongs in the message you get when a call fails for want of credits — so those messages now name /wallet/topup, the page that actually sells credits, rather than /wallet.
wallet and wallet_balance both answered "what is in my wallet" with different halves of the answer, which left the planner flipping a coin. Credits are what calls are charged in and USDC is how you top them up: two fields of one thing. They are merged into wallet_balance, with
walletkept as an alias so "my wallet" still resolves, and it is no longer path-backed — pointing a tool at /wallet is what made it scrape the wallet web page for data in the first place.A test now fails if any tool moves money out of an account.
payis named as the deliberate exception rather than pattern-matched: settling a call to another server against your own wallet is buying something, not giving it away, and the spend limit bounds it.Verified against a running binary: 56 tools listed, wallet_transfer and wallet_topup answer "Tool not found", the
walletalias still resolves, and /wallet, /wallet/transfer and /wallet/topup all still serve.Claude-Session: https://claude.ai/code/session_01KdcPjN9ndJwMGKQSRrAGPE