eng-3151-add-coinbase-connector#22
Conversation
There was a problem hiding this comment.
Stale comment
Risk: medium. Cursor Security Agent completed with no findings; Cursor Bugbot was not present on the latest commit. Risk exceeds the low auto-approval threshold, so not approving; reviewers are already assigned for human review.
Sent by Cursor Approval Agent: Pull Request Router and Approver
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 45dd5fc. Configure here.
|
@Ishita-02 Also one more comment here on reworking the base plugin skills so we don't have to copy them here and deal with drift later on: https://yieldxyz.slack.com/archives/C0A9REH4WNQ/p1783618537195029?thread_ts=1783613994.087279&cid=C0A9REH4WNQ |
There was a problem hiding this comment.
Stale comment
Risk: medium. Cursor Security Agent completed with no findings; Cursor Bugbot was not present on the latest commit. Risk exceeds the low auto-approval threshold, so not approving; Philippoes is already assigned for human review.
Sent by Cursor Approval Agent: Pull Request Router and Approver
There was a problem hiding this comment.
Stale comment
Risk: medium. Cursor Security Agent completed with no findings; Cursor Bugbot was not present on the latest commit. Risk exceeds the low auto-approval threshold, so not approving; Philippoes is already assigned for human review.
Sent by Cursor Approval Agent: Pull Request Router and Approver
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 6e34032. Configure here.
…entry Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Stale comment
Risk: medium. Cursor Security Agent completed with no findings; Cursor Bugbot was not present on the latest commit. Risk exceeds the low auto-approval threshold, so not approving; Philippoes and ajag408 are already assigned for human review.
Sent by Cursor Approval Agent: Pull Request Router and Approver
ajag408
left a comment
There was a problem hiding this comment.
Thanks for addressing all the feedback. Non-blocking comments
| "license": "MIT", | ||
| "keywords": ["defi", "yield", "base", "coinbase", "connector", "wallet", "signing", "web3"], | ||
| "skills": "./skills", | ||
| "mcpServers": "./.mcp.json", |
There was a problem hiding this comment.
Since the reference files are now symlinks, three install paths depend on dereferencing working: Claude marketplace install (documented, but there's an open issue — anthropics/claude-code#57339 — reporting dangling symlinks in the cache for exactly this cross-plugin case), the Codex plugin path added here, and standalone npx skills add. Could we do one real install from this branch on each path and confirm the five files land as content, not broken links?
There was a problem hiding this comment.
Ran a real install from this branch on all three paths and checked what actually lands in references/:
- Claude marketplace (claude plugin install …@agentkit): ✅ all five files land as content (regular files, real sizes). Couldn't repro #57339 on claude 2.1.159.
- npx skills add: ✅ all five land as content.
- Codex (codex plugin add …@agentkit): ❌ references/ installs empty — none of the five land. Codex snapshots only the plugin's own subtree, and these symlinks point outside it (to the base skill), so they're dropped rather than dereferenced.
Proposal: drop the .codex-plugin/plugin.json for coinbase and note in the README that Codex isn't a supported install target for this connector (cross-plugin symlinks).
There was a problem hiding this comment.
This was the reason i didn't add .codex-plugin for this connector, but i then saw philip added that
There was a problem hiding this comment.
Cool we can either: (a) drop this file per your proposal and add a one-line "Codex isn't a supported install target for this connector (cross-plugin symlinks)" note to the README, or (b) if Codex support matters enough, materialize real copies for this plugin (e.g. a small sync script + CI drift check, the approach we considered before symlinks). Since @Philippoes added the manifest, flagging for his call — but let's not merge with the known-broken path in place.
There was a problem hiding this comment.
ow that the three-path verification is done, this skill ReadMe dev note can state results instead of asking for them — e.g. "verified on Claude Code 2.1.159 (marketplace) and npx skills add" — and, depending on the Codex decision, add the "Codex isn't a supported install target for this connector" line here. Turns the caveat into documentation of what's actually known.
| yield-xyz-agentkit-coinbase/ | ||
| ├── SKILL.md # discover via Yield.xyz MCP, sign/broadcast via Base Account | ||
| ├── README.md # This file | ||
| └── references/ # → symlinks into ../../yield-xyz-agentkit/skills/yield-xyz-agentkit/references/ |
There was a problem hiding this comment.
Tiny accuracy nit: the actual symlinks resolve from inside references/, so they use ../../../../yield-xyz-agentkit/… — the ../../ shown here would be a broken link if someone copied it when adding a new reference. Suggest either showing the real four-level path or dropping the path and keeping just "symlinks into the core skill's references/".
There was a problem hiding this comment.
Good catch — fixed in 24f7420. Dropped the path entirely rather than spelling out the four-level ../../../../, so the comment now reads:
└── references/ # → symlinks into the core yield-xyz-agentkit skill's references/
That avoids the copy-paste-a-broken-link trap you flagged.


Note
Medium Risk
Changes are docs and agent instructions for transaction signing and fund safety, not runtime code, but errors in batching or
unsignedTransactionhandling could mislead agents handling real wallets.Overview
Adds
yield-xyz-agentkit-coinbase, a sixth marketplace plugin that runs the full yield flow without the base plugin: Yield.xyz via the Coinbase partner MCP (https://mcp.yield.xyz/p/coinbase/mcp) plus Base MCP for session,send_calls/sign, and hash reporting.The new skill documents orchestration end-to-end—wallet from
get_wallets, chain checks againstsupportedChains, preferred atomic batch ofunsignedTransactions with individual fallback, mandatorysubmit_hash, and RWA gating—with shared reference content linked from the core skill. Top-level README / plugins README / marketplace.json are updated to list the connector and note it is self-contained (unlike Privy/MoonPay/Robinhood).Small shared reference edits: clearer Rule 5 for sequential signing when not batching, RWA docs warn that Base Accounts (smart wallets) may fail issuer allowlists, and a few pointers now say “read MCP tool schema” instead of
SKILL.md.Reviewed by Cursor Bugbot for commit 6e34032. Configure here.