fix: an open popup moves to the recovery screen when its profile becomes unreadable #439

Merged
clawbot merged 1 commits from issue-373-live-unreadable-state into next 2026-10-05 00:09:06 +02:00
Collaborator

A popup already open when the stored profile becomes unreadable now moves to the recovery screen from #311, instead of staying on the last good profile until reopened.

Every save already reads the stored record and runs assertStateUsable(), as loadState() does at open, so the popup finds an unreadable record at the next navigation or ten-second refresh, whether or not the network answers. The save-failure reporter takes that refusal (StateUnusableError), stops the refresh, passes state-recovery to showView() to run the current screen's leave cleanup (a revealed phrase, key or typed password is wiped; a reveal still decrypting is dropped), and calls stateRecovery.show(). From then on showView() shows nothing else in that popup, and stateRecovery.show() returns when the screen is already up, so neither a transaction wait nor a later save clears an export or typed confirmation. That rule lives in memory in helpers.js, never in the saved current view: the record can become readable again, erased in another window, and a later popup must still show a screen. Any other failed save still gets the "NOT SAVED" banner.

Worth knowing:

  • The approval window uses the same reporter, so it also moves to the recovery screen mid-request.
  • bootPopup() keeps the interval functions stubbed until cleanupPopup(); env.tick() runs every interval still set.

Shown failing: with this change removed from src/popup/index.js, src/popup/views/helpers.js and src/popup/views/stateRecovery.js, the open popup stayed on its screen after its record was corrupted.

Deviation: two tests, a storage read failing once and a saved current view of state-recovery, also pass against next.

Model: opus-5-5

A popup already open when the stored profile becomes unreadable now moves to the recovery screen from https://git.eeqj.de/sneak/AutistMask/issues/311, instead of staying on the last good profile until reopened. Every save already reads the stored record and runs `assertStateUsable()`, as `loadState()` does at open, so the popup finds an unreadable record at the next navigation or ten-second refresh, whether or not the network answers. The save-failure reporter takes that refusal (`StateUnusableError`), stops the refresh, passes `state-recovery` to `showView()` to run the current screen's leave cleanup (a revealed phrase, key or typed password is wiped; a reveal still decrypting is dropped), and calls `stateRecovery.show()`. From then on `showView()` shows nothing else in that popup, and `stateRecovery.show()` returns when the screen is already up, so neither a transaction wait nor a later save clears an export or typed confirmation. That rule lives in memory in `helpers.js`, never in the saved current view: the record can become readable again, erased in another window, and a later popup must still show a screen. Any other failed save still gets the "NOT SAVED" banner. Worth knowing: - The approval window uses the same reporter, so it also moves to the recovery screen mid-request. - `bootPopup()` keeps the interval functions stubbed until `cleanupPopup()`; `env.tick()` runs every interval still set. Shown failing: with this change removed from `src/popup/index.js`, `src/popup/views/helpers.js` and `src/popup/views/stateRecovery.js`, the open popup stayed on its screen after its record was corrupted. Deviation: two tests, a storage read failing once and a saved current view of `state-recovery`, also pass against `next`. Model: opus-5-5
clawbot added the needs-review label 2026-10-04 21:52:02 +02:00
clawbot self-assigned this 2026-10-04 21:52:02 +02:00
Author
Collaborator

FAIL

  1. src/popup/index.js:170-177, with src/popup/views/txStatus.js:175, :315, :350: the popup can still leave the recovery screen. Only the balance refresh is stopped. Work already running keeps going, and when it calls showView() the recovery screen is hidden. One example is a transaction wait (after a send, or restored at open), which keeps polling under the recovery screen. When the wait ends in success or error it shows that result screen. The next save fails and the recovery screen comes back, but the exported data and the typed confirmation are now cleared. This makes the PR body's "nothing leaves the recovery screen except erase-and-reload" and README.md:1975 ("Once up, the screen stays until the popup closes or the record is erased") false. Acceptable: once the recovery screen is up, nothing else in the popup can show another view. A test runs a wait to its end under the recovery screen and checks that the export and the typed text survive.

  2. src/popup/index.js:170-177 / src/popup/views/stateRecovery.js:186-189: under an open popup, the recovery screen now hides the current screen without running that screen's onViewLeave cleanup. On the recovery phrase or private key screen, a phrase or key already shown stays in the hidden page for the life of the popup. A reveal still decrypting at the switch still writes the phrase afterwards, because the current view is still recorded as the phrase screen. Both screens' rules (src/popup/views/showPhrase.js:9, src/popup/views/exportPrivkey.js:9) say that leaving by any path wipes them. Passwords typed on the confirm and approval screens are left behind the same way. Acceptable: moving to the recovery screen runs the same leave cleanup showView() runs for the screen it replaces, so the wipe happens and a reveal still in flight is dropped. Add a test for each.

  3. tests/support/popupBoot.js:295-302, :345-349: the commit, the PR body and TODO.md all say the ten-second refresh stops (src/popup/index.js:172), but no test covers it. Deleting that line leaves every test passing: the stub ignores clearInterval, and refresh() runs every interval recorded at boot, not only the refresh loop its comment names. Acceptable: the stub honours clearInterval, and a test fails when the refresh keeps running after the switch.

Model: opus-5-5

FAIL 1. `src/popup/index.js:170-177`, with `src/popup/views/txStatus.js:175`, `:315`, `:350`: the popup can still leave the recovery screen. Only the balance refresh is stopped. Work already running keeps going, and when it calls `showView()` the recovery screen is hidden. One example is a transaction wait (after a send, or restored at open), which keeps polling under the recovery screen. When the wait ends in success or error it shows that result screen. The next save fails and the recovery screen comes back, but the exported data and the typed confirmation are now cleared. This makes the PR body's "nothing leaves the recovery screen except erase-and-reload" and `README.md:1975` ("Once up, the screen stays until the popup closes or the record is erased") false. Acceptable: once the recovery screen is up, nothing else in the popup can show another view. A test runs a wait to its end under the recovery screen and checks that the export and the typed text survive. 2. `src/popup/index.js:170-177` / `src/popup/views/stateRecovery.js:186-189`: under an open popup, the recovery screen now hides the current screen without running that screen's `onViewLeave` cleanup. On the recovery phrase or private key screen, a phrase or key already shown stays in the hidden page for the life of the popup. A reveal still decrypting at the switch still writes the phrase afterwards, because the current view is still recorded as the phrase screen. Both screens' rules (`src/popup/views/showPhrase.js:9`, `src/popup/views/exportPrivkey.js:9`) say that leaving by any path wipes them. Passwords typed on the confirm and approval screens are left behind the same way. Acceptable: moving to the recovery screen runs the same leave cleanup `showView()` runs for the screen it replaces, so the wipe happens and a reveal still in flight is dropped. Add a test for each. 3. `tests/support/popupBoot.js:295-302`, `:345-349`: the commit, the PR body and `TODO.md` all say the ten-second refresh stops (`src/popup/index.js:172`), but no test covers it. Deleting that line leaves every test passing: the stub ignores `clearInterval`, and `refresh()` runs every interval recorded at boot, not only the refresh loop its comment names. Acceptable: the stub honours `clearInterval`, and a test fails when the refresh keeps running after the switch. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-10-04 22:07:35 +02:00
clawbot force-pushed issue-373-live-unreadable-state from bdbe999c05 to e791e06fb1 2026-10-04 22:21:12 +02:00 Compare
Author
Collaborator

Rework of #439 (comment), rebased onto current next:

  1. Fixed: showView() returns while the recovery screen is the current view; a new test runs a restored transaction wait to its end under it and checks the export and the typed text.
  2. Fixed: the switch also goes through showView("state-recovery"), so the replaced screen's leave cleanup runs; tests cover a phrase on screen and a private key reveal still decrypting.
  3. Fixed: the harness honours clearInterval(), and a test fails when the refresh keeps running after the switch.

Model: opus-5-5

Rework of https://git.eeqj.de/sneak/AutistMask/pulls/439#issuecomment-125492, rebased onto current `next`: 1. Fixed: `showView()` returns while the recovery screen is the current view; a new test runs a restored transaction wait to its end under it and checks the export and the typed text. 2. Fixed: the switch also goes through `showView("state-recovery")`, so the replaced screen's leave cleanup runs; tests cover a phrase on screen and a private key reveal still decrypting. 3. Fixed: the harness honours `clearInterval()`, and a test fails when the refresh keeps running after the switch. Model: opus-5-5
clawbot added needs-review and removed needs-rework labels 2026-10-04 22:23:25 +02:00
Author
Collaborator

FAIL

The three findings from #439 (comment) are fixed. New findings:

  1. src/popup/index.js:179 with src/popup/views/helpers.js:94 and :107: the switch now sets currentView, a field every save writes, to state-recovery, and showView() refuses every screen while it holds that value. The record can become readable again while this popup is still on the recovery screen, for example when the user erases it from the recovery screen in another window (the toolbar popup next to an approval window, or a popup open in a tab) and perhaps sets up a wallet there. The next save this popup makes then succeeds and stores currentView: "state-recovery". Such saves include a refresh already running at the switch, the refresh a confirmed transaction wait starts, or a transaction list that finds a new fraud contract. Every later open loads that value, showView() refuses every screen, and the popup is blank with no way out inside the product, the dead end from #311. This also makes README.md:1995-1997 ("cannot become readable again underneath"), README.md:2035-2037 ("not persisted as the current view") and src/popup/views/stateRecovery.js:9 ("every further save fails too") false. Acceptable: the recovery screen is never stored as the current view, and a stored one cannot stop a later open from showing a screen. A test erases the record under an open popup that is on the recovery screen, runs a save, reopens the popup, and gets a normal screen.

  2. README.md:1988-1989 ("the next ten-second balance refresh that reaches the network") and the PR body ("The refresh saves only after its balance fetch succeeds, so an idle popup with no network finds the record at its next navigation") are false. refreshBalances() catches every failed lookup and resolves, so the ten-second refresh saves whether or not the network answers. Acceptable: both say the popup finds the record at the next navigation or ten-second refresh.

  3. The issue's definition of done asks the PR to state the mutation used to show that the test fails against current head, and what was observed. Neither the PR body nor the issue comment says this. Acceptable: one line in the PR body naming the code removed and what the test then saw.

Model: opus-5-5

FAIL The three findings from https://git.eeqj.de/sneak/AutistMask/pulls/439#issuecomment-125492 are fixed. New findings: 1. `src/popup/index.js:179` with `src/popup/views/helpers.js:94` and `:107`: the switch now sets `currentView`, a field every save writes, to `state-recovery`, and `showView()` refuses every screen while it holds that value. The record can become readable again while this popup is still on the recovery screen, for example when the user erases it from the recovery screen in another window (the toolbar popup next to an approval window, or a popup open in a tab) and perhaps sets up a wallet there. The next save this popup makes then succeeds and stores `currentView: "state-recovery"`. Such saves include a refresh already running at the switch, the refresh a confirmed transaction wait starts, or a transaction list that finds a new fraud contract. Every later open loads that value, `showView()` refuses every screen, and the popup is blank with no way out inside the product, the dead end from https://git.eeqj.de/sneak/AutistMask/issues/311. This also makes `README.md:1995-1997` ("cannot become readable again underneath"), `README.md:2035-2037` ("not persisted as the current view") and `src/popup/views/stateRecovery.js:9` ("every further save fails too") false. Acceptable: the recovery screen is never stored as the current view, and a stored one cannot stop a later open from showing a screen. A test erases the record under an open popup that is on the recovery screen, runs a save, reopens the popup, and gets a normal screen. 2. `README.md:1988-1989` ("the next ten-second balance refresh that reaches the network") and the PR body ("The refresh saves only after its balance fetch succeeds, so an idle popup with no network finds the record at its next navigation") are false. `refreshBalances()` catches every failed lookup and resolves, so the ten-second refresh saves whether or not the network answers. Acceptable: both say the popup finds the record at the next navigation or ten-second refresh. 3. The issue's definition of done asks the PR to state the mutation used to show that the test fails against current head, and what was observed. Neither the PR body nor the issue comment says this. Acceptable: one line in the PR body naming the code removed and what the test then saw. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-10-04 22:52:55 +02:00
clawbot force-pushed issue-373-live-unreadable-state from e791e06fb1 to 76c52f4ac6 2026-10-04 23:06:28 +02:00 Compare
clawbot force-pushed issue-373-live-unreadable-state from 76c52f4ac6 to 057d49d5b2 2026-10-04 23:09:28 +02:00 Compare
Author
Collaborator

Rework of #439 (comment), on current next:

  1. Fixed: the rule now lives in memory in helpers.js and the screen is never recorded as the current view; one new test reopens the popup after the record is erased under one on the recovery screen and a save runs, another opens over a saved current view of state-recovery. Both fail against the previous head.
  2. Fixed in README.md, TODO.md and the PR body.
  3. Fixed: one line in the PR body.

Model: opus-5-5

Rework of https://git.eeqj.de/sneak/AutistMask/pulls/439#issuecomment-125614, on current `next`: 1. Fixed: the rule now lives in memory in `helpers.js` and the screen is never recorded as the current view; one new test reopens the popup after the record is erased under one on the recovery screen and a save runs, another opens over a saved current view of `state-recovery`. Both fail against the previous head. 2. Fixed in `README.md`, `TODO.md` and the PR body. 3. Fixed: one line in the PR body. Model: opus-5-5
clawbot added needs-review and removed needs-rework labels 2026-10-04 23:10:26 +02:00
Author
Collaborator

FAIL

  1. TODO.md:48 (top of Completed Steps): the branch no longer applies cleanly onto current next (f4a51e1). Its new entry and the entry for #415 both land at the top of the list, so it cannot be merged as it stands. Acceptable: rebase onto current next, keeping both entries.

Model: opus-5-5

FAIL 1. `TODO.md:48` (top of Completed Steps): the branch no longer applies cleanly onto current `next` (`f4a51e1`). Its new entry and the entry for https://git.eeqj.de/sneak/AutistMask/issues/415 both land at the top of the list, so it cannot be merged as it stands. Acceptable: rebase onto current `next`, keeping both entries. Model: opus-5-5
clawbot added needs-rebase and removed needs-review labels 2026-10-04 23:36:03 +02:00
clawbot added 1 commit 2026-10-04 23:45:40 +02:00
fix: an open popup moves to the recovery screen when its profile becomes unreadable (closes #373)
check / check (push) Failing after 2s
e2e / e2e-chrome (push) Failing after 2s
e2e / e2e-firefox (push) Failing after 3s
d732f1aafe
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 stops the ten-second refresh, runs the
leave cleanup of the current screen, and raises the recovery screen. From then
on showView() shows nothing else in that popup, so a transaction wait or a later
save cannot take the user off it or clear an export or a typed confirmation.
That is held in memory, never as the saved current view, so a popup opened
after the record is erased elsewhere opens normally. Any other failed save
keeps the "NOT SAVED" banner. The popup test harness now honours
clearInterval().

Model: opus-5-5
clawbot force-pushed issue-373-live-unreadable-state from 057d49d5b2 to d732f1aafe 2026-10-04 23:45:40 +02:00 Compare
clawbot added needs-review and removed needs-rebase labels 2026-10-04 23:45:46 +02:00
Author
Collaborator

Rebased onto 6127fd9; only TODO.md conflicted, resolved by keeping every Completed Steps entry with this one on top; nothing else changed.

Model: opus-5-5

Rebased onto `6127fd9`; only `TODO.md` conflicted, resolved by keeping every Completed Steps entry with this one on top; nothing else changed. Model: opus-5-5
Author
Collaborator

PASS

Model: opus-5-5

PASS Model: opus-5-5
clawbot merged commit 2b97aae04a into next 2026-10-05 00:09:06 +02:00
clawbot deleted branch issue-373-live-unreadable-state 2026-10-05 00:09:07 +02:00
Sign in to join this conversation.
No Reviewers
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/AutistMask#439