test: the e2e suite waits for each save before it closes the popup (closes #446)

The Settings round trip switched the theme and the network and closed
the popup at once. A close before the change handler's save lands loses
the switch, and the suite then ran on Sepolia.

tests/e2e/run.js now has one helper that polls a field of the stored
record until it holds the expected value, in place of the wait that
only read viewStack. Each Settings switch and spam-filter toggle waits
for its save, the recovery-phrase reopen waits for its saved view, and
reopenPopup() waits until the view it expects to reopen on is the saved
one. README.md no longer lists #446 among the open reports of the
Chrome suite failing under load.

Model: opus-5-5

.gitea/workflows/e2e.yml

@@ -22,11 +22,10 @@ on: [push]
 # These jobs REPORT, they do not gate. Whether a check blocks a merge is
 # Gitea branch protection, which this repo does not configure, so a failure
 # here is a red mark a reviewer has to account for rather than a hard
-# block. Making e2e-chrome a required check is blocked on the measured
-# flake in the dApp signing wait -- two of six runs of unmutated code on a
-# loaded machine -- tracked as
-# https://git.eeqj.de/sneak/AutistMask/issues/287. A gate that fails at
-# random teaches people to merge past red.
+# block. Making e2e-chrome a required check is blocked while reports of the
+# Chrome suite failing under load are still open; the "In CI" section of
+# README.md names them. A gate that fails at random teaches people to merge
+# past red.
 #
 # Nothing here may pass vacuously. There is no continue-on-error and no
 # `|| true`. Both scripts exit non-zero when docker is missing, when the

README.md

@@ -619,14 +619,13 @@ The jobs **report, they do not gate.** A failure is a red mark against the
 commit that a reviewer has to account for, not a hard block: whether a check
 blocks a merge is Gitea branch protection, which this repo does not configure.
 
-That is not only a statement about configuration. The Chrome suite is
-**measurably flaky under load** — two of six runs of unmutated code on a busy
-machine lost the approval popup out from under the dApp signing wait, always in
-the `#183` section, tracked as
-[#287](https://git.eeqj.de/sneak/AutistMask/issues/287). So a red `e2e-chrome`
-has to be read before it is believed, and that flake is the blocker to ever
-making this a required check. Do not answer it with a retry wrapper: a suite
-that reruns until it is green stops being evidence.
+That is not only a statement about configuration. A report of the Chrome suite
+**failing under load** is still open:
+[#290](https://git.eeqj.de/sneak/AutistMask/issues/290), runs on a busy machine
+failing with `the extension opened no approval window within 30000ms`. So a red
+`e2e-chrome` has to be read before it is believed, and those failures are the
+blocker to ever making this a required check. Do not answer them with a retry
+wrapper: a suite that reruns until it is green stops being evidence.
 
 Nothing in either job can pass vacuously. There is no `continue-on-error` and no
 `|| true`; both scripts exit non-zero when docker is missing, when the image

TODO.md

@@ -45,6 +45,43 @@ but the review is broader than any of them.
 
 # Completed Steps
 
+- 2026-10-05: The e2e suite waits for a save to land before it closes the popup
+  ([#446](https://git.eeqj.de/sneak/AutistMask/issues/446)). The Settings round
+  trip switched the theme and the network and closed the popup at once, and a
+  close before the save lands loses the switch; with the network left on
+  Sepolia, a dozen later tests failed too. Each Settings switch and spam-filter
+  toggle is now waited for in storage before the close, and `reopenPopup()`
+  waits until the view it expects to reopen on is the saved one. The restore
+  half of the round trip and the second filter toggle change a setting right
+  after a reopen, while the reopened popup's own saves may still be running;
+  they rely on the fix for
+  [#448](https://git.eeqj.de/sneak/AutistMask/issues/448).
+
+- 2026-10-05: A change made while an earlier save from the same page is still
+  running is stored ([#448](https://git.eeqj.de/sneak/AutistMask/issues/448)).
+  `saveStateOnce()` took its baseline from the page's state after the write, so
+  a change made while the save waited on storage counted as already stored and
+  the save queued after it wrote nothing. A setting changed during the read was
+  lost; so was a wallet added, a site revoked or an endpoint changed during the
+  write, and a wallet deleted then stayed in storage. The save now copies the
+  page's fields when it starts, writes from that copy, and keeps the copy as the
+  baseline.
+
+- 2026-10-05: The extension no longer opens a window for a site-connection
+  prompt already answered
+  ([#287](https://git.eeqj.de/sneak/AutistMask/issues/287)). When the prompt was
+  decided before the toolbar popup raised for it had loaded, that popup was torn
+  down, `chrome.action.openPopup()` rejected, and the background opened its
+  fallback window for the answered approval and then removed it. In the Chrome
+  end-to-end suite the next test could take that window for its own prompt and
+  lose it under its wait. `openApprovalWindow()` now opens nothing for an
+  approval that is no longer pending. The blocklist test's Reject, whose window
+  closes itself, is clicked as the other site Reject is, with the click
+  witnessed. Making `e2e-chrome` a required check is still blocked: other
+  reports of the Chrome suite failing under load are open, among them
+  [#290](https://git.eeqj.de/sneak/AutistMask/issues/290) and
+  [#446](https://git.eeqj.de/sneak/AutistMask/issues/446), as `README.md` says.
+
 - 2026-10-05: The lost-password delete confirmation refuses an empty field and
   ignores characters that paint nothing
   ([#336](https://git.eeqj.de/sneak/AutistMask/issues/336)). A wallet named only

src/background/index.js

@@ -413,7 +413,7 @@ function releaseApproval(approval) {
     }
 }
 
-// Open approval in a separate popup window.
+// Open approval in a separate popup window, unless it is no longer pending.
 // This is the primary mechanism for tx/sign approvals (triggered programmatically,
 // not from a user gesture) and the fallback for site-connection approvals.
 // Never rejects. Its callers raise it from inside a Promise executor and drop
@@ -446,6 +446,10 @@ async function openApprovalWindow(id) {
         );
     }
 
+    // Already answered: a site-connection prompt decided before the toolbar
+    // popup raised for it had loaded, whose openPopup() rejects only now.
+    if (!pendingApprovals[id]) return;
+
     let win = null;
     try {
         win = await windowsCreate(opts);

src/shared/state.js

@@ -122,9 +122,9 @@ function currentNetwork() {
     return networkById(state.networkId);
 }
 
-// The persisted fields as they stood at the end of this page's last
-// loadState() or saveState(). saveState() diffs the live state against this
-// to find only the fields THIS page actually changed.
+// The persisted fields as this page held them when its last loadState()
+// finished, or when its last successful saveState() began. saveState() diffs
+// the live state against this to find only the fields THIS page changed since.
 //
 // Deep-cloned, not a reference: callers mutate persisted objects and arrays
 // in place (state.wallets.push(...)), and a reference baseline would mutate
@@ -464,7 +464,10 @@ function mergeNetworkEndpoints(base, ours, theirs) {
 // does not own goes on being whatever its last loadState() saw, same as
 // before this fix; only the persisted record is guaranteed current.
 async function saveStateOnce() {
-    const current = snapshotPersisted();
+    // A copy, so what this save compares and writes is the page's state as it
+    // stood when the save began. A change made while it waits on storage is
+    // left for the next save, which compares against this copy.
+    const current = structuredClone(snapshotPersisted());
     const result = await storageGet("autistmask");
     // The record in storage right now is about to be merged into and written
     // back, so it is validated exactly like a load validates it. Without this,
@@ -521,7 +524,11 @@ async function saveStateOnce() {
     // exactly as it stood; see the note above.
     rawState.hasWallet = rawState.wallets.length > 0;
 
-    baseline = structuredClone(snapshotPersisted());
+    // What this save compared and wrote, not the page's state now: a change
+    // made during the save must still differ from the baseline, or the save
+    // queued after it finds nothing to store
+    // (https://git.eeqj.de/sneak/AutistMask/issues/448).
+    baseline = current;
 }
 
 // showView() calls saveState() on every navigation without awaiting it, so

tests/backgroundApproval.test.js

@@ -2235,6 +2235,27 @@ describe("a site connection decided as the popup closes", () => {
         });
     });
 
+    // The prompt is decided before the toolbar popup raised for it has
+    // loaded; that popup is torn down and openPopup() rejects only after.
+    test("a toolbar prompt already decided opens no window when openPopup() rejects", async () => {
+        const bg = loadBackground({ actionPopup: true });
+        const opening = deferred();
+        bg.openPopup.mockImplementation(() => opening.promise);
+        const pending = bg.requestSite();
+        await settle();
+
+        const port = bg.connectApproval(pending.id());
+        port.decide(true, false);
+        port.disconnect();
+        await settle();
+        expect(pending.result()).toEqual({ result: [signer.address] });
+
+        opening.reject(new Error("the toolbar popup closed before it loaded"));
+        await settle();
+
+        expect(bg.created).toHaveLength(0);
+    });
+
     // The port carries a decision now, so it carries the sender check the
     // one-off message used to carry. A content script that guessed an
     // approval id must not be able to connect the site it is running on.

tests/e2e/run.js

@@ -204,40 +204,51 @@ async function goHome(page) {
     await visible(page, "#view-main");
 }
 
-// The navigation stack as it was actually persisted, read out of extension
-// storage rather than inferred from which screen is showing. A stale entry
-// left behind by a forward navigation that threw is invisible on screen
-// until the user presses Back one time too many — which is exactly the
-// second-order damage #150 did — so the stack itself is what gets asserted.
-function persistedViewStack(page) {
+// One field of the popup's state as it was actually persisted, read out of
+// extension storage rather than inferred from what is on screen.
+function persistedField(page, field) {
     return page.evaluate(
-        () =>
+        (key) =>
             new Promise((resolve) => {
                 chrome.storage.local.get("autistmask", (r) => {
-                    resolve((r.autistmask && r.autistmask.viewStack) || []);
+                    resolve(r.autistmask ? r.autistmask[key] : undefined);
                 });
             }),
+        field,
     );
 }
 
-// saveState() is fired from showView() without being awaited, so the write
-// lands shortly after the screen does. Polling for the expected stack keeps
-// that race out of the assertion; a stack that never becomes the expected
-// one fails with what it actually was.
-const VIEW_STACK_SETTLE_MS = 5000;
+// The navigation stack as it was actually persisted. A stale entry left
+// behind by a forward navigation that threw is invisible on screen until
+// the user presses Back one time too many — which is exactly the
+// second-order damage #150 did — so the stack itself is what gets asserted.
+async function persistedViewStack(page) {
+    return (await persistedField(page, "viewStack")) || [];
+}
 
-async function waitForViewStack(page, expected, where) {
+// Nothing here can await the popup's saves. showView() fires saveState()
+// without awaiting it, and a Settings control's "change" handler awaits its
+// save only after click() or selectOption() has already returned. So the
+// write lands shortly after the screen or the control changes, and a popup
+// closed before then loses it. Polling for the expected value keeps that
+// race out of the assertion and out of the close; a value that never
+// arrives fails with what it actually was.
+const SAVE_SETTLE_MS = 5000;
+
+async function waitForPersisted(page, field, expected, where) {
     const want = JSON.stringify(expected);
-    const deadline = Date.now() + VIEW_STACK_SETTLE_MS;
+    const deadline = Date.now() + SAVE_SETTLE_MS;
     let seen;
     for (;;) {
-        seen = await persistedViewStack(page);
+        seen = await persistedField(page, field);
         if (JSON.stringify(seen) === want) return;
         if (Date.now() >= deadline) break;
         await sleep(50);
     }
     throw new Error(
-        "navigation stack " +
+        "the persisted " +
+            field +
+            " " +
             where +
             " is " +
             JSON.stringify(seen) +
@@ -257,12 +268,18 @@ test("Back from Add Token unwinds the stack exactly once (#150)", async (env) =>
 
     await env.page.locator("#wallet-list .btn-addr-info").first().click();
     await visible(env.page, "#view-address");
-    await waitForViewStack(env.page, base.concat("main"), "on address detail");
+    await waitForPersisted(
+        env.page,
+        "viewStack",
+        base.concat("main"),
+        "on address detail",
+    );
 
     await env.page.click("#btn-add-token");
     await visible(env.page, "#view-add-token");
-    await waitForViewStack(
+    await waitForPersisted(
         env.page,
+        "viewStack",
         base.concat("main", "address"),
         "on the add token screen",
     );
@@ -273,15 +290,16 @@ test("Back from Add Token unwinds the stack exactly once (#150)", async (env) =>
         !(await env.page.isVisible("#view-add-token")),
         "the add token screen is still showing after Back",
     );
-    await waitForViewStack(
+    await waitForPersisted(
         env.page,
+        "viewStack",
         base.concat("main"),
         "after Back from add token",
     );
 
     await env.page.click("#btn-address-back");
     await visible(env.page, "#view-main");
-    await waitForViewStack(env.page, base, "after a second Back");
+    await waitForPersisted(env.page, "viewStack", base, "after a second Back");
 });
 
 test("a common-token quick-pick fills in the contract address (#150)", async (env) => {
@@ -679,6 +697,12 @@ test("reopening the popup never lands on the phrase screen (#161)", async (env)
     await openPhraseScreen(env.page);
     await revealPhrase(env.page);
 
+    await waitForPersisted(
+        env.page,
+        "currentView",
+        "show-phrase",
+        "before closing the popup",
+    );
     await env.page.close();
     env.page = await openPopup(env.ctx, env.popupUrl);
     await visible(env.page, "#view-main");
@@ -714,10 +738,20 @@ function addressScreenState(page) {
 
 // Close and reopen the page rather than reload it: that is what the toolbar
 // popup does, and it is the only thing that produces the unrendered views.
-async function reopenPopup(env, restoredView) {
+// The popup reopens on the view it last saved, so the close waits until
+// `view` is the one saved. That wait cannot see a save that leaves the value
+// as it was: a caller whose popup already had `view` saved waits for a
+// screen in between first.
+async function reopenPopup(env, view) {
+    await waitForPersisted(
+        env.page,
+        "currentView",
+        view,
+        "before closing the popup",
+    );
     await env.page.close();
     env.page = await openPopup(env.ctx, env.popupUrl);
-    await visible(env.page, restoredView);
+    await visible(env.page, "#view-" + view);
 }
 
 // The reproduction from the issue, step for step.
@@ -732,7 +766,7 @@ test("Back after reopening the popup renders the address screen (#268)", async (
     await env.page.click("#btn-settings");
     await visible(env.page, "#view-settings");
 
-    await reopenPopup(env, "#view-settings");
+    await reopenPopup(env, "settings");
 
     await env.page.click("#btn-settings-back");
     await visible(env.page, "#view-address");
@@ -789,10 +823,18 @@ test("Back after reopening the popup renders the Receive screen (#268)", async (
             JSON.stringify(before.address),
     );
 
+    // The test above left `settings` saved too, so reopenPopup() could not
+    // tell this page's save of it from that one without a save in between.
+    await waitForPersisted(
+        env.page,
+        "currentView",
+        "receive",
+        "on the Receive screen",
+    );
     await env.page.click("#btn-settings");
     await visible(env.page, "#view-settings");
 
-    await reopenPopup(env, "#view-settings");
+    await reopenPopup(env, "settings");
 
     await env.page.click("#btn-settings-back");
     await visible(env.page, "#view-receive");
@@ -1175,11 +1217,24 @@ const NONDEFAULT_NETWORK = "sepolia";
 test("the theme and network selectors carry a non-default persisted value (#229)", async (env) => {
     await openSettings(env.page);
 
-    // selectOption() fires "change", which is what the handlers bind.
+    // selectOption() fires "change", which is what the handlers bind. It
+    // returns before the handler's save lands, hence each wait.
     await env.page.selectOption("#settings-theme", NONDEFAULT_THEME);
+    await waitForPersisted(
+        env.page,
+        "theme",
+        NONDEFAULT_THEME,
+        "after the switch",
+    );
     await env.page.selectOption("#settings-network", NONDEFAULT_NETWORK);
+    await waitForPersisted(
+        env.page,
+        "networkId",
+        NONDEFAULT_NETWORK,
+        "after the switch",
+    );
 
-    await reopenPopup(env, "#view-settings");
+    await reopenPopup(env, "settings");
 
     assertSelectors(
         await selectorValues(env.page),
@@ -1198,9 +1253,16 @@ test("the theme and network selectors carry a non-default persisted value (#229)
     // fixture never customised and so are the mainnet defaults
     // src/shared/state.js starts with.
     await env.page.selectOption("#settings-theme", "system");
+    await waitForPersisted(env.page, "theme", "system", "after the restore");
     await env.page.selectOption("#settings-network", "mainnet");
+    await waitForPersisted(
+        env.page,
+        "networkId",
+        "mainnet",
+        "after the restore",
+    );
 
-    await reopenPopup(env, "#view-settings");
+    await reopenPopup(env, "settings");
 
     assertSelectors(
         await selectorValues(env.page),
@@ -1225,8 +1287,14 @@ test("a spam filter toggled in Settings survives a popup reopen (#229)", async (
         immediately.checked === false,
         "clicking #" + TOGGLED_FILTER + " did not clear it",
     );
+    await waitForPersisted(
+        env.page,
+        "hideDustTransactions",
+        false,
+        "after clearing #" + TOGGLED_FILTER,
+    );
 
-    await reopenPopup(env, "#view-settings");
+    await reopenPopup(env, "settings");
 
     const after = await checkboxStates(env.page);
     for (const { id } of SPAM_FILTER_CHECKBOXES) {
@@ -1243,8 +1311,14 @@ test("a spam filter toggled in Settings survives a popup reopen (#229)", async (
 test("turning the same filter back on survives a reopen too (#229)", async (env) => {
     await openSettings(env.page);
     await env.page.click("#" + TOGGLED_FILTER);
+    await waitForPersisted(
+        env.page,
+        "hideDustTransactions",
+        true,
+        "after setting #" + TOGGLED_FILTER + " again",
+    );
 
-    await reopenPopup(env, "#view-settings");
+    await reopenPopup(env, "settings");
 
     // Restores the fixture the later sections inherit, and rules out a
     // checkbox that persists "off" only because it is stuck there.
@@ -2365,7 +2439,7 @@ test("a token whose symbol() returns markup renders as text (#307)", async (env)
 
     // Close and reopen so the refresh that runs on open fetches balances
     // with the hostile symbol in them.
-    await reopenPopup(env, "#view-address");
+    await reopenPopup(env, "address");
     await env.page.waitForFunction(
         (addr) =>
             !!document.querySelector(
@@ -2431,7 +2505,7 @@ test("a token whose symbol() returns markup renders as text (#307)", async (env)
     // Put the fixture back before the next test reads it, and let the
     // stored balances be rewritten with the honest symbol.
     env.routeOpts.tokenSymbolOverride = null;
-    await reopenPopup(env, "#view-main");
+    await reopenPopup(env, "main");
     await env.page.waitForFunction(
         (addr) => {
             const row = document.querySelector(
@@ -2738,7 +2812,7 @@ async function closeApprovalPages(ctx) {
 //   #btn-reject on the site prompt — NOT self-proving. A page that went away
 //     without the click landing disconnects the approval port, the background
 //     settles that as 4001, and 4001 is exactly what assertUserRejection
-//     accepts. That call site arms the click trace below and asserts it.
+//     accepts. Both call sites arm the click trace below and assert it.
 //
 // A button that is missing or unclickable raises a different error, which is
 // rethrown.
@@ -3124,7 +3198,9 @@ test("a connect request from a blocklisted site is flagged (#219)", async (env)
             // Not remembered: a remembered decision for this origin would
             // outlive the test.
             await popup.uncheck("#approve-remember");
-            await popup.click("#btn-reject");
+            await armClickTrace(env, popup, "#btn-reject");
+            await clickAndClose(popup, "#btn-reject");
+            await assertClickLanded(env, "#btn-reject");
 
             await assertUserRejection(
                 phishingDapp,

tests/stateMerge.test.js

@@ -423,3 +423,80 @@ describe("two wallets independently created with a colliding identity", () => {
         expect(secrets).toContain("secret-b");
     });
 });
+
+// showView() saves on every navigation without waiting, so the user can change
+// something while that save is still waiting on storage. The change is followed
+// by its own saveState(), which runs after the first save; it must be stored
+// (https://git.eeqj.de/sneak/AutistMask/issues/448).
+describe("a change made while an earlier save from the same page is running", () => {
+    // Runs `change` inside the next call to `op` (the stub's get or set),
+    // before that call does its work.
+    function runInside(op, change) {
+        const real = op.getMockImplementation();
+        op.mockImplementationOnce(async (arg) => {
+            change();
+            return real(arg);
+        });
+    }
+
+    test("a network switched during the earlier save's read is stored", async () => {
+        const storage = makeStorageStub({
+            autistmask: { wallets: [W1], networkId: "sepolia" },
+        });
+        const { state, saveState, loadState } = loadPage(storage).state;
+        await loadState();
+
+        let queued;
+        runInside(storage.get, () => {
+            state.networkId = "mainnet";
+            queued = saveState();
+        });
+        state.theme = "dark";
+        await saveState();
+        await queued;
+
+        const stored = storage.read("autistmask");
+        expect(stored.theme).toBe("dark");
+        expect(stored.networkId).toBe("mainnet");
+    });
+
+    test("a wallet added during the earlier save's read is stored", async () => {
+        const storage = makeStorageStub({ autistmask: { wallets: [W1] } });
+        const { state, saveState, loadState } = loadPage(storage).state;
+        await loadState();
+
+        let queued;
+        runInside(storage.get, () => {
+            state.wallets.push(W2);
+            queued = saveState();
+        });
+        await saveState();
+        await queued;
+
+        const stored = storage.read("autistmask");
+        expect(stored.wallets.map((w) => w.encryptedSecret)).toEqual([
+            "secret-one",
+            "secret-two",
+        ]);
+    });
+
+    test("a wallet added during the earlier save's write is stored", async () => {
+        const storage = makeStorageStub({ autistmask: { wallets: [W1] } });
+        const { state, saveState, loadState } = loadPage(storage).state;
+        await loadState();
+
+        let queued;
+        runInside(storage.set, () => {
+            state.wallets.push(W2);
+            queued = saveState();
+        });
+        await saveState();
+        await queued;
+
+        const stored = storage.read("autistmask");
+        expect(stored.wallets.map((w) => w.encryptedSecret)).toEqual([
+            "secret-one",
+            "secret-two",
+        ]);
+    });
+});