feat(network): bind committed navigation origin - #261
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 |
seonghobae
left a comment
There was a problem hiding this comment.
Fresh current-head source review found a valid Rust quality-contract violation that must be repaired during the required current-parent reconstruction, not by churning this stale child.
AGENTS.md forbids unwrap and expect in first-party production libraries. Exact #261 head 127e02503e48938e29a9a07410574c7e72fc661a currently violates that contract in two production paths introduced/changed by this branch:
webdriver_bidi_command_correlation.rs: theSuccessbranch calls.expect("validated WebDriver BiDi success envelopes always carry a command id"). The parser invariant is real, but encoding it through a panic-capable API contradicts the repository contract.webdriver_bidi_navigation_document_origin.rs:remainder.find(['/', '?', '#']).unwrap_or(remainder.len())is also prohibited productionunwrapusage, even though the fallback is deterministic.
The parent #260 exact current head 56600a6fd982cfafd784f4b7bb659d918113ca90 does not contain the expect; its correlation path still uses the older fail-closed Some/None branch. #261 is already behind that parent by one commit, so do not source-fix this stale lineage or force-rebase it.
Repair acceptance for the post-#260 reconstruction:
- preserve the validated-envelope invariant without
expect,unwrap, panic, coverage exclusion, source rewriting, or a generic correlation shim; prefer making the success command id structurally non-optional at the private envelope/correlation boundary so the impossible state is not represented rather than reintroducing an unreachable recovery branch; - replace the navigation-origin delimiter fallback with a non-unwrap expression while preserving exact URL/origin semantics and hostile-input coverage;
- retain 100% owned production function/line/region/branch coverage and strict rustdoc/Clippy/repository contracts on the reconstructed exact head;
- preserve #261's unique origin-binding delta and #260's typed-correlation repair, then regenerate current-parent exact-head evidence.
This is a source-backed repair finding, not an approval, merge authority, or reason to weaken the queue/coverage gates.
seonghobae
left a comment
There was a problem hiding this comment.
Correction to my earlier review 5093907798 after checking the actual protected-main quality contract and workspace lint configuration.
The .expect(...) finding in webdriver_bidi_command_correlation.rs remains valid: protected-main AGENTS.md forbids production expect, and [workspace.lints.clippy] expect_used = "deny" enforces it.
The second finding was over-broad. remainder.find(...).unwrap_or(remainder.len()) in webdriver_bidi_navigation_document_origin.rs is not the panic-capable Option::unwrap() operation targeted by clippy::unwrap_used; protected main itself contains deliberate unwrap_or(...) uses in first-party crates. Treating every method whose name begins with unwrap as prohibited would misstate the current repository contract. No source change is required solely to replace this deterministic unwrap_or fallback.
Revised repair acceptance for the post-#260 reconstruction: preserve #261's unique origin-binding delta and #260's typed-correlation repair; remove the production .expect(...) while preserving the validated-success-envelope invariant without panic, generic compatibility shims, coverage exclusions, or source rewriting; then regenerate exact-current 100% function/line/region/branch coverage, strict rustdoc/Clippy/repository contracts, reviews, and security evidence. The stale-parent ordering remains unchanged.
Adopt the current document-advance parent without rewriting history and preserve the routing-aware command correlation contract. Signed-off-by: Seongho Bae <me@seonghobae.me>
Signed-off-by: Seongho Bae <me@seonghobae.me>
|
Exact-head CI RCA for
The parent head remained exactly |
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>
|
Restacked onto exact parent PR #260 head Review result: the implementation reuses the existing canonical Local exact-head evidence: focused navigation-origin tests passed; documentation contracts passed (22 tests, 177 subtests); full workspace tests passed; Clippy and rustdoc with |
Commit-Message-Assisted-by: Claude (via Claude Code) Signed-off-by: Seongho Bae <me@seonghobae.me>
|
Non-force restacked on PR #260 exact head |
Commit-Message-Assisted-by: Claude (via Claude Code) Signed-off-by: Seongho Bae <me@seonghobae.me>
|
Non-force restacked on PR #260 exact head |
Preserve the origin-binding implementation and tests while inheriting current connection provenance and executable release contracts. Verify 18 focused tests, 142 Python contracts, full Rust checks and numerical 100% coverage. Retain the unstable branch-measurement warning. Commit-Message-Assisted-by: Codex (via Codex) Signed-off-by: Seongho Bae <me@seonghobae.me>
Partial implementation of #28, stacked on PR #260 branch
feat/webdriver-bidi-navigation-document-advance.Buyer-visible boundary
An accepted committed-navigation observation derives its canonical HTTP(S) origin before registry mutation, advances only the exact caller-captured document epoch, and binds the observed origin only to the new document. Invalid or credential-bearing origins fail before mutation; stale epochs cannot overwrite newer document authority. This remains immediate-use registry evidence, not browser-adapter authentication, action causality, destination authorization, or reusable Agent authority.
Current parent adoption
Current contributor head is
934eb7d37568b439c442ffe1d1f6a9c8f8ed58a0, a normal non-force merge with parents323ac9e147691e9f6572711f5a748e13f1036624and actual #2602c5049aff97a90958e8262b1d403bdcbd64a1e8b. The only content conflict was crate-level documentation; both origin binding and the parent's connection-bound receive/teardown description were retained. The origin-binding implementation and its loopback test file are byte-identical to323ac9e.... No new production abstraction or authority grant was added.Before adoption, native discovery found zero inherited command-correlation release-contract tests and failed the expected-one assertion. Adopting the parent's existing fix makes the same check discover and pass one test.
Fresh local verification
The integrated tree passed 18 focused loopback tests; all 142 Python repository contracts; Rust 1.97.1 format, locked workspace check and tests, all-feature Clippy with warnings denied, rustdoc with warnings denied; Python compileall; and diff checks. Nightly 2026-08-01 production coverage is numerically 100%: 1,173 functions, 12,104 lines, 15,511 regions, and 1,334 branches. The measurement warning
--branch option is unstableremains visible.These are fresh local results only. Hosted checks for this new head must finish independently. Predecessor, queued, skipped, cancelled, status-only, or model-only results are not GREEN or counted review.
Historical evidence and governance
Earlier integration
ccfa13b95295bde4e7a93621ba9add12651aa3bfpreserved the parent's routing-aware success envelope and removed prohibited productionexpect; its CI33875521522and 12 focused tests are historical, not current-head verification. Subsequent323ac9e...had successful native checks but became conflicting after the actual parent advanced; those checks are not transferred to this new composition.The PR remains Draft and unmerged. Parent #260 must satisfy policy and integrate before retargeting or protected merge. Current GitHub review/ruleset authority remains mandatory. No self-approval, bypass, force update, destructive rebase, workflow/ruleset/secret changes, tag, release, or publication was performed.