test: take only an approval window whose approval is still pending (closes #287)
The site-connection tests settle the prompt in a tab while the toolbar popup for the same approval is still loading. Closing the tab tears that popup down, chrome.action.openPopup() rejects, and the background opens its fallback window for the settled approval and removes it again. The next test took any page at an approval URL, so it could take that window, which showed the site view and then closed under the wait for the sign view. The harness now asks the background whether a page's approval is still pending before taking it. 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
This commit is contained in:
@@ -619,14 +619,13 @@ The jobs **report, they do not gate.** A failure is a red mark against the
|
|||||||
commit that a reviewer has to account for, not a hard block: whether a check
|
commit that a reviewer has to account for, not a hard block: whether a check
|
||||||
blocks a merge is Gitea branch protection, which this repo does not configure.
|
blocks a merge is Gitea branch protection, which this repo does not configure.
|
||||||
|
|
||||||
That is not only a statement about configuration. The Chrome suite is
|
That is not only a statement about configuration. One report of the Chrome suite
|
||||||
**measurably flaky under load** — two of six runs of unmutated code on a busy
|
**failing under load** is still open:
|
||||||
machine lost the approval popup out from under the dApp signing wait, always in
|
[#290](https://git.eeqj.de/sneak/AutistMask/issues/290) records runs on a busy
|
||||||
the `#183` section, tracked as
|
machine failing with `the extension opened no approval window within 30000ms`.
|
||||||
[#287](https://git.eeqj.de/sneak/AutistMask/issues/287). So a red `e2e-chrome`
|
So a red `e2e-chrome` has to be read before it is believed, and that report is
|
||||||
has to be read before it is believed, and that flake is the blocker to ever
|
the blocker to ever making this a required check. Do not answer it with a retry
|
||||||
making this a required check. Do not answer it with a retry wrapper: a suite
|
wrapper: a suite that reruns until it is green stops being evidence.
|
||||||
that reruns until it is green stops being evidence.
|
|
||||||
|
|
||||||
Nothing in either job can pass vacuously. There is no `continue-on-error` and no
|
Nothing in either job can pass vacuously. There is no `continue-on-error` and no
|
||||||
`|| true`; both scripts exit non-zero when docker is missing, when the image
|
`|| true`; both scripts exit non-zero when docker is missing, when the image
|
||||||
|
|||||||
@@ -45,6 +45,16 @@ 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
|
||||||
|
load ([#287](https://git.eeqj.de/sneak/AutistMask/issues/287)). The suite
|
||||||
|
drives a site-connection prompt in a tab while the toolbar popup for the same
|
||||||
|
approval is still loading. When the tab settled it first, the toolbar popup
|
||||||
|
closed unloaded, the extension opened its fallback window for the settled
|
||||||
|
approval and removed it again, and the next test could take that window for
|
||||||
|
its own prompt and lose it under its wait. The suite now takes an approval
|
||||||
|
window only while the background still holds its approval. The blocklist
|
||||||
|
test's Reject, whose window closes itself, is clicked as the other site Reject
|
||||||
|
is, with the click witnessed.
|
||||||
- 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`,
|
||||||
|
|||||||
+45
-19
@@ -2600,15 +2600,42 @@ 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(ctx, timeout = 30000) {
|
async function waitForApprovalWindow(env, timeout = 30000) {
|
||||||
const deadline = Date.now() + timeout;
|
const deadline = Date.now() + timeout;
|
||||||
for (;;) {
|
for (;;) {
|
||||||
const page = ctx
|
const page = await findPendingApprovalPage(env);
|
||||||
.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(
|
||||||
@@ -2679,9 +2706,7 @@ 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 = env.ctx
|
const existing = await findPendingApprovalPage(env);
|
||||||
.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(
|
||||||
@@ -2706,9 +2731,8 @@ async function openSiteApprovalPopup(env, timeout = 30000) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Retire every approval page still open. A settled approval whose page is
|
// Retire every approval page still open, so none outlives the test that
|
||||||
// left behind would be found by the next waitForApprovalWindow() and driven
|
// raised it.
|
||||||
// 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=")) {
|
||||||
@@ -2738,7 +2762,7 @@ async function closeApprovalPages(ctx) {
|
|||||||
// #btn-reject on the site prompt — NOT self-proving. A page that went away
|
// #btn-reject on the site prompt — NOT self-proving. A page that went away
|
||||||
// without the click landing disconnects the approval port, the background
|
// without the click landing disconnects the approval port, the background
|
||||||
// settles that as 4001, and 4001 is exactly what assertUserRejection
|
// settles that as 4001, and 4001 is exactly what assertUserRejection
|
||||||
// accepts. That call site arms the click trace below and asserts it.
|
// accepts. Both call sites arm the click trace below and assert it.
|
||||||
//
|
//
|
||||||
// A button that is missing or unclickable raises a different error, which is
|
// A button that is missing or unclickable raises a different error, which is
|
||||||
// rethrown.
|
// rethrown.
|
||||||
@@ -3124,7 +3148,9 @@ test("a connect request from a blocklisted site is flagged (#219)", async (env)
|
|||||||
// Not remembered: a remembered decision for this origin would
|
// Not remembered: a remembered decision for this origin would
|
||||||
// outlive the test.
|
// outlive the test.
|
||||||
await popup.uncheck("#approve-remember");
|
await popup.uncheck("#approve-remember");
|
||||||
await popup.click("#btn-reject");
|
await armClickTrace(env, popup, "#btn-reject");
|
||||||
|
await clickAndClose(popup, "#btn-reject");
|
||||||
|
await assertClickLanded(env, "#btn-reject");
|
||||||
|
|
||||||
await assertUserRejection(
|
await assertUserRejection(
|
||||||
phishingDapp,
|
phishingDapp,
|
||||||
@@ -3144,7 +3170,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.ctx);
|
const popup = await waitForApprovalWindow(env);
|
||||||
await visible(popup, "#view-approve-sign");
|
await visible(popup, "#view-approve-sign");
|
||||||
const boundary = await watchApprovalBoundary(popup, env);
|
const boundary = await watchApprovalBoundary(popup, env);
|
||||||
|
|
||||||
@@ -3221,7 +3247,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.ctx);
|
const popup = await waitForApprovalWindow(env);
|
||||||
await visible(popup, "#view-approve-sign");
|
await visible(popup, "#view-approve-sign");
|
||||||
await clickAndClose(popup, "#btn-reject-sign");
|
await clickAndClose(popup, "#btn-reject-sign");
|
||||||
|
|
||||||
@@ -3249,7 +3275,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.ctx);
|
const popup = await waitForApprovalWindow(env);
|
||||||
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
|
||||||
@@ -3295,7 +3321,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.ctx);
|
const popup = await waitForApprovalWindow(env);
|
||||||
await visible(popup, "#view-approve-sign");
|
await visible(popup, "#view-approve-sign");
|
||||||
const boundary = await watchApprovalBoundary(popup, env);
|
const boundary = await watchApprovalBoundary(popup, env);
|
||||||
|
|
||||||
@@ -3382,7 +3408,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.ctx);
|
const popup = await waitForApprovalWindow(env);
|
||||||
await visible(popup, "#view-approve-sign");
|
await visible(popup, "#view-approve-sign");
|
||||||
await clickAndClose(popup, "#btn-reject-sign");
|
await clickAndClose(popup, "#btn-reject-sign");
|
||||||
|
|
||||||
@@ -3401,7 +3427,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.ctx);
|
const popup = await waitForApprovalWindow(env);
|
||||||
await visible(popup, "#view-approve-tx");
|
await visible(popup, "#view-approve-tx");
|
||||||
const boundary = await watchApprovalBoundary(popup, env);
|
const boundary = await watchApprovalBoundary(popup, env);
|
||||||
|
|
||||||
@@ -3542,7 +3568,7 @@ test("eth_sendTransaction rejected broadcasts nothing (#183)", async (env) => {
|
|||||||
data: TX_DATA,
|
data: TX_DATA,
|
||||||
},
|
},
|
||||||
]);
|
]);
|
||||||
const popup = await waitForApprovalWindow(env.ctx);
|
const popup = await waitForApprovalWindow(env);
|
||||||
await visible(popup, "#view-approve-tx");
|
await visible(popup, "#view-approve-tx");
|
||||||
await clickAndClose(popup, "#btn-reject-tx");
|
await clickAndClose(popup, "#btn-reject-tx");
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user