refactor(cli): remove dead code paths

[h1-hunt]

Summary

• remove the production-dead full-snapshot config writer and its obsolete merge semantics
• keep config-lock/security coverage on the supported updateConfig() path
• remove unreachable amount validation and unrelated keypress listener cleanup
• centralize wallet adoption writes without changing create, restore, or resolution behavior

Validation

• focused config/wallet/payment tests — 79/79 passed
• npm test — CLI 212/212 and core 14/14 passed
• npm run typecheck
• npm run lint
• npm run build
• npm run verify:pack
• npm run smoke:pack
• git diff --cached --check

Closes #86

[h1-hunt]

GPT Reviewer

Verdict: REJECT

Reviewed head 667928300fbfa11c2784d2340a1287b3e379eb86 against review snapshot base e426cef77d9fa2b1a5e846c15def15cbd4bbba25 using a bounded, tool-free review bundle.

Wallet creation no longer preserves the required in-memory config mutation, although all supplied deterministic checks pass.

Findings

1. MINOR — Wallet creation leaves the loaded config snapshot stale (packages/cli/src/commands.ts:247)
   • Evidence: Previously, a successful create assigned config.wallets[name] before invoking updateConfig. The refactored branch now calls adoptWallet only on the updater's fresh current draft, so the config loaded at command entry is not mutated. This violates the stated behavior-preserving boundary and changes mutation ordering when the durable update fails. The added create test checks only the updater result, so it misses this regression.
   • Suggested fix: Call adoptWallet(config, name, wallet.address) immediately after successful OWS creation and before updateConfig, while retaining the helper call inside the updater. Extend the test to assert that the exact object returned by loadConfig is also mutated.

Shared exact-snapshot validation

• diff-check: PASS (0.0s)
• npm-ci: PASS (1.969s)
• typecheck: PASS (2.967s)
• lint: PASS (2.265s)
• test: PASS (17.758s)
• build: PASS (2.248s)
• No paid h402/provider calls were made.

[h1-hunt]

Claude Reviewer

Verdict: REJECT

Reviewed head 667928300fbfa11c2784d2340a1287b3e379eb86 against review snapshot base e426cef77d9fa2b1a5e846c15def15cbd4bbba25 using a bounded, tool-free review bundle.

Three of the four mandated cleanup boundaries are met exactly: saveConfig/mergeConfigForSave are removed with no remaining references and every lock/permission/security test was converted to updateConfig with equivalent assertions (concurrency coverage retained); the parseUsdMicros negative branch is provably unreachable behind the sign-free regex with error text unchanged; and the promptHidden keypress cleanup removal cannot leak listeners since only a data listener is registered. However, the wallet-create branch violates the fourth boundary: it deletes the in-memory config.wallets[name] mutation and keeps only the durable updateConfig write, while issue #86 explicitly requires preserving both for the create path (as the by-name, by-address, and restore paths still do). The deviation is behaviorally inert today because create has no subsequent reader of config, but it silently exceeds the authorized behavior-preserving cleanup and drops the memory/disk coherence pattern, so it must be restored or explicitly re-scoped with the owner. Rejecting per the zero-findings bar.

Findings

1. MINOR — wallet create drops the in-memory config.wallets[name] adoption required by the CLI cleanups: dead config writer, unreachable branch, dead listener cleanup, adoption double-write DRY #86 boundary (packages/cli/src/commands.ts:247)
   • Evidence: Base code in the create branch performed both writes: config.wallets[name] = { address: wallet.address }; followed by await updateConfig((current) => { current.wallets[name] = { address: wallet.address }; });. The head deletes the in-memory line and keeps only the durable write (await updateConfig((current) => { adoptWallet(current, name, wallet.address); });). The mandatory invariant from issue CLI cleanups: dead config writer, unreachable branch, dead listener cleanup, adoption double-write DRY #86 states the centralized helper "must preserve both in-memory config.wallets[name] mutation and the same durable updateConfig write for create, by-name/by-address adoption, and restore paths" — and the other three paths do keep both (e.g. const resolved = adoptWallet(config, ...) before their updateConfig calls), leaving create asymmetric. No observable behavior changes today because the create branch prints and returns without any subsequent reader of config, and the new "persists and prints a newly created wallet" test asserts only the durable write, so this silent scope-exceed is not caught by tests.
   • Suggested fix: Restore the in-memory adoption in the create branch via the helper so create matches the other adoption paths and the CLI cleanups: dead config writer, unreachable branch, dead listener cleanup, adoption double-write DRY #86 boundary: adoptWallet(config, name, wallet.address); immediately before await updateConfig((current) => { adoptWallet(current, name, wallet.address); });. Alternatively, if the dead-store removal is intended, get the issue CLI cleanups: dead config writer, unreachable branch, dead listener cleanup, adoption double-write DRY #86 owner to explicitly amend the cleanup boundary and note the deviation in the PR description instead of claiming create behavior is unchanged.

Shared exact-snapshot validation

• diff-check: PASS (0.0s)
• npm-ci: PASS (1.969s)
• typecheck: PASS (2.967s)
• lint: PASS (2.265s)
• test: PASS (17.758s)
• build: PASS (2.248s)
• No paid h402/provider calls were made.

[clawd800]

Rework: the shared finding addressed (head 2ef5fbf)

Both reviewers flagged the same boundary violation and they are right: the create branch kept only the durable updateConfig write, dropping the in-memory config.wallets[name] adoption that issue #86 explicitly requires for all four paths.

Fixed by restoring the symmetric shape: adoptWallet(config, name, wallet.address); now runs immediately after successful OWS creation and before updateConfig — the same memory-then-durable ordering as the base code and as the by-name/by-address/restore paths, so mutation ordering under a failing durable write is also back to base behavior.

Test extended as requested: the "persists and prints a newly created wallet" test now pins the exact object returned by loadConfig via mockResolvedValueOnce and asserts loaded.wallets was mutated, in addition to the existing updater-result assertion — so a future dead-store removal cannot silently exceed the boundary again.

Gates: typecheck 0 · lint 0 · diff-check clean · 212+14 tests green · build green.