docs(trade): spell out --reduce-only side semantics (close = opposite side)#61
Open
psmiratisu wants to merge 1 commit into
Open
docs(trade): spell out --reduce-only side semantics (close = opposite side)#61psmiratisu wants to merge 1 commit into
psmiratisu wants to merge 1 commit into
Conversation
… side)
The help text ("Only reduce an existing perp position") and README example
never stated the rule that matters: a reduce-only order's --side must be the
OPPOSITE of the position (close a long with --side short, close a short with
--side long), and the flag must never be set when opening. LLM agents reading
the docs were sending side == position when closing, which HL rejects with
"Reduce only order would increase position" — our most frequent perp error.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Zuhwa
approved these changes
Jul 2, 2026
Zuhwa
pushed a commit
that referenced
this pull request
Jul 17, 2026
…submit (consolidates #61 #63 #64 #66) (#67) * fix(wallet): retry degraded stock-portfolio fetch + feat(trade): handle price-impact confirm action (#65) * feat(trade): handle the server's price-impact confirm action /trade/plan now returns kind:'confirm' when a route loses more value than the server's warn threshold (PRICE_IMPACT_WARN_BPS). The published CLI died with "Unknown action kind: confirm" — it predates the action. Now: - without --accept-impact: exit cleanly with the server's warning and a recovery hint (PRICE_IMPACT_HIGH) - with --accept-impact: re-plan once echoing the confirm's acceptImpactBps, then execute as normal Hit live on a $15 Base→Treasures AAPL buy where Relay's flat fee crossed the 5% warn line. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(trade): bugbot — loop the confirm re-plan, forward --accept-impact to withdraw 1. After an --accept-impact re-plan the fresh quote can come back with a WORSE loss (another confirm); the single-shot handling let it fall into the trade loop and crash with 'Unknown action kind: confirm'. Loop the gate (max 3 plans) and exit with a clean PRICE_IMPACT_HIGH either way. 2. withdraw-from-hl builds its own opts for runTrade and only forwarded dryRun from optsWithGlobals(); --accept-impact stayed on the parent and never reached the gate, so a high-impact bridged withdrawal could not be unblocked. Forward it the same way. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(trade): declare --accept-impact on withdraw-from-hl (bugbot) optsWithGlobals only merges a parent flag when commander PARSES it — a flag placed after the subcommand must also be declared there or it errors as unknown, exactly like the existing --dry-run duplication. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(trade): parse the endpoint URL in the plaintext guard (bugbot) The https/localhost allowlist was a prefix regex, so http://127.0.0.1:80@evil.com passed while fetch connected to evil.com in the clear (the '127.0.0.1:80' is userinfo). Parse with URL and check protocol + hostname instead; both post() and get() share the one guard. Pre-existing on main, surfaced by this diff — fixed here since it guards the calldata-signing path this PR touches. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(wallet): retry assets fetch when stock-portfolio side degrades The backend's Treasures (tokenized-stock) fetch is best-effort: on upstream failure it returns empty positions with asOf null instead of erroring, which the CLI rendered as "no stock" — holdings intermittently vanished from `wallet balance` (~1 in 3 calls). Detect that failure signature, retry the assets fetch up to twice, and when it still fails surface an explicit warning (TTY note / stderr line in piped mode / stocksUnavailable flag in JSON) instead of silently omitting real holdings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(trade): document sell-proceeds destination (--token-out/--chain-out) Tokenized-stock sells already accept --token-out/--chain-out to deliver proceeds as a chosen token/chain (the eth venue sells, then bridges the USDC onward in one command), but neither the README nor SKILL.md mentioned it, so agents never offered it. Document it in both: prose, runnable eth-venue examples (USDC->Base, ETH->Arbitrum), and the SKILL flags table. Redirect is eth-venue only; a --chain sol sell delivers USDC to the Solana wallet and can't bridge onward. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(trade): surface sell-destination example in `acp trade --help` The trade help listed only a plain stock sell; add a --token-out/--chain-out example so redirecting sale proceeds to a chosen token/chain (eth venue) is discoverable from the CLI too, matching the README/SKILL.md docs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * chore: bump version to 1.0.25 in package.json and package-lock.json * feat(trade): route trade sends through sendCalls (app-sponsored gas) - trade loop: send + sendBatch now use provider.sendCalls (the app-sponsored Alchemy Gas Manager client) instead of sendTransaction (the ERC20 paymaster that bills the user's USDC) — so trades are gasless and don't need the user to hold USDC on every source chain (e.g. a BNB->SOL swap originating on BSC). - attach solWallet for a Solana TOKEN ref (--token-out sol / a base58 mint), not just an explicit Solana chain ref — fixes 'recipient required for non-EVM destinations' on a swap delivering to Solana. - createProviderAdapter builds sponsored clients for ALL sponsored chains (ERC20_SPONSORED_CHAINS), not just Base, so a non-Base source tx doesn't fail 'ACP not configured for chainId <n>'. - env-gated local-test overrides (ACP_TRADE_BASE_URL, ACP_WALLET_RPC_URL, allow http://localhost) — inert in prod without the env set. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(trade): --take-profit / --stop-loss perp flags Forward TP/SL trigger prices to the planner. With a size (--size/--amount-usdc) they attach to the opening order; alone they attach to an existing position. Each accepts an optional --*-limit companion to rest a limit on trigger instead of a market close. Maps to the backend's takeProfitPrice / stopLossPrice (+ *LimitPrice); the proxy forwards them verbatim. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(trade): spell out --reduce-only side semantics (close = opposite side) The help text ("Only reduce an existing perp position") and README example never stated the rule that matters: a reduce-only order's --side must be the OPPOSITE of the position (close a long with --side short, close a short with --side long), and the flag must never be set when opening. LLM agents reading the docs were sending side == position when closing, which HL rejects with "Reduce only order would increase position" — our most frequent perp error. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: bugbot findings on #67 — Solana job evaluator defaults to the agent's Solana wallet (not the System Program); register-erc8004 picks the first EVM client instead of supportedChainIds[0] Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: bugbot round 2 — EVM-only agents skip the optional Solana provider; Solana tokenize honors --60-days/--airdrop-percent createAgentFromConfig treated the Solana provider as required, so any v2-agent command threw NO_SOLANA_WALLET for agents without a Privy Solana wallet. The provider is optional in CreateAgentInput — omit it on NO_SOLANA_WALLET and let every other error propagate. The 60-days/airdrop parsing was guarded to non-Solana chains, but tokenizeOnSolana forwards both fields — drop the guard so the flags parse and validate identically on every venue. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: SOL token symbol no longer forces a Solana wallet when the leg's chain flag pins an EVM chain isSolanaTokenRef(tokenIn/tokenOut) made solanaExplicit true even when --chain-in/--chain-out explicitly named an EVM chain, so an EVM-only agent trading a wrapped-SOL token on Base failed with NO_SOLANA_WALLET. The token ref now only counts when its own leg's chain flag is absent or Solana. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(trade): forward the server's exact blockhash expiry to sponsored Solana submits The planner now sends lastValidBlockHeight (decimal string) on solana-tx legs whose tx it built itself (tax skim, Relay SVM assembly). Convert to bigint and pass it to sendSponsoredSignedTransaction so acp-node-v2 bounds confirmation at the tx's TRUE expiry instead of the current tip's window (a safe upper bound it falls back to when the field is absent — LiFi/ Jupiter prebuilt txs bake their own blockhash). A digits guard keeps a malformed server value from failing the leg in BigInt(). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: tokenize offers only chains the execution path serves (bugbot round 4) agent tokenize listed chains from createProviderAdapter, which now registers the ERC20-sponsored-gas superset for trades — on mainnet that includes Base Sepolia, so the picker could offer a chain that createAgentFromConfig's getClient() (built from EVM_MAINNET_CHAINS) rejects after selection. Filter the options through the same EVM_MAINNET_CHAINS / EVM_TESTNET_CHAINS split the execution agent uses. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: strict IS_TESTNET check for agent-create's auto ERC-8004 chain pick (bugbot round 5) The truthy check meant IS_TESTNET=false selected Base Sepolia, which the mainnet agent doesn't carry, so registration silently skipped. Matches the === "true" convention used everywhere else in the CLI. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: bugbot round 6 — whoami shows execution-set + Solana token status; mid-trade 401 refreshes against the real auth server - whoami listed Token/ERC-8004 rows from createProviderAdapter's ERC20-sponsored trade superset (10 chains on mainnet, testnets included) and omitted Solana entirely. Now it uses the same EVM_MAINNET_CHAINS / EVM_TESTNET_CHAINS split the execution agent serves, plus a Token row for Solana when the agent has a Solana wallet (no ERC-8004 row — the registry is EVM-only). - runTradeLoop's 401 recovery called forceTokenRefresh(url), which is the ACP_TRADE_BASE_URL shim in local setups — a shim can't mint tokens, so long trades died at token expiry. New forceApiTokenRefresh mirrors getApiContext: trade calls ride the shim, auth stays on the real server. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: Solana tokenize preflights the quote-token balance (bugbot round 7) EVM tokenize checks VIRTUAL covers launch fee + prebuy before sending; the Solana path broadcast blind, so an underfunded wallet died on an opaque on-chain error. Check the quoteMint balance (missing ATA = zero) inside the same approval gate before sendInstructions, and let the CliError verdict surface unwrapped. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: tokenize offers Solana only when the launch path can run (bugbot round 8) The picker added Solana on any SOLANA walletProvider record, but the launch resolves the wallet via getSolanaWalletInfo, which needs a populated solWalletAddress AND the provider's Privy walletId — without them the user picked Solana and failed late with a wallet error. Gate the option on the same requirements. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Zuhwa <zuhwa@virtuals.io>
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.
Problem
We're seeing a high volume of
Reduce only order would increase positionrejections from Hyperliquid. Root cause: the--reduce-onlyhelp text ("Only reduce an existing perp position") and the README example ("Reduce-only (close part of a position)") never state the one rule that matters — the order side must be the opposite of the position. An LLM agent reasoning "my position is long, so --side long" when closing gets this rejection 100% of the time, and the docs are exactly what agents read to compose these commands.Change
--reduce-onlyhelp string now reads: "Close/shrink an existing perp position. --side must be the OPPOSITE of the position (close a long with --side short, close a short with --side long). Never set this when opening or adding to a position — HL rejects it."Docs-only; no behavior change.
tsc --noEmitclean.Companion server-side fix: internal-trading-bot#59 adds a plan-time position check so a wrong reduce-only order fails instantly with the correct close order spelled out, instead of after the full sign roundtrip.
🤖 Generated with Claude Code
Note
Low Risk
Documentation-only changes to README and Commander option help; no trade execution or validation logic modified.
Overview
Clarifies Hyperliquid perp
--reduce-onlyusage in docs and CLI help so agents stop submitting orders with the wrong--side.The
--reduce-onlyoption description inacp tradenow states that--sidemust be opposite to the open position (short to close a long, long to close a short), and warns not to use the flag when opening or adding.The README perp section replaces a single reduce-only example with both close directions and adds a callout that mismatched sides trigger Hyperliquid’s "Reduce only order would increase position" rejection.
No runtime or API behavior changes — documentation and help text only.
Reviewed by Cursor Bugbot for commit 2c78eb9. Bugbot is set up for automated code reviews on this repo. Configure here.