Logging the background worker on runs starved of CPU showed two causes:
The background centred each approval window on the last focused window. Under load that was often the previous approval window, still closing. Headless Chrome reports that 360x600 popup as 1280x720, so chrome.windows.create refused the centred position ("Bounds must be at least 50% within visible screen space") and the request got -32603 with no window: the 30000ms timeout.
waitForApprovalWindow() took the first page at an approval URL, which could be the previous test's closing window: the "has been closed" form of the same race.
What changed:
The background centres only on a normal window. When the browser refuses a position, it asks once more without one and lets the browser place the window; only if that also fails does the request get -32603.
After a passed test, the e2e runner gives approval windows five seconds to close and fails the test if one is still open. After a failed test it closes them unreported, so an unanswered prompt cannot refuse the next test's.
Unit tests cover where the window opens, a refused position included.
A new e2e test raises a transaction prompt while a sign window has focus; on the old code it fails with -32603.
Deviation: the load used to reproduce this came from a local --cpuset-cpus edit to script/test-e2e, not in this PR.
Unverified: whether desktop Chrome hits the same refusal.
Model: opus-5-5
Closes https://git.eeqj.de/sneak/AutistMask/issues/290.
Logging the background worker on runs starved of CPU showed two causes:
- The background centred each approval window on the last focused window. Under load that was often the previous approval window, still closing. Headless Chrome reports that 360x600 popup as 1280x720, so `chrome.windows.create` refused the centred position ("Bounds must be at least 50% within visible screen space") and the request got -32603 with no window: the 30000ms timeout.
- `waitForApprovalWindow()` took the first page at an approval URL, which could be the previous test's closing window: the "has been closed" form of the same race.
What changed:
- The background centres only on a `normal` window. When the browser refuses a position, it asks once more without one and lets the browser place the window; only if that also fails does the request get -32603.
- After a passed test, the e2e runner gives approval windows five seconds to close and fails the test if one is still open. After a failed test it closes them unreported, so an unanswered prompt cannot refuse the next test's.
- Unit tests cover where the window opens, a refused position included.
- A new e2e test raises a transaction prompt while a sign window has focus; on the old code it fails with -32603.
- Deviation: the load used to reproduce this came from a local `--cpuset-cpus` edit to `script/test-e2e`, not in this PR.
- Unverified: whether desktop Chrome hits the same refusal.
Model: opus-5-5
src/background/index.js lines 446-489: an approval request can still fail because of where its window is placed. The window is still centred on the last focused browser window, and when the browser refuses that position (for example a browser window partly off the screen edge, or a small one low on the screen in the e2e browser) the request still fails with -32603 and no window, with no second try. Acceptable: if creating the window with a position fails, create it again without left/top so the browser places it, and fail the request only when that also fails; a unit test where the browser refuses the positioned window and the request still gets one.
tests/e2e/run.js lines 4056-4061: closing every approval window after each test, without a report, hides an approval window that should have closed itself and did not. A regression that leaves the window open after a signature now passes the suite. Acceptable: after a test that passed, give approval windows a short time to close by themselves and fail that test if one is still open; close them without a report only after a test that already failed.
The PR body is about 280 words, over the limit of about 250. Acceptable: 250 words or fewer.
Model: opus-5-5
FAIL
1. `src/background/index.js` lines 446-489: an approval request can still fail because of where its window is placed. The window is still centred on the last focused browser window, and when the browser refuses that position (for example a browser window partly off the screen edge, or a small one low on the screen in the e2e browser) the request still fails with -32603 and no window, with no second try. Acceptable: if creating the window with a position fails, create it again without `left`/`top` so the browser places it, and fail the request only when that also fails; a unit test where the browser refuses the positioned window and the request still gets one.
2. `tests/e2e/run.js` lines 4056-4061: closing every approval window after each test, without a report, hides an approval window that should have closed itself and did not. A regression that leaves the window open after a signature now passes the suite. Acceptable: after a test that passed, give approval windows a short time to close by themselves and fail that test if one is still open; close them without a report only after a test that already failed.
3. The PR body is about 280 words, over the limit of about 250. Acceptable: 250 words or fewer.
Model: opus-5-5
The background centred each approval window on the last focused window,
which could be an earlier approval window still open; headless Chrome
reports one as 1280x720, the browser refused the resulting position, and
the request failed with no window. It now centres only on a browser
window, and when the browser refuses a position it asks again without one.
In the Chrome suite a test could raise its prompt while the previous
test's window was still closing. After a passed test the runner now gives
approval windows five seconds to close and fails the test if one is still
open; after a failed test it closes them.
Model: opus-5-5
Fixed in openApprovalWindow(): one retry without a position; unit test added in tests/backgroundApproval.test.js.
Fixed in main() of tests/e2e/run.js: five seconds after a passed test, an approval window still open fails it; closing unreported only after a failed test.
Fixed: PR body trimmed to 246 words.
Model: opus-5-5
Rework of https://git.eeqj.de/sneak/AutistMask/pulls/453#issuecomment-126300, rebased onto `next`:
1. Fixed in `openApprovalWindow()`: one retry without a position; unit test added in `tests/backgroundApproval.test.js`.
2. Fixed in `main()` of `tests/e2e/run.js`: five seconds after a passed test, an approval window still open fails it; closing unreported only after a failed test.
3. Fixed: PR body trimmed to 246 words.
Model: opus-5-5
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.
Closes #290.
Logging the background worker on runs starved of CPU showed two causes:
chrome.windows.createrefused the centred position ("Bounds must be at least 50% within visible screen space") and the request got -32603 with no window: the 30000ms timeout.waitForApprovalWindow()took the first page at an approval URL, which could be the previous test's closing window: the "has been closed" form of the same race.What changed:
The background centres only on a
normalwindow. When the browser refuses a position, it asks once more without one and lets the browser place the window; only if that also fails does the request get -32603.After a passed test, the e2e runner gives approval windows five seconds to close and fails the test if one is still open. After a failed test it closes them unreported, so an unanswered prompt cannot refuse the next test's.
Unit tests cover where the window opens, a refused position included.
A new e2e test raises a transaction prompt while a sign window has focus; on the old code it fails with -32603.
Deviation: the load used to reproduce this came from a local
--cpuset-cpusedit toscript/test-e2e, not in this PR.Unverified: whether desktop Chrome hits the same refusal.
Model: opus-5-5
FAIL
src/background/index.jslines 446-489: an approval request can still fail because of where its window is placed. The window is still centred on the last focused browser window, and when the browser refuses that position (for example a browser window partly off the screen edge, or a small one low on the screen in the e2e browser) the request still fails with -32603 and no window, with no second try. Acceptable: if creating the window with a position fails, create it again withoutleft/topso the browser places it, and fail the request only when that also fails; a unit test where the browser refuses the positioned window and the request still gets one.tests/e2e/run.jslines 4056-4061: closing every approval window after each test, without a report, hides an approval window that should have closed itself and did not. A regression that leaves the window open after a signature now passes the suite. Acceptable: after a test that passed, give approval windows a short time to close by themselves and fail that test if one is still open; close them without a report only after a test that already failed.Model: opus-5-5
eeb01a0401to7f9b856349Rework of #453 (comment), rebased onto
next:openApprovalWindow(): one retry without a position; unit test added intests/backgroundApproval.test.js.main()oftests/e2e/run.js: five seconds after a passed test, an approval window still open fails it; closing unreported only after a failed test.Model: opus-5-5
PASS
Model: opus-5-5