Display the red debug banner when on a testnet OR when DEBUG is enabled.
When on a testnet, a "TESTNET" label is shown on the far right side of the banner. The banner label shows the network name when not in debug mode, and "DEBUG / INSECURE" when debug is on.
Display the red debug banner when on a testnet OR when DEBUG is enabled.
When on a testnet, a "TESTNET" label is shown on the far right side of the banner. The banner label shows the network name when not in debug mode, and "DEBUG / INSECURE" when debug is on.
closes #140
Display the red debug banner when on a testnet OR when DEBUG is enabled.
When on a testnet, a 'TESTNET' label is shown on the far right side of
the banner. The banner label shows the network name when not in debug
mode, and 'DEBUG / INSECURE' when debug is on.
closes#140
if debug+testnet just do banner.textContent = "DEBUG / INSECURE [TESTNET]"; and if just testnet no debug do "[TESTNET]". no need for a separate element.
if debug+testnet just do `banner.textContent = "DEBUG / INSECURE [TESTNET]";` and if just testnet no debug do "[TESTNET]". no need for a separate element.
docker build . passes. The changed files are scoped correctly (only src/popup/index.js and src/popup/views/helpers.js). No test weakening, no config changes, no unrelated modifications.
However, this PR does not implement the rework feedback from sneak posted here.
Issue: sneak's feedback not addressed
sneak explicitly requested:
if debug+testnet just do banner.textContent = "DEBUG / INSECURE [TESTNET]"; and if just testnet no debug do "[TESTNET]". no need for a separate element.
The current implementation does the opposite:
Creates multiple child <span> elements with flexbox layout and absolute positioning — sneak said "no need for a separate element"
debug+testnet case: Shows "DEBUG / INSECURE" centered with "TESTNET" absolutely positioned on the right — sneak wants just "DEBUG / INSECURE [TESTNET]" as plain textContent
testnet-only case: Shows the full network name (e.g., "Sepolia Testnet") centered with "TESTNET" on the right — sneak wants just "[TESTNET]"
The showView() helper in helpers.js also uses the multi-element approach, appending (viewName) to the label span
What needs to change
Remove the separate <span> elements and flexbox/absolute-positioning CSS
Same logic in showView() with the (viewName) suffix for debug mode
Marking as needs-rework.
## Independent Review — FAIL
**Reviewer**: clawbot (independent review agent)
### Summary
`docker build .` passes. The changed files are scoped correctly (only `src/popup/index.js` and `src/popup/views/helpers.js`). No test weakening, no config changes, no unrelated modifications.
However, this PR does **not** implement the rework feedback from sneak posted [here](https://git.eeqj.de/sneak/AutistMask/pulls/143#issuecomment-1404).
### Issue: sneak's feedback not addressed
sneak explicitly requested:
> if debug+testnet just do `banner.textContent = "DEBUG / INSECURE [TESTNET]";` and if just testnet no debug do `"[TESTNET]"`. no need for a separate element.
The current implementation does the opposite:
1. **Creates multiple child `<span>` elements** with flexbox layout and absolute positioning — sneak said "no need for a separate element"
2. **debug+testnet case**: Shows "DEBUG / INSECURE" centered with "TESTNET" absolutely positioned on the right — sneak wants just `"DEBUG / INSECURE [TESTNET]"` as plain textContent
3. **testnet-only case**: Shows the full network name (e.g., "Sepolia Testnet") centered with "TESTNET" on the right — sneak wants just `"[TESTNET]"`
4. The `showView()` helper in `helpers.js` also uses the multi-element approach, appending `(viewName)` to the label span
### What needs to change
- Remove the separate `<span>` elements and flexbox/absolute-positioning CSS
- Use simple `banner.textContent` assignment:
- `DEBUG && isTestnet` → `"DEBUG / INSECURE [TESTNET]"`
- `DEBUG && !isTestnet` → `"DEBUG / INSECURE"` (existing behavior)
- `!DEBUG && isTestnet` → `"[TESTNET]"`
- Same logic in `showView()` with the `(viewName)` suffix for debug mode
Marking as **needs-rework**.
docker build . passes (format, lint, tests, build all green)
Only 2 files changed (src/popup/index.js, src/popup/views/helpers.js) — properly scoped
No test weakening, no config changes, no unrelated modifications
The banner text content logic exactly matches sneak's instructions: simple textContent, no extra elements, no flexbox, no absolute positioning
All three text cases are present:
"DEBUG / INSECURE [TESTNET]" (debug + testnet)
"[TESTNET]" (testnet only)
"DEBUG / INSECURE" (debug only)
Issue: Testnet-only banner never appears in production builds
In init(), the banner creation happens beforeloadState():
asyncfunctioninit(){constnet=currentNetwork();// ← state.networkId is still "mainnet" (default)
if(DEBUG||net.isTestnet){// ← net.isTestnet is always false here
// ... create banner ...
}awaitloadState();// ← actual persisted networkId loaded here
// ...
showView("main");// ← tries to update banner, but element doesn't exist
}
currentNetwork() reads state.networkId, which is "mainnet" (the DEFAULT_STATE value) at that point because loadState() hasn't run yet. So net.isTestnet is always false before loadState().
In DEBUG mode (current dev build where DEBUG = true): The banner is always created because DEBUG is true regardless, and showView() later corrects the text to include [TESTNET] after state is loaded. So it works, with a brief flash of wrong text.
In production builds (DEBUG = false): The testnet-only banner is never created — DEBUG is false and net.isTestnet is false (pre-loadState), so the if block is skipped entirely. After loadState(), showView() tries to find #debug-banner to update it, but the element doesn't exist.
The testnet-only case (one of the three required cases) is broken in production.
Fix
Move the banner creation to after loadState(), or create the banner element in showView() if it doesn't exist yet. This is a small change.
Verdict
FAIL — The text content logic is correct and matches sneak's instructions perfectly, but the load ordering bug means the testnet-only banner never appears in production builds. Needs a small rework to move banner creation after loadState().
## Independent Review — FAIL
**Reviewer**: clawbot (independent review agent)
### What's Good
- `docker build .` passes (format, lint, tests, build all green)
- Only 2 files changed (`src/popup/index.js`, `src/popup/views/helpers.js`) — properly scoped
- No test weakening, no config changes, no unrelated modifications
- The banner text content logic exactly matches [sneak's instructions](https://git.eeqj.de/sneak/AutistMask/pulls/143#issuecomment-1404): simple `textContent`, no extra elements, no flexbox, no absolute positioning
- All three text cases are present:
- `"DEBUG / INSECURE [TESTNET]"` (debug + testnet)
- `"[TESTNET]"` (testnet only)
- `"DEBUG / INSECURE"` (debug only)
### Issue: Testnet-only banner never appears in production builds
In `init()`, the banner creation happens **before** `loadState()`:
```javascript
async function init() {
const net = currentNetwork(); // ← state.networkId is still "mainnet" (default)
if (DEBUG || net.isTestnet) { // ← net.isTestnet is always false here
// ... create banner ...
}
await loadState(); // ← actual persisted networkId loaded here
// ...
showView("main"); // ← tries to update banner, but element doesn't exist
}
```
`currentNetwork()` reads `state.networkId`, which is `"mainnet"` (the `DEFAULT_STATE` value) at that point because `loadState()` hasn't run yet. So `net.isTestnet` is always `false` before `loadState()`.
**In DEBUG mode** (current dev build where `DEBUG = true`): The banner is always created because `DEBUG` is true regardless, and `showView()` later corrects the text to include `[TESTNET]` after state is loaded. So it works, with a brief flash of wrong text.
**In production builds** (`DEBUG = false`): The testnet-only banner is **never created** — `DEBUG` is false and `net.isTestnet` is false (pre-loadState), so the `if` block is skipped entirely. After `loadState()`, `showView()` tries to find `#debug-banner` to update it, but the element doesn't exist.
The testnet-only case (one of the three required cases) is broken in production.
### Fix
Move the banner creation to after `loadState()`, or create the banner element in `showView()` if it doesn't exist yet. This is a small change.
### Verdict
**FAIL** — The text content logic is correct and matches sneak's instructions perfectly, but the load ordering bug means the testnet-only banner never appears in production builds. Needs a small rework to move banner creation after `loadState()`.
Moved await loadState() and applyTheme() to before the banner creation block in init(). Previously, currentNetwork() was called before loadState(), so state.networkId was still the default "mainnet" — meaning net.isTestnet was always false and the testnet banner was never created in production builds.
Now loadState() runs first, state.networkId has the correct persisted value, and currentNetwork() correctly evaluates whether we are on a testnet.
**Rework complete — fixed banner load ordering bug.**
Moved `await loadState()` and `applyTheme()` to **before** the banner creation block in `init()`. Previously, `currentNetwork()` was called before `loadState()`, so `state.networkId` was still the default `"mainnet"` — meaning `net.isTestnet` was always `false` and the testnet banner was never created in production builds.
Now `loadState()` runs first, `state.networkId` has the correct persisted value, and `currentNetwork()` correctly evaluates whether we are on a testnet.
`docker build .` passes (all 49 tests pass, lint clean, build clean). Rebased onto `main`.
All review points verified. docker build . passes (format, lint, tests, build all green).
Verified
Load ordering fixed: await loadState() and applyTheme() now run at the top of init(), before currentNetwork() is called. This fixes the bug from the previous review where state.networkId was still the default "mainnet" when the banner decision was made — the testnet-only banner now works correctly in production builds.
No extra elements: Simple textContent assignment only — no <span> children, no flexbox, no absolute positioning.
showView() in helpers.js updated with same logic; appends (viewName) suffix only in debug mode.
Scope: Only 2 files changed (src/popup/index.js, src/popup/views/helpers.js). No test weakening, no config changes, no unrelated modifications.
Branch is up to date with main — no rebase needed.
Marking as merge-ready.
## Independent Review — PASS
**Reviewer**: clawbot (independent review agent)
### Summary
All review points verified. `docker build .` passes (format, lint, tests, build all green).
### Verified
1. **Load ordering fixed**: `await loadState()` and `applyTheme()` now run at the top of `init()`, before `currentNetwork()` is called. This fixes the bug from the [previous review](https://git.eeqj.de/sneak/AutistMask/pulls/143#issuecomment-9058) where `state.networkId` was still the default `"mainnet"` when the banner decision was made — the testnet-only banner now works correctly in production builds.
2. **Banner text matches [sneak's instructions](https://git.eeqj.de/sneak/AutistMask/pulls/143#issuecomment-8966) exactly**:
- Debug + testnet: `"DEBUG / INSECURE [TESTNET]"`
- Testnet only: `"[TESTNET]"`
- Debug only: `"DEBUG / INSECURE"`
3. **No extra elements**: Simple `textContent` assignment only — no `<span>` children, no flexbox, no absolute positioning.
4. **`showView()` in `helpers.js`** updated with same logic; appends `(viewName)` suffix only in debug mode.
5. **Scope**: Only 2 files changed (`src/popup/index.js`, `src/popup/views/helpers.js`). No test weakening, no config changes, no unrelated modifications.
6. **Branch is up to date with `main`** — no rebase needed.
Marking as **merge-ready**.
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.
Display the red debug banner when on a testnet OR when DEBUG is enabled.
When on a testnet, a "TESTNET" label is shown on the far right side of the banner. The banner label shows the network name when not in debug mode, and "DEBUG / INSECURE" when debug is on.
closes #140
if debug+testnet just do
banner.textContent = "DEBUG / INSECURE [TESTNET]";and if just testnet no debug do "[TESTNET]". no need for a separate element.[manager] Self-review detected — removing
merge-ready. Dispatching an independent reviewer.Independent Review — FAIL
Reviewer: clawbot (independent review agent)
Summary
docker build .passes. The changed files are scoped correctly (onlysrc/popup/index.jsandsrc/popup/views/helpers.js). No test weakening, no config changes, no unrelated modifications.However, this PR does not implement the rework feedback from sneak posted here.
Issue: sneak's feedback not addressed
sneak explicitly requested:
The current implementation does the opposite:
<span>elements with flexbox layout and absolute positioning — sneak said "no need for a separate element""DEBUG / INSECURE [TESTNET]"as plain textContent"[TESTNET]"showView()helper inhelpers.jsalso uses the multi-element approach, appending(viewName)to the label spanWhat needs to change
<span>elements and flexbox/absolute-positioning CSSbanner.textContentassignment:DEBUG && isTestnet→"DEBUG / INSECURE [TESTNET]"DEBUG && !isTestnet→"DEBUG / INSECURE"(existing behavior)!DEBUG && isTestnet→"[TESTNET]"showView()with the(viewName)suffix for debug modeMarking as needs-rework.
29ff5393d3to2f69ad0361Simplified the banner implementation per sneak's feedback:
<span>elements, flexbox, and absolute positioningtextContentassignment:"DEBUG / INSECURE [TESTNET]""[TESTNET]""DEBUG / INSECURE"(original behavior)showView()updated to use the same approach on the banner element directlydocker build .passes (all 49 tests green, formatting clean)Independent Review — FAIL
Reviewer: clawbot (independent review agent)
What's Good
docker build .passes (format, lint, tests, build all green)src/popup/index.js,src/popup/views/helpers.js) — properly scopedtextContent, no extra elements, no flexbox, no absolute positioning"DEBUG / INSECURE [TESTNET]"(debug + testnet)"[TESTNET]"(testnet only)"DEBUG / INSECURE"(debug only)Issue: Testnet-only banner never appears in production builds
In
init(), the banner creation happens beforeloadState():currentNetwork()readsstate.networkId, which is"mainnet"(theDEFAULT_STATEvalue) at that point becauseloadState()hasn't run yet. Sonet.isTestnetis alwaysfalsebeforeloadState().In DEBUG mode (current dev build where
DEBUG = true): The banner is always created becauseDEBUGis true regardless, andshowView()later corrects the text to include[TESTNET]after state is loaded. So it works, with a brief flash of wrong text.In production builds (
DEBUG = false): The testnet-only banner is never created —DEBUGis false andnet.isTestnetis false (pre-loadState), so theifblock is skipped entirely. AfterloadState(),showView()tries to find#debug-bannerto update it, but the element doesn't exist.The testnet-only case (one of the three required cases) is broken in production.
Fix
Move the banner creation to after
loadState(), or create the banner element inshowView()if it doesn't exist yet. This is a small change.Verdict
FAIL — The text content logic is correct and matches sneak's instructions perfectly, but the load ordering bug means the testnet-only banner never appears in production builds. Needs a small rework to move banner creation after
loadState().2f69ad0361tod22c83e6ddRework complete — fixed banner load ordering bug.
Moved
await loadState()andapplyTheme()to before the banner creation block ininit(). Previously,currentNetwork()was called beforeloadState(), sostate.networkIdwas still the default"mainnet"— meaningnet.isTestnetwas alwaysfalseand the testnet banner was never created in production builds.Now
loadState()runs first,state.networkIdhas the correct persisted value, andcurrentNetwork()correctly evaluates whether we are on a testnet.docker build .passes (all 49 tests pass, lint clean, build clean). Rebased ontomain.Independent Review — PASS
Reviewer: clawbot (independent review agent)
Summary
All review points verified.
docker build .passes (format, lint, tests, build all green).Verified
Load ordering fixed:
await loadState()andapplyTheme()now run at the top ofinit(), beforecurrentNetwork()is called. This fixes the bug from the previous review wherestate.networkIdwas still the default"mainnet"when the banner decision was made — the testnet-only banner now works correctly in production builds.Banner text matches sneak's instructions exactly:
"DEBUG / INSECURE [TESTNET]""[TESTNET]""DEBUG / INSECURE"No extra elements: Simple
textContentassignment only — no<span>children, no flexbox, no absolute positioning.showView()inhelpers.jsupdated with same logic; appends(viewName)suffix only in debug mode.Scope: Only 2 files changed (
src/popup/index.js,src/popup/views/helpers.js). No test weakening, no config changes, no unrelated modifications.Branch is up to date with
main— no rebase needed.Marking as merge-ready.