test(browser): retain Agent Task process teardown evidence after failure - #143
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if browser_process_id is None or browser_process_start_time_ticks is None: | ||
| raise RuntimeError("Agent Task browser process identity was not captured") | ||
| if not _wait_for_linux_process_identity_exit( | ||
| browser_process_terminated = _wait_for_linux_process_identity_exit( | ||
| browser_process_id, | ||
| browser_process_start_time_ticks, | ||
| ): | ||
| ) | ||
| if browser_failure_type is not None: | ||
| return { | ||
| "failure_type": browser_failure_type, | ||
| "browser_process_terminated": browser_process_terminated, | ||
| } |
There was a problem hiding this comment.
📝 Info: Failure path now blocks on process-exit wait
A reviewed exception caught after identity capture now falls through to _wait_for_linux_process_identity_exit (scripts/ci/run_mv3_compatibility.py:1162), which previously was skipped when an exception propagated. If that helper raises (e.g. a /proc read error), it runs outside the try/except and propagates to _run_agent_task_trial, which records a generic failure type and loses the original browser failure evidence.
Was this helpful? React with 👍 or 👎 to provide feedback.
| return { | ||
| "trial_number": trial_number, | ||
| "passed": False, | ||
| "failure_type": returned_failure_type, | ||
| "browser_process_terminated": browser_process_terminated, | ||
| "profile_cleaned": True, | ||
| "duration_ms": duration_ms, | ||
| } |
There was a problem hiding this comment.
📝 Info: New failure dict does not affect gate outcome
The failed-trial dict adds browser_process_terminated and omits success-only surface keys. The isolation and surfaces gates iterate only over passed trials, so failed trials are skipped, and the failed dict still sets profile_cleaned True for the all-trials cleanup check. A failed trial still fails the pass-count gate, so behavior is unchanged.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Partial implementation of #28, dependency-realigned on current PR #142 head
6c01e5370b0001c58f89cd772cbaf62e197db9f5.Buyer/reliability boundary
After browser PID/start-time capture, a reviewed browser/WebDriver failure must not discard the exact root-process teardown observation. A failed task may retain only bounded typed failure evidence; it must not retain raw exception text, page content, or invent teardown evidence when identity capture never succeeded.
Implemented contract
_run_agent_task_browser_pass:/proc/<pid>/statstart-time identity before controlled task work proceeds;failure_typeplusbrowser_process_terminatedon reviewed failed browser work; and_run_agent_task_trialvalidates and propagates the bounded failure/teardown result after temporary-profile cleanup.TDD provenance
Exact test-only head
e847d0768d7006771b6ee10d56c52e555c0fc168produced the intended repository-contract RED before failure-path teardown retention existed. The production repair then established the reviewed failure envelope and exact root-process evidence.Stack repair
The branch previously remained rooted at obsolete #142 head
a66fb27532a689e2e2458c7ff3cb79b340d113ce. After #142 moved, GitHub produced a clean merged treef904eb983bd714bc70ccb3b20ccf5b09a6a4edd2; this branch was advanced non-destructively with merge commit452927dd0c6040fc7814b277e553ce6eff9e1479, preserving the unique #143 delta while incorporating current #142.Fresh ancestry reports exact #142 as the merge base and this branch ahead-only.
Exact-current proof
On unchanged exact head
452927dd0c6040fc7814b277e553ce6eff9e1479against exact live base6c01e5370b0001c58f89cd772cbaf62e197db9f5:32606887326: success;97113111019: repository contracts, formatting, workspace/all-target check, full tests, strict Clippy, and rustdoc success;97113110897: exact production function/line/region/branch measurement and enforcement success;32606887322: success;No predecessor/prerequisite check, synthetic merge result, status, comment, or model output is treated as approval.
Stack / authority
This remains Draft while #142 and earlier prerequisites remain active. PR #144 is independently realigned/revalidated on this fresh head. No merge, approval, workflow, secret, ruleset, tag, release, or publication mutation is included.
Truth boundary
This lane remains limited to the exact Chrome root identity already introduced by #142. Sampled renderer/GPU/utility process-set teardown, broader process ownership, cross-platform supervision, authenticated browser provenance, and OS-wide orphan absence remain separate boundaries.