diff --git a/TODO.md b/TODO.md index fa567d5..198c70a 100644 --- a/TODO.md +++ b/TODO.md @@ -45,6 +45,16 @@ but the review is broader than any of them. # Completed Steps +- 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 diff --git a/src/shared/state.js b/src/shared/state.js index 2b6c78c..c580819 100644 --- a/src/shared/state.js +++ b/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 diff --git a/tests/stateMerge.test.js b/tests/stateMerge.test.js index 66a6b1f..26756cc 100644 --- a/tests/stateMerge.test.js +++ b/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", + ]); + }); +});