Repository navigation
fix(html): let the browser start use the check's time budget - #629
Open
ShotaroKataoka wants to merge 3 commits into
Open
ShotaroKataoka wants to merge 3 commits into
ShotaroKataoka wants to merge 3 commits into
Conversation
…time The browser check started the local Chrome or Edge once and waited a fixed 15 s for DevToolsActivePort. On GitHub-hosted runners Chrome sometimes does not get there in time, and the CI browser job reported "Browser check could not run (Google Chrome): TimeoutError: DevTools did not start within 15 s" on PRs that do not touch it (#625). local_browser now stops a launch that times out, removes its profile and starts once more with a fresh one; the second timeout raises "DevTools did not start within N s (tried 2 times)". A browser that exits before DevTools starts is not retried. SDPM_BROWSER_START_TIMEOUT sets the wait per launch (default 15 s, capped at 120 s; invalid values fall back to 15), and the CI browser job sets it to 45 s. Closes #625
ShotaroKataoka
added a commit
that referenced
this pull request
Nov 14, 2026
apply_transforms truncated each channel, so 4F81BD at lumMod 75% came out 366092 instead of PowerPoint's 376092. Now that every transformed color goes through it, rounding there fixes them all (at most 1 per channel). The CHANGELOG no longer claims every color element: table borders and text of a theme table style still ignore transforms. The docstring says which paths pass satMod. SPEC: 20261114-2000_pr-trio-takeover Progress: #634 R5 R6 R7 Next: #629
#625's failures were all the first Chrome start of the job; later starts took 0.7-1 s. The fixed 15 s wait failed a slow cold start with most of the check's budget left, and the retry plus a longer SDPM_BROWSER_START_TIMEOUT ate into that budget: with 45 s, a hung first start left the check no time and it timed out even when the second start worked. BrowserSource now takes start_timeout, and check_deck passes what is left of its budget (min(30 + 3 x slides, 110) s, unchanged). A start that does not finish names the browser; a check that times out after a slow start says how long the start took. The retry, the env var and the CI setting are removed. SPEC: 20261114-2000_pr-trio-takeover Progress: #629 R2 R3 R4 Next: PR bodies, CI
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The CI
browserjob flaked withTimeoutError: DevTools did not start within 15 son PRs that do not touch the browser check (#625). In all three failed runs it was the job's first Chrome start (test_real_browser_reports_layout_and_js); every later test in the same job started Chrome with a fresh profile in 0.7–1 s, and on passing runs the first test still takes 14–33 s. So the first start on a fresh runner is slow, not hung.The 15 s was a fixed wait in
local_browser, unrelated to the check's own time budget (min(30 + 3 × slides, 110)s, below AgentCore Browser's 120 s session). The failing test had a 48 s budget and gave up after 15 s. The same path runs for a local user's first HTML check after installing.Changes
sdpm/sdpm/engine/html/browser.py: aBrowserSourceis now called as(width, height, *, start_timeout). The local source passes it tolocal_browser; a browser that does not start in time is named:Google Chrome did not start within 36 s (no DevTools port).sdpm/sdpm/engine/html/browser_check.py:check_deckpasses what is left of its budget asstart_timeout, so the start shares the budget instead of having its own 15 s. The longest a check can take is unchanged. A check that times out after a start that took at least a third of the budget saysThe browser took N s of it to start.instead of pointing at a slide script.servers/remote/tools/agentcore_browser.py: acceptsstart_timeoutand does not use it (start_browser_sessionis bounded by the client's API timeout).local_browser's timeout message and cleanup, the local source passes the timeout; the fake sources take the new argument.CHANGELOG.md(Fixed),docs/en/architecture.md: the budget includes starting the browser.An earlier commit on this branch retried a launch that timed out and added
SDPM_BROWSER_START_TIMEOUT(45 s in CI). Both are removed: a retry restarts a slow start from zero, and a longer wait came out of the check's budget, so with 45 s a 2-slide check (36 s) timed out even when the second start worked.Testing
ok(start 21.1 s, 22.0 s in total). Before, it failed at 15 s.failedafter 36 s:Browser check could not run (hang.sh): TimeoutError: hang.sh did not start within 36 s (no DevTools port).SDPM_TEST_BROWSER=1 pytest tests/test_html_browser_check.py -k real_browseron macOS with Chrome: 13 passed.pytest tests/test_html_cdp.py tests/test_html_browser_check.py tests/test_remote_browser.py tests/test_remote_html.py: 92 passed, 13 skipped. Full suite: the only failure,test_html_assemble.py::test_svg_icon_is_inlined_with_prefixed_ids, fails the same way onmainin a full run and passes alone.make lintNotes
A start slower than the whole budget still fails, now with a message that names the browser. The CI job is not warmed up on purpose, so it keeps covering the cold first start a user gets.
Closes #625