feat: acp trade (swap + Hyperliquid) + sync dev to main, with copy polish#35
Open
psmiratisu wants to merge 115 commits into
Open
feat: acp trade (swap + Hyperliquid) + sync dev to main, with copy polish#35psmiratisu wants to merge 115 commits into
acp trade (swap + Hyperliquid) + sync dev to main, with copy polish#35psmiratisu wants to merge 115 commits into
Conversation
…ion, streamline offering creation process
refactor: remove job offering isPrivate field
feat: add update agent details command
…ndle errors appropriately
…the active wallet
…fined values gracefully
… including interactive funding options
…oken retrieval and storage
feat: enhance token management by adding wallet address support for t…
…ion steps and legacy agent search instructions
feat: apply builder code
* feat: update tokenization process and documentation * fix: update tokenization docs * fix: ensure transaction receipt handling in tokenization functions * fix: improve error handling for reverted transactions in receipt processing * refactor: remove unused ABI files and streamline tokenization functions * docs: enhance tokenization documentation * docs: clarify and expand tokenization options in SKILL and tokenization documentation --------- Co-authored-by: Zuhwa <zuhwa@virtuals.io>
feat: erc8004 identity & reputation
Wire up the new agent-email and agentcard APIs from agentic-commerce-be:
- email: whoami, provision, inbox, compose, search, thread, reply,
extract-otp, extract-links
- card: signup, signup-poll, whoami, profile (get/set/reset),
payment-method, limit (get/set), issue, list, get
Card uses the spend-request model. The BE returns a `nextStep` breadcrumb
on every mutating response; the CLI surfaces it so agents can walk the
setup flow (profile -> payment method -> limit -> issue) without
introspecting profile fields.
Also adds PATCH support to ApiClient for profile updates.
acp email attachment --attachment-id <id> [--output <dir>] Two-step download wired to the new BE routes: GET /agents/:id/email/attachments/:id → metadata GET /agents/:id/email/attachments/:id/download → binary stream Streams bytes straight to disk with stream.pipeline (back-pressure respecting) instead of buffering, so large attachments don't sit in memory. Filename comes from the upstream Content-Disposition header (RFC 5987 aware), falling back to the metadata filename if missing. path.basename() strips any separator upstream might inject, so a malicious header can't traverse the caller's filesystem. Requires agentic-commerce-be#94.
card.ts and email.ts both defined an identical formatDate helper. Moving to lib/output.ts removes the duplication and keeps a single place to evolve ISO-timestamp rendering (locale, formatting). Addresses bugbot review on #8.
`new Date(bad)` doesn't throw — it returns an Invalid Date object, and `.toLocaleString()` on that returns the literal string "Invalid Date". The try/catch fallback was dead code, so callers with bad server timestamps saw "Invalid Date" in tables instead of the raw ISO string the comment promised. Switch to an `isNaN(d.getTime())` check, which is the standard way to detect an Invalid Date, and return the raw string when it fires. Addresses bugbot review on #8.
The `whoami` handler formatted `identity.createdAt` with raw `new Date(iso).toLocaleString()` instead of the `formatDate` helper it already imports — so a malformed server timestamp would render as "Invalid Date" in the table rather than falling back to the raw ISO string. Addresses bugbot review on #8.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…trade` (#30) Adds a Treasures Finance route to `acp trade` — `--ticker` triggers a new "treasures-stock" intent and runs ownership-proof → quote → per-leg EIP-712 sign → submit → poll-status in one command. New `src/lib/treasures/client.ts` wraps the public Treasures API over `fetch` with HTTPS enforcement and TREASURES_API_URL / IS_TESTNET host selection. Also hardens the flow per review: - poll status once more after the final sleep so a late fill isn't misreported as TIMEOUT - non-zero exit code on terminal partial_failed / all_failed - validate --slippage-bps as a non-negative integer instead of sending null
A stock symbol is now named with --token everywhere; the companion flag picks the venue: --side -> HL perp, --amount-usdc/--amount-shares -> Treasures tokenized stock (spot). Drops the separate --ticker flag so an agent picks the asset once and the mode second, matching the CLI's params-decide-the-venue model. https://claude.ai/code/session_01C56LaYyFW7iRxtdY5tY3uA
Move the Treasures tokenized-stock flow out of the CLI and into the trading-agent planner. The CLI is now a thin signer: it POSTs a plan with a `treasures` block and drives the same /trade/plan + /trade/next loop as a swap, signing whatever the server asks via the new `sign` action. - runTradeLoop: handle `sign` actions (signTypedData / signMessage → post signature back). - runTreasuresStock: build a plan + run the loop; delete the ~150 lines of direct quote/sign/submit/poll and the now-dead lib/treasures/client.ts. - detectIntent: route a funded buy (--token ticker + --token-in/--chain-in/ --amount-in, no --token-out/--chain-out) to Treasures, so the server can bridge any chain's funds to USDC@eth before buying. - scripts/: tsx harnesses for the sign branch + detectIntent routing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Rebasing the trade feature onto main surfaced two integration fixes: - package-lock.json: regenerate to include @nktkas/hyperliquid (the lock was taken from main during conflict resolution and lacked it). - getAgentApi: drop the walletAddress arg — main simplified getClient to (unauthenticated?: boolean), so passing a string no longer typechecks. The param was always ignored (resolveToken uses the config owner wallet). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…nature collection and provisioning steps
feat: add signer with policy flag
- Add chain ID legend to --help (1 ETH, 42161 ARB, 8453 Base, 1337 HL) - Lead command description with cross-chain support - Clarify --chain-in/--chain-out flag descriptions with named chains - Clarify --amount-in is spend amount, not receive amount - Add --size/--leverage note in perp routing table - ensureHlFunds: actionable error with deposit command + leverage tip - --size error: explicitly say 'token units, not USD', show --leverage example - Progress labels: human-readable (Opening long / Buying / Selling vs Market long/buy/sell) - Spot validation: plain English instead of internal jargon
Fix/trade copywriting
- chains: parseChainArg() accepts named chain aliases (hyperliquid/hl, base, arb, eth, optimism, …) alongside numeric ids, case-insensitive. trade normalizes --chain-in/--chain-out up front so intent routing and the backend both get a clean id — `--chain-out hyperliquid` now works. - trade: new --dry-run flag previews any trade without signing/submitting. Swap/deposit/Treasures send dryRun to /trade/plan and render the server's new `preview` action; HL perp/spot/withdraw (signed client-side) compute and print their preview locally — size, notional, and margin at the effective leverage — without setting leverage, moving funds, or placing the order. - help: clarify --size is in token units (not USD); document aliases + --dry-run. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Removes two "two-ways-to-do-one-thing" footguns flagged in review: - Withdraw: collapse to one command. `acp trade withdraw-from-hl --amount <n>` is now the ONLY way to move USDC off Hyperliquid. Dropped the duplicate `--chain-in 1337 --chain-out <evm>` intent route and the ceremony it required (an explicit --chain-out whose only legal value was 42161 — HL's withdraw3 always settles to Arbitrum, so it was never a chain choice). --chain-in 1337 now means spot only; anything else points to the command. Adds --dry-run. - Slippage: one flag instead of two. `--slippage <pct>` (percent) now covers every route; removed `--slippage-bps` (the duplicate with a different unit that made `--slippage 300` mean wildly different things per route). Swaps/ Treasures convert pct→bps via slippageBpsFromPct; HL already used percent. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…thdraw Backward-compat: existing callers/agents that move USDC off Hyperliquid via the chain-in/chain-out combo shouldn't break now that `withdraw-from-hl` is the canonical command. detectIntent routes `1337 → 42161` to the same withdrawal. HL's withdraw3 only ever settles to Arbitrum, so we honor 42161 and reject any other chain-out (with a message pointing at withdraw-from-hl) rather than silently sending to a chain HL can't reach. Docs/help updated to show the combo as an alias of withdraw-from-hl. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…dge onward) - Default: a bare HL withdraw (no --chain-out / --to-chain) settles to Arbitrum, where HL's withdraw3 always lands. - --chain-in 1337 --chain-out <evm> (or withdraw-from-hl --to-chain <id>) to a NON-Arbitrum chain now withdraws to Arbitrum first, waits for the funds to land (polls Arbitrum USDC balance, ~few min), then bridges the actual arrived amount onward via the normal swap/LiFi route. On settle-timeout it returns the exact follow-up bridge command — funds are never stranded without a next step. - detectIntent: chain-in 1337 with a non-USDC token-out still errors as a spot order missing --chain-out 1337, so a typo can't silently move funds off HL. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`acp trade` with no flags previously dropped into a readline wizard. Drop it: the CLI is agent-first, and the wizard was a parallel input surface that could drift from the flag flows. With no intent in the flags, the command now errors with a pointer to `--help` instead. Removes runInteractive, the "interactive" intent, and the now-unused readline/prompt imports (the prompt lib stays — other commands use it). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The CLI is now a pure signer for Hyperliquid too. runPerp / runHlSpot / runWithdraw no longer touch the @nktkas ExchangeClient — they POST hl / hlSpot / hlWithdraw to /trade/plan and drive the existing runTradeLoop sign loop (the server builds each EIP-712 action; the CLI signs with the agent wallet). - Deletes the client-side execution: order placement, leverage, spot↔perp auto-balance (ensureHlFunds), the client withdraw-then-bridge, and the exitAfterOrder/placeHlOrder SDK-quirk workarounds. - lib/hl/client.ts shrinks to a read-only InfoClient for `trade status`; the ExchangeClient/signer wiring and order/price helpers are gone. - --dry-run, chain aliases, and the unified --slippage flag all keep working, now fully server-driven (the backend returns a `preview` action). Net −495 lines. `trade status` stays a client-side read (no signing). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…s intent The CLI no longer routes. `acp trade` collects whatever flags were passed into a flat body and POSTs to /trade/plan; the backend detects the venue and drives the sign loop. Deletes detectIntent + all per-venue run* functions (runSwap / runHlSpot / runPerp / runWithdraw / runTreasuresStock) and their helpers (isUsdcSymbol, validateTreasures*, parseSlippage, slippageBpsFromPct, parseChainArg usage). `withdraw-from-hl` stays as a thin alias that forwards the equivalent flat request; `status` stays a read-only HL query. Net big deletion; CLI is now a pure flag-forwarder + signer. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`acp trade status` calls the new POST /trade/hl-status instead of reading HL directly. Deletes src/lib/hl/client.ts (the last HL SDK usage) and removes @nktkas/hyperliquid from package.json. The CLI now has zero Hyperliquid deps — all HL access (trade + status) goes through the backend. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
detectIntent was deleted (intent detection moved to the backend), so its harness no longer applies. tradeLoopSign.test.ts stays — runTradeLoop is still the CLI's sign loop and it passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sync package-lock.json after dropping the HL dependency, so `npm ci` no longer installs @nktkas/hyperliquid (or its transitive valibot). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…olygon, Optimism, and Monad
…competent-raman-aac988
…policy, trim spot copy - Rename `acp trade status` to `acp trade hl-status`; clarify it's HL-account-only and that on-chain balances come from `acp wallet balance`. - add-signer: make the help/docs urge an explicit --policy (fallback is restricted, changing it later is manual); note unrestricted is for non-Virtuals contracts. - Remove concrete HL-spot how-to (examples + USDC-quoted note + table row); keep spot as a conceptual venue. Trim backend-plumbing prose to observable behavior. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 49a9b65. Configure here.
| `Trade ${plan.tradeId.slice(0, 8)}` + | ||
| (plan.direction && plan.route ? ` (${plan.direction} via ${plan.route})` : "") | ||
| ); | ||
| const result = await runTradeLoop(apiUrl, token, provider, plan, json); |
There was a problem hiding this comment.
Dry-run still requires signer
Medium Severity
runTrade always calls createProviderAdapter() before /trade/plan, so --dry-run fails with NO_SIGNER even when the plan would only return a preview and never sign. Help text says dry-run previews without submitting anything.
Reviewed by Cursor Bugbot for commit 49a9b65. Configure here.
| } catch (err) { | ||
| outputError(json, err instanceof Error ? err : String(err)); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Missing trade interactive picker
Medium Severity
README and SKILL state that bare acp trade in a TTY opens an interactive picker, but the command action always calls runTrade with parsed flags only. With no flags, the CLI posts a minimal body to /trade/plan instead of prompting.
Reviewed by Cursor Bugbot for commit 49a9b65. Configure here.
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
6 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.
Brings
devcurrent withmainand lands the unifiedacp tradecommand (swaps + Hyperliquid deposit/spot/perp/withdraw, chain aliases, Treasures tokenized stocks,--dry-run), plus the agent-facing copy polish.devwas ~91 commits behindmain, so this PR is large by design — it syncs everythingmainalready has, mergesfeat/trade-command, and adds the documentation/UX edits on top. CompanionmainPR: #34.Copy changes (on top of the feature)
acp trade status→acp trade hl-status— renamed and clarified as the Hyperliquid account view only (HL perp positions, margin, HL spot balances); on-chain token balances come fromacp wallet balance.--policyover therestrictedfallback (changing it later is manual);unrestricteddocumented as the choice for transacting outside Virtuals-approved contracts. Applied toagent add-signerandagent create --signer.🤖 Generated with Claude Code
Note
High Risk
Introduces automated signing and broadcasting for swaps, bridges, Hyperliquid, and Treasures stock flows via a new backend-driven trade loop—high financial and key-handling exposure despite tests covering only the sign branch.
Overview
This PR syncs
devwithmainand ships the package as@virtuals-protocol/acp-cli(bundleddist/bin/acp.js, Node ≥20.19, version 1.0.14), with a much broader CLI surface and agent-oriented docs.The headline feature is unified
acp trade: one entry point where chain IDs route intent (EVM↔EVM swaps, deposit to Hyperliquid via 1337, HL spot, perps via--side,withdraw-from-hl, plus Treasures tokenized stock flows). The CLI calls/trade/plan+/trade/next, signs send txs and newsignsteps (EIP-712 / personal proof), supports--dry-run, and exposesacp trade hl-status(renamed from status) as the HL-only account read.scripts/tradeLoopSign.test.tsexercises the sign branch.Beyond trading,
bin/acp.tswires subscription, email, card, compute, skill, and trade commands;ACP_CONFIG_DIRoverrides config location;configure start/configure completesupport non-blocking OAuth for harnesses; headless partner bootstrap (generate-signer-key,link, token flags) is documented indocs/headless-deployment.md.README and SKILL.md are rewritten around identity vs marketplace, wallet/email/card/compute, subscriptions, reviews,
job watch, and strict “agent runs CLI, human only clicks URLs” guidance (includingacp skill checkfor stale skills). Copy polish: explicitadd-signer --policy, and docs thathl-statusis not on-chainwallet balance.Reviewed by Cursor Bugbot for commit 49a9b65. Bugbot is set up for automated code reviews on this repo. Configure here.