fix(browser): bound forced-close teardown to one deadline - #147
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks 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 |
…lure-teardown-evidence' into test/agent-task-forced-close-shared-teardown-deadline Signed-off-by: Seongho Bae <me@seonghobae.me>
Adopt the repaired forced-close failure-evidence parent without rewriting history. Preserve #147's one-deadline teardown observer while carrying the parent's current release, doctoring, and failure-observation contract. Signed-off-by: Seongho Bae <me@seonghobae.me>
Adopt the repaired shared-deadline parent without rewriting history. Preserve the crash/pidfd child delta and its release record while carrying the current failure-observation documentation and contract. Signed-off-by: Seongho Bae <me@seonghobae.me>
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head review after parent-first repair. Current #146 11944410450684809ee1a71a35c77abafc5358db is the merge base and the effective child delta remains the one shared-deadline teardown observer plus its two focused test paths. Fresh CI 33957022858 and MV3 33957022821 are queued, so no predecessor GREEN transfers and this is not approval. Keep Draft until exact-head hosted/browser/security evidence and dependency gates are current.
Adopt the concurrent reviewed-cleanup parent without rewriting history. The existing shared-deadline tree already contains the integrated source semantics, so preserve it exactly while advancing ancestry to the current parent. Signed-off-by: Seongho Bae <me@seonghobae.me>
Adopt the concurrent cleanup-evidence parent advancement without rewriting history. The crash/recovery tree already carries the integrated source semantics, so preserve it exactly while advancing the stack edge. Signed-off-by: Seongho Bae <me@seonghobae.me>
|
Current-parent adoption regression at exact #147 head 46e7585 (parent includes #146 812c002): ancestry alone does not preserve the valid parent delta. The child deletes all 169 lines / six tests in tests/test_agent_task_review_evidence_contract.py and reverses the parent pre-shutdown count range, ordinary cleanup-field forwarding, terminal HTTPException handling in browser/trial/aggregate boundaries, and obsolete HTTP-body recognizer removal. Native discovery of that inherited filename collects zero tests (exit 5). Replaying the exact parent test module in memory against the current child runner, without writing files or launching a browser, executes six tests and reproduces 10 assertion failures plus 8 uncaught protocol-error cases. The preceding 222-test child snapshot does not cover these omitted regressions. Required repair is non-lossy reconciliation: retain the combined forced-close observer and its deadline tests, restore the parent's review repairs and six tests, then adapt only the forced-close observer injection to the child's actual shared observer if needed. Do not remove tests, weaken the count/type gate, restore raw diagnostics, or treat base ancestry as successful delta inheritance. Source changes are not started here while the current writer route is being checked. This is a repair finding, not a Close condition or eligible approval. Follow-up at exact #147 af1b98b: the synthesized parent runtime repairs and all six inherited methods are restored. Native focused discovery executes six methods; full native discovery executes 228. Both fail only the two forced-close BadStatusLine / IncompleteRead subcases at inherited line 108: the test patches _wait_for_linux_process_identity_exit, but production correctly calls _wait_for_linux_process_teardown. The unpatched reader remains live for both timeout budgets, which also makes the focused run take about 10 seconds. Preserve the ordinary-lane mock/assertion; inject the combined observer with (False, False) for forced-close and assert its (321, 654, ((321, 654),)) arguments. Keep all existing failed-denominator, cleanup, root-alive, driver-terminated and diagnostic-redaction assertions. This is a minimal test reconciliation, not a production timeout/retry change. No source write is active here on #147; #251 remains this task's sole writer lane. Resolved test reconciliation at current #147 3dff28d: after the temporary route closed, exact source stayed unchanged and contacted tasks confirmed read-only ownership, this task took the isolated test-only lane. Fresh af1b98b RED reproduced both failures; the repair specifies each lane's actual observer/result/arguments and preserves every assertion and all six methods. All six focused and all 228 native tests now pass; compileall and complete Rust 1.97.1 gates pass. Numerical production coverage is 415/415 functions, 3555/3555 lines, 4444/4444 regions and 476/476 branches. Production and workflow blobs are unchanged. The pinned branch warning remains explicit, hosted checks need the new exact head, and the separate ordinary-pass deadline thread remains unresolved. This is a verified forward test repair, not approval, browser acceptance or protected-main delivery. |
Preserve the new #255 owner verification separately from old quality failures and experimental branch instrumentation. Record the exact #147 lossy-adoption RED and required non-lossy reconciliation without claiming review resolution or delivery. Commit-Message-Assisted-by: Codex (via Codex) Signed-off-by: Seongho Bae <me@seonghobae.me>
For merge synthesis only, use the current parent forced-close contract unchanged. The child-specific waiter assertions remain fully represented by the dedicated shared-teardown-deadline contract, avoiding duplicate overlap while preserving the valid acceptance semantics. Signed-off-by: Seongho Bae <me@seonghobae.me>
Merge the original shared-deadline child onto exact current #146 without rewriting either history. The existing overlapping forced-close assertion is delegated to the dedicated shared-deadline contract so GitHub can produce a clean three-way integration; no product acceptance or protected-branch change is implied. Signed-off-by: Seongho Bae <me@seonghobae.me>
Replace the ancestry-only merge tree with the content-clean GitHub three-way synthesis built from exact current #146 and the original shared-deadline child. Preserve all reviewed parent cleanup-evidence semantics and the child shared-deadline runtime/test delta without force-pushing history. Signed-off-by: Seongho Bae <me@seonghobae.me>
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head review on af1b98ba377c73b88baa9633e2232e7f76e76f36: the parent-synthesis artifact was a real repair finding, not a reason to Close or rewrite history. Production already used _wait_for_linux_process_teardown, while the inherited process-termination contract still required the obsolete two-waiter sequence. Commit af1b98ba... restores only that child-owned shared-deadline assertion and retains #146's newer http.client.HTTPException evidence boundary. Fresh compare against exact parent 812c0020... is ahead-only with exactly three semantic paths. Current CI/MV3 runs are non-terminal, so this review records source consistency only, not hosted GREEN, approval, or merge readiness.
Keep every parent review-evidence assertion and all six methods while injecting the actual individual or combined teardown observer for each lane. Preserve exact root-only identity arguments after an early forced-close protocol error. Production code, deadline, retry policy and failed denominators are unchanged. Fresh exact-parent RED: six methods, two forced-close failures in 10.417 seconds. GREEN: six focused and all 228 Python tests, compileall, full Rust 1.97.1 fmt/check/tests/Clippy/rustdoc and enforced 100% coverage (415 functions, 3555 lines, 4444 regions, 476 branches). Branch instrumentation warning remains explicit. The ordinary-pass two-deadline issue stays separate. Commit-Message-Assisted-by: Codex (via Codex) Signed-off-by: Seongho Bae <me@seonghobae.me>
Record #252 current-parent verification and #147 test-only reconciliation while retaining the preceding lossy-integration and two-failure evidence. Keep ordinary-pass deadlines, instrumentation warning, hosted checks and protected delivery separate. Four missing current-evidence assertions failed before this update; all 169 repository tests, compileall and diff checks now pass. Commit-Message-Assisted-by: Codex (via Codex) Signed-off-by: Seongho Bae <me@seonghobae.me>
Adopt current #147 without discarding cleanup regressions and remove the crash launch sandbox bypass. An executed startup regression first fails on the disabling argument, then proves one failed attempt and cleanup. Supplemental Linux runs all 247 contracts without skips; macOS full Rust and exact numerical coverage pass. Commit-Message-Assisted-by: Codex (via Codex) Signed-off-by: Seongho Bae <me@seonghobae.me>
|
Ordinary teardown shared-deadline writer RELEASED — 2026-09-06 18:07 UTC OriginWeave hourly writer releases the bounded #147 source/test/PR-state slice at unchanged exact head Fresh exact-head hosted proof is now terminal GREEN: CI Preserved acquisition and repair historyThis lease was originally acquired for the ordinary Agent Task teardown double-budget finding. The hosted RED The invalid overlapping later claim recorded in comment |
|
Ordinary teardown shared-deadline writer RELEASED — overlap correction Fresh coordination re-read found an earlier unreleased lease in comment Before detecting the overlap, normal publication advanced the branch from the earlier writer's RED Exact hosted RED on predecessor |
Partial implementation of #28. This Draft now owns the ordinary and forced-close Agent Task teardown observers under one shared process-exit deadline while preserving exact root/set cleanup evidence.
Current exact parent and repair
Exact head
364912b7ff23b8ac88832d0f078e00d975707407, exact live base #146812c0020bd2ecdb3eda39d999ed3327647550bdf. All prior #147 history remains ancestral, including the recovered parent synthesis, forced-close repair, ordinary-pass RED3f483abb27eb8ae4abc6a2a7fe71d7632bc21e68, and ordinary production repair9d07ca7b59553ad21c85b6efcb72c8154cb55c2f.The ordinary pass previously performed
_wait_for_linux_process_identity_exitand_wait_for_linux_process_identity_set_exitsequentially, allowing a surviving sampled descendant to consume two fullPROCESS_EXIT_TIMEOUT_SECONDSwindows. The production repair reuses the existing_wait_for_linux_process_teardownobserver with one deadline. If a full sampled set was captured, root and set outcomes are preserved; if sampling failed before a full set was captured, the exact root identity is still observed whilechromium_process_set_terminatedremains unknown rather than fabricated.Exact hosted CI on production-repair head
9d07ca7b...checked out that SHA and reproduced seven repository-contract failures. They were stale expectations for the replaced individual waiters, not new production failures: two static process-set assertions, three root-observer mock subcases, and two ordinary protocol-fault mock subcases. Production coverage on the same head succeeded and Manifest V3 Compatibility succeeded.The current head reconciles only those inherited tests with the combined observer. The process-set contract now binds the full-set flag, fallback identity tuple, shared observer and conditional set evidence. Failure-path tests inject the shared observer and distinguish observed root exit/survival from observation errors without inventing set evidence. Protocol-fault regressions use the same combined observer for ordinary and forced-close lanes while retaining failure type, driver cleanup and diagnostic-redaction assertions. No timeout value, retry policy, workflow, dependency or production behavior changed after
9d07ca7b....Current verification
3f483abb...: CI34038312988, Rust-contract job101500347732, exact checkout, 229 tests with sole failuretest_browser_pass_uses_only_the_combined_teardown_waiter; coverage succeeded.9d07ca7b...: MV334043022052succeeded; CI34043022061failed only at Python repository contracts with seven stale waiter expectations; production coverage job101513049234succeeded.34046017203is terminal success. Rust-contract job101521080626passed Python repository contracts, canonical formatting, workspace check, full tests, Clippy and API docs. Production coverage job101521080749is terminal success. Manifest V3 Compatibility34046017253is also terminal success.PRRT_kwDOTulPlM6coZwcis resolved only after that exact-head hosted GREEN.Keep Draft. No counted approval, protected-main merge, release, cross-platform supervision, cgroup ownership, post-sampling descendant guarantee, or OS-wide orphan-absence claim is made.