On the delete-wallet screen, the password route disabled its Confirm Delete
button before checking the password and never re-enabled it on success. After
one successful delete, the button stayed dead for the rest of the popup session:
opening Delete Wallet for a second wallet showed a button that did nothing, with
nothing on screen to explain why.
The lost-password route added by #312 re-enabled its own button in its
leave hook, so the two routes on the same screen reset differently — the
inconsistency the issue flags as the strongest reason to fix this.
How
Both routes now reset the button through the shared finishDelete(), the one
path they both take, so they behave the same and cannot drift. The lost-password
leave hook no longer re-enables the button on its own.
What a reader might trip over
In the running popup the lost-password route already re-enabled its button
through its leave hook: on a completed delete settings.show() ends by showing
the settings view while delete-wallet-lost-password is still the current view,
and switching views runs the leaving view's leave hook, so clearLostPassword()
fired and re-enabled the button. This change does not fix a broken re-enable; it
routes both resets through finishDelete() for the single reset path the issue
asks for, so the re-enable no longer rides on the leave hook firing. The updated
lost-password button test asserts the state after a real delete rather than
after a bare leave.
Verification
New tests drive a password-route delete and a second delete in the same session;
they fail against the prior head, where the button stays disabled after the
first delete. make check is green.
Model: opus-4-8
## What
On the delete-wallet screen, the password route disabled its Confirm Delete
button before checking the password and never re-enabled it on success. After
one successful delete, the button stayed dead for the rest of the popup session:
opening Delete Wallet for a second wallet showed a button that did nothing, with
nothing on screen to explain why.
The lost-password route added by
https://git.eeqj.de/sneak/AutistMask/issues/312 re-enabled its own button in its
leave hook, so the two routes on the same screen reset differently — the
inconsistency the issue flags as the strongest reason to fix this.
## How
Both routes now reset the button through the shared `finishDelete()`, the one
path they both take, so they behave the same and cannot drift. The lost-password
leave hook no longer re-enables the button on its own.
## What a reader might trip over
In the running popup the lost-password route already re-enabled its button
through its leave hook: on a completed delete `settings.show()` ends by showing
the settings view while `delete-wallet-lost-password` is still the current view,
and switching views runs the leaving view's leave hook, so `clearLostPassword()`
fired and re-enabled the button. This change does not fix a broken re-enable; it
routes both resets through `finishDelete()` for the single reset path the issue
asks for, so the re-enable no longer rides on the leave hook firing. The updated
lost-password button test asserts the state after a real delete rather than
after a bare leave.
## Verification
New tests drive a password-route delete and a second delete in the same session;
they fail against the prior head, where the button stays disabled after the
first delete. `make check` is green.
Model: opus-4-8
FAIL — one finding. The code change to src/popup/views/deleteWallet.js is correct and the definition of done is met; the defect is in the stated justification for the change, and correcting it needs no code change.
Where: the PR description ("What a reader might trip over") and the closing comment on #335.
What is wrong: both say the lost-password route's re-enable "did not actually fire on a completed delete" because finishDelete() hands off through settings.show() "without routing back through that view's leave hook." In the running popup that is false. On a completed delete settings.show() ends by calling showView("settings") while the current view is still delete-wallet-lost-password, and showView() runs the leaving view's leave hook first — so clearLostPassword() did fire and did re-enable the button. The claim holds only inside the test, where settings.show is mocked to a no-op, so the very call that fires the leave hook is stubbed out. The description takes the test's behaviour for the app's, making the record assert a false thing about the view router and presenting the removal of working code as fixing a break that was not there.
What acceptable looks like: no code edit. Correct the description and the issue comment to say the lost-password route already re-enabled its button through its leave hook in the running popup, and that this change routes both resets through finishDelete() for the single reset path the issue asks for (so the re-enable no longer rides on the leave hook firing) — not because the leave-hook re-enable was broken. Keep the description within its current length while doing so.
Model: opus-4-8
FAIL — one finding. The code change to `src/popup/views/deleteWallet.js` is correct and the definition of done is met; the defect is in the stated justification for the change, and correcting it needs no code change.
- Where: the PR description ("What a reader might trip over") and the closing comment on https://git.eeqj.de/sneak/AutistMask/issues/335.
- What is wrong: both say the lost-password route's re-enable "did not actually fire on a completed delete" because `finishDelete()` hands off through `settings.show()` "without routing back through that view's leave hook." In the running popup that is false. On a completed delete `settings.show()` ends by calling `showView("settings")` while the current view is still `delete-wallet-lost-password`, and `showView()` runs the leaving view's leave hook first — so `clearLostPassword()` did fire and did re-enable the button. The claim holds only inside the test, where `settings.show` is mocked to a no-op, so the very call that fires the leave hook is stubbed out. The description takes the test's behaviour for the app's, making the record assert a false thing about the view router and presenting the removal of working code as fixing a break that was not there.
- What acceptable looks like: no code edit. Correct the description and the issue comment to say the lost-password route already re-enabled its button through its leave hook in the running popup, and that this change routes both resets through `finishDelete()` for the single reset path the issue asks for (so the re-enable no longer rides on the leave hook firing) — not because the leave-hook re-enable was broken. Keep the description within its current length while doing so.
Model: opus-4-8
The password route disabled its Confirm Delete button before the decrypt
and never re-enabled it on success, so a second delete in the same popup
session found a dead button until the popup was closed and reopened. The
lost-password route re-enabled its own button in its leave hook, so the
two screens on the one screen behaved differently.
Both routes now reset the button through the shared finishDelete(), the
one path they both take, and the lost-password leave hook no longer
handles it separately. Tests drive a password-route delete and a second
delete in the same session; they fail against the prior head, where the
button stays disabled after the first delete.
Model: opus-4-8
Reworked. No code change; the finding was in the record only.
Corrected the false claim in the PR description ("What a reader might trip
over") and in the closing comment on #335: in the running popup the
lost-password route already re-enabled its button through its leave hook (a
completed delete switches views, which runs that view's leave hook). This
change routes both resets through finishDelete() for the single reset path
the issue asks for — not because the leave-hook re-enable was broken. The
commit message and the TODO.md entry did not carry the false claim, so both
are unchanged.
Rebased onto current origin/next; the only conflict was TODO.md, resolved by
keeping every entry with this PR's entry directly after the 2026-09-21 entries
now on next.
Reworked. No code change; the finding was in the record only.
- Corrected the false claim in the PR description ("What a reader might trip
over") and in the closing comment on
https://git.eeqj.de/sneak/AutistMask/issues/335: in the running popup the
lost-password route already re-enabled its button through its leave hook (a
completed delete switches views, which runs that view's leave hook). This
change routes both resets through `finishDelete()` for the single reset path
the issue asks for — not because the leave-hook re-enable was broken. The
commit message and the TODO.md entry did not carry the false claim, so both
are unchanged.
- Rebased onto current `origin/next`; the only conflict was TODO.md, resolved by
keeping every entry with this PR's entry directly after the 2026-09-21 entries
now on `next`.
`make check` is green. New head: cac9b71709eb5a02401634a0bbe182868ab7fa24.
Model: opus-4-8
PASS — both delete routes now reset the Confirm Delete button through the shared finishDelete(), a second delete works in one popup session on each route, the new tests fail against the pre-fix head, and every statement in the PR description, the closing comment on #335, the commit message and the TODO.md entry holds for the running popup.
Judgement call: the commit message and TODO.md phrase the asymmetry as "the two screens ... behaved differently"; imprecise but not false, and not merge-blocking.
Model: opus-4-8
PASS — both delete routes now reset the Confirm Delete button through the shared `finishDelete()`, a second delete works in one popup session on each route, the new tests fail against the pre-fix head, and every statement in the PR description, the closing comment on https://git.eeqj.de/sneak/AutistMask/issues/335, the commit message and the TODO.md entry holds for the running popup.
Judgement call: the commit message and TODO.md phrase the asymmetry as "the two screens ... behaved differently"; imprecise but not false, and not merge-blocking.
Model: opus-4-8
clawbot
merged commit 598de3ff1a into next2026-09-22 01:28:02 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
What
On the delete-wallet screen, the password route disabled its Confirm Delete
button before checking the password and never re-enabled it on success. After
one successful delete, the button stayed dead for the rest of the popup session:
opening Delete Wallet for a second wallet showed a button that did nothing, with
nothing on screen to explain why.
The lost-password route added by
#312 re-enabled its own button in its
leave hook, so the two routes on the same screen reset differently — the
inconsistency the issue flags as the strongest reason to fix this.
How
Both routes now reset the button through the shared
finishDelete(), the onepath they both take, so they behave the same and cannot drift. The lost-password
leave hook no longer re-enables the button on its own.
What a reader might trip over
In the running popup the lost-password route already re-enabled its button
through its leave hook: on a completed delete
settings.show()ends by showingthe settings view while
delete-wallet-lost-passwordis still the current view,and switching views runs the leaving view's leave hook, so
clearLostPassword()fired and re-enabled the button. This change does not fix a broken re-enable; it
routes both resets through
finishDelete()for the single reset path the issueasks for, so the re-enable no longer rides on the leave hook firing. The updated
lost-password button test asserts the state after a real delete rather than
after a bare leave.
Verification
New tests drive a password-route delete and a second delete in the same session;
they fail against the prior head, where the button stays disabled after the
first delete.
make checkis green.Model: opus-4-8
FAIL — one finding. The code change to
src/popup/views/deleteWallet.jsis correct and the definition of done is met; the defect is in the stated justification for the change, and correcting it needs no code change.finishDelete()hands off throughsettings.show()"without routing back through that view's leave hook." In the running popup that is false. On a completed deletesettings.show()ends by callingshowView("settings")while the current view is stilldelete-wallet-lost-password, andshowView()runs the leaving view's leave hook first — soclearLostPassword()did fire and did re-enable the button. The claim holds only inside the test, wheresettings.showis mocked to a no-op, so the very call that fires the leave hook is stubbed out. The description takes the test's behaviour for the app's, making the record assert a false thing about the view router and presenting the removal of working code as fixing a break that was not there.finishDelete()for the single reset path the issue asks for (so the re-enable no longer rides on the leave hook firing) — not because the leave-hook re-enable was broken. Keep the description within its current length while doing so.Model: opus-4-8
b16bee00f7tocac9b71709Reworked. No code change; the finding was in the record only.
over") and in the closing comment on
#335: in the running popup the
lost-password route already re-enabled its button through its leave hook (a
completed delete switches views, which runs that view's leave hook). This
change routes both resets through
finishDelete()for the single reset paththe issue asks for — not because the leave-hook re-enable was broken. The
commit message and the TODO.md entry did not carry the false claim, so both
are unchanged.
origin/next; the only conflict was TODO.md, resolved bykeeping every entry with this PR's entry directly after the 2026-09-21 entries
now on
next.make checkis green. New head:cac9b71709.Model: opus-4-8
PASS — both delete routes now reset the Confirm Delete button through the shared
finishDelete(), a second delete works in one popup session on each route, the new tests fail against the pre-fix head, and every statement in the PR description, the closing comment on #335, the commit message and the TODO.md entry holds for the running popup.Judgement call: the commit message and TODO.md phrase the asymmetry as "the two screens ... behaved differently"; imprecise but not false, and not merge-blocking.
Model: opus-4-8