fix: repair wallet state on delete (closes #156) #194

clawbot · 2026-08-11T14:18:21+02:00

clawbot commented

2026-08-11 14:18:21 +02:00

Closes #156.

The three fixes

All in the delete path; the transition moved into
src/shared/walletDelete.js (removeWalletFromState) so it is testable
without a DOM. src/popup/views/deleteWallet.js calls it and keeps the
password gate, the view stack handling and the site-permission cleanup
unchanged.

hasWallet is now state.wallets.length > 0 after the splice.
Deleting the last wallet sets it false, so the next popup open hits
if (!state.hasWallet) in src/popup/index.js and renders Welcome
instead of Home's "No wallets yet".
Selection is only moved when it was deleted. Rule:
selectedWallet === walletIdx falls back to the first remaining
wallet's first address (0 / 0); selectedWallet > walletIdx is
decremented to follow the splice; otherwise it is untouched.
activeAddress moves only when it is one of the deleted wallet's
addresses (fallback: first remaining wallet's first address, or null
when no wallet remains).
AUTISTMASK_ACTIVE_CHANGED is broadcast whenever the active
address actually changed, in the same call shape as the address
switch in src/popup/views/home.js. It is sent after saveState(),
because the background reads the active address back out of storage
to build accountsChanged; the existing handler in
src/background/index.js then calls broadcastAccountsChanged(), so
a connected site sees the change (or [] when the last wallet is
gone) instead of a deleted address. No background change was needed.

Site-permission cleanup already mapped over wallet.addresses, so it
covers every address of an HD wallet; a test now pins that.

Tests

New tests/walletDelete.test.js, one case per behaviour: last wallet,
non-selected wallet before the selection (index shift), wallet after the
selection, active wallet (fallback + change flag), permission cleanup for
every address of a two-address HD wallet, and the broadcast call shape.

Demonstrated failing against the unfixed code

src/shared/walletDelete.js was temporarily replaced with the pre-fix
logic transcribed from the old inline block in deleteWallet.js
(unconditional selectedWallet = 0 / selectedAddress = 0, no
hasWallet update, no broadcast), and make test run:

FAIL tests/walletDelete.test.js
  * removeWalletFromState > deleting the last wallet clears hasWallet
      expect(state.hasWallet).toBe(false)
      Expected: false
      Received: true

  * removeWalletFromState > deleting a non-selected wallet leaves the selection intact
      expect(state.selectedWallet).toBe(1)
      Expected: 1
      Received: 0

  * removeWalletFromState > deleting a wallet after the selection does not shift it
      expect(state.selectedWallet).toBe(1)
      Expected: 1
      Received: 0

  * removeWalletFromState > deleting the active wallet falls back to the first remaining address
      expect(activeAddressChanged).toBe(true)
      Expected: true
      Received: false

  * broadcastActiveChanged > sends AUTISTMASK_ACTIVE_CHANGED to the background
      expect(jest.fn()).toHaveBeenCalledWith({"type": "AUTISTMASK_ACTIVE_CHANGED"})
      Number of calls: 0

Test Suites: 1 failed, 7 passed, 8 total
Tests:       5 failed, 144 passed, 149 total

The permission-cleanup case passes pre-fix, as expected — that part was
already correct and the test exists to keep it that way.

`make check`

Host, on the rebased branch:

Test Suites: 8 passed, 8 total
Tests:       149 passed, 149 total
Linting...
$ prettier --check .
All matched files use Prettier code style!
Checking formatting...
All matched files use Prettier code style!

And in the container, via script/cibuild (docker build ., exit 0) on
the same tree:

#11 [7/8] RUN make check
#11 2.107 Test Suites: 8 passed, 8 total
#11 2.107 Tests:       149 passed, 149 total
#11 2.390 Linting...
#11 6.457 All matched files use Prettier code style!
#11 11.32 All matched files use Prettier code style!
#11 DONE 11.7s

Not verified

Manually in a real browser, and not by make test-e2e — the end-to-end
suite has no wallet-deletion scenario. The wiring inside
deleteWallet.js (that the click handler calls the transition and the
broadcast) is covered by reading, not by a test: importing that view
pulls in the DOM helpers and the vault.

The README TODO checkbox for "Delete wallet (with confirmation)" is
ticked, since the feature exists.

Closes [#156](https://git.eeqj.de/sneak/AutistMask/issues/156). ## The three fixes All in the delete path; the transition moved into `src/shared/walletDelete.js` (`removeWalletFromState`) so it is testable without a DOM. `src/popup/views/deleteWallet.js` calls it and keeps the password gate, the view stack handling and the site-permission cleanup unchanged. 1. **`hasWallet`** is now `state.wallets.length > 0` after the splice. Deleting the last wallet sets it false, so the next popup open hits `if (!state.hasWallet)` in `src/popup/index.js` and renders Welcome instead of Home's "No wallets yet". 2. **Selection is only moved when it was deleted.** Rule: `selectedWallet === walletIdx` falls back to the first remaining wallet's first address (`0` / `0`); `selectedWallet > walletIdx` is decremented to follow the splice; otherwise it is untouched. `activeAddress` moves only when it is one of the deleted wallet's addresses (fallback: first remaining wallet's first address, or null when no wallet remains). 3. **`AUTISTMASK_ACTIVE_CHANGED` is broadcast** whenever the active address actually changed, in the same call shape as the address switch in `src/popup/views/home.js`. It is sent after `saveState()`, because the background reads the active address back out of storage to build `accountsChanged`; the existing handler in `src/background/index.js` then calls `broadcastAccountsChanged()`, so a connected site sees the change (or `[]` when the last wallet is gone) instead of a deleted address. No background change was needed. Site-permission cleanup already mapped over `wallet.addresses`, so it covers every address of an HD wallet; a test now pins that. ## Tests New `tests/walletDelete.test.js`, one case per behaviour: last wallet, non-selected wallet before the selection (index shift), wallet after the selection, active wallet (fallback + change flag), permission cleanup for every address of a two-address HD wallet, and the broadcast call shape. ### Demonstrated failing against the unfixed code `src/shared/walletDelete.js` was temporarily replaced with the pre-fix logic transcribed from the old inline block in `deleteWallet.js` (unconditional `selectedWallet = 0` / `selectedAddress = 0`, no `hasWallet` update, no broadcast), and `make test` run: ``` FAIL tests/walletDelete.test.js * removeWalletFromState > deleting the last wallet clears hasWallet expect(state.hasWallet).toBe(false) Expected: false Received: true * removeWalletFromState > deleting a non-selected wallet leaves the selection intact expect(state.selectedWallet).toBe(1) Expected: 1 Received: 0 * removeWalletFromState > deleting a wallet after the selection does not shift it expect(state.selectedWallet).toBe(1) Expected: 1 Received: 0 * removeWalletFromState > deleting the active wallet falls back to the first remaining address expect(activeAddressChanged).toBe(true) Expected: true Received: false * broadcastActiveChanged > sends AUTISTMASK_ACTIVE_CHANGED to the background expect(jest.fn()).toHaveBeenCalledWith({"type": "AUTISTMASK_ACTIVE_CHANGED"}) Number of calls: 0 Test Suites: 1 failed, 7 passed, 8 total Tests: 5 failed, 144 passed, 149 total ``` The permission-cleanup case passes pre-fix, as expected — that part was already correct and the test exists to keep it that way. ## `make check` Host, on the rebased branch: ``` Test Suites: 8 passed, 8 total Tests: 149 passed, 149 total Linting... $ prettier --check . All matched files use Prettier code style! Checking formatting... All matched files use Prettier code style! ``` And in the container, via `script/cibuild` (`docker build .`, exit 0) on the same tree: ``` #11 [7/8] RUN make check #11 2.107 Test Suites: 8 passed, 8 total #11 2.107 Tests: 149 passed, 149 total #11 2.390 Linting... #11 6.457 All matched files use Prettier code style! #11 11.32 All matched files use Prettier code style! #11 DONE 11.7s ``` ## Not verified Manually in a real browser, and not by `make test-e2e` — the end-to-end suite has no wallet-deletion scenario. The wiring inside `deleteWallet.js` (that the click handler calls the transition and the broadcast) is covered by reading, not by a test: importing that view pulls in the DOM helpers and the vault. The README TODO checkbox for "Delete wallet (with confirmation)" is ticked, since the feature exists.

clawbot added the needs-review label 2026-08-11 14:18:21 +02:00

clawbot added 1 commit 2026-08-11 14:18:21 +02:00

fix: repair wallet state on delete (closes #156 )

check / check (push) Successful in 30s

Details

bcaf0cb1f5

Deleting a wallet left three defects in the same path:

- `hasWallet` stayed true after the last wallet was deleted, so the next
  popup open rendered Home with "No wallets yet" instead of Welcome. It
  now tracks whether any wallet remains.
- The selection was reset to wallet 0 / address 0 unconditionally.
  `selectedWallet` now follows the splice (decremented when a wallet
  before it is removed), and only falls back to the first remaining
  wallet's first address when the selection itself was deleted;
  `activeAddress` only moves when it belonged to the deleted wallet.
- No `AUTISTMASK_ACTIVE_CHANGED` was sent, so connected sites kept
  reporting a deleted address. The delete path now broadcasts it after
  the state is persisted, whenever the active address actually changed,
  and the background re-emits `accountsChanged` from there.

The transition moved into `src/shared/walletDelete.js` so it can be
tested without a DOM. Site-permission cleanup is unchanged and still
covers every address of an HD wallet.

clawbot self-assigned this 2026-08-11 14:18:27 +02:00

clawbot commented

2026-08-11 14:22:36 +02:00

Independent review: PASS — index arithmetic, broadcast wiring, test teeth (5/6 reproduced failing against pre-fix logic in my own clone), policy and containerized make check all verified.

Disclosure: two DoD items of #156 — "reopen lands on Welcome" and "a connected dApp observes the account change" — were verified by tracing the code path only (src/popup/index.js:265; AUTISTMASK_ACTIVE_CHANGED -> src/background/index.js:804 -> broadcastAccountsChanged()), not in a real browser, as the author disclosed.

Independent review: PASS — index arithmetic, broadcast wiring, test teeth (5/6 reproduced failing against pre-fix logic in my own clone), policy and containerized `make check` all verified. Disclosure: two DoD items of [#156](https://git.eeqj.de/sneak/AutistMask/issues/156) — "reopen lands on Welcome" and "a connected dApp observes the account change" — were verified by tracing the code path only (`src/popup/index.js:265`; `AUTISTMASK_ACTIVE_CHANGED` -> `src/background/index.js:804` -> `broadcastAccountsChanged()`), not in a real browser, as the author disclosed.

clawbot merged commit b882cede9f into next

2026-08-11 14:23:07 +02:00

clawbot deleted branch fix/issue-156-delete-last-wallet

2026-08-11 14:23:07 +02:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/AutistMask#194