add zerion-consolidate skill + CLI command#83
Merged
Conversation
Sweep all wallet positions on a single chain into one target token. Dry-run by default; --execute broadcasts sequentially after a single passphrase prompt. Filters: target token (by symbol and on-chain address), native gas, stables (flag/prompt/non-TTY default exclude), dust below --min-value, max-loss per quote. - cli/utils/trading/consolidate.js: pure helpers (filterCandidates, evaluateQuote, computeNativeSweepAmount, parseMaxLoss dual-form, DEFAULT_GAS_RESERVES, buildConsolidatePlan with injectable quoteFn). - cli/commands/trading/consolidate.js: CLI shell. Reuses getSwapQuote and executeSwap, coerceBoolFlag pattern from bridge.js, TTY check before confirm() for stables. - formatConsolidatePlan / formatConsolidateResult appended to cli/utils/common/format.js (mirrors formatBridgeOffers). - Registered between send and search in cli/zerion.js + router help. - skills/zerion-consolidate/SKILL.md with Safety section; umbrella zerion SKILL.md table updated. - Unit tests: 50+ assertions covering stables case-insensitivity, --max-loss dual form, gas-reserve math, target-by-address exclusion, sequential quote ordering, no_route resilience, coerceBoolFlag rejection via subprocess. Resolves PLT-676
Dev keys (zk_dev_*) stay sequential; paid keys (other zk_*) fan out to 5 by default. --concurrency <n> overrides (1..10). The --execute broadcast phase remains strictly sequential regardless — parallel signed broadcasts would race EVM nonces. - cli/utils/api/auth.js: export getApiKeyTier() with a keyOverride test seam so tests don't observe whatever config the dev has stored. - cli/utils/trading/consolidate.js: add parseConcurrency, generalise buildConsolidatePlan to accept a concurrency option, refactor per-row work into buildCandidateRow + a small in-line worker pool that preserves row order. Default concurrency=1 preserves the prior sequential contract. - cli/commands/trading/consolidate.js: auto-pick from tier when --concurrency is unset (AUTO_CONCURRENCY_BY_TIER), surface concurrency + apiKeyTier + concurrencySource in plan output. - cli/utils/common/format.js: pretty header line for concurrency + tier provenance. - skills/zerion-consolidate/SKILL.md: --concurrency row, tier-aware defaults explainer, invalid_concurrency in error table. - cli/router.js: help row mentions --concurrency. Tests: 17 new assertions across getApiKeyTier classification (6), parseConcurrency validation (5), bounded-fan-out cap (2), CLI invalid_ concurrency rejection (2), auto-pick mapping (1), and a source-grep test pinning the broadcast loop to for-await (1). 241/241 passing.
The table's Error column truncates at ~27 chars + ellipsis, which
buries the reason for any failure ("Quote not executable: Input asset
balance is not enough" → "Quote not executable: Input…"). Append a
Failures block under the totals line that prints the full error string
per non-success row, leaving the compact table intact for the
quick-scan view.
…ue-on-error flag
Each swap is an independent on-chain transaction. One failing quote
should not gate the rest of a sweep — that was a user-blocking issue
in a local test where WSTETH failed and the remaining 6 ready rows
never broadcast.
- cli/commands/trading/consolidate.js: drop continueOnError parsing and
the two conditional break statements. Broadcast loop is now extracted
into executeReadyRows for testability.
- cli/utils/trading/consolidate.js: new executeReadyRows(readyRows,
executeFn, { walletName, passphrase, timeout }) helper. Iterates
every ready row, captures per-row failures into the results array
with the full error string, never aborts.
- skills/zerion-consolidate/SKILL.md: drop the --continue-on-error
flag row, drop it from the boolean-flag pitfall sentence, remove the
example that used it. Add a paragraph above the concurrency block
explaining the partial-success contract. Safety section split into
separate bullets for sequential-broadcast and partial-success.
- cli/tests/unit/cli/utils/trading/consolidate.test.mjs: 3 new tests
on executeReadyRows pinning (a) 5 rows where row 2 throws all fire,
(b) non-success status ("reverted") counts as failed without
halting, (c) bare-string throws still produce a usable error string.
AC 21e source-grep test updated to look at the new helper location.
246/246 tests passing.
Three independent bugs surfaced in a local --execute test. All three fold into PR #83. 1) Nonce reuse across the broadcast loop. Back-to-back approvals read RPC `latest` which lags the previous swap's submission — row K+1 collides on the prior nonce. Fix: additive `approvalNonceOverride` on executeSwap (default behaviour unchanged for swap/bridge); the consolidate loop seeds from `pending` once, then tracks +2 per row with an approval and +1 per row without. On thrown error, re-read pending so recovery is at worst as good as the RPC-latest path. Skipped on Solana (no EVM nonce); falls back to RPC-latest with a stderr warning if the initial pending read fails. 2) Number(quantity.float) loses precision past ~15 sigfigs on 18-dp balances; the API then over-reconstructs wei and rejects with "Input asset balance is not enough." Fix: new rawWeiToDecimalString helper; classifyPosition pulls quantity.int + impl decimals and carries both a precise decimal string (the swap amount) and a Number (display-only). Native sweep math moves to BigInt — the float→wei conversion of the user-typed --gas-reserve is the only Number step and is well within precision limits. 3) USDT0 (LayerZero-bridged USDT, e.g. on base) wasn't in STABLE_SYMBOLS, so it leaked through into the sweep even with the default stables-exclude. Added usdt0 plus usd0 (Usual), bold (Liquity v2), usdy (Ondo) — all real stables; flagged here so reviewers can prune if the additions feel too aggressive. Extracted executeReadyRows now takes walletAddress + chain + an optional clientFactory test seam. Tests inject a fake getPublicClient to drive the nonce-tracking behaviour without touching RPC. 263/263 passing.
bashalex
changed the title
…paid zk_dev_* is the only paid-key prefix documented in this codebase (skills/zerion/SKILL.md). The previous classifier extrapolated zk_prod_ and zk_live_ — speculative — and returned "unknown" (→ dev concurrency) for anything else, which would silently cap real paid customers whose key shapes don't match. Invert the default: only zk_dev_* is dev; anything else with a non- empty key is paid. False positives (typo'd / garbage keys) burn a few extra request slots before the API errors — strictly safer than throttling a paying customer. - cli/utils/api/auth.js: drop the zk_-prefix gate; any non-empty non-dev key now returns "paid". - cli/tests/unit/cli/utils/api/auth.test.mjs: flipped the "non-zk_ key → unknown" assertion to "→ paid" with extra coverage (pk_prod_*, arbitrary prefix). Added an edge case pinning the acceptable startsWith match on bare "zk_dev_" (no suffix). 264/264 passing.
graysonhyc
previously approved these changes
May 22, 2026
graysonhyc
reviewed
May 22, 2026
…ddress
Symbol-based searchFungibles matching surfaced two failure modes during the
polygon dry-run: case-sensitivity smell on `USDC.E` vs `USDC.e`, and broad
ambiguity from symbol collisions. The new resolver in
cli/utils/trading/consolidate-targets.js accepts only:
1. a curated symbol — looked up in a flat SYMBOL→fungibleId map, with
the per-chain impl address fetched live from getFungible. Whatever
Zerion lists as USDC's impl on a given chain (bridged USDC.e on
some, Circle-native on others) is what the address-based target
exclusion catches.
2. a raw contract address — for any token outside the curated set, or
to override Zerion's choice.
Anything else throws target_token_not_found and names the curated list.
Reviewer feedback (graysonhyc): show how common natural-language requests (clear dust, treasury sweep, conservative slippage, send-to-address, etc.) map to consolidate invocations.
--min-value alone can only express \"sweep everything above $N\" — the opposite of what \"clear dust\" actually means. Adding --max-value flips the dust-clearing prompt from \"sweep my main bags\" (broken) to \"sweep rows up to $N, leave main bags alone\" (correct). Combined with --min-value the two flags form a band: rows below min are dust, rows above max are above_max (surfaced as skipped plan rows so the operator sees what was excluded). --max-value defaults to Infinity, so existing callers see no behavior change. Also fixes the AI example I just added in 36e0efd — \"clear dust\" was mapped to --min-value 5, which sweeps the opposite of dust.
Before: 19 symbols, including a long tail of niche / declining stables (USDD, GUSD, USDB, FRAX, LUSD, BOLD, USDY, …). Most just bloated the auto-exclude blast radius; legacy DAI also fell into "rarely worth auto-protecting" since USDS supersedes it. After: USDC, USDT, USDC.e, USDT0, USDS, TUSD, USDe. - Bridged variants (USDC.e, USDT0) stay because they're the user-visible symbols on positions; without them the operator gets surprised sweeps. - Anything else is now a sweep candidate by default — operators add legacy bags to `--exclude` when they want to protect them. Code, SKILL.md parenthetical, the interactive TTY prompt example list, and the test fixtures (positive list + non-stable list + bridge-variant suite) are all synced.
graysonhyc
reviewed
May 25, 2026
graysonhyc
reviewed
May 25, 2026
Fixes from PR review: - H1 (swap.js): when allowance already covers the swap and no approval tx is sent, swap signing now falls through to the caller's approvalNonceOverride instead of undefined. Two back-to-back allowance-covered rows in a consolidate batch no longer race RPC `latest`. Source-pinned in consolidate.test.mjs. - M1 (consolidate.js executeReadyRows): after a row throws, invalidate the tracked nextNonce instead of re-reading `pending` — `pending` can transiently include a failed submission for several seconds and over-shoot the counter. Next row falls back to the signer default. Test updated to match the new contract. - L1 (format.js lossCell): a gain (loss_pct < 0) previously rendered as `+-2.50%` because toFixed preserved the negative sign. Now displays as `+2.50%` using Math.abs. Test added for both gain and loss cases. - H2 / H3 (SKILL.md): the "fresh quotes on --execute" bullet contradicted the implementation — quotes are taken from the plan phase. Rewritten to describe actual behavior + staleness window. Added a Safety bullet explaining that loss_pct uses the quote's expected output, not the on-chain `outputMin` floor. - M3 (SKILL.md + consolidate.js): --slippage table cell now states the real 0–100 range and warns that values above ~5 compound across an N-position sweep. The command shell also prints a stderr warning when slippage > 5. 299/299 unit tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
the fix LGTM, let's merge |
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.
Implement zerion-consolidate skill +
zerion consolidateCLI commandSweep all wallet positions on a single chain into one target token.
Dry-run by default;
--executebroadcasts each ready row sequentiallyafter a single agent-token passphrase prompt.
Changes
cli/utils/trading/consolidate.js— pure helpers (filterCandidates,evaluateQuote,computeNativeSweepAmount,parseMaxLossdual-form,DEFAULT_GAS_RESERVES,buildConsolidatePlanwith injectablequoteFnso tests don't need network mocks).cli/commands/trading/consolidate.js— CLI shell. ReusesgetSwapQuoteandexecuteSwap, thecoerceBoolFlagpattern frombridge.js, checksprocess.stdin.isTTYbeforeconfirm()for thestables prompt, does NOT call
enforceExecutablePolicies(executeSwapalready does).
formatConsolidatePlan/formatConsolidateResultappended tocli/utils/common/format.js(mirrorsformatBridgeOfferspalette +padded columns).
sendandsearchincli/zerion.js; help rowadded to
cli/router.js.skills/zerion-consolidate/SKILL.mdwith the documented frontmattertrigger phrases and a
## Safetysection.skills/zerion/SKILL.mdtable updated with a row pointingto
zerion-consolidatedirectly after thezerion-tradingrow.Acceptance Criteria
registerSingle("consolidate", consolidate)between
sendandsearch.symbol/quantity/value_usd/ expected_output/expected_output_usd/loss_pct/status.native, stables, dust,
--include,--exclude,--gas-reserve,non-wallet position types skipped entirely.
quoteFn; per-rowno_routedoes not bail the plan.skipped: no_pricefallback when inputs missing.
formatConsolidatePlanmirroringformatBridgeOffers.--executere-uses freshly-fetched quotes within theinvocation, single passphrase prompt, sequential
executeSwap,--continue-on-errorsemantics,formatConsolidateResultoutput.(
invalid_min_value,invalid_max_loss,invalid_gas_reserve,target_token_not_found,conflicting_flags,invalid_flag_value).TTY check happens BEFORE
confirm().--include-nativeopt-in,--gas-reserve(orper-chain default),
skipped: below_reservewhen reserve ≥ quantity.zerion-tradingtemplate with Safetysection.
npm testpasses (224/224 in worktree).counter in the test that drives
buildConsolidatePlanwith a fakequoteFn).Testing
npm testin the worktree passes 224/224.npm linkfrom.claude/worktrees/PLT-676, then exercise via/pre-releaseordirect
zerion consolidate ...invocations.coerceBoolFlagAC is exercised end-to-end via a spawnedsubprocess test (
--include-stables something-bad→ exit codenon-zero + stderr contains
invalid_flag_value).Design notes
evaluateQuotetreatsnull/undefined/empty inputs as missingup-front (Number(null) is 0, which would compute a 100%-loss row
and surface as
blockedrather than the documentedskipped: no_price). The numeric path still runs throughNumber.isFinitesoNaN/Infinityare also caught.0.01 - 0.001produces0.009 + 1e-18. Testsassert proximity (
< 1e-12) rather than exact decimal equality —the contract is "qty minus reserve", not a specific rendering.
Resolves PLT-676