fix(browser): prove PID-safe browser crash recovery evidence

[seonghobae]

Partial implementation of #28. Current exact head is 5a86b28bb84643c3296c937a0480d70c9229ed6c, base is current #147 3dff28d9bf2dd27b72507e39979d51b8bf140fb4, and the branch remains open, Draft and mergeable.

Buyer/security boundary

This slice proves browser-crash recovery and teardown without disabling Chromium sandboxing. All four real-Chromium launch paths contain no --no-sandbox escape. A rejected browser startup remains one failed trial; ChromeDriver/process teardown and temporary-profile cleanup remain evidence about cleanup only and are never promoted to browser/task success.

The browser-crash lane retains PID/start-time identity before signalling, uses the existing PID-safe process observers, preserves the first causal browser failure when cleanup also fails, bounds secondary cleanup evidence to typed failure names, and keeps the 3-trial denominator unchanged.

Credential-safe session-start diagnosis

Predecessor 2323ff54e7bcad4c37eddfeb9274562982d52ac5 repaired the W3C response boundary: structured session not created responses become _WebDriverSessionNotCreatedError with closed error code session_not_created, while only sandbox_unavailable and unknown may enter evidence. The exact No usable sandbox response diagnostic maps to sandbox_unavailable; all other response detail maps to unknown. Raw response text, arbitrary diagnostics, profile/executable paths and secret-shaped values are not retained.

Review 5143478858 identified the remaining blind spot: all four ChromeDriver launch sites discard process output with stdout=subprocess.DEVNULL / stderr=subprocess.STDOUT, so response-level classification cannot consume ChromeDriver process diagnostics.

Current commit 5a86b28bb84643c3296c937a0480d70c9229ed6c is the deliberate test-first RED for that finding. It adds tests/test_chromedriver_process_diagnostic_contract.py and requires:

• a bounded _ChromeDriverStartupDiagnostic that retains only a closed startup reason and no raw diagnostic/path/secret state;
• No usable sandbox recognition even when the reviewed marker crosses read chunks;
• a continuously drained ChromeDriver output pipe that consumes a 262,144-byte stream without deadlock or payload retention;
• unreviewed process text remaining unknown; and
• one shared ChromeDriver launch/session boundary used by all four real-browser lanes, with stdout=subprocess.PIPE replacing discarded process output.

No production runner source changed in this RED commit. Do not weaken or delete these contracts to restore CI.

Exact-head repository RED

CI 34249189712 is terminal failure on exact 5a86b28... as required by TDD.

• Rust contracts job 102138926823 checked out exact 5a86b28... and failed at Check Python repository contracts after running 259 tests: four errors because _ChromeDriverStartupDiagnostic / _drain_chromedriver_diagnostics do not yet exist, plus one failure because the runner still contains stdout=subprocess.DEVNULL.
• Formatting, workspace Rust tests, strict Clippy and rustdoc were correctly skipped after the repository-contract RED; they are not current-head GREEN.
• Production coverage job 102138926424 completed success because the commit is test-only. That does not override the repository-contract RED.

The next causal source repair is the minimum implementation needed to satisfy these five contracts while preserving no-retry/no---no-sandbox, four browser lanes and all 3-trial denominators.

Exact-head real-Chromium RED

Manifest V3 Compatibility 34249189613, job 102138926057, also checked out exact 5a86b28..., downloaded pinned Chrome/ChromeDriver 150.0.7871.129, executed the real fixture and failed. Artifact 10065380771, digest sha256:b01486c703d2614fd2444ad41785812c0e966779465a70792e53e02551ea9a2f, records all four browser lanes at 0/3 during session creation; browser-crash trials remain _WebDriverSessionNotCreatedError / session_not_created / startup_reason=unknown with profiles cleaned.

This repeat confirms that the test-only commit did not alter browser behavior. It still does not prove that the startup cause is the sandbox helper, AppArmor, harden-runner, or another runner restriction. Do not relabel unknown as sandbox_unavailable or infer causality from cleanup success.

Canonical sandbox-helper owner boundary

Issue #212 remains the canonical current-workflow owner. PR #43 exact 6f3134d18d3118aab33d28048671dc71a5f47b77 separately proved that this pinned Chrome generation can run sandboxed when the archive chrome_sandbox helper is root-owned, mode 4755, and bound through CHROME_DEVEL_SANDBOX. That is technical feasibility evidence for #43's leaf workflow generation, not transferable workflow authority or GREEN for this branch.

Protected AGENTS.md reserves .github/workflows/** mutation for the authorized workflow-owner path. The current order remains two-lane rather than circular: complete #148's bounded credential-safe process-diagnostic source repair and independently reconstruct/adopt the validated least-privilege helper mechanics against the current protected workflow generation under #212 without weakening lifecycle/concurrency, harden-runner/egress or immutable pins. Only a fresh exact-head pinned-Chromium run after a causal change may establish browser GREEN.

Do not restore --no-sandbox, reduce trials, broaden egress/permissions, copy an obsolete workflow blob, force-push/destructively rebase, or treat session/driver/profile cleanup as browser success.

Review and remaining gates

All currently returned inline review threads are resolved. Review 5143478858 remains the predecessor exact-head COMMENT finding now materialized as the 5a86b28... RED contract; it is not an approval. Historical CHANGES_REQUESTED submissions on predecessor heads are not current approval. Keep Draft.

Merge/release readiness still requires the bounded process-diagnostic implementation, a fresh exact-head repository GREEN (Python contracts, formatting, Rust workspace tests, strict Clippy, rustdoc and exact owned-production coverage), canonical current-generation sandbox-helper owner repair if fresh evidence establishes it as causal, a new exact-head real pinned-Chromium GREEN, current central security/review/ruleset satisfaction, protected-parent integration in normal dependency order, and the separate #28 action → observed post-condition → evidence → browser/process/profile teardown composition.

No workflow/ruleset/secret mutation, Ready transition, self-approval, merge, tag, release, publication, force update or destructive restack is performed by this PR-state refresh.

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[opencode-agent]

Pull request overview

OpenCode cannot approve yet because required coverage evidence did not pass.

Review outcome

1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence

• Problem: The required coverage-evidence job result was failure, so OpenCode cannot establish approval sufficiency for this head.

• Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.

• Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports success with required evidence or explicit no-source not-applicable evidence.

• Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present.

• Result: REQUEST_CHANGES

• Reason: coverage-evidence result was failure, so required test/docstring evidence was not proven for current head cb49d4c6c26b32e2bdab5634622624b47a6ca8f3.

• Head SHA: cb49d4c6c26b32e2bdab5634622624b47a6ca8f3

• Workflow run: 32237343063

• Workflow attempt: 1

Coverage evidence

Coverage Decision

• Result: FAIL
• Test evidence: not proven passing
• Docstring evidence: not proven passing when configured
• Failure count: 1

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["CI script: run_mv3_compatibility.py"]
  S1 --> I1["review and security gate shell path"]
  I1 --> R1["Review risk: CI script: run_mv3_compatibility.py"]
  R1 --> V1["bash -n plus Strix self-test"]
  Evidence --> S2["Test: test_agent_task_browser_crash_recovery_contract.py"]
  S2 --> I2["regression suite"]
  I2 --> R2["Review risk: Test: test_agent_task_browser_crash_recovery_contract.py"]
  R2 --> V2["targeted test run"]

Loading

[opencode-agent]

OpenCode Review Overview

• Head SHA: cb49d4c6c26b32e2bdab5634622624b47a6ca8f3
• Workflow run: 32237343063
• Workflow attempt: 1
• Gate result: REQUEST_CHANGES (approval step)

Pull request overview

OpenCode cannot approve yet because required coverage evidence did not pass.

Review outcome

1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence

• Problem: The required coverage-evidence job result was failure, so OpenCode cannot establish approval sufficiency for this head.

• Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.

• Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports success with required evidence or explicit no-source not-applicable evidence.

• Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present.

• Result: REQUEST_CHANGES

• Reason: coverage-evidence result was failure, so required test/docstring evidence was not proven for current head cb49d4c6c26b32e2bdab5634622624b47a6ca8f3.

• Head SHA: cb49d4c6c26b32e2bdab5634622624b47a6ca8f3

• Workflow run: 32237343063

• Workflow attempt: 1

Coverage evidence

Coverage Decision

• Result: FAIL
• Test evidence: not proven passing
• Docstring evidence: not proven passing when configured
• Failure count: 1

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["CI script: run_mv3_compatibility.py"]
  S1 --> I1["review and security gate shell path"]
  I1 --> R1["Review risk: CI script: run_mv3_compatibility.py"]
  R1 --> V1["bash -n plus Strix self-test"]
  Evidence --> S2["Test: test_agent_task_browser_crash_recovery_contract.py"]
  S2 --> I2["regression suite"]
  I2 --> R2["Review risk: Test: test_agent_task_browser_crash_recovery_contract.py"]
  R2 --> V2["targeted test run"]

Loading

[seonghobae]

Current-head operability finding on 0135984f1bc1f68d89d7777f49c4999474105a12, using the already-executed real-browser RED rather than another blind rerun.

MV3 artifact 9977680352 records Chrome/ChromeDriver 150.0.7871.129, all four sandbox-enabled browser lanes at 0/3, and browser-crash trials at failure_stage=session_create with only failure_type=RuntimeError / reason_code=runtime_error. That evidence proves fail-closed startup failure, but it cannot distinguish the causal classes #212 must choose between.

The current runner makes that ambiguity structural: every ChromeDriver process sends stdout/stderr to DEVNULL, while _json_request parses a WebDriver error payload only to special-case no such window; every other value.error/message, including session not created, is collapsed to generic RuntimeError and the response details are discarded. The crash classifier therefore cannot produce more specific startup evidence even when ChromeDriver supplied it. Re-running the unchanged workflow cannot improve the RCA.

Preserve the no---no-sandbox boundary. Before another hosted browser rerun, add the smallest credential-safe diagnostic boundary: retain the bounded WebDriver error code as a typed value (at minimum session not created) and classify only an allowlisted set of startup causes from bounded ChromeDriver diagnostics into closed reason codes, with raw response/log text, profile paths, page data and credentials excluded from the artifact. A focused contract should prove that a representative session-creation error produces the safe typed reason while arbitrary/unrecognized text remains session_not_created/unknown and is never emitted. Then the next real pinned-Chromium run can distinguish a sandbox/helper failure from a different browser-startup defect and hand the exact cause to #212 instead of treating the current coarse category as root cause.

This is diagnostic/operability authority only. It must not add a fallback launch, restore --no-sandbox, weaken the 3-trial denominator, or turn cleanup success into browser success.

[seonghobae]

Exact-head diagnostic repair landed as 2323ff54e7bcad4c37eddfeb9274562982d52ac5, a single-parent non-force fast-forward from 0135984f1bc1f68d89d7777f49c4999474105a12.

The prior real-browser failure remains the executed RED: Manifest V3 run 33990522248, job 101371812631, reached the pinned Chrome/ChromeDriver fixture and recorded every sandbox-enabled lane at 0/3; browser-crash trials failed at session_create but the runner collapsed structured WebDriver session not created responses into generic RuntimeError.

This repair keeps the sandbox and all repeatability/cleanup gates unchanged. It adds a typed, redacted session_not_created boundary and emits only the closed startup_reason values sandbox_unavailable or unknown. Raw ChromeDriver-controlled prose, paths and secret-shaped markers are not retained. Contracts cover HTTP-error and 2xx WebDriver error objects, allowlisted sandbox diagnosis, unknown diagnosis, a single failed startup attempt, driver/profile cleanup, and the absence of an unsandboxed retry.

TDD evidence:

• pre-implementation diagnostic RED: 2 expected assertion failures;
• pre-classifier browser-crash RED: runtime_error != session_not_created;
• current focused/relevant result: 31/31 tests pass;
• all tests outside the executor-dependent exact-exit module: 250/250 pass;
• full local discovery: 252/254 pass. The two failures are isolated to test_pidfd_termination_does_not_credit_signal_delivery_as_exit and test_pidfd_termination_refuses_stale_identity_without_signalling: this executor intermittently exposes no /proc/<Popen.pid> entry for live child processes (independent 5-child probe observed 1/5). This is recorded as an environment limitation, not source GREEN and not a relaxed assertion.

compileall and git diff --check pass. CHANGELOG and docs/doctoring.md record the bounded evidence contract. No workflow, sandbox flag, permission, timeout, trial denominator, threshold or success predicate changed.

Successor runs are newly materialized and remain non-terminal:

• CI 34232690663: queued;
• Manifest V3 Compatibility 34232690664: queued.

Keep Draft. These queued runs are not passing evidence; the next real pinned-Chromium artifact must establish the allowlisted startup cause before #212 chooses any workflow-side helper repair.

[seonghobae]

Exact-head RCA update — PR #148 @ 2323ff54e7bcad4c37eddfeb9274562982d52ac5.

Manifest V3 Compatibility run 34232690664, job 102082520922, checked out that exact SHA, downloaded matching Chrome/ChromeDriver 150.0.7871.129, and reached the real fixture. Every real-browser lane failed 0/3 during session creation. The new bounded evidence consistently reports _WebDriverSessionNotCreatedError / session_not_created; the allowlisted startup_reason is unknown, so this run does not independently prove the helper-specific reason. Raw ChromeDriver text and paths remain excluded.

The paired causal boundary is unchanged and stronger than a blind retry: current #148 fails closed without sandbox-disabling arguments, while #43 @ 6f3134d18d3118aab33d28048671dc71a5f47b77 already proved the same pinned browser generation can start sandboxed when the archive helper is root-owned, mode 4755, and bound through CHROME_DEVEL_SANDBOX. The smallest true repair remains canonical/current-workflow adoption under #212, followed by non-destructive consumer adoption and a new exact-head replay. No --no-sandbox, trial reduction, egress relaxation, stale workflow copy, or cleanup-as-success interpretation is acceptable.

[seonghobae]

Exact-head RCA extension on 2323ff54e7bcad4c37eddfeb9274562982d52ac5: the bounded session_not_created repair is working, but the current runner still cannot refine startup_reason=unknown from ChromeDriver process diagnostics. Both _run_browser_pass and _run_agent_task_browser_pass launch ChromeDriver with stdout=subprocess.DEVNULL and stderr=subprocess.STDOUT; _classify_webdriver_session_startup_reason therefore sees only the W3C response value.message. The exact hosted artifact from MV3 run 34232690664 remains 0/3 at session creation with _WebDriverSessionNotCreatedError / session_not_created / startup_reason=unknown. Re-running the unchanged head cannot recover process-output evidence that this code discards.

Treat this as a source-side diagnostic RED distinct from #212's workflow-helper authority. The smallest safe repair should stream/drain ChromeDriver diagnostics through a bounded closed-vocabulary classifier while retaining no raw diagnostic bytes, paths, profile names, capabilities, page data, or secret-shaped text. The regression should include the sandbox marker split across read chunks plus hostile path/secret markers and prove that only reviewed reason codes survive; unrecognized diagnostics remain unknown. The capture must not introduce a PIPE deadlock, retry, alternate launch, broader egress, or --no-sandbox fallback. After exact repository GREEN, rerun pinned Chrome/ChromeDriver 150.0.7871.129; only then may a classified helper/AppArmor diagnostic be used as causal evidence for #212. Until then, keep the helper repair technically plausible but not proven by #148's current unknown evidence.

[seonghobae]

Writer lease ACTIVE — exact 2323ff54e7bcad4c37eddfeb9274562982d52ac5, bounded to review 5143478858 only. Scope: establish the credential-safe ChromeDriver process-diagnostic RED before any production repair. The regression must fail while ChromeDriver launch output is discarded, require a bounded streaming classifier that retains only closed reason codes (including split No usable sandbox markers), reject raw diagnostic/path/secret retention, and require continuous pipe draining so the browser runner cannot deadlock. Preserve all four sandbox-enabled launch paths, 3-trial denominators, cleanup semantics, #147 base, Draft state, workflows/rulesets/secrets, and no-retry/no---no-sandbox policy. No merge, Ready transition, tag or release.

[seonghobae]

Writer lease RELEASED — current exact 5a86b28bb84643c3296c937a0480d70c9229ed6c. Test-first bounded ChromeDriver process-diagnostic RED is committed and hosted failure is observed; PR body is current with CI 34249189712, MV3 34249189613, artifact 10065380771, and the remaining minimal source repair. Production runner/workflow were not changed in this slice. No force update, Ready transition, merge, tag, release, workflow/ruleset/secret mutation.

[seonghobae]

Writer lease ACTIVE — exact 5a86b28bb84643c3296c937a0480d70c9229ed6c, source/docs/PR-state bounded to the existing credential-safe ChromeDriver process-diagnostic RED. Scope: implement the minimum causal production repair for _ChromeDriverStartupDiagnostic, continuous pipe drain, and one shared ChromeDriver launch/session boundary across all four real-browser lanes; preserve sandbox-enabled launch, 3-trial denominators, cleanup semantics, #147 base, Draft state, workflows/rulesets/secrets, no retry and no --no-sandbox. Intervening commits will be read and adopted rather than overwritten. No force update, Ready transition, merge, tag or release until exact-head GREEN and real-browser evidence justify it.

[seonghobae]

Current exact-head RCA for CI 34256771021: the bounded process-diagnostic production repair is present, but the two follow-up contracts added on 2b8e0e2... and 0e320aa... are not yet implemented. tests/test_chromedriver_process_diagnostic_contract.py now requires (1) the shared launch argv to include --verbose without --log-path, and (2) _drain_chromedriver_diagnostics to accept existing text-mode process doubles while retaining no raw diagnostic state. At 0e320aa..., _start_chromedriver still launches only [chromedriver, --port, --allowed-ips], and _drain_chromedriver_diagnostics raises TypeError for every non-bytes chunk. Therefore the Python repository-contract failure is causal and local; do not weaken either regression. Minimal repair: add --verbose only to the shared ChromeDriver argv, and normalize a str chunk to UTF-8 bytes inside the drain boundary before feeding _ChromeDriverStartupDiagnostic, while rejecting other chunk types and continuing to discard raw content. Preserve the binary PIPE in production, no --log-path, no retry/no --no-sandbox, 3-trial denominators, and the closed sandbox_unavailable|unknown evidence vocabulary. Exact real-browser artifact 10068188981 still reports session-create 0/3 with startup_reason=unknown, so no sandbox-helper causality is established until this repository RED is fixed and the pinned browser is rerun.

[seonghobae]

Current-head follow-up on the credential-safe ChromeDriver diagnostic repair: the async drain and the session-failure classifier are not synchronized. _start_chromedriver launches a daemon thread, but _create_chromedriver_session checks diagnostic.startup_reason immediately when the W3C session not created response arrives. If ChromeDriver emits No usable sandbox just before/after that response and the drain thread has not consumed the relevant bytes yet, a genuine reviewed process diagnostic is still recorded as unknown. Adding --verbose and StringIO normalization alone can therefore make repository contracts GREEN while leaving the real-browser classifier timing-dependent.

Please keep the existing active source lease and add a RED that deterministically delays the diagnostic feed until the session error path has begun, then proves classification waits only for a bounded, explicit diagnostic handoff—not an arbitrary sleep and not process EOF. The minimum repair should expose a bounded synchronization primitive/state from the drain boundary (or an equivalent reviewed design), preserve continuous draining and raw-byte non-retention, and return unknown on timeout. Do not wait indefinitely for ChromeDriver to exit, do not retain logs, and do not change the browser trial denominator or sandbox policy. This is independent of whether the eventual root cause is the setuid helper/AppArmor; it is about making the closed classifier deterministic on the evidence it already owns.