Add CLI on top of config keys refactor - #6082
paullinator wants to merge 25 commits into
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
67406a5 to
35eeb43
Compare
a8819b9 to
60adf1e
Compare
7d3a2ba to
c0c5f13
Compare
0574066 to
4bb295e
Compare
`typechain` emits `export * as factories from './factories'`, and the React Native preset does not transform namespace re-exports. The plugin was already a transitive dependency but never enabled, so `TransactionListTop` failed to parse the moment `src/plugins/contracts` existed — which it does after any `npm install`, since `prepare` generates it. Metro needs that transform as much as jest does, so it belongs in the shared config.
src/util/hmacAuth.ts imports hashjs directly, but the package was only ever resolved transitively. Declare it so a clean install and the Node CLI bundle both get it.
70e9045 to
b53d382
Compare
`network.ts`, `utils.ts` and the locale boot each pulled React Native in through their module load paths, so nothing outside the app could fetch from the info server, format an amount, or pick a language table. Fiat constants and helpers move to `fiatConstants.ts`, and `getOsVersion` to `rnUtils.ts` alongside the other React Native-only helpers, with `keysStore.ts` following it there. `network.ts` takes its server lists and device fields through `configureNetwork` and `initInfoServer(params)` rather than reading `appConfig` and `react-native-device-info` at module scope. Locale boot splits the same way: `bootLocale.ts` applies a language table and number format with no React Native imports, and `initLocale.ts` stays GUI-only, feeding it what `react-native-localize` reports. Capturing those device fields is separate from starting the poll: `configureInfoServer` records them synchronously, so `fetchPublicRollup` works for whoever calls it first, while `initInfoServer` owns the polling and the decision to skip the unsigned launch fetch. `fetchWaterfall` refuses an empty server list rather than handing it to `asyncWaterfall`, which awaits `Promise.race([])` and never settles.
Keep exchangeRates on network.fetchRates and utils.removeIsoPrefix; inject only Airship showError via exchangeRatesGui at app start.
`initLocale` reaches for `react-native-localize`, so nothing outside the app could ask which locale to use. The decision itself is pure: read a tag from argv, config or the environment, normalize it, and pick a language table. `nodeLocale.ts` holds that decision with no React Native imports, and feeds the same `applyLocale` that the GUI's device lookup already calls, so the GUI and any Node caller resolve a locale the same way rather than approximately the same way. Precedence is explicit and tested: an explicit tag, then config, then `EDGE_CLI_LOCALE`, then `LC_ALL` / `LC_MESSAGES` / `LANG`, then `Intl`, then `en-US`. `es_MX.UTF-8@euro` and `C` both resolve, which is what the POSIX forms actually look like. `env` is typed as the variables it reads rather than `NodeJS.ProcessEnv`, which in this repo demands `NODE_ENV` and would make every caller invent one.
`CategoriesActions.ts` held five hundred lines deciding what a transaction should be called: the category, the payee, the direction, and the label for each action type. All of it is a pure function of the transaction, the wallet and the account, but it sat behind Redux imports, so nothing outside the app could ask the same question and get the same answer. `src/util/txDisplay/` holds that logic now — `displayInfo` for the derivation, `category` for the category strings, `txActionLabels` for the action names, and `currencyCodes` for the ticker lookups. `CategoriesActions` re-exports what the GUI already imported, so no scene changed. The point is that two callers cannot drift. A transaction rendered in a list, exported to CSV, or printed by a script now describes itself identically, because it is the same code deciding.
Three small pieces of GUI state that any caller reading transactions needs, and none of which had a reason to be Redux-only. `exchangeDenom` picks the denomination a currency or token reports amounts in. `DenominationSelectors` keeps its selector shape and calls it, so the two cannot disagree about what a multiplier is. `spamThreshold` decides which incoming transactions are dust worth hiding. The GUI applies it to every list; a caller that reads the same wallet and does not apply it sees a different set of transactions, which is the sort of difference that looks like a bug in whichever one you did not write. `localAccountSettings` reads the device-local settings file that holds the spam filter toggle, and `LocalSettingsActions` reads through it rather than duplicating the format. The threshold needs a rate, and asks for it at the current hour rather than the current millisecond, so repeated listings share one cache entry instead of missing on every call. It is a different source from the GUI's, which reads live rates from Redux, and a lookup that fails yields no filtering — stated in the module, because the two can legitimately disagree. `syncedSettingsFile` holds the one definition of the synced `Settings.json` that Node-safe code reads. The GUI's own cleaner sits behind an Airship import and cannot be loaded here, so this is a two-field view of the same file with the same defaults, and a test asserts those defaults still match the GUI's.
`TransactionExportActions.tsx` was a five-hundred-line thunk that did four separable jobs: fill in historical fiat values, render CSV, render QBO, and render the Bitwave format. Only the last step needed React Native, and only for writing the file. `fillTxsFiat` asks the rates server what each transaction was worth on the day it happened, which is the part that makes an export more than a dump of native amounts. `txExport/format` renders the three formats. `exportTxInfo` holds the Bitwave account mapping the exporter needs. The thunk keeps the file-writing and the share sheet, and calls the same renderers. `TransactionsExportScene` follows it. A test now covers `fillTxsFiat` against a partial wallet, which is all it reads. The formats matter here: an export that a person reconciles against their books has to be byte-identical whichever tool produced it, and the only way to be sure of that is for one renderer to produce both.
`SendScene2` saved a sent transaction and then attached its metadata, its category, its notes and any swap details in a sequence that had to happen in a particular order and had grown inline in the scene. A second caller that saved a transaction and got the order wrong would produce a transaction that looks right until someone exports it. `txTagging/apply` holds that sequence. `SendScene2` calls it and loses two dozen lines. The scene was the only definition of what a correctly tagged transaction is, and now it is not the only caller that can produce one. It re-applies only the fields the caller actually supplied. Under `EdgeMetadataChange` an empty string is a value rather than "leave unchanged", so passing all three through would let a caller who set one field erase the other two that core derived. The trigger is any non-empty name, notes or category, which is wider than the scene's old `payeeName != null` check and preserves a category that would otherwise be lost; callers pass the metadata they were given, never computed display metadata.
A long-lived engine daemon owns the `EdgeContext` and answers a JSON REST API
over a Unix socket; the `edge-cli` binary is a thin one-shot client that spawns
the engine on demand and keeps a session id in `session.json` so commands
chain. `docs/EDGE_CLI.md` describes that architecture and deliberately
documents no endpoints — the reference is generated.
The point of this commit is the declaration format, so it carries thirteen
calls rather than all of them. Each is one `route({…})`: the core call it
fronts, the HTTP method and path, how it appears on the command line, cleaners
for the query, body and response, and its error codes. The prose lives inside
the declaration, beside the field it describes, and the JSDoc above carries
what belongs to the call as a whole.
Nothing is written twice. The command line, the help text, the OpenAPI
document and the HTML reference are all derived from these declarations, and
the derived artifacts are committed so a fresh clone needs no build step. Five
gates run in the pre-commit hook and reject the ways they could drift apart: a
route with no command, a handler reading a field its cleaner would strip, a
request parameter the core call does not have, a generated file that is stale,
and a command no test exercises.
The thirteen cover the shapes worth reviewing:
- no arguments, engine-local — `engine-status`, `engine-config`
- no arguments, reaching core — `local-users`, `fetch-login-messages`
- one named argument — `username-available`
- a body, and the session it establishes — `create-account`,
`login-with-password`, `logout`
- a positional path parameter — `object-get`, `object-delete`
- a held-open stream — `subscribe`
Path parameters are base58 identifiers and nothing else, because base64 wallet
ids and free-text usernames contain `/` and cannot survive a URL unescaped.
Everything else is a named argument. A positional is declared once as an
ordinary field and the path is derived from it, so the two cannot disagree.
`--fake` serves an in-process `makeFakeEdgeWorld`, which is what lets the CLI
tests run in a hook with no network, no server and no API key.
Core values with methods on them cannot cross JSON, so the engine keeps them
and hands back a handle: a staged transaction, a pending login, a swap quote, a
lobby. A handle carries its own TTL and is released when the caller finishes
with it, and a call that consumes one — approving a swap — marks it in flight
first, so a client that retries after its own socket timeout is refused with
`OBJECT_IN_USE` rather than spending twice. A call that keeps its handle holds
it the same way, so a broadcast that outlives the TTL still returns its txid
instead of expiring between the send and the reply. Reading a handle returns a
projection, never the live object: serializing an `EdgeAccount` would walk its
`otpKey` and `recoveryKey` getters, and a swap quote reaches both wallets and
every token they know.
Responses are validated against the same cleaners that document them.
`checkResponse` runs each one and discards the cleaned value, since a cleaner
strips unknown keys and returning it would delete fields the engine means to
send; `EDGE_CLI_CHECK_RESPONSES` decides whether a mismatch warns, fails the
request, or is skipped. Drift shows up in the log rather than reaching a caller
unnoticed.
One engine serves one profile, and it claims the profile by creating its run
file exclusively before opening the data directory, so two cold invocations
cannot hold two `EdgeContext`s on one set of repos or unlink each other's
socket. The idle timer counts in-flight requests as well as sessions and
subscribers, so a cold login cannot be shut down underneath itself. Everything
the engine reads from disk — its run file, the client's session file, the
account's synced settings — goes through a cleaner, and the bearer tokens in an
OTP challenge are masked on their way to a terminal while staying in the REST
body the commands read them from.
b53d382 to
f746c04
Compare
Target Node in the CLI bundle, so the built engine survives its first event. QA round 1, finding 1 (severity 0): `npm run build:cli` succeeded but `node lib/edgeEngine.js` — the documented "Built artifact" and `npx edge-cli` run modes — died seconds after printing `Ready` with `Cannot read properties of undefined (reading 'writableEnded')`. `@babel/preset-env` was configured with `loose: true` and no target, so it downlevelled for browsers and compiled `[...this.clients]` to `[].concat(this.clients)`. `Array.prototype.concat` does not spread a `Set`, so the loop iterated the `Set` object itself and `client.res` was undefined. Six sites were affected: `EventHub.emit`, `closeScope` and `closeAll`, the object handle sweep's `[...this.handles.keys()]`, and both session listings. Only the `sucrase/register` dev path was unaffected, which is why every test in the branch passed while the shipped artifact was dead. The bundle runs on Node — `engines` already says `>=18` — so it targets Node 18 and leaves spread native. Verified on the built artifacts: the engine stays up through `engine-status`, `local-users`, `engine-sessions` and `engine-stop`, and no `[].concat(this.clients|handles|sessions)` remains in the bundle.
Make --solve-captcha, the prompt and the event stream behave as documented.
QA round 1, findings 2, 3, 4 and 13, all severity 2.
**2** — `--solve-captcha` could not work for `username-available`, one of the
three calls the guide names as CAPTCHA-raising. The retry set
`ctx.challengeId`, but only the five hand-written login commands read it; the
generated runner built its request from argv alone, so the retry re-sent the
call unchallenged and the server issued a fresh challenge. Generated commands
now inject a solved challenge when the route declares a `challengeId` field and
the caller did not pass `--challenge-id` — general, so it holds for any route a
challenge can reach.
**3** — the prompt ignored the flag entirely: it invoked commands directly
while one-shot mode wrapped them in the retry. Both now go through
`invokeCommand`, which is the guide's claim that "the engine, session and flags
are the same as one-shot mode".
**13** — a piped script ran only its first command. `rl.question` took one line
while `rl.on('close')` resolved the loop at EOF, which arrived while that first
command was still awaiting. The loop is now `for await (const text of rl)`, so
lines run in order to the end, and the `> ` prompt is only printed to a TTY.
**4** — `subscribe` pretty-printed each frame across a dozen lines, so none of
them parsed on their own, while the guide, the route note and the command's own
comment all promise newline-delimited JSON for `jq -c` and `while read`.
`printJsonLine` emits one object per line and the stream command uses it.
Verified: every line of a live subscription now parses.
Report a bad --config through the error envelope, and print usage on a one-shot usage error. QA round 1, findings 10 and 11 (severity 2). **10** — `loadConfig` runs at import time, from `bootNodeLocale`, which is the first import in both binaries. A nonexistent or malformed `-c/--config` file therefore threw before `main()` had a handler, and the user got Node's uncaught-exception format — source line, caret, stack — and exit 1, where the documented failure model is the JSON envelope and exit 2 for bad argv. It now reports that way and stops. **11** — a one-shot usage error printed `"Incorrect arguments"` and nothing else, naming no flag, while the interactive prompt printed the usage line for the same mistake. One-shot mode now prints it too, on stderr so stdout stays machine-readable. Wiring that up exposed a bug in `formatUsage`: it prepended the command name to a usage string that already starts with it, so the prompt had been printing `Usage: edge-cli create-account create-account …` all along. Every usage string starts with its own command name — `docs:api:verify` enforces it — so the name is no longer prepended.
Measure CLI coverage from the call sites, not from every quoted string.
QA round 1, finding 17 (severity 2): the gate built its "covered" set from
`[...tests.matchAll(/'([a-z0-9-]+)'/g)]` — every single-quoted kebab-case
string anywhere in the two offline suites, including comments, labels and URL
fragments. A command that was merely mentioned passed; the `114/118 commands
run offline` figure, which is the branch's main coverage claim, was not
established by it.
It now reads the command argument at each helper's known position — `ok`,
`refuses`, `notInFakeWorld`, `cli` — plus the `spawn('node', [...])` shape
`testCliSubscribe` uses for the held-open command. A command invoked through a
variable is invisible to this, which under-counts rather than over-counts, the
safe direction for a coverage gate.
The figure is unchanged at 114/118, so the claim was true; it is now measured.
Answer help locally, and leave nothing behind when an engine stops. QA round 1, findings 24, 20 and 21 (severity 3). **24** — `main()` built a context before dispatching, so `edge-cli help` spawned an engine and left it running; without `-t` that daemon pointed at production. `help` is answered from the committed help table and needs no engine, so it is dispatched first. **20** — `engine-startup.log` sat alongside three `0600` files at `0644`, though it captures the engine's stdout and stderr. It is created `0600` now, and it is removed with the rest of the profile's artifacts, so `rmdir` succeeds and a profile directory no longer outlives its engine. `session.json` goes with them: a sessionId cannot outlive the engine that issued it, so keeping it only made the next command fail `INVALID_SESSION` for no reason a user could see. That accumulation was real — this machine had 415 profile directories. **21** — `engine-status` and `engine-config` reported `testMode: true` under `--fake` while `engine.json` for the same engine said `false`, because one came from the core descriptor and the other from argv. The run file now records the core's effective value. `--fake` reports true, which is what a caller checking this field wants to know: it is not production.
Show both forms of a command two routes share. QA round 1, finding 25 (severity 3): `local-settings` reads on GET and writes on POST, and the help table kept whichever route was declared last — so `help local-settings` showed only `--spam-filter-on=true|false` and the read form looked invalid. QA found it by diffing all 117 reference usage lines against the help table; it was the only mismatch. Both forms are carried now, and `help` prints the second as `alsoUsage`.
Cover the auto-logout timer with tests. QA round 1, finding 15 (severity 2, filed as NOT TESTED): no CLI route writes the account's synced `Settings.json`, so QA could verify everything around auto-logout — the reported `autoLogoutSeconds`, `expiresAt` arithmetic, the `0` sentinel — but could not make a session actually expire without editing a shared account's settings, which the handoff forbids. `sessions.test.ts` drives `SessionStore` directly under fake timers: the 3600 default when `Settings.json` is absent, the account's own value when present, a session surviving to its window and being logged out past it, `touch` extending it, `0` disabling it entirely, and the distinction finding 18 is about — an idle session answers `SESSION_EXPIRED` while a logged-out one answers `INVALID_SESSION`. `setImmediate` stays real, because `create` drains the core pixie stack through it before exposing a session.
f746c04 to
2a7d387
Compare
Keep the error classes real in the CLI bundle, so error dispatch survives the build. QA round 2, finding 28 (severity 0): the bundle applied `babel-plugin-transform-fake-error-class`, which rewrites `class X extends Error` into a function returning a plain `Error`. Its own README says the consequence plainly — "`instanceof MyError` will always return false" — and both binaries dispatch on exactly that. So in the published artifacts nine `instanceof` branches were dead. The engine answered `INTERNAL_ERROR` 500 for every error it raised itself, including the 404s and 405s the reference documents. The client collapsed everything to `INTERNAL_ERROR` 500 exit 1, which made exit codes 2 through 6 unreachable, suppressed every usage line, and stopped `--solve-captcha` retrying — because the `ApiClientError` it tests for was never an instance of anything. Round 1's crash had masked all of it: the engine died before it could answer anything. The plugin exists for the app's ES5 build, which keeps it; this bundle targets Node 18, where native subclasses work and give good stack traces. Verified on the built artifacts: an engine-raised `NOT_FOUND` comes back as 404 with exit 4, and a usage error prints its usage line and exits 2.
The other hundred and four calls, in the format the previous commit
established: account and session management, credentials, 2FA and vouchers,
the data store, keys and wallets, tokens, URIs, transactions and their export,
the staged spend path, swaps, exchange rates, and the `$internalStuff` admin
calls.
Nothing here changes the framework. Every call is a `route({…})` of the same
shape, and the same five gates hold across all of them: 117 routes, 117 of 118
commands, 245 response fields described, 87 routes matching their core
signature or recording why they differ, and 114 of 118 commands exercised
offline against the fake world.
Four cannot be exercised in a hook and say so: the two rates calls, swap quotes
and payment-protocol requests each reach a third-party API that the fake world
does not intercept. They have their own `test:cli:network` script, alongside
the suites that need a real login server.
Where the API departs from `edge-core-js` it is recorded in `coreExtra` with
the reason — a wallet object that cannot cross HTTP as anything but an id, the
`to`/`amount` shorthand that expands into `spendTargets`, engine-side paging
and export on `get-transactions`. Anything not listed there fails the build.
Where a value cannot be derived safely the call refuses rather than guesses.
`rates-usd-to-native` requires its `multiplier`, because this route has no
logged-in account to read a denomination from and an assumed one returns a
`nativeAmount` wrong by orders of magnitude. `sign-bytes` rejects malformed
base64 instead of signing whatever decoded. Handles record the resolved
`wallet.id`, so a wallet id and a unique prefix of it name the same wallet on
every step of a staged spend.
Answer three calls with the errors they document instead of a 500. QA round 1, findings 6, 7 and 8, all severity 2. Each declared a clean error and then let a core failure surface as `INTERNAL_ERROR`, which tells a caller the engine broke when the request was at fault or the network was. - `get-max-spendable` with no destination handed core an empty `spendTargets` and surfaced its TypeError. The route already documents that a destination is required because fees depend on one, so it now says so with `BAD_REQUEST`. - `get-item` on an absent item returned 500 while declaring `NOT_FOUND`; core throws a plain Error there. - `get-payment-protocol-info` reported an unreachable payment URL as an engine fault rather than the `NETWORK_ERROR` it declares.
Declare rates-usd-to-native's multiplier as required, rather than saying so in prose. QA round 1, finding 9 (severity 2): the usage line offered `[--multiplier=<multiplier>]` and the OpenAPI `required` array listed only `usdAmount` and `pluginId`, while the field's own description said it was required and the handler rejected a request without it. Following the usage line failed with `BAD_REQUEST`. The cleaner now requires it, so the generated usage, the OpenAPI document and the runtime agree, and the redundant hand-rolled guard is gone. The old message also blamed a missing login — "cannot derive without a logged-in account" — which misled QA, who was logged in. This route is context-level and has no session at all; the description says that now.
Correct the guide's create-account example and describe what request-edge-login actually does. QA round 1, findings 11 and 12 (severity 2). The tester-servers section's only `create-account` example passed the username as a bare positional, which fails with a usage error — and contradicts the same guide's own rule that a username is free text and therefore a flag. It now uses `--username` and `--solve-captcha`, which is what the tester login server requires. The Edge-login section said the command "prints JSON" and described a poll-it- yourself flow, never mentioning that the default blocks for up to five minutes polling every two seconds, or that `--no-wait` exists. QA followed the guide and had to kill the command. Both are documented now, along with `poll-edge-login`, `fetch-lobby` and `cancel-request`.
Stop promising a subscription exit code and an error code that cannot happen. QA round 1, findings 5 (severity 2), 18, 22 and 23 (severity 3). **5** — the guide and the route both published `subscribe` exit `3` for a session ending the stream, two paragraphs after the guide correctly says every subscription is context-scoped and a logout does not end one. Both are true of the code, and the second one wins: nothing but the engine closes a stream, so `exitCodeForClose` had three unreachable branches. The contract is now exit `0` on interrupt and `7` otherwise, in the guide and in the declaration. **18** — the catalogue said `SESSION_EXPIRED` covers "auto-logged-out, or explicitly logged out". An explicit logout deletes the record, so the next call gets `INVALID_SESSION`; `SESSION_EXPIRED` is only reachable on the idle path. The entry says which is which. **22** — `subscribe` prints a `subscription.ended` frame of its own after the stream closes, which was in no frame list and which `--type` does not filter, because the engine never sent it. Documented as a client frame. **23** — the guide's sample CAPTCHA envelope showed a `challengeUri` with an `/api/v2` segment the login server does not use.
Say what two error codes actually mean. QA round 1, findings 19 and 26 (severity 3), both rebutted as behaviour and corrected as documentation. `OBJECT_EXPIRED` was documented as what a handle past its 5-minute TTL returns. The 15-second sweeper usually gets there first and releases the handle, so the common answer is `OBJECT_NOT_FOUND`; 410 means it was read inside that window. The alternative — a tombstone per expired handle — is state with no other purpose, so the entry describes the race instead of hiding it. `SWAP_BELOW_LIMIT`'s `nativeMin` arrives empty when the plugin reports no minimum. Nothing is dropped on the way out: core's `SwapBelowLimitError` constructor defaults it with `nativeMin ?? ''` and the engine passes the value through. The entry says the field can be empty and why.
Finish the round-1 fixes that were only half done. QA round 2, findings 27, 29, 30, 31, 32 and 33 — each the remainder of a round-1 fix rather than new ground. - **27** the exit-3 contract came out of the declaration's `exits` but stayed in its prose, so the generated reference and `help subscribe` still promised it. - **29** the command name was resolved *after* `buildContext`, so a typo spawned a daemon — and with no `-t`, a production daemon on the default data directory. Resolved first now: `edge-cli no-such-command` exits 2 and starts nothing. - **30** the guide's sample payloads still disagreed with the server: a `pendingId` prefix that does not exist, and a CAPTCHA `message` shorter than the one `mapCoreError` composes. - **31** clearing a stale lock unlinked the `engine-startup.log` the client had just opened for the replacement engine — losing the boot diagnostics in the one case they exist for. The stale-lock path keeps it; a clean shutdown still removes it. Both it and `session.json`'s lifetime are now documented. - **32** `testMode` was documented as "pointed at the tester fleet" while `--fake` reports true pointed at `fake://login`. It means "not production", and the guide's verification step now says to read the server list. - **33** a shared command name merged only its usage lines, so `help local-settings` listed the write form without describing any of it. Params, returns, errors and REST routes merge too.
b03e1bb to
adaa403
Compare
Summary
edge-cli/ engine).develop.testModeconfig, etc.).Notes for reviewers
develop(config/keys, native HMAC, Node-safe splits, CLI). It is intentionally draft-style until dependencies / base strategy are finalized; there is nofuture!pseudo-merge in the history.publish:cli) is a placeholder until packaging/bin metadata is restored.edgeKey.json(build:cli:native); stub builds are refused.Test plan
npm run test:cli:node-safenpm run build:cli/npm run build:cli:native(withedgeKey.json)npm run test:cli:node-hmac(withedgeKey.json)npm run test:cli/npm run test:cli:edge-loginas applicablenpm test/tsc