Skip to content

test(idea-posterior): budget browser-probe timeouts for slow CI Chrome starts - #50

Merged
fkguo merged 1 commit into
mainfrom
claude/ci-chrome-subprocess-timeout
Aug 7, 2026
Merged

test(idea-posterior): budget browser-probe timeouts for slow CI Chrome starts#50
fkguo merged 1 commit into
mainfrom
claude/ci-chrome-subprocess-timeout

Conversation

@fkguo

@fkguo fkguo commented Aug 7, 2026

Copy link
Copy Markdown
Owner

Problem

skills/idea-posterior/tests/test_reader_surface_interactions.py::test_interactive_visible_probabilities_share_static_precision fails on GitHub Actions (job python-contracts-test, step "Python tests (interactive argument-graph reader surface)") since ~2026-08-07 with subprocess.TimeoutExpired ... timed out after 5 seconds while launching google-chrome --headless=new ... --virtual-time-budget=3000 --dump-dom, preceded by a D-Bus connect error on stderr. PR #49 run 31140161012 hit it twice (fresh run + rerun, different runner instances, identical failure); the same test on the same code passes locally. Main's last green CI was 2026-08-03, so the regression tracks a runner-image/Chrome update, not a code change.

Diagnosis

The uncaught exception saying "after 5 seconds" can only originate from the post-SIGTERM cleanup communicate(timeout=5): the main communicate(timeout=15)'s own TimeoutExpired is always caught by the surrounding except. So on the updated runner Chrome first stalled past the 15 s main timeout (consistent with a startup D-Bus connect wait; the libdbus default method-call timeout is 25 s), was SIGTERMed, and then also failed to exit within 5 s. Raising only the 5 s cleanup value would therefore not make CI green — Chrome would still be killed at 15 s mid-startup, --dump-dom would never emit, and the test would fail on assert stdout, stderr instead.

Fix

Raise both waits to 30 s (main communicate 15 → 30, post-SIGTERM cleanup 5 → 30). This does not weaken what the test asserts: the in-page work is bounded by Chrome's --virtual-time-budget=3000 (virtual ms), communicate() returns as soon as the process exits, and every DOM-content assertion (probability precision, control inventory, reader-surface contract) is untouched. The wall-clock values are purely a guard against a wedged browser.

Repo-wide, this file is the only place that launches a browser subprocess with fixed timeouts (grep -rn "communicate(timeout=" — the two review-swarm hits are parameterized launcher code, not browser tests).

Verification

  • python3 -m pytest skills/idea-posterior/tests/test_reader_surface_interactions.py -q → 2 passed locally
  • node scripts/check-idea-posterior-reader-surface-anti-drift.mjs → passes (the lock pins interaction/contract anchors, not timeout values, and forbids pytest.skip here — so "skip on CI" was never a legal alternative)
  • git diff --check clean
  • Independent read-only review of the diff returned SHIP with two non-blocking notes, dispositioned as follows:
    1. 30 s is calibrated to a single ~25 s D-Bus stall and cannot be validated locally (macOS has no D-Bus); this PR's own CI run is the acceptance check. If the runner ever stalls on two buses (~50 s), the right next step is an environment-level fix (e.g. point DBUS_SESSION_BUS_ADDRESS at an unreachable address so connects fail fast), not a larger number.
    2. The cleanup path has no SIGKILL escalation (pre-existing shape, unchanged here). Left out to keep the reviewed diff minimal; on an ephemeral CI VM the residual process-group leak is inert. skills/review-swarm/scripts/bin/run_multi_task.py:834 already has the stronger pattern if a follow-up wants it.

Note for PR #49

PR #49's python-contracts-test failure is this same environment issue, not a defect in that branch. It should clear once this lands on main and #49's CI runs against an updated merge base (branch update / rebase / new push — a plain re-run of the old run reuses the old merge commit).

…e starts

Since ~2026-08-07 the ubuntu-latest runner image ships a Chrome whose
headless cold start can stall past 15 s on D-Bus connect waits (the
libdbus default method-call timeout is 25 s). The reader-surface probe
then hit the 15 s main communicate() timeout, SIGTERMed the browser
group, and the 5 s cleanup communicate() raised the uncaught
TimeoutExpired that failed python-contracts-test twice on independent
runner instances (PR #49 run 31140161012). Main's last green run was
2026-08-03, so the regression tracks the runner image, not code.

Raise both waits to 30 s. Wall-clock budgets only guard against a
wedged browser: the in-page work is bounded by --virtual-time-budget
and communicate() returns on process exit, so the content assertions
are unchanged and a healthy run is not slowed.
@cursor

cursor Bot commented Aug 7, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@fkguo
fkguo merged commit 3d37b7d into main Aug 7, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant