Skip to content

feat(network): bind typed transport evidence to BiDi teardown - #255

Draft
seonghobae wants to merge 127 commits into
feat/webdriver-bidi-session-end-responsefrom
feat/webdriver-bidi-teardown-transport-binding
Draft

feat(network): bind typed transport evidence to BiDi teardown#255
seonghobae wants to merge 127 commits into
feat/webdriver-bidi-session-end-responsefrom
feat/webdriver-bidi-teardown-transport-binding

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Problem and preserved lineage

Session-end acknowledgement, WebSocket Close exchange, TCP closure, browser-process exit and profile cleanup require separate evidence. This Draft retains exact base #252 363a78e36e7690e9ed5bf49829567e00e2ec5d59 and the connection-generation, fresh masking, 64-control budget and one operation-wide deadline protections. Current head: ca490312647516d94a8edc5854be1d5f77eb5b6d.

Server Close-role repair

Real loopback predecessor 10d5e1ff78d46bbad004d4e0971d0ffceac757fa executed hosted RED: a WebDriver/WebSocket server Close(1010) was mirrored, while valid server Close(1011) passed. RFC 6455 §7.4.1 defines 1010 as a valid client Close status and explicitly says it is not used by a server; generic frame/wire validity therefore remains separate from the known peer-role boundary.

Test-first ce4ef7188a289bca836cb8f00cd76737b4eb4196 required the real 1010/1011 loopback contract to return PeerCloseStatusNotAllowed { status_code: 1010 }. Production repair 30b45879e8f3cc3b24244a85524f36ee8898c784 added that role-specific transport-closure error before write_close_frame, preserves Error::source() == None, and leaves the frame serializer able to represent a legitimate client-originated 1010. The public loopback contract proves no 1010 reply while 1011 is still mirrored and followed by clean EOF.

Exact coverage RCA and repair

9e51537946f85874e340b484843a80c82ef42b3d repaired the strict-Clippy failure in the public integration fixture. Its CI 34195499595 is GREEN for repository contracts, rustfmt, full workspace tests, strict Clippy and warnings-denied rustdoc, while Production coverage was one aggregate region short inside the duplicate inline 1010 assertion.

a4371639d4977d98862339656996489780f00daa then removed that inline fixture, but hosted CI 34198089883 falsified that repair. Rust contracts job 101970263206 was exact-head GREEN. Production coverage job 101970263501 measured successfully then failed exact enforcement; artifact 10045289607 (sha256:d18709e2ea30f1d003444ed9e58cf77b716a72d7a68ad102dea89d5b5e841dd6) reported functions 1104/1104, lines 11558/11562, regions 14760/14764, branches 1217/1218. The missing coordinates were the production server-1010 role guard in the crate unit-test instantiation. The separate public integration binary executed that path, but LLVM coverage kept the crate unit-test instantiation separately summarized.

Current exact ca490312647516d94a8edc5854be1d5f77eb5b6d changes only test compilation wiring in originweave-network: the existing realistic public webdriver_bidi_transport_close_role_validation.rs 1010/1011 contract is also compiled into the crate unit-test binary through a test-only self alias/path. This avoids a second synthetic fixture and preserves the public test as the single behavioral source for no 1010 reply, exact typed role error, Display/source semantics, and the valid 1011 mirrored-Close/clean-EOF control. Production WebSocket behavior is unchanged.

Exact-head GREEN — 2026-09-08

CI 34202965629 completed success on exact ca490312647516d94a8edc5854be1d5f77eb5b6d. Rust contracts job 101985783879 passed Python repository contracts, canonical rustfmt, workspace check, full tests, strict Clippy and warnings-denied rustdoc. Production coverage job 101985783660 passed measurement, diagnostics upload and exact production coverage enforcement. The shared-fixture repair is therefore verified on the same head; predecessor evidence is no longer needed for acceptance of this slice.

Both inline review threads currently shown by GitHub are resolved. Keep Draft because this is a stacked child of #252 and because browser-process termination, task-profile cleanup and the complete real Chromium action → observed post-condition → evidence → teardown composition remain separate #28 buyer gaps. The protected parent stack must adopt the valid delta in dependency order; exact-head GREEN is not permission to bypass that stack or claim a release.

No merge, tag, release, workflow/ruleset/secret change, self-approval, force push or destructive restack is claimed.

@coderabbitai

coderabbitai Bot commented Aug 30, 2026

Copy link
Copy Markdown

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

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

Copy link
Copy Markdown
Contributor Author

Test-first peer-role checkpoint published at exact 10d5e1ff78d46bbad004d4e0971d0ffceac757fa (test(network): reject server Close 1010). Production is unchanged. The new real loopback WebDriver/WebSocket fixture completes the opening exchange, sends a server Close(1010), requires no mirrored client Close and a typed malformed/role-invalid failure before closure evidence; the paired Close(1011) control requires the normal masked reply plus PeerCloseThenEof(Some(1011)). This targets the current production validator, which still rejects 1004..=1006 | 1015..=2999 but admits 1010.

RFC 6455 §7.4.1 explicitly defines 1010 for the client and says the server does not use it; 1011 remains the defined server unexpected-condition code. IANA (registry last updated 2026-06-10) likewise lists 1010 Mandatory Ext. and 1011 Internal Error. Hosted CI 34185660439 exists for the exact test-only head, but Production coverage 101933375768 and Rust contracts 101933375996 are still queued with no steps, so neither the intended RED nor any GREEN is claimed. Predecessor hosted run 34184829970 was cancelled after the new head and is not transferred as acceptance.

Writer lease 5578986283 is RELEASED at this checkpoint because no production mutation is authorized until the exact realistic RED actually executes. A successor writer must fresh-read the head/run, reproduce the role-invalid failure, then apply the minimal server-role Close-status admission repair without weakening unassigned-code, repeated-control, masking, shared-deadline, connection-generation, EOF, coverage, or Draft invariants. No merge/release claim.

Copy link
Copy Markdown
Contributor Author

PR-state writer lease ACTIVE only for stale checkpoint repair on current exact 10d5e1ff78d46bbad004d4e0971d0ffceac757fa. Source remains inactive. The body still names predecessor 3a8e6f4... as current and says hosted run 34184829970 is queued even though that predecessor run is now cancelled. I am updating only PR metadata to the exact test-first head, current queued CI, standards role finding, and preserved predecessor evidence; no code/ref/workflow/base/Draft mutation.

Copy link
Copy Markdown
Contributor Author

PR-state-only lease 5579065088 RELEASED. The PR body now names exact 10d5e1ff78d46bbad004d4e0971d0ffceac757fa, current base, the server-1010 test-first contract, predecessor run 34184829970 as cancelled rather than accepted, current queued jobs 101933375768 / 101933375996, preserved transport invariants, and the exact next causal sequence. Source/ref/base/Draft state were not changed.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Writer lease ACTIVE — exact 10d5e1f. I am first reproducing the published real-server Close 1010 test locally. If and only if it fails for the documented role-validation cause, scope is the minimal shared server-role status guard plus focused regression, full coverage, docs/CHANGELOG/AGENTS and visual verification. Preserve #252 parent, all masking/deadline/generation/evidence invariants and Draft state. No force push, workflow/ruleset/secret change, self-approval, merge or release.

@seonghobae seonghobae left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exact-head review of the test-first server-1010 checkpoint: the realistic RED shape is appropriate, but the later production repair must preserve the distinction between wire-code validity and peer-role validity. RFC 6455 §7.4.1 defines 1010 as a legitimate client Close code and explicitly says the server does not use it; 1011 is the defined server unexpected-condition code. Therefore do not repair this by simply adding 1010 to the generic validate_close_frame invalid-code range while retaining the current "Close frame status code is not valid on the wire" diagnosis. That would encode a false global invariant and could later reject a valid client-originated 1010 path for the wrong reason.

After this exact realistic test actually executes RED, keep generic framing/registry validation separate and add the narrow server-to-client role check at the trusted client receive boundary (or make the existing helper explicitly peer-role aware). Reject server 1010 before any mirrored Close or final closure evidence, with a role-accurate bounded diagnostic; preserve server 1011, the existing 1016..2999 unassigned rejection, caller-owned masking entropy, repeated-control budget, shared operation deadline, exact connection generation, and clean-EOF requirement. No production mutation should precede the executed RED.

@seonghobae seonghobae left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exact-head hosted gate has now executed and confirms the intended RED on 10d5e1ff78d46bbad004d4e0971d0ffceac757fa; the earlier queued state is stale. Run 34185660439 Production coverage job 101933375768 reached the real loopback role test: server_close_1011_remains_valid_and_is_mirrored passed, while server_close_1010_is_rejected_before_reply_or_closure_evidence failed at the explicit no-reply assertion because OriginWeave mirrored server Close(1010). This is the documented peer-role defect, not a coverage-threshold failure; measurement stops on test exit 101 before diagnostics/enforcement. The causal production repair is therefore authorized under the active writer lease, but keep wire-code validity separate from server-role validity as in review 5137320615: 1010 remains a legitimate client-originated code, while a server-sent 1010 must fail at the client receive/peer-role boundary before reply or closure evidence. Preserve server 1011 and all existing unassigned-code, repeated-control, masking, deadline, generation, and EOF invariants.

The same exact run also exposes an independent test-source formatting gate: Rust contracts 101933375996 passes all 142 repository contracts, then cargo fmt --all --check fails only on the multiline formatting of the new assert!(reply.is_none(), ...) in webdriver_bidi_transport_close_role_validation.rs; workspace tests/Clippy/rustdoc are therefore not exact-head evidence yet. Canonical rustfmt artifact 10040955946 was emitted. Apply that mechanical formatting together with the minimal role-aware production fix, rerun the focused role fixture first, then full Rust contracts and exact production coverage. Do not transfer predecessor/local GREEN as acceptance.

@seonghobae seonghobae left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One correction is needed in the test-first contract itself before the production repair is finalized. Exact webdriver_bidi_transport_close_role_validation.rs currently asserts that server 1010 returns WebDriverBiDiWebSocketTransportClosureError::Frame { source: WebDriverBiDiWebSocketFrameError::MalformedFrame { .. } }. That over-constrains the RED to the same false global invariant called out in review 5137320615: a Close(1010) frame is syntactically/wire-valid RFC 6455 data, but 1010 is client-only and therefore invalid specifically for this server→OriginWeave-client role.

The executed hosted RED is still valid because the primary post-condition failed first: the client actually mirrored 1010. In the minimal repair, update this secondary assertion with the production boundary rather than forcing the role rule into validate_close_frame. The exact topology already gives the narrow location: in WebDriverBiDiWebSocketTransportClosureObservation::observe_pre_close, after the validated opcode-0x8 frame yields peer_close_status_code and before write_close_frame, reject Some(1010) with a dedicated transport-closure role error (for example a bounded PeerCloseStatusNotAllowed { status_code } / equivalent), no nested MalformedFrame source. Then keep write_close_frame generic so a legitimate client-originated 1010 remains representable. The real peer test should assert (1) no reply, (2) the role-specific typed error, and (3) 1011 still mirrors and requires clean EOF. This preserves the distinction between WebSocket framing validity and BiDi endpoint-role admission instead of encoding policy in the frame serializer.

Co-authored-by: OpenAI Codex <noreply@openai.com>
@seonghobae

Copy link
Copy Markdown
Contributor Author

Writer lease RELEASED after publishing exact 6b6c90e. The real-server 1010 test reproduced RED before the minimal role guard; 1010 now fails before reply/evidence and the 1011 control remains valid. Exact-head local coverage is functions 1105/1105, lines 11574/11574, regions 14787/14787, branches 1218/1218. 142 repository contracts, strict all-target/all-feature Clippy, canonical formatting, warnings-denied rustdoc and doctests pass. Actual isolated Edge visual inspection passed; the owned browser process was terminated. Hosted run 34188785932 is queued, so keep Draft and do not treat this as hosted, protected-main or release acceptance. No source writer lease remains.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Fresh exact-head verification: run 34188785932 is terminal SUCCESS for 6b6c90e. Rust contracts job 101942381020 and Production coverage job 101942381148 both passed. The PR remains Draft/CLEAN on parent 363a78e and is not protected-main or release acceptance.

Copy link
Copy Markdown
Contributor Author

PR-state writer lease ACTIVE on exact 6b6c90ed3919ea84b69527ab688a087eeb45224d only to replace the stale hosted-CI checkpoint in the PR body. Source/ref/base/Draft/workflow/ruleset/secret state remains untouched. Fresh run 34188785932 is terminal success on this exact head; after the body records exact Rust-contract and production-coverage GREEN plus the remaining parent/process/profile gates, this metadata-only lease will be released.

Copy link
Copy Markdown
Contributor Author

PR-state-only lease 5580119992 RELEASED. The body now records exact 6b6c90ed3919ea84b69527ab688a087eeb45224d hosted CI 34188785932 as terminal GREEN, including Rust contracts 101942381020 and Production coverage 101942381148, while retaining Draft and the parent/process/profile/review boundaries. Source/ref/base/workflow/ruleset/secret state was not changed.

Copy link
Copy Markdown
Contributor Author

Source writer lease ACTIVE on exact 6b6c90ed3919ea84b69527ab688a087eeb45224d for one bounded endpoint-role error repair. Fresh source review after hosted GREEN found that server-sent RFC 6455 status 1010 is correctly rejected before reply/evidence, but the closure state machine still reports it as Frame { MalformedFrame }. That collapses wire validity into peer-role admission despite the current AGENTS lesson and earlier review 5137352544. Scope: realistic existing 1010/1011 loopback contract first requires a role-specific transport-closure error; observe RED locally; add the smallest PeerCloseStatusNotAllowed { status_code }-style variant at the closure boundary; focused/full verification; no frame serializer, workflow, base, ref rewrite, policy, release, or other lane changes.

Copy link
Copy Markdown
Contributor Author

Source writer lease 5580138856 RELEASED at exact 30b45879e8f3cc3b24244a85524f36ee8898c784. The bounded delta is three ordinary commits from test-first ce4ef7188...: role-specific PeerCloseStatusNotAllowed { status_code } at the transport-closure boundary, matching Display/Error-source contract, and the real loopback 1010/1011 assertion. Generic frame/wire validation and client-side 1010 representability were not changed. Compare ce4ef...30b458... touches only the closure owner and its role-validation integration test. Current CI 34194095253 is queued; predecessor GREEN is not transferred. No source writer remains from this slice.

@seonghobae seonghobae left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exact-head review of the role-error repair: the current source now preserves the distinction required by RFC 6455 §7.4.1. Server-sent 1010 is rejected in the WebDriver BiDi client transport-closure owner before write_close_frame, as typed PeerCloseStatusNotAllowed { status_code: 1010 }; it is no longer misclassified as wire MalformedFrame. Generic frame validation/serialization remains unchanged, so a legitimate client-originated 1010 is not globally forbidden. The real loopback regression also asserts no 1010 reply, exact typed error/diagnostic with no causal frame source, and retained 1011 masked Close + clean EOF behavior. Compare from test-first ce4ef7188... to this head touches only the closure owner and role-validation integration test.

This review is not GREEN or approval. The test-first run was cancelled while queued after the ordinary successor commits, and current CI 34194095253 is still queued with no job steps. Keep Draft and do not transfer predecessor 6b6c90ed... GREEN. If current Rust/coverage gates fail, repair the exact failure before any restack/acceptance; process exit and profile cleanup remain separate #28 evidence.

Copy link
Copy Markdown
Contributor Author

Writer lease ACTIVE — exact 30b45879e8f3cc3b24244a85524f36ee8898c784, bounded hosted-gate repair only. Fresh CI 34194095253 is terminal FAILURE: repository contracts, rustfmt, workspace check and full tests pass, then strict Clippy rejects the new role-validation integration fixture's expect_err() under -D clippy::expect-used; production coverage measures successfully but exact enforcement reports one region short. Exact coverage artifact 10043530514 (sha256:753ad7128f44cada2d9b8aa30693b122368833f938b15975a315ba8a72c60168) localizes the sole aggregate region gap to the new inline role-error test assertion in webdriver_bidi_websocket_transport_closure.rs, not the production 1010 guard. Scope: remove the duplicate inline test-only coverage artifact while preserving the public real-loopback 1010/1011 contract, replace expect_err with a strict-Clippy-compatible explicit error extraction, run focused/local gates where available, publish ordinary child commits, and leave Draft/base/protocol behavior/workflow/ruleset/secret/merge/release untouched. No competing source writer was present in the fresh comment sweep.

@seonghobae seonghobae left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exact-head gate repair finding. Predecessor CI 34194095253 proves the production role boundary itself behaves correctly under the real loopback 1010/1011 tests, but exact coverage artifact 10043530514 (sha256:753ad7128f44cada2d9b8aa30693b122368833f938b15975a315ba8a72c60168) is functions 1105/1105, lines 11579/11579, branches 1218/1218, regions 14796/14797. The sole aggregate missing region is the false/panic arm generated by the duplicate inline assert!(matches!(...PeerCloseStatusNotAllowed { status_code: 1010 })) at webdriver_bidi_websocket_transport_closure.rs:477, while the production guard at lines 154–159 is executed. Current head leaves that source blob unchanged (5efb37b4...) and fixes the separate strict-Clippy expect_err failure in the public integration regression. Next minimal repair should remove the duplicate inline assertion/test-only coverage artifact or express the public error contract without manufacturing a permanently unexecuted assertion panic region; do not add a deliberately failing test, weaken coverage, or alter the 1010 production guard. Keep the real loopback integration test as the behavioral authority and require same-head Clippy/rustdoc/exact coverage before clearance.

Copy link
Copy Markdown
Contributor Author

Writer lease RELEASED — bounded hosted-gate repair published as ordinary child 9e51537946f85874e340b484843a80c82ef42b3d. The strict-Clippy failure from predecessor 30b45879e8f3cc3b24244a85524f36ee8898c784 is repaired in the public real-loopback role-validation fixture by explicit Result error extraction; no production behavior changed. PR body is code-current and CI 34195499595 is queued. Exact coverage predecessor artifact still proves one separate test-only region gap in the unchanged inline source assertion; review 5138235182 records the minimal next repair and explicitly forbids coverage weakening or a deliberately failing assertion. I am releasing source ownership rather than holding the branch while hosted runners are queued. Draft/base remain unchanged; no force push, destructive rebase, self-approval, merge, workflow/ruleset/secret change, tag or release.

Copy link
Copy Markdown
Contributor Author

Writer lease ACTIVE — exact 9e51537946f85874e340b484843a80c82ef42b3d, bounded hosted-coverage repair only. Fresh CI 34195499595 is terminal FAILURE: Rust contracts 101962201222 is exact-head GREEN through repository contracts, canonical rustfmt, full workspace tests, strict Clippy and warnings-denied rustdoc; Production coverage 101962201239 measures 1105/1105 functions, 11579/11579 lines, 1218/1218 branches, but 14796/14797 regions. Exact artifact 10044341836 (sha256:ec1b2d8117ca254622b91dae77966a9e4f520e4476754d54eb15408d5776170b) localizes the sole aggregate miss to line 477 in the duplicate inline client_only_close_status_is_rejected_before_reply test assertion. The public real-loopback webdriver_bidi_transport_close_role_validation.rs already owns the same 1010 no-reply/type/display/source contract and the paired valid 1011 control. Scope: remove only that redundant inline test artifact, preserve all production/WebSocket behavior and the real-peer regression, publish an ordinary child, and require fresh exact-head Rust/coverage GREEN. No force/rebase, workflow/ruleset/secret/gate change, self-approval, base/Draft mutation, merge, tag or release.

Copy link
Copy Markdown
Contributor Author

Writer lease RELEASED — bounded hosted-coverage repair published as ordinary child a4371639d4977d98862339656996489780f00daa. Exact compare from 9e51537946f85874e340b484843a80c82ef42b3d is one file, +0/-27: only the redundant inline client_only_close_status_is_rejected_before_reply unit fixture was removed. Production code and the public real-loopback 1010/1011 regression are unchanged. Predecessor CI 34195499595 proves Rust contracts GREEN and localizes the sole remaining aggregate region to that removed assertion; current CI 34198089883 is queued, so no exact-head GREEN/Ready/merge claim is made. PR body now records the current head and evidence. No source/docs/refs/PR-state authority is retained by this lease.

Copy link
Copy Markdown
Contributor Author

Writer lease ACTIVE — exact a4371639d4977d98862339656996489780f00daa, bounded exact-coverage RCA/repair only. Fresh hosted CI 34198089883 is terminal FAILURE: Rust contracts 101970263206 is GREEN through repository contracts, rustfmt, workspace tests, strict Clippy and warnings-denied rustdoc; Production coverage 101970263501 measures successfully then fails exact enforcement. Artifact 10045289607 (sha256:d18709e2ea30f1d003444ed9e58cf77b716a72d7a68ad102dea89d5b5e841dd6) shows the previous +0/-27 deletion was not causal: removing the inline 1010 fixture exposed the crate unit-test instantiation of the production server-role guard as uncovered (aggregate functions 1104/1104, lines 11558/11562, regions 14760/14764, branches 1217/1218), while the public integration binary still executes the realistic 1010 RED/GREEN path. Scope: restore only a behavior-minimal real-loopback unit-instantiation witness for server Close(1010) without duplicating the public integration's wire/no-reply/error-shape assertions, verify locally where available, publish one ordinary child, and require fresh exact-head Rust/coverage GREEN. Production behavior, base/Draft, workflow/ruleset/secrets, merge/release remain untouched; no force/rebase/self-approval.

Copy link
Copy Markdown
Contributor Author

Writer lease RELEASED — bounded exact-coverage repair published as ordinary child ca490312647516d94a8edc5854be1d5f77eb5b6d. The prior a437... hosted result falsified deletion-only repair: Rust contracts are GREEN, but exact coverage lost the server-1010 guard in the crate unit-test instantiation. Current child changes only test compilation wiring: the existing realistic public 1010/1011 loopback source is reused inside the crate unit-test binary through a test-only self alias/path, avoiding a second behavioral fixture while exercising the same production instantiation LLVM summarizes separately. Production code/base/Draft are unchanged. CI 34202965629 is queued, so no exact-head GREEN/Ready/merge claim is made. No source/docs/refs/PR-state authority is retained while runners are pending.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant