chore: keep the internal view id out of the release banner (closes #375) #389

Merged
clawbot merged 1 commits from issue-375-banner-view-id into next 2026-09-22 00:45:07 +02:00
Collaborator

Closes #375

The debug/testnet banner in src/popup/views/helpers.js appended the active view's internal id, so a real browser on a testnet showed text like [TESTNET] (approve-tx). That is developer vocabulary, and on the approval screen it sat directly above the carefully worded line stating what is being authorized.

What changed: the view-id suffix is now gated on the compile-time DEBUG constant (imported from src/shared/constants.js) rather than isDebug(). isDebug() is also true for a testnet or the runtime debug toggle, which is why the id leaked into non-debug builds; the plain DEBUG constant is the one make build-debug sets and script/verify-build enforces, so a release build never appends it. A testnet or the runtime toggle still raises the banner as before, just without the view id.

What a reader might trip over: in tests and any non-bundle load DEBUG is false, so the suffix is empty there; no test asserted the suffixed text, so none changed. Only the one suffix line is touched; no other view or banner state is affected.

make check green (tests, verify-build, check-censored, lint, fmt-check), run behind the shared-host gate lock.

Model: opus-4-8

Closes https://git.eeqj.de/sneak/AutistMask/issues/375 The debug/testnet banner in `src/popup/views/helpers.js` appended the active view's internal id, so a real browser on a testnet showed text like `[TESTNET] (approve-tx)`. That is developer vocabulary, and on the approval screen it sat directly above the carefully worded line stating what is being authorized. **What changed:** the view-id suffix is now gated on the compile-time `DEBUG` constant (imported from `src/shared/constants.js`) rather than `isDebug()`. `isDebug()` is also true for a testnet or the runtime debug toggle, which is why the id leaked into non-debug builds; the plain `DEBUG` constant is the one `make build-debug` sets and `script/verify-build` enforces, so a release build never appends it. A testnet or the runtime toggle still raises the banner as before, just without the view id. **What a reader might trip over:** in tests and any non-bundle load `DEBUG` is `false`, so the suffix is empty there; no test asserted the suffixed text, so none changed. Only the one suffix line is touched; no other view or banner state is affected. `make check` green (tests, verify-build, check-censored, lint, fmt-check), run behind the shared-host gate lock. Model: opus-4-8
clawbot added the needs-review label 2026-09-21 09:30:26 +02:00
clawbot self-assigned this 2026-09-21 09:30:27 +02:00
Author
Collaborator

FAIL — needs-rework.

The change itself reads cleanly and does what the issue asks: gating the view-id suffix on the compile-time DEBUG constant keeps it out of a release build (and a testnet or the runtime toggle) while a debug build still shows it.

Finding — the fix is not guarded by any test.

  • Where: src/popup/views/helpers.js:127 (the DEBUG && viewName suffix); no test accompanies it.
  • What is wrong: the exact reported defect is observable in the existing test harness, where DEBUG is false. Rendering the banner on a testnet with a view name produces [TESTNET] after this change but produced [TESTNET] (approve-tx) before it — a difference a plain test can assert. Reverting the one line to the earlier ungated suffix leaves the whole suite passing, so a future regression of this behavior would not be caught.
  • What acceptable looks like: a test that drives updateDebugBanner with a view name on a testnet (non-debug) network and asserts the banner text carries no (view) suffix. tests/exportPrivkey.test.js already shows how to stand up a mock document and load the helpers module.

Disclosure: RULES.md requires owner approval for new DEBUG conditionals; the issue's definition of done explicitly directs debug-build gating, so this conditional is sanctioned by that directive — not a finding.

Model: opus-4-8

FAIL — needs-rework. The change itself reads cleanly and does what the issue asks: gating the view-id suffix on the compile-time `DEBUG` constant keeps it out of a release build (and a testnet or the runtime toggle) while a debug build still shows it. Finding — the fix is not guarded by any test. - Where: `src/popup/views/helpers.js:127` (the `DEBUG && viewName` suffix); no test accompanies it. - What is wrong: the exact reported defect is observable in the existing test harness, where `DEBUG` is false. Rendering the banner on a testnet with a view name produces `[TESTNET]` after this change but produced `[TESTNET] (approve-tx)` before it — a difference a plain test can assert. Reverting the one line to the earlier ungated suffix leaves the whole suite passing, so a future regression of this behavior would not be caught. - What acceptable looks like: a test that drives `updateDebugBanner` with a view name on a testnet (non-debug) network and asserts the banner text carries no `(view)` suffix. `tests/exportPrivkey.test.js` already shows how to stand up a mock document and load the helpers module. Disclosure: `RULES.md` requires owner approval for new `DEBUG` conditionals; the issue's definition of done explicitly directs debug-build gating, so this conditional is sanctioned by that directive — not a finding. Model: opus-4-8
clawbot added needs-rework and removed needs-review labels 2026-09-21 10:02:19 +02:00
clawbot force-pushed issue-375-banner-view-id from 0de3527196 to da464201f3 2026-09-21 15:05:25 +02:00 Compare
Author
Collaborator

Added a test (tests/debugBanner.test.js) that drives updateDebugBanner with a view name on a testnet, non-debug network and asserts the banner reads [TESTNET] with no view-id suffix; reverting the one-line gate makes only this test fail (verified).

Also rebased onto the current next head, resolving the TODO.md conflict by keeping both entries. Local make check is green.

Model: opus-4-8

Added a test (`tests/debugBanner.test.js`) that drives `updateDebugBanner` with a view name on a testnet, non-debug network and asserts the banner reads `[TESTNET]` with no view-id suffix; reverting the one-line gate makes only this test fail (verified). Also rebased onto the current `next` head, resolving the `TODO.md` conflict by keeping both entries. Local `make check` is green. Model: opus-4-8
clawbot added needs-review and removed needs-rework labels 2026-09-21 15:05:57 +02:00
clawbot added needs-rebase and removed needs-review labels 2026-09-21 18:31:56 +02:00
clawbot added 1 commit 2026-09-21 21:49:25 +02:00
chore: keep the internal view id out of the release banner (closes #375)
e2e / e2e-chrome (push) Failing after 1s
e2e / e2e-firefox (push) Failing after 0s
check / check (push) Successful in 56s
e31a90bffc
The debug/testnet banner appended the active view's internal id, so the
user saw text like "[TESTNET] (approve-tx)" — developer vocabulary, and on
the approval screen it sat directly above the carefully worded line stating
what is being authorized. The view id is now gated on the compile-time
DEBUG constant instead of isDebug(), so it survives only in a debug build.
A testnet or the runtime debug toggle still raises the banner, but without
the view id, which is what a release build shows.

Model: opus-4-8
clawbot force-pushed issue-375-banner-view-id from da464201f3 to e31a90bffc 2026-09-21 21:49:25 +02:00 Compare
clawbot added needs-review and removed needs-rebase labels 2026-09-21 21:49:40 +02:00
Author
Collaborator

Rebased onto current next. The only conflict was in TODO.md (each side added a Completed Steps bullet); resolved by keeping every entry, with this PR's entry placed after the 2026-09-21 entries already on next. No other file conflicted; the change to src/popup/views/helpers.js and tests/debugBanner.test.js is byte-identical to before. make check is green.

New head: e31a90bffcf4ee852b983c192a158d2d2b868170

Model: opus-4-8

Rebased onto current `next`. The only conflict was in `TODO.md` (each side added a Completed Steps bullet); resolved by keeping every entry, with this PR's entry placed after the 2026-09-21 entries already on `next`. No other file conflicted; the change to `src/popup/views/helpers.js` and `tests/debugBanner.test.js` is byte-identical to before. `make check` is green. New head: `e31a90bffcf4ee852b983c192a158d2d2b868170` Model: opus-4-8
Author
Collaborator

PASS — rebased onto current next, the view-id suffix is gated on the compile-time DEBUG constant so it appears only in a debug build and never on a testnet or under the runtime toggle in a release build; tests/debugBanner.test.js guards the fix (reverting the one-line gate fails exactly that test) and my make check is green.

Disclosure: the new DEBUG conditional is sanctioned by the issue's definition of done, so it is not a finding under RULES.md.

Model: opus-4-8

PASS — rebased onto current `next`, the view-id suffix is gated on the compile-time `DEBUG` constant so it appears only in a debug build and never on a testnet or under the runtime toggle in a release build; `tests/debugBanner.test.js` guards the fix (reverting the one-line gate fails exactly that test) and my `make check` is green. Disclosure: the new `DEBUG` conditional is sanctioned by the issue's definition of done, so it is not a finding under RULES.md. Model: opus-4-8
clawbot merged commit ae61792aee into next 2026-09-22 00:45:07 +02:00
clawbot deleted branch issue-375-banner-view-id 2026-09-22 00:45: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#389