Skip to content

Add CLI on top of config keys refactor - #6082

Draft
paullinator wants to merge 25 commits into
developfrom
paul/cli
Draft

paullinator wants to merge 25 commits into
developfrom
paul/cli

Conversation

@paullinator

@paullinator paullinator commented Jul 22, 2026 •

Copy link
Copy Markdown
Member

Summary

  • Adds a Node-safe engine-based Edge CLI with JSON REST API (edge-cli / engine).
  • Makes shared network/utils and exchange-rate core Node-loadable for the CLI.
  • Includes native Edge API HMAC signing (mobile + Node N-API) and config/keys split work already on this branch vs develop.
  • Follow-up fixups address review-code findings (edge-login races, infoServer interval stacking, stub signer fail-closed, testMode config, etc.).

Notes for reviewers

  • This PR currently stacks several related stacks vs develop (config/keys, native HMAC, Node-safe splits, CLI). It is intentionally draft-style until dependencies / base strategy are finalized; there is no future! pseudo-merge in the history.
  • CLI publish (publish:cli) is a placeholder until packaging/bin metadata is restored.
  • Production Node HMAC addon must be built from edgeKey.json (build:cli:native); stub builds are refused.

Test plan

  • npm run test:cli:node-safe
  • npm run build:cli / npm run build:cli:native (with edgeKey.json)
  • npm run test:cli:node-hmac (with edgeKey.json)
  • npm run test:cli / npm run test:cli:edge-login as applicable
  • npm test / tsc

@socket-security

socket-security Bot commented Jul 22, 2026 •

Copy link
Copy Markdown

@socket-security

socket-security Bot commented Jul 22, 2026 •

Copy link
Copy Markdown

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.

Priority Alert  (click "▶" to expand/collapse) Action
Low priority
High CVE: npm undici vulnerable to TLS certificate validation bypass via dropped connect options in BalancedPool

CVE: GHSA-w293-vg96-wgc3 undici vulnerable to TLS certificate validation bypass via dropped connect options in BalancedPool (HIGH)

Affected versions: >= 7.24.1 < 7.29.1; >= 8.0.0 < 8.10.2

Patched version: 8.10.2

From: package-lock.json → npm/node-gyp@13.0.1 → npm/undici@8.9.0

ℹ Read more on: This package | This alert | What is a CVE?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev.

Suggestion: Remove or replace dependencies that include known high severity CVEs. Consumers can use dependency overrides or npm audit fix --force to remove vulnerable dependencies.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/undici@8.9.0. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

Warn
Low priority
Low adoption: npm lib-cmdparse

Location: Package overview

From: package-lock.json → npm/lib-cmdparse@0.1.0

ℹ Read more on: This package | This alert | What are unpopular packages?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev.

Suggestion: Unpopular packages may have less maintenance and contain other problems.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/lib-cmdparse@0.1.0. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

Warn
Low priority
Low adoption: npm babel-plugin-transform-fake-error-class

Location: Package overview

From: package-lock.json → npm/babel-plugin-transform-fake-error-class@1.0.0

ℹ Read more on: This package | This alert | What are unpopular packages?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev.

Suggestion: Unpopular packages may have less maintenance and contain other problems.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/babel-plugin-transform-fake-error-class@1.0.0. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

Warn

View full report

@paullinator
paullinator force-pushed the paul/cli branch 11 times, most recently from 67406a5 to 35eeb43 Compare August 8, 2026 07:06
@paullinator
paullinator force-pushed the paul/cli branch 10 times, most recently from a8819b9 to 60adf1e Compare September 3, 2026 05:13
@paullinator
paullinator force-pushed the paul/cli branch 2 times, most recently from 7d3a2ba to c0c5f13 Compare September 5, 2026 00:08
@paullinator
paullinator force-pushed the paul/cli branch 3 times, most recently from 0574066 to 4bb295e Compare September 24, 2026 20:49
`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.
@paullinator
paullinator force-pushed the paul/cli branch 2 times, most recently from 70e9045 to b53d382 Compare September 30, 2026 17:24
`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.
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.
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant