Apply browser-leak fix to claude_code_harness + pi_harness (extends PR #14)#15
Open
Alezander9 wants to merge 1 commit into
Open
Conversation
The cubic-ai review caught this leak class in codex_harness (commit e3a3ebf), but claude_code_harness and pi_harness share the same start_remote_daemon API and the same execute() shape: _start_browser is called before env/cmd build and before subprocess.create_subprocess_exec, but _stop_browser only runs in an inner try/finally that begins AFTER subprocess spawn. A failure in env build, system_prompt read, _build_*_cmd, or create_subprocess_exec leaks the provisioned cloud browser. Wraps env+cmd build in try/except _stop_browser, and the subprocess spawn + stderr_task creation in a separate try/except that also kills any partially-started proc before tearing down the browser. Same pattern as the codex_harness fix; symmetric now for all three browser-harness frameworks (cch, pi_harness, codex_harness).
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
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
1 participant
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
Extends PR #14's framework verifier review fixes. The cubic-ai review caught the browser-leak class in
codex_harness(fix in commite3a3ebf), butclaude_code_harnessandpi_harnessshare the samestart_remote_daemonAPI and the sameexecute()shape —_start_browseris called before env/cmd build and beforeasyncio.create_subprocess_exec, but_stop_browseronly runs in an innertry/finallythat begins AFTER subprocess spawn. A failure in env build,system_prompt.read_text(),_build_*_cmd, orcreate_subprocess_execleaks the provisioned cloud browser session — exactly the bug cubic flagged on codex_harness.This PR applies the same try/except
_stop_browserwrap tocchandpi_harness. After this, all three browser-harness frameworks (cch, pi_harness, codex_harness) have parallel structure: three_stop_browser(browser_name, bu_name)call sites each — env-build early-exit, subprocess-spawn early-exit, normal finally.This was discovered while porting PR #14's fixes to the internal
benchmark-x-laminarrepo (browser-use/benchmark-x-laminar#24). The audit there ran an AST comparison of everyexecute()body and found these two were the only ones still vulnerable.Targeting
This PR targets
codex/bu-bench-framework-reverification(PR #14's head branch) so the fix merges into the existing review thread rather than landing as a separate PR after #14 merges.Validation
python3 -m compileall -q frameworks/claude_code_harness/run_task.py frameworks/pi_harness/run_task.pycleancodex_harnessfix ine3a3ebf(just adapted for cch's / pi_harness's slightly different env dict shape)Related