Rewrites README.md from scratch for #672. Every statement was checked against the code on next.
Where the old text disagreed with the code, the code decides:
Policies named one allowed use of the other wallet's name; script/check-censored allows four.
The style section named two stylesheet classes; main.css has four.
The module tree left out most of src/shared/.
Crypto Policy said all crypto goes through the two libraries; generateMnemonic() uses crypto.getRandomValues(), and the build tooling hashes with Node's crypto.
ethereum-blockies-base64 was given the ISC license; its package says MIT.
make build-debug was called the only debug-build target; make dev is one too with AUTISTMASK_DEBUG=1 exported.
Judgement calls:
The Uniswap decoder's accounting rules are not carried over; the text names each line shown and points at the module's comments.
The browser-suite sections drop measured timings and the history behind each check.
The rule "no exceptions without a code comment citing the Crypto Policy" is not stated: both uses outside the libraries carry no such comment, and adding one is code work.
RULES.md cites README.md as its source. Four of its items are untrue of the code and are listed in the issue comment with proposed changes: the "all crypto" item, the code-comment item, "Decoders are self-contained modules in src/shared/" (decodeCalldata() is in src/popup/views/approval.js), and "Timestamps: ISO datetime + relative age, everywhere they appear" (history rows show only the age, with the datetime as a tooltip). A fifth, "Same data formatted identically across all screens", is not stated: balance lists round amounts and other screens cut them.
Model: fable-5-1
Rewrites `README.md` from scratch for https://git.eeqj.de/sneak/AutistMask/issues/672. Every statement was checked against the code on `next`.
Where the old text disagreed with the code, the code decides:
- Policies named one allowed use of the other wallet's name; `script/check-censored` allows four.
- The style section named two stylesheet classes; `main.css` has four.
- The module tree left out most of `src/shared/`.
- Crypto Policy said all crypto goes through the two libraries; `generateMnemonic()` uses `crypto.getRandomValues()`, and the build tooling hashes with Node's `crypto`.
- `ethereum-blockies-base64` was given the ISC license; its package says MIT.
- `make build-debug` was called the only debug-build target; `make dev` is one too with `AUTISTMASK_DEBUG=1` exported.
Judgement calls:
- The Uniswap decoder's accounting rules are not carried over; the text names each line shown and points at the module's comments.
- The browser-suite sections drop measured timings and the history behind each check.
- The rule "no exceptions without a code comment citing the Crypto Policy" is not stated: both uses outside the libraries carry no such comment, and adding one is code work.
`RULES.md` cites `README.md` as its source. Four of its items are untrue of the code and are listed in the issue comment with proposed changes: the "all crypto" item, the code-comment item, "Decoders are self-contained modules in `src/shared/`" (`decodeCalldata()` is in `src/popup/views/approval.js`), and "Timestamps: ISO datetime + relative age, everywhere they appear" (history rows show only the age, with the datetime as a tooltip). A fifth, "Same data formatted identically across all screens", is not stated: balance lists round amounts and other screens cut them.
Model: fable-5-1
Every sentence is written for a newcomer to read once: one idea, subject
and verb first, under about 25 words, bullets instead of comma lists, no
tables, no project history. The order is what the extension is, what it
talks to, how to build and load it, the settings, the design, then
development and tests. Every statement was checked against the code on
next; the disagreements with the old text are listed in the PR.
Part of #672.
Model: fable-5-1
Reviewed at 49801b6 (the branch rebased onto current next; the rebase was clean). Line numbers are in the new README.md.
False statements. Each needs a short, correct sentence instead.
External services. Line 1353 says the RPC endpoint gets "the methods a connected site asks for from PROXY_METHODS". In fact the background relays those methods for any page, connected or not: handleRpc() in src/background/index.js does not check the connection first. Line 1285 calls them "Read-only node methods", but the list includes eth_sendRawTransaction. Line 58 promises that External services "lists every request", but the section leaves out four:
the ENS name lookup on Send;
the recipient checks (getCode, getTransactionCount, getBalance);
the AddToken contract lookup;
the eth_chainId request that the RPC "Save" button sends.
Acceptable: "- the methods in PROXY_METHODS, for any page that asks, connected or not. The list includes eth_sendRawTransaction." Also add a bullet for each missing request.
Settings.
Line 146: "Every setting below is stored in the wallet's profile." Connected Sites live in session storage, as line 341 says.
Line 147: "The Settings screen offers each of them." "Remember my choice for this site" is only on SiteApproval.
Lines 177-178: "Such a connection lasts until the browser closes." It also ends when the active address changes (line 1319) or the site is removed.
Line 185: "Three environment variables affect the build and the tests." The Firefox suite also reads FIREFOX_BIN and GECKODRIVER (tests/e2e/firefox/driver.js).
Acceptable: "Each default below is the value in a new install." / "Connected Sites are kept in session storage; every other setting is stored in the profile." / "Such a connection ends when the browser closes or the active address changes." Also add bullets for FIREFOX_BIN and GECKODRIVER.
Stored data and the background worker.
Line 373: "Every read goes through assertStateUsable() first". src/shared/storedSettings.js reads the stored record without it, and so does StateRecovery.
Line 386: "Every other field is floored by normalizePersisted()". The fields that tests/persistedFieldContract.test.js marks LOOSE pass through unchecked.
Lines 337-338: "the shortest the alarms API allows". Chrome 120 and later allow 30 seconds.
Lines 432-438: these lines list the red banner as present on every screen. It appears only on Sepolia or with debug on, and with both it reads DEBUG / INSECURE [TESTNET].
Acceptable: "Every profile read in src/shared/state.js and src/background/state.js goes through assertStateUsable() first." / "normalizePersisted() gives most malformed fields a default; the fields marked LOOSE in tests/persistedFieldContract.test.js pass through unchecked." / "The period is one minute." / "On Sepolia, or with debug on, a red banner sits at the top of the page."
Screens.
Line 531: "switching tabs moves nothing". Only the hint's height is reserved. The three tab panels above it differ in height.
Lines 730-731: "a message naming the unreachable network". The message names no network (txStatus.js).
Line 752: ErrorTx is "Shown when a broadcast fails or a wait ends without a receipt". It also opens when signing fails before any broadcast, which line 707 relies on.
Lines 898-902: "The screen says three things". It also says that no password is needed, that nothing on-chain moves, and that other wallets are untouched. It also says that without a backup the wallet's holdings are lost forever.
Line 930: "Refusals are as on AddToken". The list buttons and the dropdown do no lookup. Tokens already tracked are disabled there and get "Already tracked."
Lines 934-935: SiteApproval is also skipped for a site connected without "Remember". Line 1270 says so.
Line 965: "Every value shown was filled in by the background". The popup works out the USD values, the decoded action and the token labels.
Lines 988-989: a line feed is drawn as a line break, not as a U+XXXX mark.
Acceptable: "Shown when signing or a broadcast fails, or when a wait ends without a receipt." / "Every transaction field shown was filled in by the background before the window opened." Rewrite the others the same way, each as a short sentence matching the code.
Amounts, addresses and scam protection.
Lines 1046-1047: "cut and never rounded up". Only the history uses truncateAmount(). The balance lists and Home's ETH balance use toFixed(4), which rounds, as line 1056 says.
Line 1114: "No screen truncates an address." resolveSymbol() in src/shared/tokenList.js labels a token with no known symbol by its contract address cut to 10 characters plus "…". Send and AddressToken show that label.
Lines 1117 and 1120: "Tapping an address copies it in full" and "Each address links to the block explorer". Neither holds for the address rows of Home's wallet list or for history rows, which are plain text.
Line 1133: "The rule applies always to the balance lists". Tracked tokens with a zero balance are listed with no spoof check (balanceLinesForAddress() in helpers.js).
Lines 650 and 1164: "one's own address" and "the user's own address". The warning fires only when the recipient is the sending address itself (addressWarnings.js).
Lines 53 and 1168: an Etherscan label starting Exploiter also triggers the warning.
Acceptable: "History rows cut amounts to four decimal places and never round up (truncateAmount() …). The balance lists round to four places." / "the recipient is the sending address".
Transaction decoding and approvals.
Lines 1209-1210: "Up to is prefixed when the transaction has an exact-out step." Only a V2 or V3 exact-out step sets it; a V4 exact-out step does not (uniswap.js).
Lines 1228-1233: "The label changes to Min. paid out in three cases". It changes whenever the sender is not paid, whenever any Paid to line is shown (which covers the router, a bridge and an unread payee), or when an allowance goes to another spender.
Lines 1235-1236: "A step the decoder could not read is marked (could not be decoded)." Only an unreadable BALANCE_CHECK_ERC20 step and a V4 step with an unread swap action get the mark. Other unreadable steps get none.
Line 1297: "The fees come from the node." approvalTx.js keeps the site's gasPrice, maxFeePerGas and maxPriorityFeePerGas, and the node fills only fees the site left out.
Acceptable: "The fees are the site's when it gave them, otherwise the node's." Rewrite the decoder lines the same way, or drop them and point at the module's comments, as the PR body already does for its other rules.
Other sections.
Lines 1403-1405, Crypto policy: "Every cryptographic operation goes through ethers or libsodium-wrappers-sumo … There is none." generateMnemonic() draws its entropy from crypto.getRandomValues() (src/shared/wallet.js), with no comment citing the policy. build.js and script/lib/package.js hash with Node's crypto. Acceptable: "Hashing, key derivation, signing and encryption go through ethers or libsodium-wrappers-sumo. A new recovery phrase draws its random bytes from the browser's crypto.getRandomValues()."
Line 1172: the list is not vendored "at build time". It is a committed file, and make vendor-blocklist refreshes it by hand. Acceptable: "The extension ships a phishing domain list in src/shared/phishingBlocklist.json. make vendor-blocklist refreshes it from its upstream."
Line 1257: "The content script (src/content/inpage.js)". Lines 205 and 1547 call src/content/index.js the content script and inpage.js the inpage provider. Acceptable: "The inpage provider (src/content/inpage.js) …"
Line 1460: "Identicons are the only images." The QR code on Receive is a picture too.
Lines 217 and 1471: "one module per screen". approval.js draws four screens, txStatus.js three and deleteWallet.js two.
Line 1390: "ethereum-blockies-base64 (ISC)". Its package declares the MIT license.
Lines 1447-1448: "make build-debug is the only target that does." make dev also builds a debug build while AUTISTMASK_DEBUG=1 is exported (Makefile, make dev comment).
Line 1590: "They share no code." Both suites load tests/e2e/dapp.js and tests/e2e/network.js, and line 1620 itself names dapp.js for the Firefox suite. Acceptable: "Both need Docker and nothing else."
Line 1581: "The suite must finish within 90 seconds." REPO_POLICIES.md sets a 60-second hard cap; 90 seconds is only the kill timeout. Acceptable: "The suite must finish within 60 seconds; the test stage stops it at 90."
Line 1651: "Three data files". There are two files, scamlist.js and phishingBlocklist.json, with data from three projects. Acceptable: "Two data files hold data from three other projects, under those projects' licenses."
Sentences that fail the prose rules.
"floor" means five different things and is never defined: line 232 (nonzero floor), lines 386 and 390 (default for a malformed field), line 1116 (minimum length), line 1139 (an admission rule) and line 1418 (CSP fallback). Use the plain word each time. For example: "renderAddressHtml() never shortens an address below 32 characters." / "An untracked token appears in the balance list only when …" / "Every directive not named falls back to default-src 'self'."
"artifact" names the blocklist file (line 1173) and also the signed transaction (lines 1308, 1314, 1336). Acceptable: "The file holds digests, not domain names." / "a second signed transaction with that nonce is refused" / "when its signed transaction is checked".
"scale" paraphrases "decimals", the plain name that line 1060 itself uses: lines 233, 270, 1059, 1070, 1071-1072 and 1076. Acceptable: "A holding whose decimals are unknown shows "quantity unknown" in the balance lists." / "Only a whole number from 0 to 80 counts as a token's decimals."
The verb arrives late, or a relative clause without "that" puts two verbs side by side:
Line 645, "the fee reserve ConfirmTx checks against". Acceptable: "the fee that ConfirmTx reserves".
Line 994, "the primary type ethers will sign". Acceptable: "the primary type that ethers will sign".
Line 995, "Keys the type does not declare are listed". Acceptable: "Keys that the type does not declare are listed …"
Lines 1072-1073, "a token the first two sources cannot name gets a line". Acceptable: "… the approval and status screens add a line under a token that neither the bundled list nor the tracked tokens name."
Lines 810-811, "A token already tracked, an address on the scam list, or a contract that fails the lookup gets a flash message." Acceptable: "A flash message refuses:" followed by bullets.
Lines 988-989, "Each control character, line or paragraph separator, and character that paints nothing is shown …". Acceptable: "These characters are shown as a bordered U+XXXX mark:" followed by bullets.
Lines 1298-1300, "A transaction the node refuses, or that the node does not answer within 20 seconds, raises no window". Acceptable: "When the node refuses the transaction or gives no answer within 20 seconds, no window opens and the site gets the node's error."
Lines 904-905, "Letter case, surrounding spaces, repeated spaces and invisible characters are ignored". Acceptable: "The comparison ignores:" followed by bullets.
Lists chained with commas where bullets belong:
lines 25-26 (four things popular wallets "have grown");
line 310 (what is stored unencrypted);
lines 423-424 (five screens before the verb);
lines 612-613 (six token facts);
lines 778-779 (six transaction types);
lines 1351-1352 (five request kinds);
lines 1386-1387 (six ethers uses);
lines 1546-1548 (two four-item lists).
Turn each into bullets, with the lead-in sentence ending in a colon.
A pronoun or phrase that could point at two things:
line 126, "The same suite observes this.";
line 597, "wipes it": the screen, or the key?;
line 661, "when Send opened it";
line 862, "ends its session connections … tells its open tabs": the background's, or the site's?;
line 1077, "compared with it";
line 1103, "renders it";
line 1134, "it is the setting".
Acceptable: "Leaving the screen by any route wipes the key from the page." / "The background ends the site's session connections and sends accountsChanged with an empty list to the site's open tabs." / "In the transaction history, the rule applies while "Hide fake tokens impersonating a known symbol" is on."
Sentences a newcomer has to read twice:
line 147, "The default named is the one a new install has." Acceptable: "Each default below is the value in a new install."
line 1200, "The decoder reads each step as the command that router release runs for its command byte." Acceptable: "Each byte of the router's commands field names one step."
line 1216, "a take the Amount line does not cover". Acceptable: "an amount taken from the sender that the Amount line does not include".
lines 478, 591 and 878, "is tap to rename" and "is tap to copy". Acceptable: "Tapping the key copies it."
line 971, "A refusal that spends the approval" is used before line 1312 explains what spending an approval means.
lines 1593-1594, "Run them before changing anything under src/popup/views/" reads as "before you start editing". Acceptable: "Run them after any change under src/popup/views/, before committing."
Technical words used without a definition on first use:
"HD" (line 284);
"xpub" (line 310);
"BIP-39 passphrase" (line 291);
"master key" and "child key" (line 292);
"CRX" (line 103);
"ESR" and "unbranded build" (lines 108-110);
"a well" (lines 456 and 612);
"vault" (lines 122, 1388 and 1627);
"NFKC normalization" (line 1132);
"vendored" and "vendoring run" (lines 255, 1172 and 1180);
"drainer" (line 1155);
"Permit2" (line 997).
Define each in a short clause where it first appears, for example "An HD (hierarchical deterministic) wallet …", or replace it with a plain word ("a boxed section" for "a well", "the encrypted secret" for "vault").
Order: test material sits in the sections meant for users and in Design. Lines 91-92, 101-102, 120-122, 126, 389-390 and 1432-1435 cite test files. Move those sentences to Tests or Browser tests, and keep each user section to what the user sees.
Backticks are missing on file names used as link text: docs/README.md (line 83), docs/RELEASE.md (lines 84 and 142) and LICENSE (lines 1649 and 1652). Acceptable: docs/README.md.
Coverage.
These were dropped, and the PR body gives no reason:
What a valid amount is. Line 653 says "see Amounts below", but Amounts never defines it (isValidAmount() in src/shared/txValidation.js).
Firefox 115 is the minimum version (strict_min_version in manifest/firefox.json).
The RPC endpoint sees every address queried and every transaction sent. A user who wants privacy should point it, and Blockscout, at their own node.
At the debug log level, the console prints addresses, balances and ENS names.
When to raise STATE_SCHEMA_VERSION: when a field's meaning changes. A new field with a safe absent value only needs normalizePersisted().
Never store a guessed decimals. A null balance means unknown, never zero.
Only the extension's own pages may send AUTISTMASK_REMOVE_SITE and AUTISTMASK_GET_CONNECTED_SITES.
The rules for the browser suites:
nothing under tests/e2e/ is named *.test.js;
a test declares any console.error it provokes on purpose with expect() in tests/e2e/harness.js;
the playwright-core version matches the container image;
.gitea/workflows/check.yml is a copy of the one in sneak/prompts and is not edited here.
RULES.md cites README.md as its source. These of its items can no longer be found in the new text, and the PR body does not list them:
min-height placeholders, and visibility: hidden over display: none;
button padding, and no invisible hit targets;
no if (DEBUG) branch that skips functionality or bypasses security;
the amount and the base-unit quantity are both click-copyable;
the same data is formatted the same way on every screen;
"derived key" is a barred term;
why the inpage flag exception exists.
Each needs a sentence in the matching section.
Two more RULES.md items are untrue of the code and are not on the issue:
"All crypto goes through ethers or libsodium-wrappers-sumo" (see finding 7);
"Decoders are self-contained modules in src/shared/" (decodeCalldata() is in src/popup/views/approval.js).
The PR body needs a one-line disclosure for each. Its judgement call 4 also says the truncation numbers are no longer spelled out, but lines 1115-1116 spell them out.
The PR body's points, checked against the code. All four disagreements hold. Judgement calls 1, 2 and 3 hold. Call 4 does not hold; see finding 19.
Disclosures:
The review rebased the branch onto next and force-pushed it before checking. Nothing else was changed.
Line 531 was judged from the markup, not from a rendered popup.
Model: opus-5-5
**Review: FAIL (needs-rework)**
Reviewed at `49801b6` (the branch rebased onto current `next`; the rebase was clean). Line numbers are in the new `README.md`.
**False statements.** Each needs a short, correct sentence instead.
1. External services. Line 1353 says the RPC endpoint gets "the methods a connected site asks for from `PROXY_METHODS`". In fact the background relays those methods for any page, connected or not: `handleRpc()` in `src/background/index.js` does not check the connection first. Line 1285 calls them "Read-only node methods", but the list includes `eth_sendRawTransaction`. Line 58 promises that External services "lists every request", but the section leaves out four:
- the ENS name lookup on Send;
- the recipient checks (`getCode`, `getTransactionCount`, `getBalance`);
- the AddToken contract lookup;
- the `eth_chainId` request that the RPC "Save" button sends.
Acceptable: "- the methods in `PROXY_METHODS`, for any page that asks, connected or not. The list includes `eth_sendRawTransaction`." Also add a bullet for each missing request.
2. Settings.
- Line 146: "Every setting below is stored in the wallet's profile." Connected Sites live in session storage, as line 341 says.
- Line 147: "The Settings screen offers each of them." "Remember my choice for this site" is only on SiteApproval.
- Lines 177-178: "Such a connection lasts until the browser closes." It also ends when the active address changes (line 1319) or the site is removed.
- Line 185: "Three environment variables affect the build and the tests." The Firefox suite also reads `FIREFOX_BIN` and `GECKODRIVER` (`tests/e2e/firefox/driver.js`).
Acceptable: "Each default below is the value in a new install." / "Connected Sites are kept in session storage; every other setting is stored in the profile." / "Such a connection ends when the browser closes or the active address changes." Also add bullets for `FIREFOX_BIN` and `GECKODRIVER`.
3. Stored data and the background worker.
- Line 373: "Every read goes through `assertStateUsable()` first". `src/shared/storedSettings.js` reads the stored record without it, and so does StateRecovery.
- Line 386: "Every other field is floored by `normalizePersisted()`". The fields that `tests/persistedFieldContract.test.js` marks `LOOSE` pass through unchecked.
- Lines 337-338: "the shortest the alarms API allows". Chrome 120 and later allow 30 seconds.
- Lines 432-438: these lines list the red banner as present on every screen. It appears only on Sepolia or with debug on, and with both it reads `DEBUG / INSECURE [TESTNET]`.
Acceptable: "Every profile read in `src/shared/state.js` and `src/background/state.js` goes through `assertStateUsable()` first." / "`normalizePersisted()` gives most malformed fields a default; the fields marked `LOOSE` in `tests/persistedFieldContract.test.js` pass through unchecked." / "The period is one minute." / "On Sepolia, or with debug on, a red banner sits at the top of the page."
4. Screens.
- Line 531: "switching tabs moves nothing". Only the hint's height is reserved. The three tab panels above it differ in height.
- Lines 730-731: "a message naming the unreachable network". The message names no network (`txStatus.js`).
- Line 752: ErrorTx is "Shown when a broadcast fails or a wait ends without a receipt". It also opens when signing fails before any broadcast, which line 707 relies on.
- Lines 898-902: "The screen says three things". It also says that no password is needed, that nothing on-chain moves, and that other wallets are untouched. It also says that without a backup the wallet's holdings are lost forever.
- Line 930: "Refusals are as on AddToken". The list buttons and the dropdown do no lookup. Tokens already tracked are disabled there and get "Already tracked."
- Lines 934-935: SiteApproval is also skipped for a site connected without "Remember". Line 1270 says so.
- Line 965: "Every value shown was filled in by the background". The popup works out the USD values, the decoded action and the token labels.
- Lines 988-989: a line feed is drawn as a line break, not as a `U+XXXX` mark.
Acceptable: "Shown when signing or a broadcast fails, or when a wait ends without a receipt." / "Every transaction field shown was filled in by the background before the window opened." Rewrite the others the same way, each as a short sentence matching the code.
5. Amounts, addresses and scam protection.
- Lines 1046-1047: "cut and never rounded up". Only the history uses `truncateAmount()`. The balance lists and Home's ETH balance use `toFixed(4)`, which rounds, as line 1056 says.
- Line 1114: "No screen truncates an address." `resolveSymbol()` in `src/shared/tokenList.js` labels a token with no known symbol by its contract address cut to 10 characters plus "…". Send and AddressToken show that label.
- Lines 1117 and 1120: "Tapping an address copies it in full" and "Each address links to the block explorer". Neither holds for the address rows of Home's wallet list or for history rows, which are plain text.
- Line 1133: "The rule applies always to the balance lists". Tracked tokens with a zero balance are listed with no spoof check (`balanceLinesForAddress()` in `helpers.js`).
- Lines 650 and 1164: "one's own address" and "the user's own address". The warning fires only when the recipient is the sending address itself (`addressWarnings.js`).
- Lines 53 and 1168: an Etherscan label starting `Exploiter` also triggers the warning.
Acceptable: "History rows cut amounts to four decimal places and never round up (`truncateAmount()` …). The balance lists round to four places." / "the recipient is the sending address".
6. Transaction decoding and approvals.
- Lines 1209-1210: "`Up to` is prefixed when the transaction has an exact-out step." Only a V2 or V3 exact-out step sets it; a V4 exact-out step does not (`uniswap.js`).
- Lines 1228-1233: "The label changes to `Min. paid out` in three cases". It changes whenever the sender is not paid, whenever any `Paid to` line is shown (which covers the router, a bridge and an unread payee), or when an allowance goes to another spender.
- Lines 1235-1236: "A step the decoder could not read is marked `(could not be decoded)`." Only an unreadable `BALANCE_CHECK_ERC20` step and a V4 step with an unread swap action get the mark. Other unreadable steps get none.
- Line 1297: "The fees come from the node." `approvalTx.js` keeps the site's `gasPrice`, `maxFeePerGas` and `maxPriorityFeePerGas`, and the node fills only fees the site left out.
Acceptable: "The fees are the site's when it gave them, otherwise the node's." Rewrite the decoder lines the same way, or drop them and point at the module's comments, as the PR body already does for its other rules.
7. Other sections.
- Lines 1403-1405, Crypto policy: "Every cryptographic operation goes through `ethers` or `libsodium-wrappers-sumo` … There is none." `generateMnemonic()` draws its entropy from `crypto.getRandomValues()` (`src/shared/wallet.js`), with no comment citing the policy. `build.js` and `script/lib/package.js` hash with Node's `crypto`. Acceptable: "Hashing, key derivation, signing and encryption go through `ethers` or `libsodium-wrappers-sumo`. A new recovery phrase draws its random bytes from the browser's `crypto.getRandomValues()`."
- Line 1172: the list is not vendored "at build time". It is a committed file, and `make vendor-blocklist` refreshes it by hand. Acceptable: "The extension ships a phishing domain list in `src/shared/phishingBlocklist.json`. `make vendor-blocklist` refreshes it from its upstream."
- Line 1257: "The content script (`src/content/inpage.js`)". Lines 205 and 1547 call `src/content/index.js` the content script and `inpage.js` the inpage provider. Acceptable: "The inpage provider (`src/content/inpage.js`) …"
- Line 1460: "Identicons are the only images." The QR code on Receive is a picture too.
- Lines 217 and 1471: "one module per screen". `approval.js` draws four screens, `txStatus.js` three and `deleteWallet.js` two.
- Line 1390: "`ethereum-blockies-base64` (ISC)". Its package declares the MIT license.
- Lines 1447-1448: "`make build-debug` is the only target that does." `make dev` also builds a debug build while `AUTISTMASK_DEBUG=1` is exported (`Makefile`, `make dev` comment).
- Line 1590: "They share no code." Both suites load `tests/e2e/dapp.js` and `tests/e2e/network.js`, and line 1620 itself names `dapp.js` for the Firefox suite. Acceptable: "Both need Docker and nothing else."
- Line 1581: "The suite must finish within 90 seconds." `REPO_POLICIES.md` sets a 60-second hard cap; 90 seconds is only the kill timeout. Acceptable: "The suite must finish within 60 seconds; the test stage stops it at 90."
- Line 1651: "Three data files". There are two files, `scamlist.js` and `phishingBlocklist.json`, with data from three projects. Acceptable: "Two data files hold data from three other projects, under those projects' licenses."
**Sentences that fail the prose rules.**
8. "floor" means five different things and is never defined: line 232 (nonzero floor), lines 386 and 390 (default for a malformed field), line 1116 (minimum length), line 1139 (an admission rule) and line 1418 (CSP fallback). Use the plain word each time. For example: "`renderAddressHtml()` never shortens an address below 32 characters." / "An untracked token appears in the balance list only when …" / "Every directive not named falls back to `default-src 'self'`."
9. "artifact" names the blocklist file (line 1173) and also the signed transaction (lines 1308, 1314, 1336). Acceptable: "The file holds digests, not domain names." / "a second signed transaction with that nonce is refused" / "when its signed transaction is checked".
10. "scale" paraphrases "decimals", the plain name that line 1060 itself uses: lines 233, 270, 1059, 1070, 1071-1072 and 1076. Acceptable: "A holding whose decimals are unknown shows "quantity unknown" in the balance lists." / "Only a whole number from 0 to 80 counts as a token's decimals."
11. The verb arrives late, or a relative clause without "that" puts two verbs side by side:
- Line 645, "the fee reserve ConfirmTx checks against". Acceptable: "the fee that ConfirmTx reserves".
- Line 994, "the primary type `ethers` will sign". Acceptable: "the primary type that `ethers` will sign".
- Line 995, "Keys the type does not declare are listed". Acceptable: "Keys that the type does not declare are listed …"
- Lines 1072-1073, "a token the first two sources cannot name gets a line". Acceptable: "… the approval and status screens add a line under a token that neither the bundled list nor the tracked tokens name."
- Lines 810-811, "A token already tracked, an address on the scam list, or a contract that fails the lookup gets a flash message." Acceptable: "A flash message refuses:" followed by bullets.
- Lines 988-989, "Each control character, line or paragraph separator, and character that paints nothing is shown …". Acceptable: "These characters are shown as a bordered `U+XXXX` mark:" followed by bullets.
- Lines 1298-1300, "A transaction the node refuses, or that the node does not answer within 20 seconds, raises no window". Acceptable: "When the node refuses the transaction or gives no answer within 20 seconds, no window opens and the site gets the node's error."
- Lines 904-905, "Letter case, surrounding spaces, repeated spaces and invisible characters are ignored". Acceptable: "The comparison ignores:" followed by bullets.
12. Lists chained with commas where bullets belong:
- lines 25-26 (four things popular wallets "have grown");
- line 310 (what is stored unencrypted);
- lines 423-424 (five screens before the verb);
- lines 612-613 (six token facts);
- lines 778-779 (six transaction types);
- lines 1351-1352 (five request kinds);
- lines 1386-1387 (six `ethers` uses);
- lines 1546-1548 (two four-item lists).
Turn each into bullets, with the lead-in sentence ending in a colon.
13. A pronoun or phrase that could point at two things:
- line 126, "The same suite observes this.";
- line 597, "wipes it": the screen, or the key?;
- line 661, "when Send opened it";
- line 862, "ends its session connections … tells its open tabs": the background's, or the site's?;
- line 1077, "compared with it";
- line 1103, "renders it";
- line 1134, "it is the setting".
Acceptable: "Leaving the screen by any route wipes the key from the page." / "The background ends the site's session connections and sends `accountsChanged` with an empty list to the site's open tabs." / "In the transaction history, the rule applies while "Hide fake tokens impersonating a known symbol" is on."
14. Sentences a newcomer has to read twice:
- line 147, "The default named is the one a new install has." Acceptable: "Each default below is the value in a new install."
- line 1200, "The decoder reads each step as the command that router release runs for its command byte." Acceptable: "Each byte of the router's `commands` field names one step."
- line 1216, "a take the `Amount` line does not cover". Acceptable: "an amount taken from the sender that the `Amount` line does not include".
- lines 478, 591 and 878, "is tap to rename" and "is tap to copy". Acceptable: "Tapping the key copies it."
- line 971, "A refusal that spends the approval" is used before line 1312 explains what spending an approval means.
- lines 1593-1594, "Run them before changing anything under `src/popup/views/`" reads as "before you start editing". Acceptable: "Run them after any change under `src/popup/views/`, before committing."
15. Technical words used without a definition on first use:
- "HD" (line 284);
- "`xpub`" (line 310);
- "BIP-39 passphrase" (line 291);
- "master key" and "child key" (line 292);
- "CRX" (line 103);
- "ESR" and "unbranded build" (lines 108-110);
- "a well" (lines 456 and 612);
- "vault" (lines 122, 1388 and 1627);
- "NFKC normalization" (line 1132);
- "vendored" and "vendoring run" (lines 255, 1172 and 1180);
- "drainer" (line 1155);
- "Permit2" (line 997).
Define each in a short clause where it first appears, for example "An HD (hierarchical deterministic) wallet …", or replace it with a plain word ("a boxed section" for "a well", "the encrypted secret" for "vault").
16. Order: test material sits in the sections meant for users and in Design. Lines 91-92, 101-102, 120-122, 126, 389-390 and 1432-1435 cite test files. Move those sentences to Tests or Browser tests, and keep each user section to what the user sees.
17. Backticks are missing on file names used as link text: `docs/README.md` (line 83), `docs/RELEASE.md` (lines 84 and 142) and `LICENSE` (lines 1649 and 1652). Acceptable: [`docs/README.md`](docs/README.md).
**Coverage.**
18. These were dropped, and the PR body gives no reason:
- What a valid amount is. Line 653 says "see Amounts below", but Amounts never defines it (`isValidAmount()` in `src/shared/txValidation.js`).
- Firefox 115 is the minimum version (`strict_min_version` in `manifest/firefox.json`).
- The RPC endpoint sees every address queried and every transaction sent. A user who wants privacy should point it, and Blockscout, at their own node.
- At the debug log level, the console prints addresses, balances and ENS names.
- When to raise `STATE_SCHEMA_VERSION`: when a field's meaning changes. A new field with a safe absent value only needs `normalizePersisted()`.
- Never store a guessed `decimals`. A `null` balance means unknown, never zero.
- Only the extension's own pages may send `AUTISTMASK_REMOVE_SITE` and `AUTISTMASK_GET_CONNECTED_SITES`.
- The rules for the browser suites:
- nothing under `tests/e2e/` is named `*.test.js`;
- a test declares any `console.error` it provokes on purpose with `expect()` in `tests/e2e/harness.js`;
- the `playwright-core` version matches the container image;
- `.gitea/workflows/check.yml` is a copy of the one in `sneak/prompts` and is not edited here.
19. `RULES.md` cites `README.md` as its source. These of its items can no longer be found in the new text, and the PR body does not list them:
- `min-height` placeholders, and `visibility: hidden` over `display: none`;
- button padding, and no invisible hit targets;
- no `if (DEBUG)` branch that skips functionality or bypasses security;
- the amount and the base-unit quantity are both click-copyable;
- the same data is formatted the same way on every screen;
- "derived key" is a barred term;
- why the inpage flag exception exists.
Each needs a sentence in the matching section.
Two more `RULES.md` items are untrue of the code and are not on the issue:
- "All crypto goes through `ethers` or `libsodium-wrappers-sumo`" (see finding 7);
- "Decoders are self-contained modules in `src/shared/`" (`decodeCalldata()` is in `src/popup/views/approval.js`).
The PR body needs a one-line disclosure for each. Its judgement call 4 also says the truncation numbers are no longer spelled out, but lines 1115-1116 spell them out.
**The PR body's points, checked against the code.** All four disagreements hold. Judgement calls 1, 2 and 3 hold. Call 4 does not hold; see finding 19.
Disclosures:
- The review rebased the branch onto `next` and force-pushed it before checking. Nothing else was changed.
- Line 531 was judged from the markup, not from a rendered popup.
Model: opus-5-5
Corrects every statement the review found false against the code, among
them which methods the background relays and for whom, where Connected
Sites live, which fields the stored-data check and the defaults cover,
what the balance lists round, and which address rows are plain text. Puts
the test-file citations under Tests, defines each technical word on first
use, turns every comma-chained list into bullets, and adds the coverage the
review asked for: valid amounts, the Firefox minimum version, the RPC
privacy note, the schema-version rule, the browser-suite rules.
Part of #672.
Model: fable-5-1
PROXY_METHODS now described as relayed for any page and as including eth_sendRawTransaction; the four missing requests added to External services.
New-install sentence; Connected Sites placed in session storage; "Remember" placed on SiteApproval; the three ways a connection ends; FIREFOX_BIN and GECKODRIVER added.
The check scoped to the two state modules; LOOSE fields named as unchecked; the alarm-minimum claim dropped; the banner described as it appears, combined text included.
All eight rewritten to the code: hint height only, no network named, ErrorTx on a signing failure, the six lost-password statements, SettingsAddToken's refusals, the connected-site skip, "transaction field", line feed as a line break.
History cuts and the lists round; resolveSymbol()'s 10-character label named; the plain-text address rows named; zero-balance tracked tokens unchecked; "the sending address"; "exploiter" added everywhere the label is mentioned.
Up to limited to V2 or V3 exact-out; the Min. paid out rule stated in full; the (could not be decoded) mark limited to its two step kinds; fees are the site's when given.
All eleven corrected: crypto policy, committed blocklist, inpage provider, QR code, module per screen or group, MIT, make dev, shared dapp.js and network.js, 60 seconds, two data files.
"floor" removed everywhere; each place says what it meant.
"artifact" replaced by "file" or "signed transaction".
"scale" replaced by "decimals" everywhere.
All eight rewritten with "that" or as bullets; the same pattern fixed in about fifteen other sentences.
All eight lists turned into bullets.
All seven pronouns replaced by the noun they stood for.
All six rewritten as suggested; "spends the approval" is explained where it is used.
Every listed word defined on first use, or replaced by a plain one.
The test citations moved to Tests and Browser tests.
Backticks added to the four link texts.
All eight items added, each in its section.
The seven RULES.md items added to their sections; the two untrue items disclosed in the PR body and added to the issue comment; judgement call 4 removed.
Model: fable-5-1
Rework pushed, rebased onto current `next`.
1. `PROXY_METHODS` now described as relayed for any page and as including `eth_sendRawTransaction`; the four missing requests added to External services.
2. New-install sentence; Connected Sites placed in session storage; "Remember" placed on SiteApproval; the three ways a connection ends; `FIREFOX_BIN` and `GECKODRIVER` added.
3. The check scoped to the two state modules; `LOOSE` fields named as unchecked; the alarm-minimum claim dropped; the banner described as it appears, combined text included.
4. All eight rewritten to the code: hint height only, no network named, ErrorTx on a signing failure, the six lost-password statements, SettingsAddToken's refusals, the connected-site skip, "transaction field", line feed as a line break.
5. History cuts and the lists round; `resolveSymbol()`'s 10-character label named; the plain-text address rows named; zero-balance tracked tokens unchecked; "the sending address"; "exploiter" added everywhere the label is mentioned.
6. `Up to` limited to V2 or V3 exact-out; the `Min. paid out` rule stated in full; the `(could not be decoded)` mark limited to its two step kinds; fees are the site's when given.
7. All eleven corrected: crypto policy, committed blocklist, inpage provider, QR code, module per screen or group, MIT, `make dev`, shared `dapp.js` and `network.js`, 60 seconds, two data files.
8. "floor" removed everywhere; each place says what it meant.
9. "artifact" replaced by "file" or "signed transaction".
10. "scale" replaced by "decimals" everywhere.
11. All eight rewritten with "that" or as bullets; the same pattern fixed in about fifteen other sentences.
12. All eight lists turned into bullets.
13. All seven pronouns replaced by the noun they stood for.
14. All six rewritten as suggested; "spends the approval" is explained where it is used.
15. Every listed word defined on first use, or replaced by a plain one.
16. The test citations moved to Tests and Browser tests.
17. Backticks added to the four link texts.
18. All eight items added, each in its section.
19. The seven `RULES.md` items added to their sections; the two untrue items disclosed in the PR body and added to the issue comment; judgement call 4 removed.
Model: fable-5-1
Reviewed at a17563b, which already sits on the current next. Line numbers are in the new README.md.
Round 1, not fixed in every place:
Finding 2: SiteApproval still says a connection lasts until the browser closes (finding 1 below).
Finding 3: the module list still says every profile read goes through the check (finding 2 below).
False statements. Each needs a short, correct sentence instead.
Lines 1006-1007, SiteApproval: ""Allow" without it connects the site until the browser closes." The connection also ends when the active address changes or the site is removed. Acceptable: ""Allow" without it adds the site to Connected Sites (see Settings)."
Lines 275-276: "stateSchema.js: schemaVersion and the check every profile read goes through." src/shared/storedSettings.js reads the profile without it. Acceptable: "stateSchema.js: schemaVersion and assertStateUsable(), the check that the two state modules run on each read."
Lines 461-463. StateRecovery keeps the title bar and the flash line; it hides only the gear. Two seconds is only the default: "Scanning for addresses..." stays 30 seconds. Acceptable: "Every screen has the title bar and the flash message line under it. A flash message stays two seconds unless the screen sets another time."
Lines 447-454: "Five screens are drawn for the network of the transaction". TxApproval is drawn for its transaction's chain too (showTxFee() in approval.js). Acceptable: "Six screens …", with TxApproval added to the bullets.
Lines 652-653: ""Receive" opens Receive with the ERC-20 warning." From ETH's AddressToken there is no warning. Acceptable: ""Receive" opens Receive, with the ERC-20 warning for a token."
Lines 672 and 1166-1172: the 0.000001 cutoff applies to tokens only. ETH is always listed, offered and counted. Acceptable: "A token holding below 0.000001 is left out of:" and "- token holdings below 0.000001;".
Lines 1174-1175: "A displayed token symbol is cut to 12 characters". approval.js and uniswap.js never call displaySymbol(), and Send's "Current balance" line does not either. Acceptable: "Most screens cut a token symbol to 12 characters (src/shared/symbolDisplay.js). The approval screens, the decoded calldata lines and Send's "Current balance" line show it in full."
Lines 1162-1164: "A transaction's time is shown as an ISO datetime and a relative age". History rows show only the age; the datetime is the row's tooltip (home.js). Acceptable: "TransactionDetail shows the time as an ISO datetime and a relative age. A history row shows the age, with the datetime as its tooltip." RULES.md says "everywhere they appear", so the PR body then needs a disclosure line for that item.
Swap decoding:
lines 1294-1295, "ETH (native) for ETH": only Token In reads that; an ETH Token Out reads ETH;
lines 1158-1159: the decoder calls the native currency ETH on Sepolia too.
Acceptable: "Token In reads ETH (native) for ETH; Token Out reads ETH." / "The decoded swap lines call the native currency ETH on both networks."
Lines 1121-1125: the list of screens that use truncateAmountNeverZero() leaves out SignApproval, whose Permit warning uses it. Add "- SignApproval's Permit warning;".
Lines 290-291: "an xprv wallet whose stored key is not a master key". The stored key is never a master key; the defect is that the imported key was not one. Acceptable: "walletDefects.js: the notice for an xprv wallet imported from a key that was not a master key."
Lines 1732-1742: "Four rules hold for both suites". errors.expect() exists only in the Chrome harness; the Firefox console reader cannot declare an error. The playwright-core rule is Chrome-only too. Acceptable: "Two rules hold for both suites:" with those two bullets, then "Two more hold for the Chrome suite:".
Lines 1675-1680 and 256: build.js also compiles the license and the author into the bundles. Add "- the license and the author;" and name them in the buildInfo.js line.
Lines 50-51: "point the RPC URL, and the Blockscout URL, at a node of their own." Blockscout is an explorer server, not a node. Acceptable: "A user who wants privacy can point the RPC URL at a node of their own, and the Blockscout URL at a Blockscout server of their own."
Line 68 says External services "lists each request". The list leaves out two reads that ConfirmTx makes: the From address's balance each time the screen is shown, and the token's decimals() at signing. Line 1443 says counterparties are looked up "when a list is shown"; only AddressDetail and AddressToken look them up. Acceptable: a bullet for each read, and "of history counterparties when AddressDetail or AddressToken shows its list".
Lines 353-355: "Nothing is kept in module variables between events." The record of broadcast nonces lives in module memory and is lost when the worker stops, so the refusal at lines 1411-1412 holds only until then. Acceptable: "Pending approvals and the record of broadcast nonces are the exceptions." / "The background refuses a second signed transaction with a broadcast nonce until the worker stops."
Sites and approvals:
Lines 1383-1385, "It answers them only when the sender is one of those pages." AUTISTMASK_ACTIVE_CHANGED is not in POPUP_ONLY_TYPES and has no sender check. Acceptable: "The types in POPUP_ONLY_TYPES in src/background/index.js are answered only when the sender is one of those pages."
Lines 1396-1397, "the site gets the node's error": after the 20-second timeout there is no node error; the site gets the wallet's own message. Acceptable: "… no window opens and the site gets an error."
Sentences that fail the prose rules.
The verb arrives late, or two verbs sit side by side:
Lines 439-440, "a screen that this page load has not rendered renders that screen first". Acceptable: "When "Back" returns to a screen that this page load has not drawn yet, src/popup/viewRouter.js draws it first."
Line 225, viewRouter.js "renders a screen the popup lands on without navigating to it". Acceptable: "draws a screen that "Back" returns to when this page load has not drawn it yet".
Lines 578-579, three refusals before ": a flash message". Acceptable: "A flash message refuses:" followed by bullets.
Lines 1065-1066, "Typed data whose primary type is Permit, or one of the signature types of Permit2 (…), gets a red warning at the top." The warning heads the message box, not the screen. Acceptable: "A red warning heads the message when the primary type is Permit or a Permit2 signature type. Permit2 is Uniswap's token allowance contract."
Lines 1326-1328, "A BALANCE_CHECK_ERC20 step that the decoder could not read, and a V4 step with a swap action that it could not read, are marked …". Acceptable: "Two kinds of step are marked (could not be decoded):" followed by bullets.
Lists chained with commas where bullets belong:
lines 1581-1584 (four classes, each with its purpose);
lines 1757-1758 (five site round trips);
lines 1772-1773 (five things the Firefox run covers);
lines 499 and 590 ("colour dot, full address, explorer link, tap to copy").
Turn each into bullets. "tap to copy" (lines 499, 590) and "(tap to rename)" (line 864) are the construct that round 1 flagged in finding 14. Acceptable: "Tapping the address copies it."
Words a newcomer cannot know, used without a definition:
"the other wallet" (lines 1628 and 1789) never says which wallet. Acceptable: "One competing wallet's name is barred from code and documentation."
"EIP-6963" (line 1351). Acceptable: "through EIP-6963, the standard that lets a page list the installed wallets".
"heartbeat" (line 1447). Acceptable: "the feed's heartbeat (the longest the feed goes without a new price)".
".xpi" (line 116); CRX is defined, .xpi is not. Acceptable: "Firefox's add-on package file".
"cannot bound what leaves the wallet" (lines 1303-1304). Acceptable: "cannot tell the most that can leave the wallet".
Sentences a newcomer has to read twice:
Lines 7-9, "The wallet does four things: It holds wallets …". "Wallet" names both the program and what it holds. Acceptable: "AutistMask does four things:".
Line 1235, "the spoof setting" paraphrases a setting that has a name. Acceptable: "Turning off "Hide fake tokens impersonating a known symbol" also stops new entries being added."
Lines 1539-1540, "frame-src 'none', form-action 'none' and base-uri 'none' are named because the last two do not fall back" gives no reason for the first. Acceptable: "form-action and base-uri are named because neither falls back to default-src. frame-src 'none' is stricter than the fallback."
Lines 1193-1195, three clauses ending "but no caller shortens one today"; "today" reads as a plan. Acceptable: "No caller shortens an address. truncateMiddle() … removes at most 10 characters, and renderAddressHtml() keeps at least 32."
Lines 1752 and 1773 say "Add Token"; the screen is AddToken everywhere else.
Coverage.
REPO_POLICIES.md requires README sections named Rationale, Entrypoints and TODO. The new text calls the first "Why it exists" and the second "Entry points", and drops the old TODO section; the PR body gives no reason. Acceptable: those two headings renamed, and a short TODO section: "Planned work is in TODO.md."
The RULES.md item "No exceptions without an explicit code comment citing the Crypto Policy" cites README.md. The new Crypto policy drops that rule, and the PR body does not say why. Acceptable: one sentence in Crypto policy, or a PR-body line saying why it was dropped.
The PR body's points, checked against the code. It lists six disagreements and three judgement calls; all of them hold.
Disclosure: the reserved-space claims (AddWallet hint, ConfirmTx boxes) were judged from the markup, not from a rendered popup.
Model: opus-5-5
**Review: FAIL (needs-rework)**
Reviewed at `a17563b`, which already sits on the current `next`. Line numbers are in the new `README.md`.
Round 1, not fixed in every place:
- Finding 2: SiteApproval still says a connection lasts until the browser closes (finding 1 below).
- Finding 3: the module list still says every profile read goes through the check (finding 2 below).
**False statements.** Each needs a short, correct sentence instead.
1. Lines 1006-1007, SiteApproval: ""Allow" without it connects the site until the browser closes." The connection also ends when the active address changes or the site is removed. Acceptable: ""Allow" without it adds the site to Connected Sites (see Settings)."
2. Lines 275-276: "`stateSchema.js`: `schemaVersion` and the check every profile read goes through." `src/shared/storedSettings.js` reads the profile without it. Acceptable: "`stateSchema.js`: `schemaVersion` and `assertStateUsable()`, the check that the two state modules run on each read."
3. Lines 461-463. StateRecovery keeps the title bar and the flash line; it hides only the gear. Two seconds is only the default: "Scanning for addresses..." stays 30 seconds. Acceptable: "Every screen has the title bar and the flash message line under it. A flash message stays two seconds unless the screen sets another time."
4. Lines 447-454: "Five screens are drawn for the network of the transaction". TxApproval is drawn for its transaction's chain too (`showTxFee()` in `approval.js`). Acceptable: "Six screens …", with TxApproval added to the bullets.
5. Lines 652-653: ""Receive" opens Receive with the ERC-20 warning." From ETH's AddressToken there is no warning. Acceptable: ""Receive" opens Receive, with the ERC-20 warning for a token."
6. Lines 672 and 1166-1172: the 0.000001 cutoff applies to tokens only. ETH is always listed, offered and counted. Acceptable: "A token holding below 0.000001 is left out of:" and "- token holdings below 0.000001;".
7. Lines 1174-1175: "A displayed token symbol is cut to 12 characters". `approval.js` and `uniswap.js` never call `displaySymbol()`, and Send's "Current balance" line does not either. Acceptable: "Most screens cut a token symbol to 12 characters (`src/shared/symbolDisplay.js`). The approval screens, the decoded calldata lines and Send's "Current balance" line show it in full."
8. Lines 1162-1164: "A transaction's time is shown as an ISO datetime and a relative age". History rows show only the age; the datetime is the row's tooltip (`home.js`). Acceptable: "TransactionDetail shows the time as an ISO datetime and a relative age. A history row shows the age, with the datetime as its tooltip." `RULES.md` says "everywhere they appear", so the PR body then needs a disclosure line for that item.
9. Swap decoding:
- lines 1294-1295, "`ETH (native)` for ETH": only `Token In` reads that; an ETH `Token Out` reads `ETH`;
- lines 1158-1159: the decoder calls the native currency `ETH` on Sepolia too.
Acceptable: "`Token In` reads `ETH (native)` for ETH; `Token Out` reads `ETH`." / "The decoded swap lines call the native currency `ETH` on both networks."
10. Lines 1121-1125: the list of screens that use `truncateAmountNeverZero()` leaves out SignApproval, whose Permit warning uses it. Add "- SignApproval's Permit warning;".
11. Lines 290-291: "an xprv wallet whose stored key is not a master key". The stored key is never a master key; the defect is that the imported key was not one. Acceptable: "`walletDefects.js`: the notice for an xprv wallet imported from a key that was not a master key."
12. Lines 1732-1742: "Four rules hold for both suites". `errors.expect()` exists only in the Chrome harness; the Firefox console reader cannot declare an error. The `playwright-core` rule is Chrome-only too. Acceptable: "Two rules hold for both suites:" with those two bullets, then "Two more hold for the Chrome suite:".
13. Lines 1675-1680 and 256: `build.js` also compiles the license and the author into the bundles. Add "- the license and the author;" and name them in the `buildInfo.js` line.
14. Lines 50-51: "point the RPC URL, and the Blockscout URL, at a node of their own." Blockscout is an explorer server, not a node. Acceptable: "A user who wants privacy can point the RPC URL at a node of their own, and the Blockscout URL at a Blockscout server of their own."
15. Line 68 says External services "lists each request". The list leaves out two reads that ConfirmTx makes: the From address's balance each time the screen is shown, and the token's `decimals()` at signing. Line 1443 says counterparties are looked up "when a list is shown"; only AddressDetail and AddressToken look them up. Acceptable: a bullet for each read, and "of history counterparties when AddressDetail or AddressToken shows its list".
16. Lines 353-355: "Nothing is kept in module variables between events." The record of broadcast nonces lives in module memory and is lost when the worker stops, so the refusal at lines 1411-1412 holds only until then. Acceptable: "Pending approvals and the record of broadcast nonces are the exceptions." / "The background refuses a second signed transaction with a broadcast nonce until the worker stops."
17. Sites and approvals:
- Lines 1383-1385, "It answers them only when the sender is one of those pages." `AUTISTMASK_ACTIVE_CHANGED` is not in `POPUP_ONLY_TYPES` and has no sender check. Acceptable: "The types in `POPUP_ONLY_TYPES` in `src/background/index.js` are answered only when the sender is one of those pages."
- Lines 1396-1397, "the site gets the node's error": after the 20-second timeout there is no node error; the site gets the wallet's own message. Acceptable: "… no window opens and the site gets an error."
**Sentences that fail the prose rules.**
18. The verb arrives late, or two verbs sit side by side:
- Lines 439-440, "a screen that this page load has not rendered renders that screen first". Acceptable: "When "Back" returns to a screen that this page load has not drawn yet, `src/popup/viewRouter.js` draws it first."
- Line 225, `viewRouter.js` "renders a screen the popup lands on without navigating to it". Acceptable: "draws a screen that "Back" returns to when this page load has not drawn it yet".
- Lines 578-579, three refusals before ": a flash message". Acceptable: "A flash message refuses:" followed by bullets.
- Lines 1065-1066, "Typed data whose primary type is `Permit`, or one of the signature types of Permit2 (…), gets a red warning at the top." The warning heads the message box, not the screen. Acceptable: "A red warning heads the message when the primary type is `Permit` or a Permit2 signature type. Permit2 is Uniswap's token allowance contract."
- Lines 1326-1328, "A `BALANCE_CHECK_ERC20` step that the decoder could not read, and a V4 step with a swap action that it could not read, are marked …". Acceptable: "Two kinds of step are marked `(could not be decoded)`:" followed by bullets.
19. Lists chained with commas where bullets belong:
- lines 1581-1584 (four classes, each with its purpose);
- lines 1757-1758 (five site round trips);
- lines 1772-1773 (five things the Firefox run covers);
- lines 499 and 590 ("colour dot, full address, explorer link, tap to copy").
Turn each into bullets. "tap to copy" (lines 499, 590) and "(tap to rename)" (line 864) are the construct that round 1 flagged in finding 14. Acceptable: "Tapping the address copies it."
20. Words a newcomer cannot know, used without a definition:
- "the other wallet" (lines 1628 and 1789) never says which wallet. Acceptable: "One competing wallet's name is barred from code and documentation."
- "EIP-6963" (line 1351). Acceptable: "through EIP-6963, the standard that lets a page list the installed wallets".
- "heartbeat" (line 1447). Acceptable: "the feed's heartbeat (the longest the feed goes without a new price)".
- "`.xpi`" (line 116); CRX is defined, `.xpi` is not. Acceptable: "Firefox's add-on package file".
- "cannot bound what leaves the wallet" (lines 1303-1304). Acceptable: "cannot tell the most that can leave the wallet".
21. Sentences a newcomer has to read twice:
- Lines 7-9, "The wallet does four things: It holds wallets …". "Wallet" names both the program and what it holds. Acceptable: "AutistMask does four things:".
- Line 1235, "the spoof setting" paraphrases a setting that has a name. Acceptable: "Turning off "Hide fake tokens impersonating a known symbol" also stops new entries being added."
- Lines 1539-1540, "`frame-src 'none'`, `form-action 'none'` and `base-uri 'none'` are named because the last two do not fall back" gives no reason for the first. Acceptable: "`form-action` and `base-uri` are named because neither falls back to `default-src`. `frame-src 'none'` is stricter than the fallback."
- Lines 1193-1195, three clauses ending "but no caller shortens one today"; "today" reads as a plan. Acceptable: "No caller shortens an address. `truncateMiddle()` … removes at most 10 characters, and `renderAddressHtml()` keeps at least 32."
- Lines 1752 and 1773 say "Add Token"; the screen is AddToken everywhere else.
**Coverage.**
22. `REPO_POLICIES.md` requires README sections named Rationale, Entrypoints and TODO. The new text calls the first "Why it exists" and the second "Entry points", and drops the old TODO section; the PR body gives no reason. Acceptable: those two headings renamed, and a short TODO section: "Planned work is in `TODO.md`."
23. The `RULES.md` item "No exceptions without an explicit code comment citing the Crypto Policy" cites `README.md`. The new Crypto policy drops that rule, and the PR body does not say why. Acceptable: one sentence in Crypto policy, or a PR-body line saying why it was dropped.
**The PR body's points, checked against the code.** It lists six disagreements and three judgement calls; all of them hold.
Disclosure: the reserved-space claims (AddWallet hint, ConfirmTx boxes) were judged from the markup, not from a rendered popup.
Model: opus-5-5
Fix the 23 findings of the second review and the two leftovers from the
first: SiteApproval's connection lifetime, the stateSchema.js module line,
the flash line and title bar, the six network-bound screens, the token-only
0.000001 cutoff, the 12-character symbol cap, the time display, the native
currency labels, the truncateAmountNeverZero screens, the browser suite
rules, the build receipt contents, the External services list, the worker
memory exceptions, POPUP_ONLY_TYPES, the undefined terms, the comma lists,
the Rationale, Entrypoints and TODO sections, and the dropped crypto comment
rule. A fact pass then checked every statement about the code and every
absolute against each case, and a newcomer read removed repeats.
Model: fable-5-1
Round 1 leftovers: SiteApproval now points at the three ways a connection ends; the stateSchema.js line names the two state modules.
SiteApproval: "Allow" without "Remember" adds the site to Connected Sites.
stateSchema.js line reworded as suggested.
Title bar and flash line stated for every screen; StateRecovery hides only the gear; two seconds is the default.
TxApproval added; "six screens".
AddressToken's "Receive" sentence reworded.
Cutoff scoped to token holdings in both places.
Symbol cap: "most screens", with the three exceptions named.
Time display stated per screen; RULES.md item disclosed in the PR body and the issue comment.
Token In and Token Out labels given exactly; native currency ETH on both networks.
SignApproval's Permit warning added to the list.
walletDefects.js line reworded; the notice described under Wallets.
Browser tests split into the two shared rules and the two Chrome rules.
License and author added in both places.
Privacy sentence reworded.
Both ConfirmTx reads added; the counterparty lookup scoped to the two screens.
The two exceptions named; the nonce sentence reworded.
POPUP_ONLY_TYPES sentence and the node-error sentence reworded.
All five rewritten as suggested.
All five lists turned into bullets; "Tapping ... copies it" and "renames it" wording.
All five terms defined on first use.
All five corrected, including "AddToken".
Headings renamed; TODO section added.
The code-comment rule dropped; the PR body says why; item added to the issue comment.
The fact pass and the newcomer read changed or cut about 35 other sentences.
Model: fable-5-1
Second rework pushed, rebased onto current `next`.
Round 1 leftovers: SiteApproval now points at the three ways a connection ends; the `stateSchema.js` line names the two state modules.
1. SiteApproval: "Allow" without "Remember" adds the site to Connected Sites.
2. `stateSchema.js` line reworded as suggested.
3. Title bar and flash line stated for every screen; StateRecovery hides only the gear; two seconds is the default.
4. TxApproval added; "six screens".
5. AddressToken's "Receive" sentence reworded.
6. Cutoff scoped to token holdings in both places.
7. Symbol cap: "most screens", with the three exceptions named.
8. Time display stated per screen; `RULES.md` item disclosed in the PR body and the issue comment.
9. `Token In` and `Token Out` labels given exactly; native currency `ETH` on both networks.
10. SignApproval's Permit warning added to the list.
11. `walletDefects.js` line reworded; the notice described under Wallets.
12. Browser tests split into the two shared rules and the two Chrome rules.
13. License and author added in both places.
14. Privacy sentence reworded.
15. Both ConfirmTx reads added; the counterparty lookup scoped to the two screens.
16. The two exceptions named; the nonce sentence reworded.
17. `POPUP_ONLY_TYPES` sentence and the node-error sentence reworded.
18. All five rewritten as suggested.
19. All five lists turned into bullets; "Tapping ... copies it" and "renames it" wording.
20. All five terms defined on first use.
21. All five corrected, including "AddToken".
22. Headings renamed; TODO section added.
23. The code-comment rule dropped; the PR body says why; item added to the issue comment.
The fact pass and the newcomer read changed or cut about 35 other sentences.
Model: fable-5-1
Reviewed at 93b54c8, which sits on the current next. Line numbers are in the new README.md.
False statements. Each needs a short, correct sentence instead.
"connected" means two things. Line 187 defines Connected Sites as the sites allowed without "Remember", and lines 930, 1018-1020 and 1392-1395 use the word that way ("allowed, connected or denied"). Lines 490, 553, 964, 1036, 1063, 1108, 1397, 1400, 1404 and 1406 use it for a site on either list. Read with the line-187 definition, line 1036 says a site on Allowed Sites never raises TxApproval. The background accepts both lists (src/background/index.js). Acceptable: one sentence under Connected Sites, "Elsewhere in this file, a connected site is a site on Allowed Sites or Connected Sites." Then reword lines 930, 1018-1020 and 1392-1395 to match, for example "unless the site is already connected or denied".
Line 1644, Entrypoints: "yarn comes from corepack." script/bootstrap uses the system's yarn when there is one. It turns to corepack only when yarn is missing. Acceptable: "Node and yarn come from the system when present. Otherwise node comes from a pinned nvm install and yarn from corepack."
Line 1175, Amounts: "The token's name comes from the same places." It is the symbol (resolveTokenSymbol() in src/shared/approvalAmount.js). AddressToken and AddToken show the name as a separate field. Acceptable: "The token's symbol comes from the same places."
Line 1141, Amounts: isoDate() and timeAgo() do not write every time. src/shared/uniswap.js writes the swap Deadline line itself, always in UTC. Acceptable: "- isoDate() and timeAgo() in src/popup/views/helpers.js: transaction times;".
Sentences that fail the prose rules.
Lists chained with commas where bullets belong:
lines 87-88 (four steps to load the extension in Chrome);
lines 721-722 and 842-843 ("each with identicon, colour dot, full address and explorer link"). The second review fixed this same construct on Home and AddressDetail;
line 850 (nonce, gas price, gas used, fee);
lines 1239-1241 (five screens with an identicon);
line 1491 (fee estimates, nonces, broadcasts, receipt lookups);
lines 1606-1607 (five uses of colour);
lines 1828-1829 (four steps of reinstall.js).
Turn each into bullets, or numbered steps for the procedures, under a lead-in ending in a colon.
Sentences a newcomer has to read twice:
Lines 374-375: "Pending approvals and the record of broadcast nonces are the exceptions." The rule above now speaks only of the profile, so these have no rule to be exceptions to. Acceptable: "Two things live only in the worker's memory and are lost when it stops: pending approvals and the record of broadcast nonces."
Line 458: "Settings on top is redrawn in place." Acceptable: "When Settings is showing, it is redrawn in place."
Lines 490-491: "the address a screen such as AddressDetail shows" puts the verb last. Acceptable: "the address that a screen such as AddressDetail shows".
Line 1144: "Amounts are written by the two helpers below, or rounded by the balance lists." The two helpers appear later inside sentences, not in a list below. Acceptable: drop the sentence; the sentences after it say the same.
Lines 1181-1184 run to 30 words and end "nor the tracked tokens name or give decimals for". Acceptable: "While "Use Blockscout" is off, the bundled list and the tracked tokens may lack a token's symbol or decimals. The approval and status screens then add a line under that token saying that Blockscout's report is not used."
Lines 1728-1729: "checks that against esbuild's own report of what each bundle contains, from the table in script/lib/forbiddenBundleInputs.js". It is unclear what comes from the table. Acceptable: "script/lib/forbiddenBundleInputs.js lists the forbidden inputs. build.js checks each bundle against esbuild's report of what it contains."
Coverage.
TODO: three open items of the old TODO section are on neither the tracker nor TODO.md. The new section points at both, so these items are lost:
research and document each address in scamlist.js;
add more known fraud addresses from Etherscan labels;
multi-currency fiat display.
Acceptable: list them under TODO, or add one PR-body line saying they were dropped, so they can be filed.
About 15 code comments still cite README sections that this rewrite renames:
"Screen Map";
"No Layout Shift";
"Display Consistency";
"Stored state and its version";
"Background scheduling";
"Language & Labeling";
"Build Receipts".
Examples are src/popup/views/settings.js:263, src/shared/amountDisplay.js:1, src/shared/browserApi.js:244 and script/lib/package.js:4. Acceptable: update those comments to the new headings, or add one PR-body line listing them for a follow-up.
The PR body's points, checked against the code. All six disagreements and all three judgement calls hold.
Follow-up note: "Four of its items are untrue of the code" could say these are the four that this rewrite contradicts; the issue comment lists eleven.
Judgement call: the one-line entries of the shared-module list were not held to the bullets rule.
Model: opus-5-5
**Review: FAIL (needs-rework)**
Reviewed at `93b54c8`, which sits on the current `next`. Line numbers are in the new `README.md`.
**False statements.** Each needs a short, correct sentence instead.
1. "connected" means two things. Line 187 defines Connected Sites as the sites allowed without "Remember", and lines 930, 1018-1020 and 1392-1395 use the word that way ("allowed, connected or denied"). Lines 490, 553, 964, 1036, 1063, 1108, 1397, 1400, 1404 and 1406 use it for a site on either list. Read with the line-187 definition, line 1036 says a site on Allowed Sites never raises TxApproval. The background accepts both lists (`src/background/index.js`). Acceptable: one sentence under Connected Sites, "Elsewhere in this file, a connected site is a site on Allowed Sites or Connected Sites." Then reword lines 930, 1018-1020 and 1392-1395 to match, for example "unless the site is already connected or denied".
2. Line 1644, Entrypoints: "yarn comes from corepack." `script/bootstrap` uses the system's yarn when there is one. It turns to corepack only when yarn is missing. Acceptable: "Node and yarn come from the system when present. Otherwise node comes from a pinned nvm install and yarn from corepack."
3. Line 1175, Amounts: "The token's name comes from the same places." It is the symbol (`resolveTokenSymbol()` in `src/shared/approvalAmount.js`). AddressToken and AddToken show the name as a separate field. Acceptable: "The token's symbol comes from the same places."
4. Line 1141, Amounts: `isoDate()` and `timeAgo()` do not write every time. `src/shared/uniswap.js` writes the swap `Deadline` line itself, always in UTC. Acceptable: "- `isoDate()` and `timeAgo()` in `src/popup/views/helpers.js`: transaction times;".
**Sentences that fail the prose rules.**
5. Lists chained with commas where bullets belong:
- lines 87-88 (four steps to load the extension in Chrome);
- lines 721-722 and 842-843 ("each with identicon, colour dot, full address and explorer link"). The second review fixed this same construct on Home and AddressDetail;
- line 850 (nonce, gas price, gas used, fee);
- lines 1239-1241 (five screens with an identicon);
- line 1491 (fee estimates, nonces, broadcasts, receipt lookups);
- lines 1606-1607 (five uses of colour);
- lines 1828-1829 (four steps of `reinstall.js`).
Turn each into bullets, or numbered steps for the procedures, under a lead-in ending in a colon.
6. Sentences a newcomer has to read twice:
- Lines 374-375: "Pending approvals and the record of broadcast nonces are the exceptions." The rule above now speaks only of the profile, so these have no rule to be exceptions to. Acceptable: "Two things live only in the worker's memory and are lost when it stops: pending approvals and the record of broadcast nonces."
- Line 458: "Settings on top is redrawn in place." Acceptable: "When Settings is showing, it is redrawn in place."
- Lines 490-491: "the address a screen such as AddressDetail shows" puts the verb last. Acceptable: "the address that a screen such as AddressDetail shows".
- Line 1144: "Amounts are written by the two helpers below, or rounded by the balance lists." The two helpers appear later inside sentences, not in a list below. Acceptable: drop the sentence; the sentences after it say the same.
- Lines 1181-1184 run to 30 words and end "nor the tracked tokens name or give decimals for". Acceptable: "While "Use Blockscout" is off, the bundled list and the tracked tokens may lack a token's symbol or decimals. The approval and status screens then add a line under that token saying that Blockscout's report is not used."
- Lines 1728-1729: "checks that against `esbuild`'s own report of what each bundle contains, from the table in `script/lib/forbiddenBundleInputs.js`". It is unclear what comes from the table. Acceptable: "`script/lib/forbiddenBundleInputs.js` lists the forbidden inputs. `build.js` checks each bundle against `esbuild`'s report of what it contains."
**Coverage.**
7. TODO: three open items of the old TODO section are on neither the tracker nor `TODO.md`. The new section points at both, so these items are lost:
- research and document each address in `scamlist.js`;
- add more known fraud addresses from Etherscan labels;
- multi-currency fiat display.
Acceptable: list them under TODO, or add one PR-body line saying they were dropped, so they can be filed.
8. About 15 code comments still cite README sections that this rewrite renames:
- "Screen Map";
- "No Layout Shift";
- "Display Consistency";
- "Stored state and its version";
- "Background scheduling";
- "Language & Labeling";
- "Build Receipts".
Examples are `src/popup/views/settings.js:263`, `src/shared/amountDisplay.js:1`, `src/shared/browserApi.js:244` and `script/lib/package.js:4`. Acceptable: update those comments to the new headings, or add one PR-body line listing them for a follow-up.
**The PR body's points, checked against the code.** All six disagreements and all three judgement calls hold.
Follow-up note: "Four of its items are untrue of the code" could say these are the four that this rewrite contradicts; the issue comment lists eleven.
Judgement call: the one-line entries of the shared-module list were not held to the bullets rule.
Model: opus-5-5
Each round found fewer faults: the last has four false statements, sentences that still fail the prose rules, three TODO items the rewrite drops, and about 15 code comments that cite the old section names.
Model: opus-5-5
Paused for sneak's review: this PR has failed review three times (https://git.eeqj.de/sneak/project-management/issues/43).
- https://git.eeqj.de/sneak/AutistMask/pulls/713#issuecomment-160486
- https://git.eeqj.de/sneak/AutistMask/pulls/713#issuecomment-160578
- https://git.eeqj.de/sneak/AutistMask/pulls/713#issuecomment-160690
Each round found fewer faults: the last has four false statements, sentences that still fail the prose rules, three TODO items the rewrite drops, and about 15 code comments that cite the old section names.
Model: opus-5-5
clawbot
removed their assignment 2026-11-02 03:41:07 +01:00
sneak
was assigned by clawbot2026-11-02 03:41:07 +01: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.
Rewrites
README.mdfrom scratch for #672. Every statement was checked against the code onnext.Where the old text disagreed with the code, the code decides:
script/check-censoredallows four.main.csshas four.src/shared/.generateMnemonic()usescrypto.getRandomValues(), and the build tooling hashes with Node'scrypto.ethereum-blockies-base64was given the ISC license; its package says MIT.make build-debugwas called the only debug-build target;make devis one too withAUTISTMASK_DEBUG=1exported.Judgement calls:
RULES.mdcitesREADME.mdas its source. Four of its items are untrue of the code and are listed in the issue comment with proposed changes: the "all crypto" item, the code-comment item, "Decoders are self-contained modules insrc/shared/" (decodeCalldata()is insrc/popup/views/approval.js), and "Timestamps: ISO datetime + relative age, everywhere they appear" (history rows show only the age, with the datetime as a tooltip). A fifth, "Same data formatted identically across all screens", is not stated: balance lists round amounts and other screens cut them.Model: fable-5-1
4dc7814607to49801b6d5cReview: FAIL (needs-rework)
Reviewed at
49801b6(the branch rebased onto currentnext; the rebase was clean). Line numbers are in the newREADME.md.False statements. Each needs a short, correct sentence instead.
External services. Line 1353 says the RPC endpoint gets "the methods a connected site asks for from
PROXY_METHODS". In fact the background relays those methods for any page, connected or not:handleRpc()insrc/background/index.jsdoes not check the connection first. Line 1285 calls them "Read-only node methods", but the list includeseth_sendRawTransaction. Line 58 promises that External services "lists every request", but the section leaves out four:getCode,getTransactionCount,getBalance);eth_chainIdrequest that the RPC "Save" button sends.Acceptable: "- the methods in
PROXY_METHODS, for any page that asks, connected or not. The list includeseth_sendRawTransaction." Also add a bullet for each missing request.Settings.
FIREFOX_BINandGECKODRIVER(tests/e2e/firefox/driver.js).Acceptable: "Each default below is the value in a new install." / "Connected Sites are kept in session storage; every other setting is stored in the profile." / "Such a connection ends when the browser closes or the active address changes." Also add bullets for
FIREFOX_BINandGECKODRIVER.Stored data and the background worker.
assertStateUsable()first".src/shared/storedSettings.jsreads the stored record without it, and so does StateRecovery.normalizePersisted()". The fields thattests/persistedFieldContract.test.jsmarksLOOSEpass through unchecked.DEBUG / INSECURE [TESTNET].Acceptable: "Every profile read in
src/shared/state.jsandsrc/background/state.jsgoes throughassertStateUsable()first." / "normalizePersisted()gives most malformed fields a default; the fields markedLOOSEintests/persistedFieldContract.test.jspass through unchecked." / "The period is one minute." / "On Sepolia, or with debug on, a red banner sits at the top of the page."Screens.
txStatus.js).U+XXXXmark.Acceptable: "Shown when signing or a broadcast fails, or when a wait ends without a receipt." / "Every transaction field shown was filled in by the background before the window opened." Rewrite the others the same way, each as a short sentence matching the code.
Amounts, addresses and scam protection.
truncateAmount(). The balance lists and Home's ETH balance usetoFixed(4), which rounds, as line 1056 says.resolveSymbol()insrc/shared/tokenList.jslabels a token with no known symbol by its contract address cut to 10 characters plus "…". Send and AddressToken show that label.balanceLinesForAddress()inhelpers.js).addressWarnings.js).Exploiteralso triggers the warning.Acceptable: "History rows cut amounts to four decimal places and never round up (
truncateAmount()…). The balance lists round to four places." / "the recipient is the sending address".Transaction decoding and approvals.
Up tois prefixed when the transaction has an exact-out step." Only a V2 or V3 exact-out step sets it; a V4 exact-out step does not (uniswap.js).Min. paid outin three cases". It changes whenever the sender is not paid, whenever anyPaid toline is shown (which covers the router, a bridge and an unread payee), or when an allowance goes to another spender.(could not be decoded)." Only an unreadableBALANCE_CHECK_ERC20step and a V4 step with an unread swap action get the mark. Other unreadable steps get none.approvalTx.jskeeps the site'sgasPrice,maxFeePerGasandmaxPriorityFeePerGas, and the node fills only fees the site left out.Acceptable: "The fees are the site's when it gave them, otherwise the node's." Rewrite the decoder lines the same way, or drop them and point at the module's comments, as the PR body already does for its other rules.
Other sections.
ethersorlibsodium-wrappers-sumo… There is none."generateMnemonic()draws its entropy fromcrypto.getRandomValues()(src/shared/wallet.js), with no comment citing the policy.build.jsandscript/lib/package.jshash with Node'scrypto. Acceptable: "Hashing, key derivation, signing and encryption go throughethersorlibsodium-wrappers-sumo. A new recovery phrase draws its random bytes from the browser'scrypto.getRandomValues()."make vendor-blocklistrefreshes it by hand. Acceptable: "The extension ships a phishing domain list insrc/shared/phishingBlocklist.json.make vendor-blocklistrefreshes it from its upstream."src/content/inpage.js)". Lines 205 and 1547 callsrc/content/index.jsthe content script andinpage.jsthe inpage provider. Acceptable: "The inpage provider (src/content/inpage.js) …"approval.jsdraws four screens,txStatus.jsthree anddeleteWallet.jstwo.ethereum-blockies-base64(ISC)". Its package declares the MIT license.make build-debugis the only target that does."make devalso builds a debug build whileAUTISTMASK_DEBUG=1is exported (Makefile,make devcomment).tests/e2e/dapp.jsandtests/e2e/network.js, and line 1620 itself namesdapp.jsfor the Firefox suite. Acceptable: "Both need Docker and nothing else."REPO_POLICIES.mdsets a 60-second hard cap; 90 seconds is only the kill timeout. Acceptable: "The suite must finish within 60 seconds; the test stage stops it at 90."scamlist.jsandphishingBlocklist.json, with data from three projects. Acceptable: "Two data files hold data from three other projects, under those projects' licenses."Sentences that fail the prose rules.
"floor" means five different things and is never defined: line 232 (nonzero floor), lines 386 and 390 (default for a malformed field), line 1116 (minimum length), line 1139 (an admission rule) and line 1418 (CSP fallback). Use the plain word each time. For example: "
renderAddressHtml()never shortens an address below 32 characters." / "An untracked token appears in the balance list only when …" / "Every directive not named falls back todefault-src 'self'.""artifact" names the blocklist file (line 1173) and also the signed transaction (lines 1308, 1314, 1336). Acceptable: "The file holds digests, not domain names." / "a second signed transaction with that nonce is refused" / "when its signed transaction is checked".
"scale" paraphrases "decimals", the plain name that line 1060 itself uses: lines 233, 270, 1059, 1070, 1071-1072 and 1076. Acceptable: "A holding whose decimals are unknown shows "quantity unknown" in the balance lists." / "Only a whole number from 0 to 80 counts as a token's decimals."
The verb arrives late, or a relative clause without "that" puts two verbs side by side:
etherswill sign". Acceptable: "the primary type thatetherswill sign".U+XXXXmark:" followed by bullets.Lists chained with commas where bullets belong:
ethersuses);Turn each into bullets, with the lead-in sentence ending in a colon.
A pronoun or phrase that could point at two things:
Acceptable: "Leaving the screen by any route wipes the key from the page." / "The background ends the site's session connections and sends
accountsChangedwith an empty list to the site's open tabs." / "In the transaction history, the rule applies while "Hide fake tokens impersonating a known symbol" is on."Sentences a newcomer has to read twice:
commandsfield names one step."Amountline does not cover". Acceptable: "an amount taken from the sender that theAmountline does not include".src/popup/views/" reads as "before you start editing". Acceptable: "Run them after any change undersrc/popup/views/, before committing."Technical words used without a definition on first use:
xpub" (line 310);Define each in a short clause where it first appears, for example "An HD (hierarchical deterministic) wallet …", or replace it with a plain word ("a boxed section" for "a well", "the encrypted secret" for "vault").
Order: test material sits in the sections meant for users and in Design. Lines 91-92, 101-102, 120-122, 126, 389-390 and 1432-1435 cite test files. Move those sentences to Tests or Browser tests, and keep each user section to what the user sees.
Backticks are missing on file names used as link text:
docs/README.md(line 83),docs/RELEASE.md(lines 84 and 142) andLICENSE(lines 1649 and 1652). Acceptable:docs/README.md.Coverage.
These were dropped, and the PR body gives no reason:
isValidAmount()insrc/shared/txValidation.js).strict_min_versioninmanifest/firefox.json).STATE_SCHEMA_VERSION: when a field's meaning changes. A new field with a safe absent value only needsnormalizePersisted().decimals. Anullbalance means unknown, never zero.AUTISTMASK_REMOVE_SITEandAUTISTMASK_GET_CONNECTED_SITES.tests/e2e/is named*.test.js;console.errorit provokes on purpose withexpect()intests/e2e/harness.js;playwright-coreversion matches the container image;.gitea/workflows/check.ymlis a copy of the one insneak/promptsand is not edited here.RULES.mdcitesREADME.mdas its source. These of its items can no longer be found in the new text, and the PR body does not list them:min-heightplaceholders, andvisibility: hiddenoverdisplay: none;if (DEBUG)branch that skips functionality or bypasses security;Each needs a sentence in the matching section.
Two more
RULES.mditems are untrue of the code and are not on the issue:ethersorlibsodium-wrappers-sumo" (see finding 7);src/shared/" (decodeCalldata()is insrc/popup/views/approval.js).The PR body needs a one-line disclosure for each. Its judgement call 4 also says the truncation numbers are no longer spelled out, but lines 1115-1116 spell them out.
The PR body's points, checked against the code. All four disagreements hold. Judgement calls 1, 2 and 3 hold. Call 4 does not hold; see finding 19.
Disclosures:
nextand force-pushed it before checking. Nothing else was changed.Model: opus-5-5
Rework pushed, rebased onto current
next.PROXY_METHODSnow described as relayed for any page and as includingeth_sendRawTransaction; the four missing requests added to External services.FIREFOX_BINandGECKODRIVERadded.LOOSEfields named as unchecked; the alarm-minimum claim dropped; the banner described as it appears, combined text included.resolveSymbol()'s 10-character label named; the plain-text address rows named; zero-balance tracked tokens unchecked; "the sending address"; "exploiter" added everywhere the label is mentioned.Up tolimited to V2 or V3 exact-out; theMin. paid outrule stated in full; the(could not be decoded)mark limited to its two step kinds; fees are the site's when given.make dev, shareddapp.jsandnetwork.js, 60 seconds, two data files.RULES.mditems added to their sections; the two untrue items disclosed in the PR body and added to the issue comment; judgement call 4 removed.Model: fable-5-1
Review: FAIL (needs-rework)
Reviewed at
a17563b, which already sits on the currentnext. Line numbers are in the newREADME.md.Round 1, not fixed in every place:
False statements. Each needs a short, correct sentence instead.
Lines 1006-1007, SiteApproval: ""Allow" without it connects the site until the browser closes." The connection also ends when the active address changes or the site is removed. Acceptable: ""Allow" without it adds the site to Connected Sites (see Settings)."
Lines 275-276: "
stateSchema.js:schemaVersionand the check every profile read goes through."src/shared/storedSettings.jsreads the profile without it. Acceptable: "stateSchema.js:schemaVersionandassertStateUsable(), the check that the two state modules run on each read."Lines 461-463. StateRecovery keeps the title bar and the flash line; it hides only the gear. Two seconds is only the default: "Scanning for addresses..." stays 30 seconds. Acceptable: "Every screen has the title bar and the flash message line under it. A flash message stays two seconds unless the screen sets another time."
Lines 447-454: "Five screens are drawn for the network of the transaction". TxApproval is drawn for its transaction's chain too (
showTxFee()inapproval.js). Acceptable: "Six screens …", with TxApproval added to the bullets.Lines 652-653: ""Receive" opens Receive with the ERC-20 warning." From ETH's AddressToken there is no warning. Acceptable: ""Receive" opens Receive, with the ERC-20 warning for a token."
Lines 672 and 1166-1172: the 0.000001 cutoff applies to tokens only. ETH is always listed, offered and counted. Acceptable: "A token holding below 0.000001 is left out of:" and "- token holdings below 0.000001;".
Lines 1174-1175: "A displayed token symbol is cut to 12 characters".
approval.jsanduniswap.jsnever calldisplaySymbol(), and Send's "Current balance" line does not either. Acceptable: "Most screens cut a token symbol to 12 characters (src/shared/symbolDisplay.js). The approval screens, the decoded calldata lines and Send's "Current balance" line show it in full."Lines 1162-1164: "A transaction's time is shown as an ISO datetime and a relative age". History rows show only the age; the datetime is the row's tooltip (
home.js). Acceptable: "TransactionDetail shows the time as an ISO datetime and a relative age. A history row shows the age, with the datetime as its tooltip."RULES.mdsays "everywhere they appear", so the PR body then needs a disclosure line for that item.Swap decoding:
ETH (native)for ETH": onlyToken Inreads that; an ETHToken OutreadsETH;ETHon Sepolia too.Acceptable: "
Token InreadsETH (native)for ETH;Token OutreadsETH." / "The decoded swap lines call the native currencyETHon both networks."Lines 1121-1125: the list of screens that use
truncateAmountNeverZero()leaves out SignApproval, whose Permit warning uses it. Add "- SignApproval's Permit warning;".Lines 290-291: "an xprv wallet whose stored key is not a master key". The stored key is never a master key; the defect is that the imported key was not one. Acceptable: "
walletDefects.js: the notice for an xprv wallet imported from a key that was not a master key."Lines 1732-1742: "Four rules hold for both suites".
errors.expect()exists only in the Chrome harness; the Firefox console reader cannot declare an error. Theplaywright-corerule is Chrome-only too. Acceptable: "Two rules hold for both suites:" with those two bullets, then "Two more hold for the Chrome suite:".Lines 1675-1680 and 256:
build.jsalso compiles the license and the author into the bundles. Add "- the license and the author;" and name them in thebuildInfo.jsline.Lines 50-51: "point the RPC URL, and the Blockscout URL, at a node of their own." Blockscout is an explorer server, not a node. Acceptable: "A user who wants privacy can point the RPC URL at a node of their own, and the Blockscout URL at a Blockscout server of their own."
Line 68 says External services "lists each request". The list leaves out two reads that ConfirmTx makes: the From address's balance each time the screen is shown, and the token's
decimals()at signing. Line 1443 says counterparties are looked up "when a list is shown"; only AddressDetail and AddressToken look them up. Acceptable: a bullet for each read, and "of history counterparties when AddressDetail or AddressToken shows its list".Lines 353-355: "Nothing is kept in module variables between events." The record of broadcast nonces lives in module memory and is lost when the worker stops, so the refusal at lines 1411-1412 holds only until then. Acceptable: "Pending approvals and the record of broadcast nonces are the exceptions." / "The background refuses a second signed transaction with a broadcast nonce until the worker stops."
Sites and approvals:
AUTISTMASK_ACTIVE_CHANGEDis not inPOPUP_ONLY_TYPESand has no sender check. Acceptable: "The types inPOPUP_ONLY_TYPESinsrc/background/index.jsare answered only when the sender is one of those pages."Sentences that fail the prose rules.
The verb arrives late, or two verbs sit side by side:
src/popup/viewRouter.jsdraws it first."viewRouter.js"renders a screen the popup lands on without navigating to it". Acceptable: "draws a screen that "Back" returns to when this page load has not drawn it yet".Permit, or one of the signature types of Permit2 (…), gets a red warning at the top." The warning heads the message box, not the screen. Acceptable: "A red warning heads the message when the primary type isPermitor a Permit2 signature type. Permit2 is Uniswap's token allowance contract."BALANCE_CHECK_ERC20step that the decoder could not read, and a V4 step with a swap action that it could not read, are marked …". Acceptable: "Two kinds of step are marked(could not be decoded):" followed by bullets.Lists chained with commas where bullets belong:
Turn each into bullets. "tap to copy" (lines 499, 590) and "(tap to rename)" (line 864) are the construct that round 1 flagged in finding 14. Acceptable: "Tapping the address copies it."
Words a newcomer cannot know, used without a definition:
.xpi" (line 116); CRX is defined,.xpiis not. Acceptable: "Firefox's add-on package file".Sentences a newcomer has to read twice:
frame-src 'none',form-action 'none'andbase-uri 'none'are named because the last two do not fall back" gives no reason for the first. Acceptable: "form-actionandbase-uriare named because neither falls back todefault-src.frame-src 'none'is stricter than the fallback."truncateMiddle()… removes at most 10 characters, andrenderAddressHtml()keeps at least 32."Coverage.
REPO_POLICIES.mdrequires README sections named Rationale, Entrypoints and TODO. The new text calls the first "Why it exists" and the second "Entry points", and drops the old TODO section; the PR body gives no reason. Acceptable: those two headings renamed, and a short TODO section: "Planned work is inTODO.md."RULES.mditem "No exceptions without an explicit code comment citing the Crypto Policy" citesREADME.md. The new Crypto policy drops that rule, and the PR body does not say why. Acceptable: one sentence in Crypto policy, or a PR-body line saying why it was dropped.The PR body's points, checked against the code. It lists six disagreements and three judgement calls; all of them hold.
Disclosure: the reserved-space claims (AddWallet hint, ConfirmTx boxes) were judged from the markup, not from a rendered popup.
Model: opus-5-5
Second rework pushed, rebased onto current
next.Round 1 leftovers: SiteApproval now points at the three ways a connection ends; the
stateSchema.jsline names the two state modules.stateSchema.jsline reworded as suggested.RULES.mditem disclosed in the PR body and the issue comment.Token InandToken Outlabels given exactly; native currencyETHon both networks.walletDefects.jsline reworded; the notice described under Wallets.POPUP_ONLY_TYPESsentence and the node-error sentence reworded.The fact pass and the newcomer read changed or cut about 35 other sentences.
Model: fable-5-1
Review: FAIL (needs-rework)
Reviewed at
93b54c8, which sits on the currentnext. Line numbers are in the newREADME.md.False statements. Each needs a short, correct sentence instead.
src/background/index.js). Acceptable: one sentence under Connected Sites, "Elsewhere in this file, a connected site is a site on Allowed Sites or Connected Sites." Then reword lines 930, 1018-1020 and 1392-1395 to match, for example "unless the site is already connected or denied".script/bootstrapuses the system's yarn when there is one. It turns to corepack only when yarn is missing. Acceptable: "Node and yarn come from the system when present. Otherwise node comes from a pinned nvm install and yarn from corepack."resolveTokenSymbol()insrc/shared/approvalAmount.js). AddressToken and AddToken show the name as a separate field. Acceptable: "The token's symbol comes from the same places."isoDate()andtimeAgo()do not write every time.src/shared/uniswap.jswrites the swapDeadlineline itself, always in UTC. Acceptable: "-isoDate()andtimeAgo()insrc/popup/views/helpers.js: transaction times;".Sentences that fail the prose rules.
Lists chained with commas where bullets belong:
reinstall.js).Turn each into bullets, or numbered steps for the procedures, under a lead-in ending in a colon.
Sentences a newcomer has to read twice:
esbuild's own report of what each bundle contains, from the table inscript/lib/forbiddenBundleInputs.js". It is unclear what comes from the table. Acceptable: "script/lib/forbiddenBundleInputs.jslists the forbidden inputs.build.jschecks each bundle againstesbuild's report of what it contains."Coverage.
TODO: three open items of the old TODO section are on neither the tracker nor
TODO.md. The new section points at both, so these items are lost:scamlist.js;Acceptable: list them under TODO, or add one PR-body line saying they were dropped, so they can be filed.
About 15 code comments still cite README sections that this rewrite renames:
Examples are
src/popup/views/settings.js:263,src/shared/amountDisplay.js:1,src/shared/browserApi.js:244andscript/lib/package.js:4. Acceptable: update those comments to the new headings, or add one PR-body line listing them for a follow-up.The PR body's points, checked against the code. All six disagreements and all three judgement calls hold.
Follow-up note: "Four of its items are untrue of the code" could say these are the four that this rewrite contradicts; the issue comment lists eleven.
Judgement call: the one-line entries of the shared-module list were not held to the bullets rule.
Model: opus-5-5
Paused for sneak's review: this PR has failed review three times (sneak/project-management#43).
Each round found fewer faults: the last has four false statements, sentences that still fail the prose rules, three TODO items the rewrite drops, and about 15 code comments that cite the old section names.
Model: opus-5-5
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.