sneak/AutistMask
1
0
Fork 0
Code Issues 42 Pull Requests 8 Actions Packages Projects Releases Wiki Activity

fix: filter the restored view stack against RESTORABLE_VIEWS (closes #224) #266

Merged
clawbot merged 1 commits from fix/issue-224-filter-restored-viewstack into next 2026-08-12 12:07:49 +02:00
Conversation 3 Commits 1 Files Changed 3 +154 -1
clawbot commented 2026-08-12 11:33:31 +02:00
Collaborator
Copy Link

Closes #224.

restoreView() refuses to reopen the popup ONTO a non-restorable view, but the
stack behind it was restored verbatim, so a screen the popup will not render —
export-privkey, show-phrase — could sit in it. Back then landed on a view
the popup had declined to restore, and show-phrase has no Back control to
leave by.

What this is not: no secret is exposed. Those screens are empty precisely
because nothing is restored into them. This is a navigation defect, not a
disclosure one.

Scope

This fixes exactly the narrowed DoD: Back after reopening never lands on a view
the popup DECLINED TO RESTORE.

It does not make Back safe in general. goBack() unhides the popped view
without re-rendering it, so a restorable view sitting in the stack —
address, receive, confirm-tx — still comes up as the blank static
template on a freshly reopened popup. That is independent of this filter and is
tracked as #268. The test
an ordinary restorable stack is restored unchanged asserts the stack contents
only, and says so.

Truncate, not splice

loadState() truncates the stored stack at the first entry outside
RESTORABLE_VIEWS — the entry and everything above it go.

The result is therefore always a prefix of what was stored, which is the whole
argument: every surviving entry keeps exactly the Back target it had before.
Splicing the bad entry out preserves more of the stack but silently re-points
the entry that sat above the hole at a different screen, so a Back press means
something new without anything visible having changed. Truncation also matches
how the stack was built: the entries above a dropped view were reached
through it, so the path they describe no longer exists once it is gone. What
truncation loses is depth, and the cost of that is bounded — Back reaches the
root sooner — whereas the cost of splicing is unbounded: it can send Back
anywhere.

Never an empty dead end: when truncation leaves nothing and the restored view
is a restorable non-root view, main is put beneath it. (An empty stack
already sends goBack() to main; making it explicit means the invariant does
not depend on that fallback, and it is behaviourally identical for stacks that
were legitimately empty.) The stack stays empty only when the popup restores
onto main, or onto a view that is not restorable at all — in which case
restoreView() will not reopen onto it and there is nothing to sit under.

A stored stack that is missing or is not an array now runs through that same
rule instead of returning early, so a corrupt stack does not fall back on the
goBack() behaviour the explicit ["main"] exists in order not to depend on.

Note on what the ["main"] injection does NOT promise: several views are in
RESTORABLE_VIEWS but still fall back to main at restore time when their
backing data is gone — address-token without selectedToken, confirm-tx
without a pending tx, transaction without a tx, a wait-tx whose resume
fails, any needsAddress view without a valid address. Those get ["main"]
injected and then land on main anyway, leaving main beneath main.
Harmless (Back from main is not reachable), but the stack is not empty in
that case and an earlier revision of this description wrongly said it was.

On load, not on save

Two reasons:

  1. Only a load-side filter is retroactive. Filtering on save fixes stacks
    written after this ships and leaves every stack already in storage exactly
    as broken as it is today — including stacks written by a build where a view
    was still restorable and has since left the set. Filtering on load means
    the guarantee holds on the first read of any profile, whatever wrote it.
  2. The live in-session stack is legitimate and must stay whole: while the
    popup is open the user really is one Back away from a screen that is
    rendered right now. The set is about what may be re-rendered from
    persisted state    , so it belongs at the point where state comes back from
    storage.

saveState() is unchanged and still persists the live stack verbatim; a test
pins that.

Tests, failing first

New cases in tests/state.test.js cover a non-restorable view at the top, in
the middle and at the bottom of the stack, a name that is not a restorable view
at all, an ordinary restorable stack restored unchanged, restoring onto main,
restoring onto a view the popup will not reopen, a non-array stack under both a
restorable and a non-restorable current view, and the save path. Against
unmodified next six of them fail:

✕ a non-restorable view at the top of the stack is dropped
✕ a non-restorable view in the middle truncates the stack there
✕ a non-restorable view at the bottom leaves main to go back to
✕ no restored stack retains a secret-bearing view
✕ a name that is not a restorable view at all is dropped
✕ restoring onto main keeps the stack empty
Tests: 6 failed, 561 passed, 567 total

src/popup/restorableViews.js is untouched, so the existing assertions that
export-privkey and show-phrase are absent and wait-tx is present continue
to hold and now also govern the stack.

Mutation pinned

The review found the RESTORABLE_VIEWS.has(currentView) clause of the
never-empty guard unpinned: dropping it left every test green. restoring onto a view the popup will not reopen keeps the stack empty now kills it. With the
clause removed:

✕ restoring onto a view the popup will not reopen keeps the stack empty
  - Expected  - 1
  + Received  + 3
  - Array []
  + Array [
  +   "main",
  + ]
Tests: 1 failed, 586 passed, 587 total

With it restored: 587 passed, 587 total.

Verification

make check on the rebased branch: 25 suites / 587 tests, test-verify-build
18 cases, prettier clean. make test-e2e: 27/27 in the pinned container.

Closes [#224](https://git.eeqj.de/sneak/AutistMask/issues/224). `restoreView()` refuses to reopen the popup ONTO a non-restorable view, but the stack behind it was restored verbatim, so a screen the popup will not render — `export-privkey`, `show-phrase` — could sit in it. Back then landed on a view the popup had declined to restore, and `show-phrase` has no Back control to leave by. What this is not: no secret is exposed. Those screens are empty precisely because nothing is restored into them. This is a navigation defect, not a disclosure one. ## Scope This fixes exactly the narrowed DoD: Back after reopening never lands on a view the popup DECLINED TO RESTORE. It does not make Back safe in general. `goBack()` unhides the popped view without re-rendering it, so a *restorable* view sitting in the stack — `address`, `receive`, `confirm-tx` — still comes up as the blank static template on a freshly reopened popup. That is independent of this filter and is tracked as [#268](https://git.eeqj.de/sneak/AutistMask/issues/268). The test `an ordinary restorable stack is restored unchanged` asserts the stack contents only, and says so. ## Truncate, not splice `loadState()` truncates the stored stack at the first entry outside `RESTORABLE_VIEWS` — the entry and everything above it go. The result is therefore always a prefix of what was stored, which is the whole argument: every surviving entry keeps exactly the Back target it had before. Splicing the bad entry out preserves more of the stack but silently re-points the entry that sat above the hole at a different screen, so a Back press means something new without anything visible having changed. Truncation also matches how the stack was built: the entries above a dropped view were reached *through* it, so the path they describe no longer exists once it is gone. What truncation loses is depth, and the cost of that is bounded — Back reaches the root sooner — whereas the cost of splicing is unbounded: it can send Back anywhere. Never an empty dead end: when truncation leaves nothing and the restored view is a restorable non-root view, `main` is put beneath it. (An empty stack already sends `goBack()` to `main`; making it explicit means the invariant does not depend on that fallback, and it is behaviourally identical for stacks that were legitimately empty.) The stack stays empty only when the popup restores onto `main`, or onto a view that is not restorable at all — in which case `restoreView()` will not reopen onto it and there is nothing to sit under. A stored stack that is missing or is not an array now runs through that same rule instead of returning early, so a corrupt stack does not fall back on the `goBack()` behaviour the explicit `["main"]` exists in order not to depend on. Note on what the `["main"]` injection does NOT promise: several views are in `RESTORABLE_VIEWS` but still fall back to `main` at restore time when their backing data is gone — `address-token` without `selectedToken`, `confirm-tx` without a pending tx, `transaction` without a tx, a `wait-tx` whose resume fails, any `needsAddress` view without a valid address. Those get `["main"]` injected and then land on `main` anyway, leaving `main` beneath `main`. Harmless (Back from `main` is not reachable), but the stack is not empty in that case and an earlier revision of this description wrongly said it was. ## On load, not on save Two reasons: 1. Only a load-side filter is retroactive. Filtering on save fixes stacks written after this ships and leaves every stack already in storage exactly as broken as it is today — including stacks written by a build where a view was still restorable and has since left the set. Filtering on load means the guarantee holds on the first read of any profile, whatever wrote it. 2. The live in-session stack is legitimate and must stay whole: while the popup is open the user really is one Back away from a screen that is rendered right now. The set is about what may be *re-rendered from persisted state*, so it belongs at the point where state comes back from storage. `saveState()` is unchanged and still persists the live stack verbatim; a test pins that. ## Tests, failing first New cases in `tests/state.test.js` cover a non-restorable view at the top, in the middle and at the bottom of the stack, a name that is not a restorable view at all, an ordinary restorable stack restored unchanged, restoring onto `main`, restoring onto a view the popup will not reopen, a non-array stack under both a restorable and a non-restorable current view, and the save path. Against unmodified `next` six of them fail: ``` ✕ a non-restorable view at the top of the stack is dropped ✕ a non-restorable view in the middle truncates the stack there ✕ a non-restorable view at the bottom leaves main to go back to ✕ no restored stack retains a secret-bearing view ✕ a name that is not a restorable view at all is dropped ✕ restoring onto main keeps the stack empty Tests: 6 failed, 561 passed, 567 total ``` `src/popup/restorableViews.js` is untouched, so the existing assertions that `export-privkey` and `show-phrase` are absent and `wait-tx` is present continue to hold and now also govern the stack. ### Mutation pinned The review found the `RESTORABLE_VIEWS.has(currentView)` clause of the never-empty guard unpinned: dropping it left every test green. `restoring onto a view the popup will not reopen keeps the stack empty` now kills it. With the clause removed: ``` ✕ restoring onto a view the popup will not reopen keeps the stack empty - Expected - 1 + Received + 3 - Array [] + Array [ + "main", + ] Tests: 1 failed, 586 passed, 587 total ``` With it restored: 587 passed, 587 total. ## Verification `make check` on the rebased branch: 25 suites / 587 tests, `test-verify-build` 18 cases, prettier clean. `make test-e2e`: 27/27 in the pinned container.
clawbot added the needs-review label 2026-08-12 11:33:31 +02:00
clawbot added 1 commit 2026-08-12 11:33:32 +02:00
fix: filter the restored view stack against RESTORABLE_VIEWS (closes #224)
All checks were successful
check / check (push) Successful in 34s
Details
e36d83627a 
restoreView() refuses to reopen the popup ONTO a non-restorable view, but
the stack behind it was restored verbatim, so a screen the popup will not
render -- export-privkey, show-phrase -- could sit in it. Back then landed
on a view whose content is deliberately never re-rendered, and show-phrase
has no Back control to leave by. No secret is exposed: those screens are
empty precisely because nothing is restored into them. This is navigation,
not disclosure.

loadState() now truncates the stored stack at the first entry outside
RESTORABLE_VIEWS rather than splicing that entry out, so the result stays a
prefix of what was stored and every surviving entry keeps exactly the Back
target it had; splicing would silently re-point the entry above the hole at
a different screen. A stack truncated to nothing under a restorable
non-root view gets main beneath it, so Back always has somewhere to go.
Stacks with no unrenderable entry are restored unchanged.

The filter is on load, not on save: the live in-session stack is
legitimate, since the screen really is rendered while the popup is open,
and only a load-side filter also repairs the stacks already in storage,
including ones written before a view left the set.
clawbot self-assigned this 2026-08-12 11:33:36 +02:00
clawbot commented 2026-08-12 11:42:43 +02:00
Author
Collaborator
Copy Link

Review: FAIL (needs-rework; a rebase is also required)

1. Does not merge into current next — blocking

origin/next is now 5af89a1 (#238 landed after this branch was cut). e36d836 neither rebases nor merges onto it:

CONFLICT (content): Merge conflict in TODO.md

Both commits add a bullet at the top of # Completed Steps. Trivial to resolve, but the tracker's mergeable: true was computed against the stale base c6a1f97. Rebase onto current next and re-run make check.

2. DoD bullet 2 is not satisfied — Back still lands on an unrendered screen

Pressing Back after reopening never lands on an unrendered screen. This is reachable with a stack that is entirely restorable, so the new filter cannot help.

goBack() (src/popup/views/helpers.js:132-143) pops the target and calls showView(target). showView() (src/popup/views/helpers.js:65-80) only toggles hidden classes and runs the leave handler — it re-renders nothing. The sole exception is main, via _renderMain(). And src/popup/index.js:266-272 renders only renderWalletList() before restoreView(), so on a fresh popup every view except main and the restored one is the blank static template from src/popup/index.html.

Reproduction:

  1. main → address detail → gear. Stack is ["main","address"], currentView is settings.
  2. Close the popup, reopen it. restoreView() shows settings; the stack is restored unchanged (correctly, per this PR).
  3. Press < Back (btn-settings-back, src/popup/views/settings.js:423).
  4. showView("address") unhides view-address (src/popup/index.html:257) with address-full empty, address-balances rendering  , and the placeholder heading "Address".

Same for address-token, receive, confirm-tx, transaction when they sit in the stack. No secret-bearing view is involved.

This is pre-existing, not introduced here — but the PR's own test an ordinary restorable stack is restored unchanged pins it as correct, and the issue's DoD claims it as done. Acceptable: either route the popped view in goBack() through the same per-view show()/data guards restoreView() already uses (falling back to main when the backing data is gone), or narrow #224's DoD to the non-restorable-view case and file the general defect as its own issue before this merges. Do not leave the DoD checked as-is.

3. Untested guard — surviving mutation

src/shared/state.js:72. Replacing the never-empty condition with kept.length === 0 && currentView !== "main" (i.e. dropping RESTORABLE_VIEWS.has(currentView)) leaves 585/585 green. Behaviourally harmless — restoreView() falls back to main, which has no Back control — but the guard is unpinned. Add a case: a stored stack that truncates to nothing under a non-restorable currentView (e.g. ["export-privkey"] with currentView: "show-phrase") must restore to [], not ["main"]. The other four mutations (truncation off-by-one, never-empty rule, !== "main", Array.isArray) each killed at least one test.

4. PR body states an invariant that does not hold

When the popup restores onto main, or onto a view that will itself fall back to main, the stack stays empty.

The second clause is false. address-token with no selectedToken, confirm-tx with no viewData.pendingTx, transaction with no viewData.tx, wait-tx whose restoreWait() fails, and any needsAddress view without a valid address are all in RESTORABLE_VIEWS, so restorableStack() injects ["main"] — and then restoreView() falls back to main anyway, leaving the popup on main with ["main"] beneath it. Harmless in practice; the claim is wrong and should be corrected or dropped.

5. Minor inconsistency

src/shared/state.js:63-65: the non-array early return bypasses the never-empty rule, so a corrupt stack under a restorable non-root view yields [] and relies on exactly the goBack() fallback that the explicit ["main"] exists in order not to depend on. Behaviourally identical; noting only.

Anomaly (not filed as a defect)

src/shared/state.js:6 is the only module under src/shared/ or src/background/ that requires from src/popup/, pulling a popup constant into the background bundle. Acknowledged in a comment, dependency-free, 11 strings — cost is nil. Raised as a layering question for the owner, not written up as an error.

Verified and passing

make check executed (not cached): 25 suites / 585 tests in 12.3s, test-verify-build 18 cases, prettier clean. make test-e2e 18/18 in the pinned container. Failing-first reproduced exactly against merge base c6a1f97: the 6 named tests fail. src/popup/restorableViews.js untouched; wait-tx present, export-privkey/show-phrase absent. Save→load round trip: saveState() persists the live stack verbatim and the next loadState() filters it, so the defect does not reappear. Malformed entries (nulls, numbers, objects) fail Set.has() and truncate safely. Truncate-over-splice and load-over-save are both the right calls and the arguments hold. Single commit, author and committer clawbot <clawbot@noreply.example.org>, title ends (closes #224), base next, one TODO.md bullet at the top of # Completed Steps with nothing lost, no attribution trailers, no competitor named, and the class is stated correctly as navigation rather than disclosure in both the body and the commit message.

## Review: FAIL (`needs-rework`; a rebase is also required) ### 1. Does not merge into current `next` — blocking `origin/next` is now `5af89a1` ([#238](https://git.eeqj.de/sneak/AutistMask/pulls/238) landed after this branch was cut). `e36d836` neither rebases nor merges onto it: ``` CONFLICT (content): Merge conflict in TODO.md ``` Both commits add a bullet at the top of `# Completed Steps`. Trivial to resolve, but the tracker's `mergeable: true` was computed against the stale base `c6a1f97`. Rebase onto current `next` and re-run `make check`. ### 2. DoD bullet 2 is not satisfied — Back still lands on an unrendered screen `Pressing Back after reopening never lands on an unrendered screen.` This is reachable with a stack that is entirely restorable, so the new filter cannot help. `goBack()` (`src/popup/views/helpers.js:132-143`) pops the target and calls `showView(target)`. `showView()` (`src/popup/views/helpers.js:65-80`) only toggles `hidden` classes and runs the leave handler — it re-renders nothing. The sole exception is `main`, via `_renderMain()`. And `src/popup/index.js:266-272` renders only `renderWalletList()` before `restoreView()`, so on a fresh popup every view except `main` and the restored one is the blank static template from `src/popup/index.html`. Reproduction: 1. `main` → address detail → gear. Stack is `["main","address"]`, `currentView` is `settings`. 2. Close the popup, reopen it. `restoreView()` shows settings; the stack is restored unchanged (correctly, per this PR). 3. Press `< Back` (`btn-settings-back`, `src/popup/views/settings.js:423`). 4. `showView("address")` unhides `view-address` (`src/popup/index.html:257`) with `address-full` empty, `address-balances` rendering ` `, and the placeholder heading "Address". Same for `address-token`, `receive`, `confirm-tx`, `transaction` when they sit in the stack. No secret-bearing view is involved. This is pre-existing, not introduced here — but the PR's own test `an ordinary restorable stack is restored unchanged` pins it as correct, and the issue's DoD claims it as done. Acceptable: either route the popped view in `goBack()` through the same per-view `show()`/data guards `restoreView()` already uses (falling back to `main` when the backing data is gone), or narrow [#224](https://git.eeqj.de/sneak/AutistMask/issues/224)'s DoD to the non-restorable-view case and file the general defect as its own issue before this merges. Do not leave the DoD checked as-is. ### 3. Untested guard — surviving mutation `src/shared/state.js:72`. Replacing the never-empty condition with `kept.length === 0 && currentView !== "main"` (i.e. dropping `RESTORABLE_VIEWS.has(currentView)`) leaves 585/585 green. Behaviourally harmless — `restoreView()` falls back to `main`, which has no Back control — but the guard is unpinned. Add a case: a stored stack that truncates to nothing under a non-restorable `currentView` (e.g. `["export-privkey"]` with `currentView: "show-phrase"`) must restore to `[]`, not `["main"]`. The other four mutations (truncation off-by-one, never-empty rule, `!== "main"`, `Array.isArray`) each killed at least one test. ### 4. PR body states an invariant that does not hold > When the popup restores onto `main`, or onto a view that will itself fall back to `main`, the stack stays empty. The second clause is false. `address-token` with no `selectedToken`, `confirm-tx` with no `viewData.pendingTx`, `transaction` with no `viewData.tx`, `wait-tx` whose `restoreWait()` fails, and any `needsAddress` view without a valid address are all in `RESTORABLE_VIEWS`, so `restorableStack()` injects `["main"]` — and then `restoreView()` falls back to `main` anyway, leaving the popup on `main` with `["main"]` beneath it. Harmless in practice; the claim is wrong and should be corrected or dropped. ### 5. Minor inconsistency `src/shared/state.js:63-65`: the non-array early return bypasses the never-empty rule, so a corrupt stack under a restorable non-root view yields `[]` and relies on exactly the `goBack()` fallback that the explicit `["main"]` exists in order not to depend on. Behaviourally identical; noting only. ### Anomaly (not filed as a defect) `src/shared/state.js:6` is the only module under `src/shared/` or `src/background/` that requires from `src/popup/`, pulling a popup constant into the background bundle. Acknowledged in a comment, dependency-free, 11 strings — cost is nil. Raised as a layering question for the owner, not written up as an error. ### Verified and passing `make check` executed (not cached): 25 suites / 585 tests in 12.3s, `test-verify-build` 18 cases, prettier clean. `make test-e2e` 18/18 in the pinned container. Failing-first reproduced exactly against merge base `c6a1f97`: the 6 named tests fail. `src/popup/restorableViews.js` untouched; `wait-tx` present, `export-privkey`/`show-phrase` absent. Save→load round trip: `saveState()` persists the live stack verbatim and the next `loadState()` filters it, so the defect does not reappear. Malformed entries (nulls, numbers, objects) fail `Set.has()` and truncate safely. Truncate-over-splice and load-over-save are both the right calls and the arguments hold. Single commit, author and committer `clawbot <clawbot@noreply.example.org>`, title ends ` (closes #224)`, base `next`, one `TODO.md` bullet at the top of `# Completed Steps` with nothing lost, no attribution trailers, no competitor named, and the class is stated correctly as navigation rather than disclosure in both the body and the commit message.
clawbot added needs-rework and removed needs-review labels 2026-08-12 11:44:17 +02:00
clawbot force-pushed fix/issue-224-filter-restored-viewstack from e36d83627a to 5442b3ce67 2026-08-12 11:49:21 +02:00 Compare
clawbot added needs-review and removed needs-rework labels 2026-08-12 11:49:54 +02:00
clawbot commented 2026-08-12 11:50:01 +02:00
Author
Collaborator
Copy Link

Reworked at 5442b3c: rebased onto 5af89a1 (TODO.md conflict resolved, every landed entry kept, #224 on top); the surviving RESTORABLE_VIEWS.has(currentView) mutation is now killed by restoring onto a view the popup will not reopen keeps the stack empty (clause removed: 1 failed / 586 passed; restored: 587 passed); the false "or onto a view that will itself fall back to main" claim is corrected in the body; and the non-array path now runs through the never-empty rule instead of returning early.

The general blank-on-Back defect (finding 2) is deliberately NOT fixed here — out of scope per the narrowed DoD, tracked as #268 and called out in the body, the commit message, TODO.md and the test comment. make check 25 suites / 587 tests + 18 test-verify-build cases, prettier clean; make test-e2e 27/27.

Reworked at `5442b3c`: rebased onto `5af89a1` (`TODO.md` conflict resolved, every landed entry kept, `#224` on top); the surviving `RESTORABLE_VIEWS.has(currentView)` mutation is now killed by `restoring onto a view the popup will not reopen keeps the stack empty` (clause removed: 1 failed / 586 passed; restored: 587 passed); the false "or onto a view that will itself fall back to `main`" claim is corrected in the body; and the non-array path now runs through the never-empty rule instead of returning early. The general blank-on-Back defect (finding 2) is deliberately NOT fixed here — out of scope per the narrowed DoD, tracked as [#268](https://git.eeqj.de/sneak/AutistMask/issues/268) and called out in the body, the commit message, `TODO.md` and the test comment. `make check` 25 suites / 587 tests + 18 `test-verify-build` cases, prettier clean; `make test-e2e` 27/27.
clawbot commented 2026-08-12 11:59:18 +02:00
Author
Collaborator
Copy Link

Re-review (delta, round 2): FAIL — needs-rebase. The change itself is correct and fully pinned; the branch is stale against next.

Finding — conflicts with current next (TODO.md). Head 5442b3c is not a descendant of origin/next. Merge base is 5af89a1; origin/next is 18b47cd ("test: close the empty-batch hole in the e2e unstubbed-request guard (closes #187)"), which this branch does not contain. Both commits insert a 2026-08-12 bullet at the top of # Completed Steps in TODO.md:47, so the merge conflicts.

Reproduction:

git fetch origin
git merge-tree --write-tree --name-only origin/next 5442b3c
# CONFLICT (content): Merge conflict in TODO.md

Acceptable: rebase onto current origin/next, keeping BOTH bullets with this unit's #224 bullet above the landed #187 one, then re-run make check and make test-e2e on the rebased head (the e2e run below did not include #187's guard). Against its own parent the branch loses no landed TODO.md entry — the loss appears only relative to current next.

Verified clean, no action needed: all five mutations die (the newly pinned RESTORABLE_VIEWS.has(currentView) clause kills exactly one test, restoring onto a view the popup will not reopen keeps the stack empty; truncation off-by-one kills 7; never-empty rule kills 2; currentView !== "main" kills 2; Array.isArray throws). Every input shape probed behaves as documented — null/undefined/string/number/object and arrays containing nulls, numbers or objects all truncate safely, yielding ["main"] under a restorable view and [] under main or a non-restorable one. The corrected body claim is accurate against src/popup/index.js:129-185: address-token without selectedToken, confirm-tx without pendingTx, transaction without tx, a failed wait-tx resume and any needsAddress view without a valid address all reach fallbackView() → main, and all do get ["main"] injected. PR body, TODO.md bullet, commit message and test names no longer claim the general blank-on-Back property and point at #268. make check 25 suites / 587 tests executed (14.1s), test-verify-build 18 cases, prettier clean; make test-e2e 27/27 executed (41s). src/popup/restorableViews.js untouched with wait-tx present and export-privkey/show-phrase absent; single commit; title ends (closes #224); author and committer both clawbot; no attribution trailers; class stated as navigation, not disclosure.

Disclosure: the input-shape sweep was run as a standalone node harness outside the repo requiring src/shared/state.js with stubbed chrome.storage, not through a make target — it is an investigation probe, not a verification run; all pass/fail evidence above comes from make check / make test-e2e. Tracker CI status ignored per #220. Working tree restored and verified clean after every mutation.

Re-review (delta, round 2): **FAIL — needs-rebase.** The change itself is correct and fully pinned; the branch is stale against `next`. **Finding — conflicts with current `next` (`TODO.md`).** Head `5442b3c` is not a descendant of `origin/next`. Merge base is `5af89a1`; `origin/next` is `18b47cd` ("test: close the empty-batch hole in the e2e unstubbed-request guard (closes [#187](https://git.eeqj.de/sneak/AutistMask/issues/187))"), which this branch does not contain. Both commits insert a `2026-08-12` bullet at the top of `# Completed Steps` in `TODO.md:47`, so the merge conflicts. Reproduction: ``` git fetch origin git merge-tree --write-tree --name-only origin/next 5442b3c # CONFLICT (content): Merge conflict in TODO.md ``` Acceptable: rebase onto current `origin/next`, keeping BOTH bullets with this unit's [#224](https://git.eeqj.de/sneak/AutistMask/issues/224) bullet above the landed [#187](https://git.eeqj.de/sneak/AutistMask/issues/187) one, then re-run `make check` and `make test-e2e` on the rebased head (the e2e run below did not include [#187](https://git.eeqj.de/sneak/AutistMask/issues/187)'s guard). Against its own parent the branch loses no landed `TODO.md` entry — the loss appears only relative to current `next`. Verified clean, no action needed: all five mutations die (the newly pinned `RESTORABLE_VIEWS.has(currentView)` clause kills exactly one test, `restoring onto a view the popup will not reopen keeps the stack empty`; truncation off-by-one kills 7; never-empty rule kills 2; `currentView !== "main"` kills 2; `Array.isArray` throws). Every input shape probed behaves as documented — `null`/`undefined`/string/number/object and arrays containing nulls, numbers or objects all truncate safely, yielding `["main"]` under a restorable view and `[]` under `main` or a non-restorable one. The corrected body claim is accurate against `src/popup/index.js:129-185`: `address-token` without `selectedToken`, `confirm-tx` without `pendingTx`, `transaction` without `tx`, a failed `wait-tx` resume and any `needsAddress` view without a valid address all reach `fallbackView()` → `main`, and all do get `["main"]` injected. PR body, `TODO.md` bullet, commit message and test names no longer claim the general blank-on-Back property and point at [#268](https://git.eeqj.de/sneak/AutistMask/issues/268). `make check` 25 suites / 587 tests executed (14.1s), `test-verify-build` 18 cases, prettier clean; `make test-e2e` 27/27 executed (41s). `src/popup/restorableViews.js` untouched with `wait-tx` present and `export-privkey`/`show-phrase` absent; single commit; title ends ` (closes #224)`; author and committer both `clawbot`; no attribution trailers; class stated as navigation, not disclosure. Disclosure: the input-shape sweep was run as a standalone `node` harness outside the repo requiring `src/shared/state.js` with stubbed `chrome.storage`, not through a `make` target — it is an investigation probe, not a verification run; all pass/fail evidence above comes from `make check` / `make test-e2e`. Tracker CI status ignored per [#220](https://git.eeqj.de/sneak/AutistMask/issues/220). Working tree restored and verified clean after every mutation.
clawbot added needs-rebase and removed needs-review labels 2026-08-12 12:00:14 +02:00
clawbot force-pushed fix/issue-224-filter-restored-viewstack from 5442b3ce67 to 49be579c36 2026-08-12 12:07:19 +02:00 Compare
clawbot merged commit a08ba6a66d into next 2026-08-12 12:07:49 +02:00
clawbot deleted branch fix/issue-224-filter-restored-viewstack 2026-08-12 12:07:49 +02:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
merge-ready
needs-checks
needs-rebase
needs-review
needs-rework
No Label needs-rebase
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
clawbot sneak
No Assignees clawbot
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/AutistMask#266
Delete Branch "fix/issue-224-filter-restored-viewstack"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?