Compare commits

..
2 Commits
Author SHA1 Message Date
clawbot df89f20099 fix: open no approval window for a site-connection prompt already answered (closes #287)
check / check (push) Failing after 3s
e2e / e2e-chrome (push) Failing after 4s
e2e / e2e-firefox (push) Failing after 5s
When a site-connection prompt was decided before the toolbar popup raised
for it had loaded, that popup was torn down, chrome.action.openPopup()
rejected, and the background opened its fallback window for the answered
approval and only then removed it. In the Chrome end-to-end suite the next
test could take that window for its own prompt and lose it under its wait.
openApprovalWindow() now returns before creating a window when the approval
is no longer pending.

The blocklist test clicked its self-closing Reject with a plain click; it
now clicks it as the other site Reject does, with the click witnessed.

Model: opus-5-5
2026-10-05 00:29:29 +00:00
clawbot f86740ce69 test: the popup boot's stand-in for filterTransactions returns the real shape (closes #429)
check / check (push) Failing after 2s
e2e / e2e-chrome (push) Failing after 2s
e2e / e2e-firefox (push) Failing after 2s
The stand-in in tests/support/popupBoot.js returned a bare list, while the
real filterTransactions returns { transactions, newFraudContracts }. Home,
AddressDetail and AddressToken read both fields, so every test boot onto
one of them threw inside its transaction loading, logged loadHomeTxs failed
or loadTransactions failed, and never ran the rest of that code. The
stand-in now returns the real shape, and tests/persistedFieldContract.test.js
boots onto each of the three views and asserts neither message is logged.

Model: opus-5-5
2026-10-05 01:59:16 +02:00
6 changed files with 90 additions and 53 deletions
+21 -10
View File
@@ -45,16 +45,27 @@ but the review is broader than any of them.
# Completed Steps # Completed Steps
- 2026-10-04: The Chrome end-to-end suite no longer loses a dApp prompt under - 2026-10-05: The extension no longer opens a window for a site-connection
load ([#287](https://git.eeqj.de/sneak/AutistMask/issues/287)). The suite prompt already answered
drives a site-connection prompt in a tab while the toolbar popup for the same ([#287](https://git.eeqj.de/sneak/AutistMask/issues/287)). When the prompt was
approval is still loading. When the tab settled it first, the toolbar popup decided before the toolbar popup raised for it had loaded, that popup was torn
closed unloaded, the extension opened its fallback window for the settled down, `chrome.action.openPopup()` rejected, and the background opened its
approval and removed it again, and the next test could take that window for fallback window for the answered approval and then removed it. In the Chrome
its own prompt and lose it under its wait. The suite now takes an approval end-to-end suite the next test could take that window for its own prompt and
window only while the background still holds its approval. The blocklist lose it under its wait. `openApprovalWindow()` now opens nothing for an
test's Reject, whose window closes itself, is clicked as the other site Reject approval that is no longer pending. The blocklist test's Reject, whose window
is, with the click witnessed. closes itself, is clicked as the other site Reject is, with the click
witnessed.
- 2026-10-04: A popup boot in the tests loads transactions without failing
([#429](https://git.eeqj.de/sneak/AutistMask/issues/429)). The stand-in for
`filterTransactions` in `tests/support/popupBoot.js` returned a bare list,
while the real one returns `{ transactions, newFraudContracts }`, so every
boot onto Home, AddressDetail or AddressToken failed inside its transaction
loading and logged `loadHomeTxs failed` or `loadTransactions failed`; the rest
of that code never ran. The stand-in now returns the real shape, and
`tests/persistedFieldContract.test.js` boots onto each of the three and
asserts neither message is logged. `make test` time did not change measurably.
- 2026-10-04: The native token's label follows the network - 2026-10-04: The native token's label follows the network
([#372](https://git.eeqj.de/sneak/AutistMask/issues/372)). `networks.js` gives ([#372](https://git.eeqj.de/sneak/AutistMask/issues/372)). `networks.js` gives
each network a `nativeCurrency` and nothing read it: every screen wrote `ETH`, each network a `nativeCurrency` and nothing read it: every screen wrote `ETH`,
+5 -1
View File
@@ -413,7 +413,7 @@ function releaseApproval(approval) {
} }
} }
// Open approval in a separate popup window. // Open approval in a separate popup window, unless it is no longer pending.
// This is the primary mechanism for tx/sign approvals (triggered programmatically, // This is the primary mechanism for tx/sign approvals (triggered programmatically,
// not from a user gesture) and the fallback for site-connection approvals. // not from a user gesture) and the fallback for site-connection approvals.
// Never rejects. Its callers raise it from inside a Promise executor and drop // Never rejects. Its callers raise it from inside a Promise executor and drop
@@ -446,6 +446,10 @@ async function openApprovalWindow(id) {
); );
} }
// Already answered: a site-connection prompt decided before the toolbar
// popup raised for it had loaded, whose openPopup() rejects only now.
if (!pendingApprovals[id]) return;
let win = null; let win = null;
try { try {
win = await windowsCreate(opts); win = await windowsCreate(opts);
+21
View File
@@ -2235,6 +2235,27 @@ describe("a site connection decided as the popup closes", () => {
}); });
}); });
// The prompt is decided before the toolbar popup raised for it has
// loaded; that popup is torn down and openPopup() rejects only after.
test("a toolbar prompt already decided opens no window when openPopup() rejects", async () => {
const bg = loadBackground({ actionPopup: true });
const opening = deferred();
bg.openPopup.mockImplementation(() => opening.promise);
const pending = bg.requestSite();
await settle();
const port = bg.connectApproval(pending.id());
port.decide(true, false);
port.disconnect();
await settle();
expect(pending.result()).toEqual({ result: [signer.address] });
opening.reject(new Error("the toolbar popup closed before it loaded"));
await settle();
expect(bg.created).toHaveLength(0);
});
// The port carries a decision now, so it carries the sender check the // The port carries a decision now, so it carries the sender check the
// one-off message used to carry. A content script that guessed an // one-off message used to carry. A content script that guessed an
// approval id must not be able to connect the site it is running on. // approval id must not be able to connect the site it is running on.
+17 -41
View File
@@ -2600,42 +2600,15 @@ function dappMessages(page, type) {
); );
} }
// The open approval page whose approval the background still holds, or null.
//
// A page at an approval URL is not enough. When a site-connection prompt is
// settled in the reserved tab (see reserveApprovalTab) before the toolbar popup
// raised for the same approval has loaded, closing the tab tears that popup
// down, chrome.action.openPopup() rejects, and src/background/index.js opens
// its fallback window for the settled approval and then removes it. That window
// can be up when the next test looks for its prompt; taken for it, it showed
// the site view and then closed under the wait for the sign view
// (https://git.eeqj.de/sneak/AutistMask/issues/287).
async function findPendingApprovalPage(env) {
for (const page of env.ctx.pages()) {
if (page.isClosed() || !page.url().includes("?approval=")) continue;
const id = new URL(page.url()).searchParams.get("approval");
const approval = await env.page.evaluate(
(approvalId) =>
new Promise((resolve) => {
chrome.runtime.sendMessage(
{ type: "AUTISTMASK_GET_APPROVAL", id: approvalId },
resolve,
);
}),
id,
);
if (approval) return page;
}
return null;
}
// The approval window the background opened. Approvals are raised from an // The approval window the background opened. Approvals are raised from an
// RPC call rather than from a user gesture, so the extension opens a real // RPC call rather than from a user gesture, so the extension opens a real
// popup window for them; it is an ordinary page in this context. // popup window for them; it is an ordinary page in this context.
async function waitForApprovalWindow(env, timeout = 30000) { async function waitForApprovalWindow(ctx, timeout = 30000) {
const deadline = Date.now() + timeout; const deadline = Date.now() + timeout;
for (;;) { for (;;) {
const page = await findPendingApprovalPage(env); const page = ctx
.pages()
.find((p) => !p.isClosed() && p.url().includes("?approval="));
if (page) return page; if (page) return page;
if (Date.now() > deadline) { if (Date.now() > deadline) {
throw new Error( throw new Error(
@@ -2706,7 +2679,9 @@ async function reserveApprovalTab(env) {
async function openSiteApprovalPopup(env, timeout = 30000) { async function openSiteApprovalPopup(env, timeout = 30000) {
const deadline = Date.now() + timeout; const deadline = Date.now() + timeout;
for (;;) { for (;;) {
const existing = await findPendingApprovalPage(env); const existing = env.ctx
.pages()
.find((p) => !p.isClosed() && p.url().includes("?approval="));
if (existing) return existing; if (existing) return existing;
const url = await env.page.evaluate( const url = await env.page.evaluate(
@@ -2731,8 +2706,9 @@ async function openSiteApprovalPopup(env, timeout = 30000) {
} }
} }
// Retire every approval page still open, so none outlives the test that // Retire every approval page still open. A settled approval whose page is
// raised it. // left behind would be found by the next waitForApprovalWindow() and driven
// as if it were the next approval.
async function closeApprovalPages(ctx) { async function closeApprovalPages(ctx) {
for (const page of ctx.pages()) { for (const page of ctx.pages()) {
if (!page.isClosed() && page.url().includes("?approval=")) { if (!page.isClosed() && page.url().includes("?approval=")) {
@@ -3170,7 +3146,7 @@ test("personal_sign signs, and the signature recovers to the address (#183)", as
SIGN_HEX, SIGN_HEX,
env.expectedAddress, env.expectedAddress,
]); ]);
const popup = await waitForApprovalWindow(env); const popup = await waitForApprovalWindow(env.ctx);
await visible(popup, "#view-approve-sign"); await visible(popup, "#view-approve-sign");
const boundary = await watchApprovalBoundary(popup, env); const boundary = await watchApprovalBoundary(popup, env);
@@ -3247,7 +3223,7 @@ test("personal_sign rejected returns a rejection to the page (#183)", async (env
SIGN_HEX, SIGN_HEX,
env.expectedAddress, env.expectedAddress,
]); ]);
const popup = await waitForApprovalWindow(env); const popup = await waitForApprovalWindow(env.ctx);
await visible(popup, "#view-approve-sign"); await visible(popup, "#view-approve-sign");
await clickAndClose(popup, "#btn-reject-sign"); await clickAndClose(popup, "#btn-reject-sign");
@@ -3275,7 +3251,7 @@ test("a personal message is laid out in the order of its bytes (#403)", async (e
hexlify(toUtf8Bytes(text)), hexlify(toUtf8Bytes(text)),
env.expectedAddress, env.expectedAddress,
]); ]);
const popup = await waitForApprovalWindow(env); const popup = await waitForApprovalWindow(env.ctx);
await visible(popup, "#view-approve-sign"); await visible(popup, "#view-approve-sign");
// The text on screen, marks included, and the left edge of each of its // The text on screen, marks included, and the left edge of each of its
@@ -3321,7 +3297,7 @@ test("eth_signTypedData_v4 signs, and the signature recovers (#183)", async (env
env.expectedAddress, env.expectedAddress,
TYPED_DATA_JSON, TYPED_DATA_JSON,
]); ]);
const popup = await waitForApprovalWindow(env); const popup = await waitForApprovalWindow(env.ctx);
await visible(popup, "#view-approve-sign"); await visible(popup, "#view-approve-sign");
const boundary = await watchApprovalBoundary(popup, env); const boundary = await watchApprovalBoundary(popup, env);
@@ -3408,7 +3384,7 @@ test("eth_signTypedData_v4 rejected returns a rejection to the page (#183)", asy
env.expectedAddress, env.expectedAddress,
TYPED_DATA_JSON, TYPED_DATA_JSON,
]); ]);
const popup = await waitForApprovalWindow(env); const popup = await waitForApprovalWindow(env.ctx);
await visible(popup, "#view-approve-sign"); await visible(popup, "#view-approve-sign");
await clickAndClose(popup, "#btn-reject-sign"); await clickAndClose(popup, "#btn-reject-sign");
@@ -3427,7 +3403,7 @@ test("eth_sendTransaction signs the approved transaction and broadcasts it (#183
data: TX_DATA, data: TX_DATA,
}; };
await startRequest(env.dapp, "tx", "eth_sendTransaction", [txParams]); await startRequest(env.dapp, "tx", "eth_sendTransaction", [txParams]);
const popup = await waitForApprovalWindow(env); const popup = await waitForApprovalWindow(env.ctx);
await visible(popup, "#view-approve-tx"); await visible(popup, "#view-approve-tx");
const boundary = await watchApprovalBoundary(popup, env); const boundary = await watchApprovalBoundary(popup, env);
@@ -3568,7 +3544,7 @@ test("eth_sendTransaction rejected broadcasts nothing (#183)", async (env) => {
data: TX_DATA, data: TX_DATA,
}, },
]); ]);
const popup = await waitForApprovalWindow(env); const popup = await waitForApprovalWindow(env.ctx);
await visible(popup, "#view-approve-tx"); await visible(popup, "#view-approve-tx");
await clickAndClose(popup, "#btn-reject-tx"); await clickAndClose(popup, "#btn-reject-tx");
+22
View File
@@ -722,6 +722,28 @@ describe("the base profile the sweep corrupts", () => {
} }
}); });
// Home, AddressDetail and AddressToken load their transactions inside a catch
// that only logs, so a boot that fails there still renders the view and passes
// the tests above while none of that code runs.
describe("the base profile loads transactions", () => {
for (const view of ["main", "address", "address-token"]) {
test(`on ${view} without logging a failure`, async () => {
const consoleError = jest.spyOn(console, "error");
try {
await bootPopup(restoringOnto(view));
const failures = consoleError.mock.calls
.map((args) => args.join(" "))
.filter((line) =>
/loadHomeTxs failed|loadTransactions failed/.test(line),
);
expect(failures).toEqual([]);
} finally {
consoleError.mockRestore();
}
});
}
});
// A field the ROUTER itself reads — the two it gates on and the two // A field the ROUTER itself reads — the two it gates on and the two
// hasValidAddress() indexes with. A hostile value in one of these legitimately // hasValidAddress() indexes with. A hostile value in one of these legitimately
// changes which view renders, so each gets its own boot per view and is held // changes which view renders, so each gets its own boot per view and is held
+4 -1
View File
@@ -263,9 +263,12 @@ async function bootPopup(stored, options) {
getProvider: () => ({}), getProvider: () => ({}),
scanForAddresses: jest.fn(async () => []), scanForAddresses: jest.fn(async () => []),
})); }));
// filterTransactions() answers in the real one's shape: Home,
// AddressDetail and AddressToken read both fields, and a bare list makes
// their transaction loading throw into a catch that only logs.
jest.doMock("../../src/shared/transactions", () => ({ jest.doMock("../../src/shared/transactions", () => ({
fetchRecentTransactions: jest.fn(async () => []), fetchRecentTransactions: jest.fn(async () => []),
filterTransactions: () => [], filterTransactions: () => ({ transactions: [], newFraudContracts: [] }),
})); }));
const storage = const storage =