Skip to content

fix(browser): tolerate ChromeDriver exit races during teardown - #153

Open
seonghobae wants to merge 5 commits into
test/agent-task-shared-teardown-deadlinefrom
fix/chromedriver-exit-race-cleanup
Open

fix(browser): tolerate ChromeDriver exit races during teardown#153
seonghobae wants to merge 5 commits into
test/agent-task-shared-teardown-deadlinefrom
fix/chromedriver-exit-race-cleanup

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Partial implementation of #28, stacked on exact PR #150 head cb03afd86c303e7217987f850c47c73089cef9bf.

Falsifiable defect

All three pinned browser lanes previously signaled ChromeDriver through raw terminate() / kill() calls. If the child exited immediately before either signal boundary, Python could raise ProcessLookupError even though the child had already terminated, converting a normal teardown race into a compatibility failure. The cleanup also granted TERM wait and KILL wait independent full five-second budgets instead of one total teardown deadline.

RED -> RCA -> GREEN

  1. Test-only head acc2d2707f2f18c2666b61ee706ab597a3bc5c8c introduced behavioral contracts for child exit immediately before TERM, child exit immediately before KILL, propagation of unrelated signal errors, and one cleanup boundary across all three pinned browser lanes.
  2. Production head f8738768d1ec94bd4f9e046220395898ea15d09a introduced _terminate_chromedriver_process, using one monotonic deadline across TERM wait and optional KILL escalation while normalizing only ProcessLookupError exit races.
  3. Exact predecessor head 4d4209eaa5ca478d3e08c2f963ab0ce3f69f72bb then exposed a repository-contract regression in CI run 31789991615: test_forced_close_browser_failure_is_returned_after_teardown_waits still searched the removed literal driver.wait(timeout=5). That was a stale test assertion, not an infrastructure failure and not a reason to weaken the production cleanup.
  4. Current head 89c1d12cc3afcdde04a565cd440c741039aa6462 updates that contract to assert ordering through _terminate_chromedriver_process(driver), preserving the required shutdown -> Linux teardown proof -> failure-return sequence.

Current exact-head evidence

Current exact head: 89c1d12cc3afcdde04a565cd440c741039aa6462.
Exact prerequisite/base head: cb03afd86c303e7217987f850c47c73089cef9bf.

  • CI run 31790801953: success on the exact head.
  • Rust contracts job 94737022518: Python repository contracts, canonical formatting, locked workspace check, full tests, strict Clippy, and rustdoc success.
  • Production coverage job 94737022579: exact owned-production function/line/region/branch enforcement success.
  • Manifest V3 Compatibility run 31790802032: success on the exact head.
  • CodeRabbit exact-head commit status: success.
  • Formal reviews: none currently returned.
  • Inline review threads: none currently returned.
  • GitHub currently reports the PR open, mergeable, and Ready for review.

Predecessor-head failures are historical only and are not promoted as current exact-head evidence. Passing automation is not independent approval.

Truth boundary

This lane hardens controlled pinned-browser CI cleanup only. It does not establish cgroup/task ownership, discover post-snapshot processes, prove OS-wide orphan absence, authenticate Chromium/ChromeDriver provenance, implement a cross-platform product process supervisor, or turn the compatibility runner into a product browser API.

The stack prerequisite PR #150 remains open. The scheduled actor does not merge or self-approve.

@coderabbitai

coderabbitai Bot commented Aug 14, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ebb140e9-0c99-41ed-be5c-64d1794a919c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@seonghobae
seonghobae marked this pull request as ready for review August 14, 2026 10:29
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