Add CLI on top of config keys refactor - #6082
Draft
paullinator wants to merge 24 commits into
Draft
paullinator wants to merge 24 commits into
paullinator wants to merge 24 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.
|
paullinator
force-pushed
the
paul/cli
branch
11 times, most recently
from
August 8, 2026 07:06
67406a5 to
35eeb43
Compare
paullinator
force-pushed
the
paul/cli
branch
10 times, most recently
from
September 3, 2026 05:13
a8819b9 to
60adf1e
Compare
paullinator
force-pushed
the
paul/cli
branch
2 times, most recently
from
September 5, 2026 00:08
7d3a2ba to
c0c5f13
Compare
paullinator
force-pushed
the
paul/cli
branch
3 times, most recently
from
September 24, 2026 20:49
0574066 to
4bb295e
Compare
paullinator
force-pushed
the
paul/cli
branch
from
September 30, 2026 06:40
4bb295e to
70e9045
Compare
paullinator
force-pushed
the
paul/cli
branch
4 times, most recently
from
October 1, 2026 18:00
e3c597d to
784c6ed
Compare
`typechain` emits `export * as factories from './factories'`, and the React Native preset does not transform namespace re-exports. The plugin was declared 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.
`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.
Exchange rates could not be fetched outside the app: the module read its server list and its error reporter at load time, both of which come from React Native. The query logic now takes what it needs through `configureExchangeRates`, and `exchangeRatesGui` supplies the GUI's Airship reporter at app start. A Node caller supplies its own, so the same rate lookup answers the same way in both places.
`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. 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.
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 — a hundred and four `route({…})` declarations, each
carrying its own cleaners, error list and prose.
The five documentation gates hold across every one of them: the surface
matches, each response field carries prose, each call either matches its
`edge-core-js` signature or records why it differs, and all but four run
offline against the fake world. `npm run docs:api:gates` reports the
counts; they are not repeated here, because a number in a commit message
goes stale the first time a field is added.
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. No suite covers them:
`test:cli:network` runs the one-shot, CAPTCHA and edge-login suites, and
none of the three names any of the four. The coverage gate records that
as the reason it excuses them.
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.
The CLI is built from this repository but is not this repository: rollup inlines every module it reaches under `src/`, so a published package is the two bundles, the native HMAC addon, the CLI document as its README and `LICENSE`. Nothing about it needs the CLI to move to a workspace or a submodule, and the app's own `package.json` stays `private: true` — what is published is a separate manifest assembled in a temporary directory, so the app itself cannot reach npm by accident. `src/cli/npmMeta.ts` holds the decisions: the scoped name, the bin name, the licence, and the per-platform native packages, which become `optionalDependencies` once they exist. The version is not among them. The CLI ships in lockstep with the app, so the manifest takes it from the app's `package.json`: there is no second number to bump, and a published CLI says which app release it corresponds to. Lockstep costs one thing worth knowing before a release — npm will not replace an existing version, so a CLI-only fix goes out on the next app version bump rather than on its own. The dependency list is derived, not written down. `rollup.config.cli.mjs` externalises every key of the app's `dependencies` — its whole runtime set — so the bundles leave all of them as bare `require`s while needing fifteen, and anything the app does not declare is inlined instead. `buildCliManifest.ts` walks the module graph from both entry points, keeps the bare specifiers the app declares as dependencies, drops builtins and type-only imports, and treats the rest as bundled. Checked against the bundles' own `require` calls: fifteen declared, fifteen required, none missing and none spare. Hand-maintaining that list fails as an `npm install` that succeeds and a CLI that cannot resolve a module on first run, so `cli:manifest:check` gates it in `verify` and in CI — where, unlike the five documentation gates, `npm run prepare` does not regenerate it first. `publishCli.ts` builds, stages and publishes. With `edgeKey.json` it runs `build:cli:all`, so a build server needs that one file to produce a CLI with full native signing; without it the addon cannot be built, so publishing takes an explicit `--allow-unsigned` and the staged README says the build cannot sign. A dirty tree is refused, since the registry copy could not then be re-derived from any commit. `--dry-run` packs without publishing and `--out` stages for inspection. Verified end to end: the staged tarball is 924 kB over six files, and installed into an empty project the client answers `--help` and the engine boots and serves `engine-status`. Two things the exercise surfaced, neither fixed here. `uuid` is imported by `src/util/utils.ts` and declared nowhere, resolved only because other packages happen to depend on it; rollup inlines it, so the published CLI is unaffected, but the build rests on a transitive resolution. And installing the package costs 2.3 GB across 69,756 files, of which about 1.3 GB is React Native mobile binaries — iOS simulator slices and Android libraries for the privacy coins — reached through `edge-currency-accountbased`, which also brings `react-native` itself. A Node CLI can load none of it.
3.review-errors.2, with 3.engine-routes.1 and 3.engine-infra.2. Five request-position amount fields were declared `asString` and handed to biggystring, which throws a plain `Error` that `toErrorBody` has no arm for — so `spend --native-amount=abc` and `get-transactions --spam-threshold=abc` answered 500 on routes declaring 400. A fractional value was worse than an error: the UTXO plugin sums fees with biggystring but builds the output with `parseInt`, so "1.5" funded the fee math at 1.5 and paid out 1, under a field documented as the chain's smallest unit. `asIntegerString` already existed for exactly this and is now exported and used by `asSpendTarget`, the spend shorthand's `nativeAmount` and `amount`, `swap-quote`, `encode-uri` and `spamThreshold`, where zero stays legal. Response-position amounts keep `asString`: a transaction's `nativeAmount` is negative for a send, and core's own values are already valid. `asEdgeTxAction` dispatched through a plain object literal, so `actionType: "toString"` resolved `Object.prototype.toString` — not null, so the unknown-actionType guard was skipped and a string was returned as an `EdgeTxAction`; `"constructor"` handed back the unvalidated body. Both reached `wallet.saveTxAction`, where core dispatches `CURRENCY_WALLET_FILE_CHANGED` before its own uncleaner rejects. Guarded with the shared `hasOwn`, as `util/exchangeDenom.ts` already does. Seven offline checks cover the new rejections, including both prototype names.
3.review-state.6, and 3.review-state.7 with 3.harness-review.8.
`--save-export-prefs` was honoured on one branch of three: the
`mergeExportTxInfo` call sat inside `if (formats.includes('bitwave'))` and
again inside the explicit-account-id test, so
`--export-format=csv,qbo --save-export-prefs` answered `ok` and wrote
nothing. The caller asked for their format choice to be remembered and the
GUI export scene's switches were untouched. The write now runs once for
every format combination, still only when the flag is given — writing
unasked turned a one-off `--bitwave-account-id` into the user's saved id.
Passing an absent id is safe because `mergeExportTxInfo` reads each field
as `patch.x ?? prev?.x`, which is what keeps a saved bitwave id through a
csv-only save.
`limit`'s published description said "Defaults to 100" while
`DEFAULT_TX_LIMIT` is 99 — deliberately, so a page prices in one rates
request. The wrong number ships in `openapi.json`, the HTML reference and
`edge-cli help`, so a caller paging on the documented default walks
`offset` 0, 100, 200 and skips one transaction per page, which `total`
cannot reveal because it is the match count. The description cannot
interpolate the constant: `extractRoutes` reads it through the checker as a
string literal, and a template literal drops the description from the
reference altogether. A test holds the two in step instead.
3.engine-routes.3, 3.engine-routes.2, 3.harness-review.4 and 3.review-state.5 — four routes whose declared reach and real reach differed. The key-export calls resolved through `findWallet`, which searches `account.currencyWallets`: core builds that only from `activeWalletIds` and only for wallets whose api exists. So `all-keys` listed an archived wallet and `get-raw-private-key` answered `WALLET_NOT_FOUND` for the exact id it had just printed — on the disaster-recovery path a CLI key export is for. Core's `getRawPrivateKey`, `getDisplayPrivateKey`, `getRawPublicKey` and `listSplittableWalletTypes` all work off `allKeys`, so `findWalletId` resolves there, with the same prefix contract and the same errors. `change-wallet-states` validated nothing. Core treats an id it has never seen as new and writes a state file for it without complaint, so a typo or a prefix answered 204 while nothing changed, and left a bogus `Keys/<hash>.json` in the account repo to sync to every device. It was also the one wallet route that ignored the prefix contract `walletId` is documented with. Every key now resolves over `allKeys` first. `save-tx` was the only handle-advancing route setting neither `consuming` nor `hold`, so `OBJECT_IN_USE`, the sweeper's skip and a bulk release's bounded wait were all blind to it. A slow save overlapping a `broadcast-tx` for the same handle removed the record under the in-flight broadcast, which then answered `OBJECT_NOT_FOUND` after the funds had left. It runs under `consume` now, which also does the delete the handler did by hand. `object-get` and `object-delete` published a reach they lost: only `transaction` and `swap` are created with a `sessionId`, so `pendingLogin` and `lobby` can only answer `OBJECT_SESSION_MISMATCH` there. Both descriptions say so, and the two dead arms of `projectHandleValue` are explicit about being unreachable and point at `pendingSummary`, which is where a pending login is really projected — the reasoning worth keeping, since serialising one walks into `otpKey` and `recoveryKey` getters.
3.review-servers.4, and 3.review-state.3 with 3.harness-review.2.
`handleRequest` ran `idle.touch()` and `idle.beginRequest()` before
`checkTcpRequest`, so every rejected request pushed `idleShutdownAt` out by
a full `--idle-timeout`. Any other local process — the threat
`transportAuth.ts` names — could poll the port once a minute with no token
and keep the daemon resident indefinitely, holding its EdgeContext and
every plugin's polling open, which is the leak `idleShutdown.ts` exists to
bound. The comment three lines below already claimed this ordering ("a
caller that cannot authenticate learns nothing about this engine"); now it
is true. A rejection is also logged at `warn`, because it is the only sign
of a probe and nothing recorded it.
The auto-logout ticker skipped any session whose window was `0`, which made
that value a one-way latch: a session created while the setting said `0`
captured it and the ticker never looked again, so a user turning
auto-logout back on from their phone had no effect on a session the engine
was already holding, while `engine-sessions` kept reporting
`autoLogoutSeconds: 0` as though it were still their choice. The cost
argument behind the skip is sound — `isExpired` answers false for `0`
before it looks at a clock, and the read is a decrypt plus a parse with no
cache — so those sessions now re-read once a minute against the ticker's
fifteen seconds. A quarter of the reads, and a bound on the staleness of a
security control.
The test that asserted the skip now asserts the cadence, and a new one
turns auto-logout back on mid-session and expects the logout.
3.engine-infra.3, 3.harness-review.6 and the closing paragraph of 3.review-async.2 — one defect found three times. `cleanupStaleLock` required a listening socket as well as a live pid, and its own comment asserted that `sweepStaleProfiles` "skips the directory for the same reason". The sweep tested the pid alone. `process.kill(pid, 0)` succeeds for ever once the OS hands that pid to something else, so the sweep permanently skipped the directories it exists to clear: a SIGKILLed engine's `session.json` — a full-account bearer token, which `removeRunArtifacts` is specifically there to delete — survived in a profile nothing would revisit, because `testCliFake` derives its data directory from the pid and every run hashes fresh. That is the 428 directories with 235 session files the sweep was written for. Both callers now share one `isClaimLive`, so they cannot drift again: a claim is live when something answers on its socket, or when it is young enough to still be booting. `sweepStaleProfiles` becomes async for the probe, which is local to its one caller in the async startup. A new case covers the recycled pid directly — live pid, nothing listening, backdated past the boot grace — and asserts the session file goes.
3.review-performance.4 with 3.review-servers.6, and 3.review-code-quality.6. `apiClient` waited 15 s for a shutting-down engine to release its socket, under a comment claiming that was "bounded by the drain the engine itself allows, plus a margin" — the drain alone is 110 s, so the margin was negative by a factor of seven. A stop or a Ctrl-C landing while any request slower than 15 s was in flight, which the guide says `get-transactions` on a whole wallet is, left the socket bound and answering 503; the next command gave up, spawned a replacement, and that child failed `claimRunFile`'s `wx` against the dying engine's run file and printed "An engine is already running … Stop it first" — advice to do what the user had just done, and the exact sequence `isShuttingDown` exists to prevent. `shutdownTiming.ts` now holds the phases that stand between `shuttingDown = true` and the listeners closing — the request drain, a bulk handle release, a logout's wait — and the client's wait is their sum rather than a fourth number that contradicted them. The module has no logic and no imports, so the client reads the engine's figures without pulling the engine into its bundle, which a check on the built client confirms. `ApiClientOptions.host` and `.port` are gone. They were unreachable — every construction passes `socketPath` — and could not have worked: the engine's TCP listener requires `X-Edge-Token` and the client never sent one, so a caller reaching for them would have got a 401 with no clue why. The two transports were also chosen two different ways, so `openStream` would have kept using the socket while `request` switched. `--tcp` is forwarded to the engine for other local scripts, which is what the architecture listing now says. `testCliSubscribe` imports `TCP_TOKEN_HEADER` instead of spelling it five times — and spelling it by hand is what let a bad replacement of mine through until two checks that need a valid token caught it.
3.review-react.1 with 3.harness-review.5. `escapeOFXString`'s non-ASCII pattern had no `u` flag, so it matched one UTF-16 code unit at a time and `codePointAt(0)` saw half a surrogate pair: `Tip 🍕` came out as `Tip ��`. A lone surrogate is not a character in SGML, XML or OFX, so no importer can turn that back into the original — which is exactly what the docblock promises, for exactly the inputs the sentence above it lists: a payee or memo set by an `edgeProvider` dapp, by the CLI's `--metadata`, or typed into the GUI's notes field. It is also a regression against base for this input, which emitted the raw UTF-8 bytes. Every character in the existing charset case is BMP, which is why it passed. A second case covers an emoji and asserts the real code points rather than surrogate halves; it fails without the flag.
3.review-performance.2.
Rollup keeps an external module it cannot prove side-effect free, even
after tree-shaking every binding away, and `external` is not declared
side-effect free — so the built client opened with six bare `require`s it
never used: `edge-core-js`, `biggystring`,
`csv-stringify/lib/browser/sync`, `sha.js`, `sprintf-js` and `date-fns`.
`src/cli/index.ts` states the opposite as its contract ("Nothing here
imports `edge-core-js`. The engine owns core; this half owns argv, the
socket and the output"), and `docs/EDGE_CLI.md` presents one process per
command as the normal mode, so a script of twenty commands paid it twenty
times.
Measured on this machine, five runs each: `node lib/edgeCli.js help` was
162 ms and is now 60 ms. All six requires are gone from the client, the
engine still requires every plugin package it genuinely loads, and both
offline suites pass against the rebuilt bundles.
`moduleSideEffects` is false for externals only. The bundle's own modules
keep theirs, because `import './bootNodeLocale'` and
`import './commands/all'` are side-effect imports and dropping them would
unregister every command.
The manifest generator now cross-checks itself against the built bundles
when they exist. It reports `date-fns` as declared but unrequired — 25 MB,
reached through `src/locales/intl.ts`, whose `format` binding no CLI path
calls, so rollup shakes it out. It stays declared on purpose: the graph
over-approximates in the safe direction, and the moment a CLI path does
call it, a missing declaration is an `npm install` that succeeds and a CLI
that dies on first run. The reverse direction — a bundle requiring
something undeclared — now fails the gate.
3.review-async.4. `addToQueue` latches `inQuery` before arming the debounce, and the only place that cleared it was `doQuery`'s terminal branch. The `.catch` around the call reported through `onQueryError` and reset nothing, so a rejection from anywhere outside `doQuery`'s per-group `try` — building the groups, or stringifying the params — left the flag latched for the life of the process. Every later arrival then took the `!inQuery` false path, armed no timer, and never settled, because `getHistoricalRate` never calls its own `reject`. In the engine that is a `get-transactions` hanging to the client's deadline, for ever, on a daemon documented as long-lived; in the GUI it is every `useHistoricalRate` row left unresolved. The sink now unlatches, settles the queue with `0` — what a rate the server cannot price already answers — and then reports, so one bad pass costs one pass. `stopRateQueue` also cleared `inQuery` without accounting for a `doQuery` already awaiting a response, so a key queued immediately after it armed a second chain running alongside the first. A pass now carries the epoch it started with and abandons its recursion if the queue was stopped. The settle loop is shared with the failure sink rather than written twice. No test constructs a live rejection, because none is reachable through the public API — which is why the finding is medium. The epoch is reachable: a new case stops the queue under a slow pass and asserts the caller settles and the module still works afterwards.
3.review-performance.3. A key the server answers for but cannot price is deliberately not cached: storing `0` would answer `0` for the life of the process, long after the rates server recovered. But nothing absorbed the repeat either, and the engine does not persist `metadata.exchangeAmount`, so an asset with no feed re-paid the entire fiat fill on every listing — measured against a stub answering 200 with `rate` absent, a 1,200-transaction wallet cost 13 upstream requests and about a second on the first, second and third listing alike, with the cache staying empty. Reachable on a hand-added custom token, a long-tail token the server has no feed for, and dates predating an asset's market, none of them rare on an old wallet. Those keys now go in a short-lived negative cache instead, bounded and cleared the same way as the rate cache. Five minutes is far longer than a listing loop and far shorter than an outage, so a repeated listing is free and a recovered server is still picked up. `engine-status` publishes `rateUnpricedCount` beside `rateCachedCount`, so an asset with no feed is visible rather than silent. The test that asserted a zero is never cached now asserts what actually matters: the zero never enters the rate cache, the repeat costs no request, the TTL is minutes rather than hours, and clearing the cache forgets it.
3.review-servers.5 and 3.review-state.9. The in-band error path never touched the logger. Anything `mapCoreError` does not recognise becomes `500 INTERNAL_ERROR` carrying only `error.message`, and `handleRequest`'s catch sent that and returned — no stack, no route, no method. `server.ts` logged only what escaped `handleRequest` itself, and `route.ts` logs response-shape drift, so the daemon recorded a documentation bug but not an actual exception. For a detached process whose one diagnostic surface is `~/.edge-cli/logs/engine-<profile>.log`, a plugin or core fault was unreproducible afterwards: the operator had the line the client printed, and the client is often a script that discarded it. A 5xx now logs the route, the code and the stack; a 4xx logs a line without one, since it is the caller's doing. The response body is unchanged. `--tcp-host=[::1]` was accepted and then answered nothing. `allowedHostnamesFor` seeded its set from the raw string, so the set held `[::1]`, while `hostnameOf` strips the brackets `URL` keeps — a caller's `Host: [::1]:9008` arrived as `::1`, missed the set, and every request was refused 403 with a message about DNS rebinding. `server.listen(port, '[::1]')` would not have bound it as an address either. The unbracketed form worked, which is what made the bracketed one a trap rather than an obvious failure. The validator moves to `tcpPort.ts`, beside the port one that exists for the same "one spelling, both entries" reason, and canonicalises once. That also makes it testable: `index.ts` runs `main()` at import, so the old private function could not be reached from a test. The new suite pins the pair that disagreed — whatever is bound must be in the allowed set under the name a real `Host` header reduces to, which for IPv6 is always bracketed.
3.review-tests.2 and 3.review-tests.3.
Seven export checks never opened a file. `ok()` asserts only `status === 0`
and the absence of an error body, and the client returns early when
`result.files == null`, so an engine that stopped assembling `files`, or a
`writeExportFiles` whose write were deleted, would have kept all seven
green. Four checks now read what was written: that `csv,qbo` produces
exactly `tx.csv` and `tx.qbo` and no extensionless `tx` — the only coverage
`exportFilePath`'s multi-format stem logic can get — that the QBO carries
`OFXHEADER:100` and is not trivially short, and that an empty wallet
exports an empty CSV.
That last one is asserted rather than fixed, deliberately. The CSV exporter
ends in `csvStringify(items, { header: true })`, and csv-stringify takes
its header from the first record's keys — so no records means no header and
a zero-byte file. It is the shared exporter the GUI uses, so emitting a
header row for an empty range would change the app's output and needs
explicit columns to do at all. Worth a decision, not a drive-by change.
The `details` half of the error-contract table could not fail.
`toHaveProperty(key)` passes for a key whose value is `undefined`, and
every arm builds `details` as a fixed object literal, so the key is always
there — the table held for a projection gutted to
`{ challengeId: undefined, challengeUri: undefined }`, which is the one
thing it existed to catch. It now carries the values core supplied and
asserts them with `toMatchObject`. `OTP_REQUIRED` listed three of its seven
fields and omitted `voucherAuth`, the second credential whose redaction
`redaction.test.ts` exists to pin, so the two halves of that guard never
met; the error is now constructed with all seven populated. Two swap-limit
cases listed one field each and in fact publish three.
`redaction.test.ts` reads its field list back out of `toErrorBody` instead
of copying it, so renaming a field in the arm fails one test or the other
rather than leaving both asserting a key nothing emits.
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
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