Compare commits

...
Author SHA1 Message Date
sneak bdbe999c05 fix: an open popup moves to the recovery screen when its profile becomes unreadable (closes #373)
check / check (push) Failing after 1s
e2e / e2e-chrome (push) Failing after 2s
e2e / e2e-firefox (push) Failing after 2s
A popup already open when the stored profile became unreadable stayed on the
last good profile until reopened. Every save already reads the stored record
and runs the same check loadState() runs at open; the popup now sends a save
refused by that check to the recovery screen and stops its ten-second refresh,
while any other failed save keeps the "NOT SAVED" banner and the screen it is
on. The recovery screen ignores a second request to show it, so a save that
was in flight does not clear an export or a typed confirmation. The test
harness records the refresh loop so a test can run one tick of it.

Model: opus-5-5
2026-10-04 19:50:14 +00:00
7 changed files with 130 additions and 22 deletions
+27 -15
View File
@@ -1114,16 +1114,18 @@ because bumping for one would send every older install to StateRecovery for
nothing. nothing.
Every read of the record goes through `assertStateUsable()` first, on the raw Every read of the record goes through `assertStateUsable()` first, on the raw
bytes, before normalization: `loadState()` for the popup and `getState()` for bytes, before normalization: `loadState()` and every `saveState()` for the
the background. It refuses a record that is not an object, a `schemaVersion` popup, and `getState()` for the background. It refuses a record that is not an
this build does not understand (a newer one included), a `wallets` that is not a object, a `schemaVersion` this build does not understand (a newer one included),
list of wallet records with address records in them, and a `networkId` that is a `wallets` that is not a list of wallet records with address records in them,
not a network in `src/shared/networks.js`. Refusing is the whole point — a and a `networkId` that is not a network in `src/shared/networks.js`. Refusing is
record the wallet cannot vouch for is never normalized, never written back, and the whole point — a record the wallet cannot vouch for is never normalized,
never half-loaded. The popup shows StateRecovery; a dApp gets a specific error never written back, and never half-loaded. The popup shows StateRecovery,
(`-32007`, an EIP-1474 server-error code the spec leaves unassigned) saying the whether it finds the record unreadable when it opens or at a save while it is
saved data cannot be read and that nothing was signed or sent, rather than the open; a dApp gets a specific error (`-32007`, an EIP-1474 server-error code the
generic `-32603` every request used to answer. 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 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 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 `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 popup puts up a persistent "NOT SAVED" banner (`showSaveFailureBanner()` in
`src/popup/views/helpers.js`). Storage can still fail for reasons no floor `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 covers — a quota, a revoked permission — and the wallet must never look healthy
wallet must never look healthy while that is true. 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 The `networkId` check is not cosmetic: that value is an object KEY into
`state.networkEndpoints`, so an unvalidated `"__proto__"` would set the map's `state.networkEndpoints`, so an unvalidated `"__proto__"` would set the map's
@@ -1961,9 +1964,18 @@ view would leave a wallet one click from deletion.
#### StateRecovery (`state-recovery`) #### StateRecovery (`state-recovery`)
- **When**: `loadState()` refused the stored profile, so the popup has no - **When**: the stored profile fails `assertStateUsable()`. At open, that is
profile at all. It is the only screen reached without one, and the only one `loadState()` refusing it, so the popup has no profile at all. While the popup
that never appears during ordinary use. 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.
Once up, the screen stays until the popup closes or the record is erased:
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 - **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 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. internal error, and no reset or wipe control existed anywhere in the product.
+10
View File
@@ -45,6 +45,16 @@ but the review is broader than any of them.
# Completed Steps # 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. Any other failed
save still gets the banner and leaves the screen alone. The recovery screen
ignores a second request to show it, so a later save does not clear an export
or a typed confirmation.
- 2026-10-04: A token that reports more than 80 decimal places has no known - 2026-10-04: A token that reports more than 80 decimal places has no known
scale ([#350](https://git.eeqj.de/sneak/AutistMask/issues/350)). The shared scale ([#350](https://git.eeqj.de/sneak/AutistMask/issues/350)). The shared
scale check `toDecimals()` accepted any `uint8`, but `formatUnits()` throws scale check `toDecimals()` accepted any `uint8`, but `formatUnits()` throws
+21 -2
View File
@@ -50,6 +50,10 @@ function renderWalletList() {
let refreshInFlight = false; 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() { async function doRefreshAndRender() {
if (refreshInFlight) return; if (refreshInFlight) return;
refreshInFlight = true; refreshInFlight = true;
@@ -155,7 +159,22 @@ async function init() {
// reported rather than being swallowed by the save queue // reported rather than being swallowed by the save queue
// (https://git.eeqj.de/sneak/AutistMask/issues/362). Registered ahead of // (https://git.eeqj.de/sneak/AutistMask/issues/362). Registered ahead of
// the approval-window branch below too, since that window saves as well. // 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). 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);
} else {
showSaveFailureBanner(e);
}
});
try { try {
await loadState(); await loadState();
} catch (e) { } catch (e) {
@@ -244,7 +263,7 @@ async function init() {
renderWalletList(); renderWalletList();
restoreView(); restoreView();
doRefreshAndRender(); doRefreshAndRender();
setInterval(doRefreshAndRender, 10000); refreshTimer = setInterval(doRefreshAndRender, 10000);
} }
} }
+1 -1
View File
@@ -54,7 +54,7 @@ const VIEWS = [
// Shown by src/popup/views/stateRecovery.js when the stored profile // Shown by src/popup/views/stateRecovery.js when the stored profile
// cannot be read. It is never reached through showView() — by then the // cannot be read. It is never reached through showView() — by then the
// state singleton this file writes on every navigation refuses to be read // state singleton this file writes on every navigation refuses to be read
// — but it is listed so that every view-hiding loop covers it. // or saved — but it is listed so that every view-hiding loop covers it.
"state-recovery", "state-recovery",
]; ];
+10 -2
View File
@@ -3,8 +3,11 @@
// Everything else in the popup assumes a loaded profile: showView() reads and // Everything else in the popup assumes a loaded profile: showView() reads and
// writes the state singleton, every view renders from it, and the Settings // 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 // 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 // the time this runs, the stored record has been REFUSED, deliberately: at
// singleton throws (https://git.eeqj.de/sneak/AutistMask/issues/311). // 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 // 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 // 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. * refused, or its sentence.
*/ */
function show(problem) { 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 = const sentence =
(problem && (problem.problem || problem.message)) || String(problem); (problem && (problem.problem || problem.message)) || String(problem);
+49
View File
@@ -148,6 +148,55 @@ 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.refresh();
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("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.refresh();
await env.click("btn-state-recovery-export");
env.node("state-recovery-reset-input").value = "erase my";
// A refresh that was already in flight when the screen went up.
await env.refresh();
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.refresh();
// 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.refresh();
expect(env.visibleViews()).toEqual(["main"]);
});
});
describe("an unversioned profile that is perfectly valid", () => { describe("an unversioned profile that is perfectly valid", () => {
// The upgrade case. Every install in the field is in this state, and the // The upgrade case. Every install in the field is in this state, and the
// popup must load it, not offer to wipe it. // popup must load it, not offer to wipe it.
+12 -2
View File
@@ -292,9 +292,14 @@ async function bootPopup(stored, options) {
}), }),
addEventListener: () => {}, addEventListener: () => {},
}; };
// The 10s refresh loop init() starts would outlive the test. // The 10s refresh loop init() starts would outlive the test, so it is
// recorded rather than started, and refresh() below runs it once.
const realSetInterval = globalThis.setInterval; const realSetInterval = globalThis.setInterval;
globalThis.setInterval = () => 0; const intervals = [];
globalThis.setInterval = (fn) => {
intervals.push(fn);
return 0;
};
require("../../src/popup/index"); require("../../src/popup/index");
@@ -337,6 +342,11 @@ async function bootPopup(stored, options) {
for (const fn of fns) await fn(); for (const fn of fns) await fn();
await settle(); await settle();
}, },
// One tick of the refresh loop, as if ten seconds had passed.
refresh: async () => {
for (const fn of intervals) await fn();
await settle();
},
settle, settle,
// The view ids whose section is not hidden, as the audit measured them. // The view ids whose section is not hidden, as the audit measured them.
visibleViews: () => { visibleViews: () => {