fix: an open popup moves to the recovery screen when its profile becomes unreadable (closes #373)

A popup already open when the stored profile became unreadable stayed on the
last good profile until reopened. Every save already runs the check loadState()
runs at open; a save refused by it now raises the recovery screen, stops the
ten-second refresh, and passes the screen to showView(), which runs the leave
cleanup of the screen it replaces and records it as the current view. From
then on showView() shows nothing else, so a transaction wait or a later save
cannot take the user off it or clear an export or a typed confirmation. Any
other failed save keeps the "NOT SAVED" banner. The popup test harness now
honours clearInterval().

Model: opus-5-5

README.md

@@ -1114,16 +1114,18 @@ because bumping for one would send every older install to StateRecovery for
 nothing.
 
 Every read of the record goes through `assertStateUsable()` first, on the raw
-bytes, before normalization: `loadState()` for the popup and `getState()` for
-the background. It refuses a record that is not an object, a `schemaVersion`
-this build does not understand (a newer one included), a `wallets` that is not a
-list of wallet records with address records in them, and a `networkId` that is
-not a network in `src/shared/networks.js`. Refusing is the whole point — a
-record the wallet cannot vouch for is never normalized, never written back, and
-never half-loaded. The popup shows StateRecovery; a dApp gets a specific error
-(`-32007`, an EIP-1474 server-error code the spec leaves unassigned) saying the
-saved data cannot be read and that nothing was signed or sent, rather than the
-generic `-32603` every request used to answer.
+bytes, before normalization: `loadState()` and every `saveState()` for the
+popup, and `getState()` for the background. It refuses a record that is not an
+object, a `schemaVersion` this build does not understand (a newer one included),
+a `wallets` that is not a list of wallet records with address records in them,
+and a `networkId` that is not a network in `src/shared/networks.js`. Refusing is
+the whole point — a record the wallet cannot vouch for is never normalized,
+never written back, and never half-loaded. The popup shows StateRecovery,
+whether it finds the record unreadable when it opens or at a save while it is
+open; a dApp gets a specific error (`-32007`, an EIP-1474 server-error code the
+spec leaves unassigned) saying the saved data cannot be read and that nothing
+was signed or sent, rather than the generic `-32603` every request used to
+answer.
 
 Every other field of the record is floored in `normalizePersisted()` rather than
 gated, and the floor is not the same for every field. Some are type-checked as a
@@ -1167,8 +1169,9 @@ now also reported rather than swallowed: `onSaveFailure()` in
 `src/shared/state.js` is called for every failed save, awaited or not, and the
 popup puts up a persistent "NOT SAVED" banner (`showSaveFailureBanner()` in
 `src/popup/views/helpers.js`). Storage can still fail for reasons no floor
-covers — a quota, a revoked permission, a record a newer build wrote — and the
-wallet must never look healthy while that is true.
+covers — a quota, a revoked permission — and the wallet must never look healthy
+while that is true. A save that fails because the stored record fails the gate,
+such as one a newer build wrote, gets StateRecovery instead of the banner.
 
 The `networkId` check is not cosmetic: that value is an object KEY into
 `state.networkEndpoints`, so an unvalidated `"__proto__"` would set the map's
@@ -1978,9 +1981,20 @@ view would leave a wallet one click from deletion.
 
 #### StateRecovery (`state-recovery`)
 
-- **When**: `loadState()` refused the stored profile, so the popup has no
-  profile at all. It is the only screen reached without one, and the only one
-  that never appears during ordinary use.
+- **When**: the stored profile fails `assertStateUsable()`. At open, that is
+  `loadState()` refusing it, so the popup has no profile at all. While the popup
+  is open, on any screen, it is a save refusing it: every `saveState()` reads
+  the stored record and runs the same check before writing, so the popup finds
+  it at the next navigation, or at the next ten-second balance refresh that
+  reaches the network ([#373](https://git.eeqj.de/sneak/AutistMask/issues/373)).
+  A save that fails for any other reason, such as a storage read or write that
+  errors, gets the "NOT SAVED" banner instead and leaves the screen as it is.
+  The screen it replaces is left as any navigation leaves it, so a revealed
+  phrase or key, or a typed password, is wiped. Once up, the screen stays until
+  the popup closes or the record is erased: work still running in the popup,
+  such as a transaction wait, cannot replace it, and nothing in AutistMask
+  writes over a record that fails the check, so it cannot become readable again
+  underneath. It is the only screen that never appears during ordinary use.
 - **Why it exists**: a record the wallet cannot read used to render nothing — no
   view, no message, no control — while every dApp call answered a generic
   internal error, and no reset or wipe control existed anywhere in the product.
@@ -2007,16 +2021,20 @@ view would leave a wallet one click from deletion.
       Nothing was erased." on the error line
 - **No other control is reachable.** The Settings gear is hidden while this
   screen is up, because every screen behind it renders from the profile that
-  could not be read, and `showView()` is not used to raise it for the same
-  reason — it reads and writes the state singleton.
+  could not be read. At open `showView()` is not used to raise it for the same
+  reason — it reads and writes the state singleton. Under an open popup it is
+  also passed to `showView()`, which runs the replaced screen's cleanup and
+  records it as the current view, and `showView()` shows nothing else after
+  that.
 - **Both controls are required.** An export with no reset leaves the user
   looking at a broken profile with no way to use the wallet again; a reset with
   no export destroys the only copy of a record that may hold recoverable key
   material. The typed phrase is the same barrier DeleteWalletLostPassword uses,
   and for the same reason: there is no password to gate this with, since there
   is no profile to check one against.
-- Not in `RESTORABLE_VIEWS`: it is never persisted as the current view, because
-  nothing on this path writes state at all.
+- Not in `RESTORABLE_VIEWS`, and not persisted as the current view: at open
+  nothing on this path writes state at all, and under an open popup the check
+  that raised the screen refuses the save that would record it.
 
 ### External Services
 

TODO.md

@@ -45,6 +45,18 @@ but the review is broader than any of them.
 
 # Completed Steps
 
+- 2026-10-04: A popup that is already open when the stored profile becomes
+  unreadable moves to the recovery screen
+  ([#373](https://git.eeqj.de/sneak/AutistMask/issues/373)). It used to stay on
+  the last good profile, with the "NOT SAVED" banner at most, until reopened.
+  Every save already ran the check the popup runs at open; a save that fails
+  that check now raises the recovery screen and stops the ten-second refresh, so
+  the popup finds the record at the next navigation or refresh. The screen it
+  replaces is left as any navigation leaves it, so a revealed phrase or key or a
+  typed password is wiped. Once up, nothing else can replace it, and a later
+  save or a transaction wait that ends does not clear an export or a typed
+  confirmation. Any other failed save still gets the banner and leaves the
+  screen alone.
 - 2026-10-04: The signature screen shows a personal message as the bytes that
   are signed ([#403](https://git.eeqj.de/sneak/AutistMask/issues/403)). It
   showed only the decoded text, with bidirectional and zero-width characters

src/popup/index.js

@@ -50,6 +50,10 @@ function renderWalletList() {
 
 let refreshInFlight = false;
 
+// The ten-second refresh init() starts, stopped when the popup moves to the
+// recovery screen: there is no profile left to refresh.
+let refreshTimer = null;
+
 async function doRefreshAndRender() {
     if (refreshInFlight) return;
     refreshInFlight = true;
@@ -155,7 +159,28 @@ async function init() {
     // reported rather than being swallowed by the save queue
     // (https://git.eeqj.de/sneak/AutistMask/issues/362). Registered ahead of
     // the approval-window branch below too, since that window saves as well.
-    onSaveFailure(showSaveFailureBanner);
+    //
+    // Every save first reads the stored record and refuses it with the same
+    // check loadState() runs below. So a record that becomes unreadable while
+    // the popup is open is found by the next save, a navigation or the
+    // ten-second refresh, and gets the screen it would get at open
+    // (https://git.eeqj.de/sneak/AutistMask/issues/373). The recovery screen
+    // is then also passed to showView(), which runs the leave cleanup of the
+    // screen it replaces, so a phrase, key or password on it is wiped, and
+    // records it as the current view, after which showView() shows nothing
+    // else. The save showView() fires fails too and comes back here, where
+    // both calls see the screen already up and do nothing. Any other failed
+    // save is a read or write that failed, and gets the banner without
+    // changing the screen.
+    onSaveFailure((e) => {
+        if (e instanceof StateUnusableError) {
+            clearInterval(refreshTimer);
+            stateRecovery.show(e);
+            showView("state-recovery");
+        } else {
+            showSaveFailureBanner(e);
+        }
+    });
     try {
         await loadState();
     } catch (e) {
@@ -244,7 +269,7 @@ async function init() {
         renderWalletList();
         restoreView();
         doRefreshAndRender();
-        setInterval(doRefreshAndRender, 10000);
+        refreshTimer = setInterval(doRefreshAndRender, 10000);
     }
 }
 

src/popup/views/helpers.js

@@ -52,9 +52,10 @@ const VIEWS = [
     "export-privkey",
     "show-phrase",
     // Shown by src/popup/views/stateRecovery.js when the stored profile
-    // cannot be read. It is never reached through showView() — by then the
-    // state singleton this file writes on every navigation refuses to be read
-    // — but it is listed so that every view-hiding loop covers it.
+    // cannot be read. At open it is not reached through showView(), since the
+    // state singleton refuses to be read; under an open popup,
+    // src/popup/index.js also passes it to showView() so the screen it
+    // replaces is left like any other.
     "state-recovery",
 ];
 
@@ -86,6 +87,11 @@ function hideError(id) {
 }
 
 function showView(name) {
+    // The recovery screen, once up, is never replaced: work still running
+    // when it went up, such as a transaction wait, must not take the user off
+    // it or clear what they exported or typed there
+    // (https://git.eeqj.de/sneak/AutistMask/issues/373).
+    if (state.currentView === "state-recovery") return;
     const leaving = state.currentView;
     if (leaving && leaving !== name) {
         const onLeave = viewLeaveHandlers.get(leaving);

src/popup/views/stateRecovery.js

@@ -3,8 +3,11 @@
 // Everything else in the popup assumes a loaded profile: showView() reads and
 // writes the state singleton, every view renders from it, and the Settings
 // gear leads to a screen that does both. None of that is available here — by
-// the time this runs, loadState() has REFUSED, deliberately, and reading the
-// singleton throws (https://git.eeqj.de/sneak/AutistMask/issues/311).
+// the time this runs, the stored record has been REFUSED, deliberately: at
+// open loadState() refused it and reading the singleton throws
+// (https://git.eeqj.de/sneak/AutistMask/issues/311), and under an open popup
+// a save refused it, so every further save fails too
+// (https://git.eeqj.de/sneak/AutistMask/issues/373).
 //
 // So this module talks to the DOM directly and touches no state at all. It is
 // the one screen that must work when nothing else can, which is also why it
@@ -170,6 +173,11 @@ function wire() {
  *     refused, or its sentence.
  */
 function show(problem) {
+    // Already up: a later save that trips over the same record, such as a
+    // refresh that was in flight when the screen went up, must not clear what
+    // the user has exported or typed here.
+    if (!$("view-state-recovery").classList.contains("hidden")) return;
+
     const sentence =
         (problem && (problem.problem || problem.message)) || String(problem);
 

tests/stateRecovery.test.js

@@ -148,6 +148,150 @@ describe("the destructive reset on the recovery screen", () => {
     });
 });
 
+describe("a popup already open when the stored profile becomes unreadable", () => {
+    // https://git.eeqj.de/sneak/AutistMask/issues/373. The popup used to stay
+    // on the wallet list with the last good balances, and only a reopen
+    // reached the recovery screen.
+    test("moves to the recovery screen at its next refresh", async () => {
+        const env = await bootPopup(unversionedValidProfile());
+        expect(env.visibleViews()).toEqual(["main"]);
+
+        env.storage.write("autistmask", CORRUPT_BLOBS[0].blob);
+        await env.tick();
+
+        expect(env.visibleViews()).toEqual(["state-recovery"]);
+        expect(env.text("state-recovery-problem").length).toBeGreaterThan(10);
+        expect(env.hidden("btn-settings")).toBe(true);
+        expect(env.storage.read("autistmask")).toEqual(CORRUPT_BLOBS[0].blob);
+    });
+
+    test("stops the ten-second refresh", async () => {
+        const env = await bootPopup(unversionedValidProfile());
+        env.storage.write("autistmask", CORRUPT_BLOBS[0].blob);
+        await env.tick();
+        const { refreshBalances } = require("../src/shared/balances");
+        const calls = refreshBalances.mock.calls.length;
+
+        await env.tick();
+
+        expect(refreshBalances).toHaveBeenCalledTimes(calls);
+    });
+
+    test("a later save does not clear what the user exported or typed", async () => {
+        const env = await bootPopup(unversionedValidProfile());
+        env.storage.write("autistmask", CORRUPT_BLOBS[2].blob);
+        await env.tick();
+
+        await env.click("btn-state-recovery-export");
+        env.node("state-recovery-reset-input").value = "erase my";
+        // Such as the save of a refresh already in flight when the screen
+        // went up.
+        const { saveState } = require("../src/shared/state");
+        await expect(saveState()).rejects.toThrow();
+        await env.settle();
+
+        expect(env.visibleViews()).toEqual(["state-recovery"]);
+        expect(env.hidden("state-recovery-blob")).toBe(false);
+        expect(env.value("state-recovery-reset-input")).toBe("erase my");
+    });
+
+    test("a transaction wait that ends under it does not replace it", async () => {
+        const env = await bootPopup(
+            unversionedValidProfile({
+                currentView: "wait-tx",
+                viewData: {
+                    pendingWait: {
+                        hash: "0x1",
+                        txInfo: { to: ADDRESS, amount: "1", token: "ETH" },
+                        broadcastTime: Date.now(),
+                    },
+                },
+            }),
+        );
+        expect(env.visibleViews()).toEqual(["wait-tx"]);
+        env.storage.write("autistmask", CORRUPT_BLOBS[2].blob);
+        await env.tick();
+        await env.click("btn-state-recovery-export");
+        env.node("state-recovery-reset-input").value = "erase my";
+
+        // The test provider answers no receipt lookup, and six that fail in
+        // a row end the wait with an error.
+        for (let i = 0; i < 6; i++) await env.tick();
+
+        expect(env.text("error-tx-message")).toMatch(/could not be reached/);
+        expect(env.visibleViews()).toEqual(["state-recovery"]);
+        expect(env.hidden("state-recovery-blob")).toBe(false);
+        expect(env.value("state-recovery-reset-input")).toBe("erase my");
+    });
+
+    test("a storage read that fails once leaves the wallet list up", async () => {
+        const env = await bootPopup(unversionedValidProfile());
+
+        env.storage.local.get.mockRejectedValueOnce(
+            new Error("IO error: storage busy"),
+        );
+        await env.tick();
+
+        // Reported as a failed save, not mistaken for an unreadable profile.
+        expect(env.visibleViews()).toEqual(["main"]);
+        expect(env.node("save-failure-banner")).not.toBeNull();
+
+        await env.tick();
+        expect(env.visibleViews()).toEqual(["main"]);
+    });
+
+    // The screen it replaces is left as any navigation leaves it: the rules
+    // at the top of src/popup/views/showPhrase.js and exportPrivkey.js hold
+    // for this way off them too.
+    describe("from a screen holding a secret", () => {
+        const PHRASE =
+            "abandon abandon abandon abandon abandon abandon abandon" +
+            " abandon abandon abandon abandon about";
+
+        afterEach(() => jest.dontMock("../src/shared/vault"));
+
+        test("a recovery phrase on screen is wiped", async () => {
+            jest.doMock("../src/shared/vault", () => ({
+                decryptWithPassword: async () => PHRASE,
+            }));
+            const env = await bootPopup(unversionedValidProfile());
+            require("../src/popup/views/showPhrase").show(0);
+            env.node("show-phrase-password").value = "password";
+            await env.click("btn-show-phrase-reveal");
+            expect(env.text("show-phrase-value")).toBe(PHRASE);
+
+            env.storage.write("autistmask", CORRUPT_BLOBS[0].blob);
+            await env.tick();
+
+            expect(env.visibleViews()).toEqual(["state-recovery"]);
+            expect(env.text("show-phrase-value")).toBe("");
+        });
+
+        test("a private key still being decrypted is never written", async () => {
+            let answer;
+            jest.doMock("../src/shared/vault", () => ({
+                decryptWithPassword: () =>
+                    new Promise((resolve) => {
+                        answer = resolve;
+                    }),
+            }));
+            const env = await bootPopup(unversionedValidProfile());
+            require("../src/popup/views/exportPrivkey").show(0, 0);
+            env.node("export-privkey-password").value = "password";
+            const revealing = env.click("btn-export-privkey-confirm");
+
+            env.storage.write("autistmask", CORRUPT_BLOBS[0].blob);
+            await env.tick();
+            expect(env.visibleViews()).toEqual(["state-recovery"]);
+            expect(env.value("export-privkey-password")).toBe("");
+
+            answer(PHRASE);
+            await revealing;
+            expect(env.text("export-privkey-value")).toBe("");
+        });
+    });
+});
+
 describe("an unversioned profile that is perfectly valid", () => {
     // The upgrade case. Every install in the field is in this state, and the
     // popup must load it, not offer to wipe it.

tests/support/popupBoot.js

@@ -40,6 +40,10 @@ jest.doMock("libsodium-wrappers-sumo", () => sodium);
 jest.doMock("qrcode", () => QRCode);
 jest.doMock("ethereum-blockies-base64", () => makeBlockie);
 
+// Taken before any boot replaces them; see bootPopup().
+const realSetInterval = globalThis.setInterval;
+const realClearInterval = globalThis.clearInterval;
+
 const POPUP_HTML = fs.readFileSync(
     path.join(__dirname, "..", "..", "src", "popup", "index.html"),
     "utf8",
@@ -292,9 +296,20 @@ async function bootPopup(stored, options) {
         }),
         addEventListener: () => {},
     };
-    // The 10s refresh loop init() starts would outlive the test.
-    const realSetInterval = globalThis.setInterval;
-    globalThis.setInterval = () => 0;
+    // The ten-second refresh init() starts, and a transaction wait's timers,
+    // would outlive the test. So every interval is recorded rather than
+    // started, clearInterval() removes it as a browser would, and tick() below
+    // runs the ones still set. Put back by cleanupPopup().
+    const intervals = new Map();
+    let lastId = 0;
+    globalThis.setInterval = (fn) => {
+        lastId += 1;
+        intervals.set(lastId, fn);
+        return lastId;
+    };
+    globalThis.clearInterval = (id) => {
+        intervals.delete(id);
+    };
 
     require("../../src/popup/index");
 
@@ -316,8 +331,6 @@ async function bootPopup(stored, options) {
     }
     await settle();
 
-    globalThis.setInterval = realSetInterval;
-
     return {
         storage,
         document,
@@ -337,6 +350,12 @@ async function bootPopup(stored, options) {
             for (const fn of fns) await fn();
             await settle();
         },
+        // Every interval still set runs once: the ten-second refresh, and a
+        // transaction wait's receipt poll and elapsed counter while one runs.
+        tick: async () => {
+            for (const fn of intervals.values()) await fn();
+            await settle();
+        },
         settle,
         // The view ids whose section is not hidden, as the audit measured them.
         visibleViews: () => {
@@ -354,6 +373,8 @@ function cleanupPopup() {
     delete globalThis.chrome;
     delete globalThis.document;
     delete globalThis.window;
+    globalThis.setInterval = realSetInterval;
+    globalThis.clearInterval = realClearInterval;
 }
 
 module.exports = {