From 8278ad50d01d27485f820561e7baa03bbc577adf Mon Sep 17 00:00:00 2001 From: Vladimir Rogojin Date: Sun, 3 May 2026 22:52:45 +0200 Subject: [PATCH 1/3] fix(legacy-cli): align imports with sphere-sdk L1 namespace + missing exports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two upstream regressions broke `sphere --help` and `npm run typecheck` against the latest @unicitylabs/sphere-sdk: 1. encrypt/decrypt/hexToWIF/generatePrivateKey/generateAddressFromMasterKey are no longer top-level exports — they live in the L1 (alpha-chain) namespace. Import L1 once and destructure at module load to keep the diff to one block (call sites unchanged). 2. CreateInvoiceRequest, InvoiceRequestedAsset, GetInvoicesOptions, PayInvoiceParams, ReturnPaymentParams are declared inside SDK's AccountingModule but not re-exported at the package root. Extract them structurally via Parameters<> on Sphere.prototype.accounting method signatures so the types stay synced to the SDK without manual mirroring — a future SDK signature change surfaces here as a typecheck error. Symptoms before this fix: $ node bin/sphere.mjs --help SyntaxError: '@unicitylabs/sphere-sdk' does not provide an export named 'decrypt' / 'generateAddressFromMasterKey' $ npm run typecheck TS2459: Module '@unicitylabs/sphere-sdk' declares 'CreateInvoiceRequest' locally, but it is not exported. (× 5 types) After: $ npm run check Lint: 0 errors (warnings pre-existing). Typecheck: clean. Tests: 94/94. $ sphere --help → exits 0, lists subcommands. $ sphere host help → reaches the DM transport layer (needs a wallet+manager). Unblocks the SkipIf-gated `sphere-cli-roundtrip.e2e-live.test.ts` in agentic-hosting PR #22 (refactor/delete-ahctl) which probes `sphere --help` exit code to decide whether to run. --- src/legacy/legacy-cli.ts | 35 ++++++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/src/legacy/legacy-cli.ts b/src/legacy/legacy-cli.ts index 8cc231b..ac0a469 100644 --- a/src/legacy/legacy-cli.ts +++ b/src/legacy/legacy-cli.ts @@ -7,14 +7,20 @@ import * as fs from 'fs'; import * as path from 'path'; import * as readline from 'readline'; -import { encrypt, decrypt } from '@unicitylabs/sphere-sdk'; +// `encrypt`, `decrypt`, `hexToWIF`, `generatePrivateKey`, and +// `generateAddressFromMasterKey` are no longer top-level exports of +// @unicitylabs/sphere-sdk — they live in the L1 (alpha-chain) namespace +// as of the latest SDK refactor. Destructuring at module load preserves +// the existing call-site shape so the diff is contained to this block. +// Other helpers (parseWalletText, isValidPrivateKey, etc.) are still +// top-level exports. +import { L1 } from '@unicitylabs/sphere-sdk'; +const { encrypt, decrypt, hexToWIF, generatePrivateKey, generateAddressFromMasterKey } = L1; import { parseWalletText, isTextWalletEncrypted, parseAndDecryptWalletText } from '@unicitylabs/sphere-sdk'; import { parseWalletDat, isSQLiteDatabase, isWalletDatEncrypted } from '@unicitylabs/sphere-sdk'; import { isValidPrivateKey, base58Encode, base58Decode } from '@unicitylabs/sphere-sdk'; -import { hexToWIF, generatePrivateKey } from '@unicitylabs/sphere-sdk'; import { toSmallestUnit, toHumanReadable, formatAmount } from '@unicitylabs/sphere-sdk'; import { getPublicKey } from '@unicitylabs/sphere-sdk'; -import { generateAddressFromMasterKey } from '@unicitylabs/sphere-sdk'; import { Sphere } from '@unicitylabs/sphere-sdk'; import { createNodeProviders } from '@unicitylabs/sphere-sdk/impl/nodejs'; import { TokenRegistry } from '@unicitylabs/sphere-sdk'; @@ -23,14 +29,21 @@ import { tokenToTxf } from '@unicitylabs/sphere-sdk'; import type { NetworkType } from '@unicitylabs/sphere-sdk'; import type { TransportProvider } from '@unicitylabs/sphere-sdk'; import type { ProviderStatus } from '@unicitylabs/sphere-sdk'; -import type { - CreateInvoiceRequest, - InvoiceRequestedAsset, - GetInvoicesOptions, - PayInvoiceParams, - ReturnPaymentParams, - TxfToken, -} from '@unicitylabs/sphere-sdk'; +import type { TxfToken } from '@unicitylabs/sphere-sdk'; + +// CreateInvoiceRequest, InvoiceRequestedAsset, GetInvoicesOptions, +// PayInvoiceParams, ReturnPaymentParams are declared inside the SDK's +// AccountingModule but are not re-exported at the package root in the +// current SDK build. Extract them structurally from the public method +// signatures on `Sphere.prototype.accounting` so the types stay synced +// to whatever the SDK exposes — no manual mirroring required, and a +// future SDK signature change surfaces here as a typecheck error. +type SphereAccounting = NonNullable; +type CreateInvoiceRequest = Parameters[0]; +type InvoiceRequestedAsset = NonNullable[number]['assets'][number]; +type GetInvoicesOptions = NonNullable[0]>; +type PayInvoiceParams = Parameters[1]; +type ReturnPaymentParams = Parameters[1]; /** * Strip prototype-pollution-prone keys from user-supplied JSON before passing From 8fa5de8e63f0b624b7c7356daaceb466e584eee0 Mon Sep 17 00:00:00 2001 From: Vladimir Rogojin Date: Sun, 3 May 2026 23:40:13 +0200 Subject: [PATCH 2/3] test(legacy-cli): smoke test for L1 destructure + InvoiceRequestedAsset note MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 1 of steelman loop on PR #6. Two findings addressed: MEDIUM — Zero behavioral test coverage of the changed code paths. The 30-test scaffold suite never imports `legacy-cli.ts` directly (every existing test goes through `createCli()` / `buildLegacyArgv()` which lazily delegate to legacy-cli at call time, AFTER the test runner has imported its own modules). A future SDK rename of any L1-destructured symbol would pass `npm run typecheck` + `npm test` while breaking `sphere wallet …` at runtime — exactly the failure mode that motivated this PR. Fix: add `src/legacy/legacy-cli-imports.test.ts` that statically imports `legacy-cli.ts` and asserts `legacyMain` is defined. If the top-of-file `const { encrypt, decrypt, hexToWIF, generatePrivateKey, generateAddressFromMasterKey } = L1;` ever fails (e.g., L1 becomes undefined or a symbol is renamed), Vitest now reports a clean TypeError at the precise line. A second test verifies each L1 symbol is a function at runtime — coverage stays shallow on purpose (we don't exercise the encrypt/decrypt round-trip; that needs a wallet fixture and crosses into integration-test territory). LOW — `Parameters<>`-extracted Invoice* types are structurally identical to the SDK's named interfaces but TS error messages at call sites display the structural expansion instead of the named type. No correctness impact, but worth a comment so a future reader knows to migrate back to direct imports if/when the SDK re-exports the names. Added a QoL note under the type-aliases block. Verified: 96/96 tests pass (was 94, +2 new), npm run check clean. --- src/legacy/legacy-cli-imports.test.ts | 61 +++++++++++++++++++++++++++ src/legacy/legacy-cli.ts | 7 +++ 2 files changed, 68 insertions(+) create mode 100644 src/legacy/legacy-cli-imports.test.ts diff --git a/src/legacy/legacy-cli-imports.test.ts b/src/legacy/legacy-cli-imports.test.ts new file mode 100644 index 0000000..fa5bac9 --- /dev/null +++ b/src/legacy/legacy-cli-imports.test.ts @@ -0,0 +1,61 @@ +/** + * Smoke test for `legacy-cli.ts` module-load. + * + * Why this exists: + * The L1-namespace destructure at the top of legacy-cli.ts + * (`const { encrypt, decrypt, hexToWIF, generatePrivateKey, + * generateAddressFromMasterKey } = L1;`) is the line that broke when + * sphere-sdk's namespace layout changed. The fix in this PR re-anchors + * those imports against the L1 namespace, but the existing 94-test + * suite never imports legacy-cli directly — every test in + * `src/index.test.ts` works through commander's `createCli()` / + * `buildLegacyArgv()` factories which lazily delegate to legacy-cli + * at call time, AFTER the test runner has already imported its own + * modules. As a result, a regression that broke the legacy-cli + * module-load (e.g., a future SDK rename of `decrypt` again) would + * pass `npm run typecheck` and `npm test` while breaking + * `sphere wallet …` at runtime — exactly the failure mode the + * legacy-cli regression that motivated this PR exhibited. + * + * This test forces a static import of legacy-cli at test load time. + * If the destructure ever fails (TypeError: Cannot destructure + * property '' of 'L1' as it is undefined), Vitest reports a + * clean module-load failure with a precise pointer to the bad symbol. + * + * Coverage scope: this is INTENTIONALLY shallow — we don't exercise + * the encrypt/decrypt round-trip (that needs a wallet fixture and + * crosses into integration-test territory). We just confirm the + * module loads, which is enough to catch SDK-export drift. + */ + +import { describe, it, expect } from 'vitest'; + +describe('legacy-cli module load', () => { + it('imports without throwing — proves the L1 destructure resolved', async () => { + // Dynamic import inside the test so a failure surfaces as a test + // failure rather than as a module-collection-time crash that + // takes down the whole vitest suite. + const mod = await import('./legacy-cli.js'); + expect(mod.legacyMain).toBeDefined(); + expect(typeof mod.legacyMain).toBe('function'); + }); + + it('exposes the SDK-derived runtime helpers as functions', async () => { + // Verifies that the `const { encrypt, decrypt, ... } = L1;` + // destructure at module top resolved every name. If any field + // were undefined, the destructure itself would throw before + // `legacyMain` ever became defined. + const sdk = await import('@unicitylabs/sphere-sdk'); + const L1 = (sdk as { L1: Record }).L1; + expect(L1).toBeDefined(); + for (const name of [ + 'encrypt', + 'decrypt', + 'hexToWIF', + 'generatePrivateKey', + 'generateAddressFromMasterKey', + ]) { + expect(typeof L1[name]).toBe('function'); + } + }); +}); diff --git a/src/legacy/legacy-cli.ts b/src/legacy/legacy-cli.ts index ac0a469..4425404 100644 --- a/src/legacy/legacy-cli.ts +++ b/src/legacy/legacy-cli.ts @@ -38,6 +38,13 @@ import type { TxfToken } from '@unicitylabs/sphere-sdk'; // signatures on `Sphere.prototype.accounting` so the types stay synced // to whatever the SDK exposes — no manual mirroring required, and a // future SDK signature change surfaces here as a typecheck error. +// +// QoL note: these aliases are STRUCTURALLY EQUIVALENT to the SDK's +// named interfaces (`interface CreateInvoiceRequest` etc.) but TS +// error messages at call sites will display the structural expansion +// rather than the SDK's name. If the SDK ever re-exports the named +// types at the package root, prefer importing them directly to +// restore the named-type error messages. type SphereAccounting = NonNullable; type CreateInvoiceRequest = Parameters[0]; type InvoiceRequestedAsset = NonNullable[number]['assets'][number]; From 0b8164000530907e940ea4887161a0734430319d Mon Sep 17 00:00:00 2001 From: Vladimir Rogojin Date: Sun, 3 May 2026 23:46:50 +0200 Subject: [PATCH 3/3] review: round 2 steelman fixes (drop redundant test, add SDK issue TODO) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 2 of steelman loop on PR #6. Two findings addressed: HIGH — `legacy-cli-imports.test.ts` had two tests, but the second ("exposes the SDK-derived runtime helpers as functions") re-imported the SDK independently and checked `sdk.L1` directly. That tests the SDK in isolation, not the legacy-cli module under review — if legacy-cli destructured from the wrong path while the SDK was fine, the second test would still pass. False sense of two coverage vectors where only one (test 1's dynamic import) actually exists. The dynamic `await import('./legacy-cli.js')` in test 1 fully subsumes the intent: any L1 destructure failure throws at module evaluation time and Vitest reports a clean error pointing at the bad symbol. Deleted test 2 and updated test 1's docstring to explain why no further checks are needed. LOW — QoL comment in legacy-cli.ts about Invoice* type extraction had no issue-tracker reference. Added a TODO line linking the intent to a future sphere-sdk re-export request, so the comment prompts action when the SDK changes rather than being silently forgotten. Verified: 95/95 tests pass (was 96; -1 from deleted redundant test), lint+typecheck clean. --- src/legacy/legacy-cli-imports.test.ts | 28 +++++++++------------------ src/legacy/legacy-cli.ts | 4 ++++ 2 files changed, 13 insertions(+), 19 deletions(-) diff --git a/src/legacy/legacy-cli-imports.test.ts b/src/legacy/legacy-cli-imports.test.ts index fa5bac9..562be7b 100644 --- a/src/legacy/legacy-cli-imports.test.ts +++ b/src/legacy/legacy-cli-imports.test.ts @@ -35,27 +35,17 @@ describe('legacy-cli module load', () => { // Dynamic import inside the test so a failure surfaces as a test // failure rather than as a module-collection-time crash that // takes down the whole vitest suite. + // + // This single test subsumes the entire scope of round-1's + // intent: if the `const { encrypt, decrypt, ... } = L1;` + // destructure at the top of legacy-cli.ts ever fails (L1 + // undefined, name renamed, etc.), the dynamic import below + // throws at evaluation time and Vitest reports a clean failure + // pointing at the bad symbol. No additional SDK-side checks + // are needed — they would test the SDK in isolation, not the + // module under review, creating false coverage signal. const mod = await import('./legacy-cli.js'); expect(mod.legacyMain).toBeDefined(); expect(typeof mod.legacyMain).toBe('function'); }); - - it('exposes the SDK-derived runtime helpers as functions', async () => { - // Verifies that the `const { encrypt, decrypt, ... } = L1;` - // destructure at module top resolved every name. If any field - // were undefined, the destructure itself would throw before - // `legacyMain` ever became defined. - const sdk = await import('@unicitylabs/sphere-sdk'); - const L1 = (sdk as { L1: Record }).L1; - expect(L1).toBeDefined(); - for (const name of [ - 'encrypt', - 'decrypt', - 'hexToWIF', - 'generatePrivateKey', - 'generateAddressFromMasterKey', - ]) { - expect(typeof L1[name]).toBe('function'); - } - }); }); diff --git a/src/legacy/legacy-cli.ts b/src/legacy/legacy-cli.ts index 4425404..77e3fbf 100644 --- a/src/legacy/legacy-cli.ts +++ b/src/legacy/legacy-cli.ts @@ -45,6 +45,10 @@ import type { TxfToken } from '@unicitylabs/sphere-sdk'; // rather than the SDK's name. If the SDK ever re-exports the named // types at the package root, prefer importing them directly to // restore the named-type error messages. +// +// TODO: open a sphere-sdk issue requesting AccountingModule's +// Invoice* interfaces be re-exported at the package root. Until +// then this `Parameters<>` extraction is the maintainable path. type SphereAccounting = NonNullable; type CreateInvoiceRequest = Parameters[0]; type InvoiceRequestedAsset = NonNullable[number]['assets'][number];