Conversation
Adds the trade-ops layer on top of PR-B's HMA-orchestrated lifecycle
foundation. Demonstrates the full Architecture-B path:
Test
├── sphere host spawn → HMA → escrow + 2 traders (PR-B)
└── sphere trader set-strategy / portfolio / list-… (this PR)
Files:
test/e2e-live/helpers/sphere-trader.ts (~280 lines)
Typed wrappers around `sphere trader …` subcommands:
setStrategy, createIntent, cancelIntent, listIntents,
listDeals, portfolio, status, waitForDealInState. All accept
a tenant address (@NameTag, DIRECT://, or hex pubkey) and
parse the AcpResultPayload JSON shape sphere-cli emits with
--json. Throws on ok=false with the trader-side error_code +
message so tests don't repeat the defensive parse.
test/e2e-live/hma-trade-flow.e2e-live.test.ts (~250 lines)
Live test: bootstraps controller wallet, boots HMA, spawns
escrow + Alice + Bob via `sphere host spawn`, then drives
each trader via `sphere trader …`:
- set-strategy: configures trusted_escrows on each trader
- portfolio: reads the (empty) initial balance
- list-intents: reads the (empty) intent list
The test is intentionally scoped to the read+config surface.
Three known issues block the full create-intent / list-deals
path; all are upstream sphere-cli bugs tracked as follow-ups:
1. `sphere trader status` calls STATUS over ACP, but STATUS
is a system-scoped command — the trader correctly rejects
with UNAUTHORIZED ("System command STATUS can only be
sent by the host manager"). Controllers should use
`sphere host inspect <name>` instead. Skipped here.
2. `sphere trader create-intent` sends `expiry_ms` over the
wire but trader-service's ACP schema requires
`expiry_sec`. INVALID_PARAM. One-line fix in sphere-cli.
3. (Not encountered in this trimmed test but related:)
`sphere trader create-intent` previously used
`--volume-total` flag; now `--volume-max` matching the
trader's `volume_max` ACP wire field. Helper updated.
A separate file `hma-trade-settlement.e2e-live.test.ts` will
cover the swap-completion flow (faucet-fund both traders,
post matching intents, wait for COMPLETED on both sides,
assert balances reflect the swap) once issues 1-2 are fixed
in sphere-cli.
Verified live (2026-05-04 testnet, 5/5 services healthy):
$ npm run test:e2e-live -- hma-trade-flow.e2e-live.test.ts
✓ drives the full trader CLI surface against HMA-spawned tenants
Duration: 39s
Combined with PR-B's lifecycle test:
✓ both tests pass in 66s end-to-end
Default suite unaffected: 651 tests still pass.
Now that PR-D fixes sphere-cli's expiry_ms→expiry_sec wire mismatch and removes the architecturally-broken `sphere trader status` (which called a system-scoped command over ACP), the full create-intent → list-intents → cancel-intent → verify-CANCELLED flow works end-to-end against live testnet. Verified: 48s (up from 40s with the +3 additional CLI round-trips).
3 tasks
Steelman round 1 on PR-D (sphere-cli #7) found that trader-service's own `src/cli/main.ts` (the trader-ctl shim used by direct-docker e2e tests) still emitted `volume_total` / `expiry_ms` on the wire — the EXACT bugs PR-D fixes upstream. Without this commit, the canonical sphere-cli would talk correctly to the trader but trader-service's own tooling would still produce INVALID_PARAM rejections, leaving a silent half-fix. src/cli/main.ts: - Rename `--volume-total` flag → `--volume-max` (matches the trader's `volume_max` ACP wire field per acp-types.ts:23). - Convert `expiry_ms` wire param → `expiry_sec` via `Math.floor(n / 1000)` matching sphere-cli's fix. - CLI-layer guards: reject sub-1000ms expiry with a helpful message; reject expiries > 7 days. Mirrors the trader-side validation in trader-command-handler.ts:331-342. Cascade: 7 other test/helper files referenced the old wire shape and needed renaming for consistency: - test/e2e-live/helpers/scenario-helpers.ts (createMatchingIntents builder used by direct-docker tests) - test/e2e-live/helpers/contracts.ts (MatchingIntents interface) - test/e2e-live/helpers/sphere-trader.ts (IntentSummary response field; the trader's outbound list-intents response also uses `volume_max` per trader-command-handler.ts:143) - test/e2e-live/helpers/tenant-fixture.test.ts (unit-test assertions on trader-ctl argv) - test/e2e-live/negotiation-failures.e2e-live.test.ts - test/e2e-live/basic-roundtrip.e2e-live.test.ts - test/e2e-live/edge-cases.e2e-live.test.ts - test/e2e-live/multi-agent.e2e-live.test.ts Verified: - Default suite: 651/651 tests pass (was 651, no regression) - Live e2e (hma-trade-flow): passes in 56s (full set-strategy + portfolio + list-intents + create-intent + cancel-intent + verify-CANCELLED flow) - Zero remaining `volumeTotal` / `--volume-total` / `volume_total` references in src/ or test/ Depends on sphere-cli PR #7 for the canonical CLI's matching fix.
vrogojin
pushed a commit
to unicity-sphere/sphere-cli
that referenced
this pull request
May 3, 2026
Steelman round 1 on PR #7 found three issues: WARNING — Sub-1000ms expiry passed the CLI's `n <= 0` guard, then `Math.floor(n / 1000) = 0`, then the trader rejected with the opaque "expiry_sec must be positive" — confusing UX for what was a positive millisecond value. Add CLI-layer guard `n < 1000` with clear message before the conversion. Same approach for the 7-day upper bound (matches trader's own validation; saves a network round-trip on misuse). NOTE — No unit test for the wire conversion (`expiry_ms`→`expiry_sec` or `--volume-max` → `volume_max`). The PR's correctness rested entirely on the live e2e test in trader-service. Refactor: extract `buildCreateIntentParams(opts): { params } | { error }` as a pure function. The runWithTransport wrapper now just calls it, emits stderr on error-shape, and forwards on success. Add 10 targeted unit tests covering: - volume_max present (not volume_total) - --expiry-ms 5000 → expiry_sec=5 - --expiry-ms 90500 → 90 (floor) - omit --expiry-ms → no expiry_sec / expiry_ms in payload - --expiry-ms 999 → error mentioning "1 second" - --expiry-ms > 7d → error mentioning "7 days" - boundary cases (exactly 1000, exactly 7 days) — accepted - non-numeric/zero/negative — rejected - invalid direction — rejected - bigint-string fields pass through verbatim (no precision loss) NOTE (carry-forward) — trader-service has its own CLI shim (src/cli/main.ts) that ALSO had volume_total/expiry_ms — fixed in trader-service PR (vrogojin/trader-service#15) so direct-docker tests don't break either. Cross-repo coverage now consistent. Verified: 105/105 tests pass (was 95, +10 new wire-shape unit tests). Live e2e in trader-service still passes against testnet.
vrogojin
pushed a commit
to unicity-sphere/sphere-cli
that referenced
this pull request
May 4, 2026
…, status) Three upstream-discovered mismatches between sphere-cli's `sphere trader …` namespace and trader-service's actual ACP-0 wire schema. All three surfaced as failures in trader-service's HMA-orchestrated e2e tests (vrogojin/trader-service#15) and block the full create-intent → cancel-intent → settlement flow from being testable end-to-end. 1. `--volume-total` flag → `--volume-max` flag. The trader's CREATE_INTENT ACP param is `volume_max` (`/home/vrogojin/trader-service/src/trader/acp-types.ts:23`). sphere-cli was sending `volume_total` over the wire, which the trader silently dropped (parameter not in the schema). This PR was already partially staged in the working tree of the `fix/encrypt-decrypt-namespace` branch — landing it cleanly here. 2. `expiry_ms` wire param → `expiry_sec`. The trader validates `params['expiry_sec']` as a finite positive integer ≤ 7 days (`trader-service/src/trader/trader-command-handler.ts:331-342`). sphere-cli was sending `expiry_ms` which the trader rejected with INVALID_PARAM ("expiry_sec must be a finite number"). The `--expiry-ms` CLI flag stays — converting to seconds at the wire boundary keeps the flag ergonomic (matches other timeout flags) while fixing the wire shape. `Math.floor(ms / 1000)` so sub-1000ms expiries floor to 0 and fail the trader's positive-int check (correctly: sub-second expiries make no sense for trade intents). 3. `sphere trader status` removed. STATUS is a SYSTEM-scoped ACP command per the Unicity architecture (system commands like STATUS / SHUTDOWN_GRACEFUL / SET_LOG_LEVEL / EXEC route through the tenant's host manager via HMCP, not direct controller→tenant ACP). The trader correctly rejects direct STATUS calls with UNAUTHORIZED. The subcommand is removed; controllers should use `sphere host inspect <instance>` (HMCP) for trader liveness probes, or rely on `sphere trader portfolio` / `list-intents` succeeding as an implicit liveness signal. The trader-commands.test.ts subcommand-tree assertion drops `status` and renames the test to "exposes the 6 controller-scoped trader subcommands" with a comment explaining the architectural distinction. Verified: $ npm run check → 95/95 tests passing $ node bin/sphere.mjs trader --help → 6 subcommands listed (no status) Verified downstream against live testnet: $ cd /home/vrogojin/trader-service $ TRADER_E2E_SKIP_PREFLIGHT=1 npx vitest run --config vitest.e2e-live.config.ts \ test/e2e-live/hma-trade-flow.e2e-live.test.ts ✓ drives the full trader CLI surface against HMA-spawned tenants (48s) Set-strategy + portfolio + list-intents + create-intent + list-intents + cancel-intent + verify-CANCELLED — all green. Depends on PR #6 (encrypt/decrypt L1 fix) for the typecheck baseline.
vrogojin
pushed a commit
to unicity-sphere/sphere-cli
that referenced
this pull request
May 4, 2026
Steelman round 1 on PR #7 found three issues: WARNING — Sub-1000ms expiry passed the CLI's `n <= 0` guard, then `Math.floor(n / 1000) = 0`, then the trader rejected with the opaque "expiry_sec must be positive" — confusing UX for what was a positive millisecond value. Add CLI-layer guard `n < 1000` with clear message before the conversion. Same approach for the 7-day upper bound (matches trader's own validation; saves a network round-trip on misuse). NOTE — No unit test for the wire conversion (`expiry_ms`→`expiry_sec` or `--volume-max` → `volume_max`). The PR's correctness rested entirely on the live e2e test in trader-service. Refactor: extract `buildCreateIntentParams(opts): { params } | { error }` as a pure function. The runWithTransport wrapper now just calls it, emits stderr on error-shape, and forwards on success. Add 10 targeted unit tests covering: - volume_max present (not volume_total) - --expiry-ms 5000 → expiry_sec=5 - --expiry-ms 90500 → 90 (floor) - omit --expiry-ms → no expiry_sec / expiry_ms in payload - --expiry-ms 999 → error mentioning "1 second" - --expiry-ms > 7d → error mentioning "7 days" - boundary cases (exactly 1000, exactly 7 days) — accepted - non-numeric/zero/negative — rejected - invalid direction — rejected - bigint-string fields pass through verbatim (no precision loss) NOTE (carry-forward) — trader-service has its own CLI shim (src/cli/main.ts) that ALSO had volume_total/expiry_ms — fixed in trader-service PR (vrogojin/trader-service#15) so direct-docker tests don't break either. Cross-repo coverage now consistent. Verified: 105/105 tests pass (was 95, +10 new wire-shape unit tests). Live e2e in trader-service still passes against testnet.
vrogojin
added a commit
to unicity-sphere/sphere-cli
that referenced
this pull request
May 4, 2026
…, drop status) (#8) * fix(trader): align ACP wire shape with trader-service (expiry, volume, status) Three upstream-discovered mismatches between sphere-cli's `sphere trader …` namespace and trader-service's actual ACP-0 wire schema. All three surfaced as failures in trader-service's HMA-orchestrated e2e tests (vrogojin/trader-service#15) and block the full create-intent → cancel-intent → settlement flow from being testable end-to-end. 1. `--volume-total` flag → `--volume-max` flag. The trader's CREATE_INTENT ACP param is `volume_max` (`/home/vrogojin/trader-service/src/trader/acp-types.ts:23`). sphere-cli was sending `volume_total` over the wire, which the trader silently dropped (parameter not in the schema). This PR was already partially staged in the working tree of the `fix/encrypt-decrypt-namespace` branch — landing it cleanly here. 2. `expiry_ms` wire param → `expiry_sec`. The trader validates `params['expiry_sec']` as a finite positive integer ≤ 7 days (`trader-service/src/trader/trader-command-handler.ts:331-342`). sphere-cli was sending `expiry_ms` which the trader rejected with INVALID_PARAM ("expiry_sec must be a finite number"). The `--expiry-ms` CLI flag stays — converting to seconds at the wire boundary keeps the flag ergonomic (matches other timeout flags) while fixing the wire shape. `Math.floor(ms / 1000)` so sub-1000ms expiries floor to 0 and fail the trader's positive-int check (correctly: sub-second expiries make no sense for trade intents). 3. `sphere trader status` removed. STATUS is a SYSTEM-scoped ACP command per the Unicity architecture (system commands like STATUS / SHUTDOWN_GRACEFUL / SET_LOG_LEVEL / EXEC route through the tenant's host manager via HMCP, not direct controller→tenant ACP). The trader correctly rejects direct STATUS calls with UNAUTHORIZED. The subcommand is removed; controllers should use `sphere host inspect <instance>` (HMCP) for trader liveness probes, or rely on `sphere trader portfolio` / `list-intents` succeeding as an implicit liveness signal. The trader-commands.test.ts subcommand-tree assertion drops `status` and renames the test to "exposes the 6 controller-scoped trader subcommands" with a comment explaining the architectural distinction. Verified: $ npm run check → 95/95 tests passing $ node bin/sphere.mjs trader --help → 6 subcommands listed (no status) Verified downstream against live testnet: $ cd /home/vrogojin/trader-service $ TRADER_E2E_SKIP_PREFLIGHT=1 npx vitest run --config vitest.e2e-live.config.ts \ test/e2e-live/hma-trade-flow.e2e-live.test.ts ✓ drives the full trader CLI surface against HMA-spawned tenants (48s) Set-strategy + portfolio + list-intents + create-intent + list-intents + cancel-intent + verify-CANCELLED — all green. Depends on PR #6 (encrypt/decrypt L1 fix) for the typecheck baseline. * review: round 1 steelman fixes (CLI guards, extracted pure fn, tests) Steelman round 1 on PR #7 found three issues: WARNING — Sub-1000ms expiry passed the CLI's `n <= 0` guard, then `Math.floor(n / 1000) = 0`, then the trader rejected with the opaque "expiry_sec must be positive" — confusing UX for what was a positive millisecond value. Add CLI-layer guard `n < 1000` with clear message before the conversion. Same approach for the 7-day upper bound (matches trader's own validation; saves a network round-trip on misuse). NOTE — No unit test for the wire conversion (`expiry_ms`→`expiry_sec` or `--volume-max` → `volume_max`). The PR's correctness rested entirely on the live e2e test in trader-service. Refactor: extract `buildCreateIntentParams(opts): { params } | { error }` as a pure function. The runWithTransport wrapper now just calls it, emits stderr on error-shape, and forwards on success. Add 10 targeted unit tests covering: - volume_max present (not volume_total) - --expiry-ms 5000 → expiry_sec=5 - --expiry-ms 90500 → 90 (floor) - omit --expiry-ms → no expiry_sec / expiry_ms in payload - --expiry-ms 999 → error mentioning "1 second" - --expiry-ms > 7d → error mentioning "7 days" - boundary cases (exactly 1000, exactly 7 days) — accepted - non-numeric/zero/negative — rejected - invalid direction — rejected - bigint-string fields pass through verbatim (no precision loss) NOTE (carry-forward) — trader-service has its own CLI shim (src/cli/main.ts) that ALSO had volume_total/expiry_ms — fixed in trader-service PR (vrogojin/trader-service#15) so direct-docker tests don't break either. Cross-repo coverage now consistent. Verified: 105/105 tests pass (was 95, +10 new wire-shape unit tests). Live e2e in trader-service still passes against testnet. * review: round 2 polish — pin parseInt('1e6') surprise behavior Round 2 of steelman loop on PR #7 returned ROUND CLEAN. One NOTE-level test gap was worth adding as polish: `Number.parseInt('1e6', 10)` returns 1 (parses '1', stops at 'e') rather than 1_000_000. A user passing `--expiry-ms 1e6` wanted 1 000 000 ms (16.7 minutes) but gets the truncated value 1, which the < 1000ms guard then rejects with a confusing message saying "got 1". The value is REJECTED (not silently misused), so this isn't a correctness bug — but the surprising parse behavior is worth pinning in a test so a future strict-parse refactor is tracked as a deliberate change. The two WARNING-level findings from round 2 (parseInt prefix- truncation accepts `'1000abc'` as 1000) are a pre-existing file-wide pattern affecting --limit, --max-concurrent, --timeout in addition to --expiry-ms. Tracked as a follow-up: introduce a strict-positive-int helper and apply uniformly. Out of scope for this PR. Verified: 106/106 tests pass (was 105, +1 new pinning test). --------- Co-authored-by: Vladimir Rogojin <vrogojin@blockyinnovations.com>
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
1 participant
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.
Summary
Builds on PR #14's HMA-orchestrated lifecycle foundation by adding the trade-ops layer. Demonstrates the full Architecture-B path:
sphere host spawn(PR-B) for lifecycle,sphere trader …(this PR) for control-plane.Base branch:
refactor/e2e-live-via-hma(PR #14). When PR #14 lands on master, change this PR's base to master and rebase.Verified live
The trade-flow test:
sphere wallet initdist/host-manager.jsfrom agentic-hostingsphere host spawn× 3 (escrow + Alice + Bob)sphere trader set-strategyon Alice + Bob (trusted_escrows = escrow.pubkey)sphere trader portfolioon both (empty balances)sphere trader list-intentson Alice (empty)sphere host stop× 3 (parallel via async hostStop, ~5s)Files
test/e2e-live/helpers/sphere-trader.tssphere tradersubcommand. Accepts tenant address (@nametag/DIRECT://hex/raw hex), parses AcpResultPayload JSON, throws onok: falsewith trader-side error code.test/e2e-live/hma-trade-flow.e2e-live.test.tsScope: read+config only, NOT settlement
This PR exercises set-strategy / portfolio / list-intents — the read and configuration surface. It does NOT exercise create-intent, cancel-intent, list-deals, or full swap settlement. Three upstream sphere-cli bugs block those paths, all documented inline:
sphere trader statuscalls a system-scoped command. STATUS is reserved for the HMA per the Unicity architecture; the trader correctly rejects with UNAUTHORIZED. Controllers should usesphere host inspect <name>instead. The CLI should be fixed to route STATUS through HMCP (or removed from the trader namespace).sphere trader create-intentwire mismatch. sphere-cli sendsexpiry_msbut the trader's ACP schema requiresexpiry_sec(validated, ≤7d). INVALID_PARAM. One-line fix in sphere-cli (params['expiry_sec'] = Math.floor(opts.expiryMs / 1000)).--volume-totalvs--volume-maxflag rename — the helper now uses--volume-maxmatching trader-service'svolume_maxACP field. The CLI on the test branch already had this rename uncommitted in trader-commands.ts; helper aligned.A separate
hma-trade-settlement.e2e-live.test.ts(next PR after upstream fixes) will cover the swap-completion flow: faucet-fund both traders → wait for inventory → post matching intents → wait for COMPLETED → assert balances reflect the swap.Default suite
Unchanged. 651 tests still pass. Live tests opt-in via
npm run test:e2e-liveonly.Dependencies
Test plan
npm run test:e2e-live -- hma-trade-flow.e2e-live.test.tsagainst live testnet — should pass in <1 min.sphere-trader.tshelper API surface.