fix(cli)(#23): detectWalletKind ignores empty {} wallet.json placeholder - #27
Merged
vrogojin merged 1 commit intoMay 23, 2026
Conversation
…older
The `sphere wallet use <name>` flow constructs a FileStorageProvider
whose `connect()` writes an empty `{}` JSON to wallet.json as a side
effect — BEFORE any wallet data exists. PR #25's `detectWalletKind`
classified that placeholder as `legacy` and tripped the migrate gate
on every fresh wallet, blocking `sphere init` for first-time users.
Caught by `manual-test-full-recovery.sh §1`: peer1-alice `sphere init`
exited 75 immediately with "Legacy wallet detected" instead of
proceeding to mint the nametag.
Fix: parse `wallet.json` and classify an empty top-level object as
`fresh`. A wallet.json with any key is still treated as `legacy`
(real wallet data → migrate triage). An unparseable / non-object
file is also conservatively routed through `legacy` so a corrupted
or unrecognized file isn't silently clobbered by a Profile boot.
Tests (5 new in `src/shared/sphere-providers.test.ts`):
• empty `{}` placeholder → fresh
• `{}` with whitespace → fresh
• single key (`{"mnemonic":"..."}`) → legacy (unchanged)
• unparseable garbage → legacy (conservative)
• array shape `[]` → legacy (unexpected shape)
All 119 unit tests pass. Typecheck clean. No new lint warnings.
Refs sphere-cli#23, PR #25.
3 tasks
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
PR #25's
detectWalletKind(sphere-cli#23) classified an empty{}wallet.json aslegacy— butsphere wallet use <name>writes that empty placeholder as a connect-time side-effect of the FileStorageProvider, BEFORE any wallet data exists. The migrate gate then fired on every fresh wallet, blockingsphere init.Repro (manual-test-full-recovery.sh §1)
Fix
Parse
wallet.jsonindetectWalletKind:{}(empty top-level object) → fresh — the no-content sentinel from a connect-time write{"mnemonic":"..."}, etc.) → legacy — real wallet state, route through migrateTests (5 new in
src/shared/sphere-providers.test.ts){}placeholder → fresh{}with whitespace → fresh{"mnemonic":"..."}) → legacy (unchanged)[]→ legacy (unexpected shape)All 119 unit tests pass.
npm run typecheckclean. No new lint warnings.Adversarial review
loadFromFile()reads first and only writes back what it read. So{}only appears if the file is genuinely fresh.{}to bypass migrate? No —{}means there's nothing to migrate. The migrate gate exists to preserve legacy wallet data, and{}carries none.{})? Theoretically possible via an admin clearing operation, but in practice the FileStorageProvider only writes afterset()calls. A legitimately emptied wallet has no token data to migrate, so treating it as fresh is correct.Refs sphere-cli#23, PR #25.