repair(network): restack committed-navigation subscription on live origin binding - #277
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 |
Signed-off-by: Seongho Bae <me@seonghobae.me>
Signed-off-by: Seongho Bae <me@seonghobae.me>
|
RCA and exact-head refresh for
Local verification on the pushed head passed: Python tests (139), rustfmt, strict workspace/all-target/all-feature Clippy, all workspace/all-target/all-feature tests, rustdoc with warnings denied, and pinned-nightly LLVM function/line/region/branch coverage at 100%. Hosted exact-head checks remain authoritative. No review threads are open; this PR remains draft and stacked on #261. |
Signed-off-by: Seongho Bae <me@seonghobae.me>
|
Parent synchronization update: exact head |
|
Correction: the exact pushed head is |
Signed-off-by: Seongho Bae <me@seonghobae.me>
|
Parent synchronization: exact head |
Signed-off-by: Seongho Bae <me@seonghobae.me>
|
Parent synchronization: exact head |
Signed-off-by: Seongho Bae <me@seonghobae.me>
|
Reconstructed on exact current parent PR #261 head The only merge conflict was CHANGELOG composition. Resolution preserved the parent navigation observation/epoch/origin entries, added the missing buyer-visible context-scoped Local exact-head evidence: all three subscription test suites passed; documentation contracts passed (22 tests, 177 subtests); strict Clippy and rustdoc passed; full workspace tests passed after one policy-permitted retry of the known unrelated macOS revoked-socket |
Commit-Message-Assisted-by: Claude (via Claude Code) Signed-off-by: Seongho Bae <me@seonghobae.me>
|
Resolved the CHANGELOG restack conflict by preserving both the navigation-subscription correlation contract and the parent's coverage/error-vocabulary records. Non-force restacked on PR #261 exact head |
Commit-Message-Assisted-by: Claude (via Claude Code) Signed-off-by: Seongho Bae <me@seonghobae.me>
|
Non-force restacked on PR #261 exact head |
Preserve typed subscription production and test files while integrating current connection provenance and executable release contracts. Verify 16 focused tests, 142 Python contracts, full Rust checks and numerical 100% coverage. Retain the branch-measurement warning. Commit-Message-Assisted-by: Codex (via Codex) Signed-off-by: Seongho Bae <me@seonghobae.me>
seonghobae
left a comment
There was a problem hiding this comment.
Current-parent read-only review finding; no source-writer takeover. The #277 adoption itself is now aligned with actual #261 and correctly retains the existing NavigationCommittedSubscription typed family. One doctoring item is stale on this exact head: ADR 0103 still cites/describes the 18 Aug 2026 WebDriver BiDi Working Draft. W3C's latest published Working Draft is 3 Sep 2026 (https://www.w3.org/TR/2026/WD-webdriver-bidi-20260903/). Recheck/update the dated reference while preserving the current product boundary. The 3 Sep draft still defines session.SubscribeParameters with required non-empty events, optional non-empty contexts/userContexts, and session.SubscribeResult { subscription }, so I found no source-shape reason to replace or rename the existing typed command family. Treat this as standards/TRACEABILITY freshness only; hosted GREEN and real-browser completion remain separate.
|
Follow-up to COMMENT review 5121358906, still read-only/no source ownership: current |
seonghobae
left a comment
There was a problem hiding this comment.
Current-head repair finding from the adopted parent, still read-only/no competing writer. WebDriverBiDiNavigationCommittedSubscriptionCommand::send currently registers NavigationCommittedSubscription correlation before calling write_text_frame, and invalid_frame_timeout_consumes_transport_and_retains_correlation asserts the id remains outstanding for Duration::ZERO. On this parent that is no longer causally correct: write_text_frame rejects zero/>max deadlines before frame construction/I/O, and rejects an adjacent reused masking key as MalformedFrame before any write. Both session.status and session.end already preflight invalid deadlines, and their frame-failure mapping retires the just-registered id when MalformedFrame proves no bytes were emitted. So #277 can leave a phantom outstanding subscription after a purely local rejection, causing avoidable id/resource exhaustion with no possible remote side effect.
Please preserve the existing NavigationCommittedSubscription family and ambiguous-write retention, but adapt the child to the current parent invariant: make invalid timeout RED expect zero correlation before I/O; add the no-write masking-key-preflight case; preflight the reviewed frame timeout before registration; and retire only this typed id when frame preflight returns MalformedFrame. Do not retire for write-mode/timeout/I/O failures after emission may have begun. Update the current body/doctoring sentence that says rejected deadlines intentionally retain correlation. This is independent of hosted GREEN and does not require workflow/provider/authority changes.
Reuse frame validation before reserving correlation. Prove invalid deadlines emit no command and preserve unrelated pending work. Keep post-registration frame-failure retention. Verify 11 focused tests, 142 Python contracts, full Rust gates and numeric 100% coverage. Commit-Message-Assisted-by: Codex (via Codex) Signed-off-by: Seongho Bae <me@seonghobae.me>
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head follow-up on 117f6414e8a6db46eb2b32f4ebae85cf2a208371, read-only/no source-writer takeover. The deadline half of the prior finding is repaired correctly: zero and >max frame deadlines now fail before NavigationCommittedSubscription registration and preserve only pre-existing correlation. One causal half remains.
post_registration_frame_failure_keeps_the_outstanding_command deliberately reuses the immediately previous masking key. The frame owner rejects that adjacent key as WebDriverBiDiWebSocketFrameError::MalformedFrame during local frame preparation, and the loopback proves no subscription command bytes are emitted, yet this test requires the new subscription id to remain outstanding. That leaves a phantom request with no possible remote effect. The adopted parent already treats this MalformedFrame class as provably no-write for session.status / session.end and retires only the just-registered typed id; ambiguous timeout/I/O failures after write may have begun remain outstanding.
Minimal repair remains: RED the adjacent-key case to require preservation of unrelated pre-existing correlation but retirement/reusability of this exact NavigationCommittedSubscription id; on frame error retire only this id when the frame error class proves no bytes were emitted, while retaining correlation for ambiguous write failures. Do not broaden authority or change workflow/provider/gates.
The standards-doctoring item also remains current on this exact head: ADR 0103 still cites the 18 Aug 2026 WebDriver BiDi Working Draft, while W3C's latest published WD is 3 Sep 2026. This does not require renaming the existing typed subscription family.
Repairs the active #28 subscription stack without force-pushing or closing stale PR #262.
Current deadline repair
Current head is
117f6414e8a6db46eb2b32f4ebae85cf2a208371, a normal follow-up commit after the verified parent adoption below. A real-socket regression observed no subscription bytes but two outstanding commands where only one pre-existing command should remain. The sender now reuses the frame owner's existing timeout validator before registration. Both zero and over-limit deadlines preserve the unrelated command and leave the rejected identifier reusable; no bound or wire format changes.Post-registration frame failures still conservatively retain correlation. A separate real-socket test covers this unchanged path using repeated masking-key rejection after a successful empty Pong. The response parser, typed command family, origin binding, and Proposed ADR are unchanged.
Fresh verification for this new source tree passed 11 focused subscription loopback tests, all 142 Python contracts, the complete locked Rust workspace check/tests, format, all-feature Clippy, warning-denying rustdoc, compileall, and diff checks. Numeric production coverage is 100%: 1,221 functions, 12,781 lines, 16,404 regions, and 1,418 branches. The unstable branch-measurement warning remains. New hosted checks must finish independently; earlier coverage is not transferred.
Historical verified parent adoption
Parent-adoption head was
3c0484174eeda0703492ba76b530be125e3e99dd, a normal merge of child01038ba71fb276426cc67f90a91a3c431e194db5and actual #261934eb7d37568b439c442ffe1d1f6a9c8f8ed58a0. Both release records and crate-level capability descriptions were retained when resolving the two documentation conflicts.The two subscription production modules, three integration-test files, and Proposed ADR text are byte-identical to
01038ba7.... The child's existingNavigationCommittedSubscriptioncommand family remains the sole two-line addition to the parent's correlation enum; no generic compatibility shim was restored. The former suggestion to addSessionSubscribeis historical: the current implementation already uses typed subscription registration and response correlation.Product boundary
The command subscribes only to committed navigation in one already-registered context, revalidates that binding before I/O, registers its typed command before a possible remote side effect, and uses the existing bounded verified WebSocket. The bounded opaque receipt remains private in diagnostics. Invalid or ambiguous results and command-kind mismatches preserve unrelated outstanding commands.
This is protocol-correlation evidence, not browser authentication, proof that an event occurred, or navigation/action/policy/secret authority. That parent-adoption commit retained the existing correlation behavior, including invalid deadlines. The subsequent repair above changes only rejection before registration; post-registration retention remains.
Parent-adoption local verification
Native discovery reproduced zero inherited command-correlation release-contract tests before adoption; after reusing the parent fix the same expected-one assertion and test pass. Fresh verification passed 16 focused loopback tests, all 142 Python contracts, Rust 1.97.1 format, locked workspace check/tests, all-feature Clippy, warning-denying rustdoc, compileall, and diff checks.
Production coverage on nightly 2026-08-01 is numerically 100%: 1,220 functions, 12,774 lines, 16,397 regions, and 1,418 branches. The measurement warning
--branch option is unstableremains visible. These results are not hosted GREEN, protected-main integration, or real-browser completion.Preserved historical repair evidence
The original #262 unique subscription delta was reconstructed rather than copied wholesale. Historical #277 head
74fdedb6ee441a4055ee335bc5a5b96dee852661failed native CI33600076214: Rust contracts job100151610266exposed formatting failures (artifact9854587900, ZIP SHA-256d1c6ff0f9ff8f0cd970a41ce61b804460987d6d7a0dee41acbf7af9a7bec3fd9), and coverage job100151609938exposed three removed generic-correlation calls. Current typed registration/correlation fixes that incompatibility without reviving confused-response routing.Historical review
5094483494corrected the overbroad objection to deterministicunwrap_or; the actual prohibited productionexpectwas removed in the parent routing repair. These findings and predecessor results are not current-head verification.Parent-first governance
Keep Draft and unmerged. Hosted checks and reviews must be independently evaluated for this new head; predecessor, queued, skipped, cancelled, model-only, or author-only results are not passing evidence or counted approval. Parent #261 must integrate under current policy before retargeting or protected merge.
Do not close #262 or promote #263+ merely because this reconstruction exists. Require current-parent hosted GREEN and complete unique-delta/successor evidence first. No self-approval, bypass, force update, destructive rebase, workflow/ruleset/secret mutation, gate weakening, tag, release, or publication was performed.