fix(browser-session): require aggregate-issued lifecycle request authority - #317
seonghobae wants to merge 95 commits into
Conversation
📝 WalkthroughWalkthrough
Changes라이프사이클 포트 바인딩
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~60 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant Caller
participant BoundBrowserSession
participant DisposableContextPort
Caller->>BoundBrowserSession: bind_lifecycle_port(port)
Caller->>BoundBrowserSession: create_disposable_context()
BoundBrowserSession->>DisposableContextPort: create_disposable_context(request)
BoundBrowserSession->>DisposableContextPort: complete_disposable_context_creation(completion)
Caller->>BoundBrowserSession: execute_authorized_context_operation(authority, operation)
BoundBrowserSession->>DisposableContextPort: execute_authorized_context_operation(request)
Caller->>BoundBrowserSession: finish()
BoundBrowserSession->>DisposableContextPort: destroy_disposable_context(request)
Merge Risk: 🟠 High · up to The change reworks browser-session lifecycle ownership so that a session's adapter is bound once and cannot be swapped, which is a solid improvement. However, the test suite for this crate does not currently build (a pattern no longer matches the changed recovery-evidence shape), and a new identity-length test asserts behavior the parser does not yet provide, so the build and test workflow is blocked. An operability counter can also over-report abandoned sessions, and the standards references in the documentation and contract test disagree with each other. These should be fixed before merging. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 51.97% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 127 functions across 14 files. (4 skipped: 4 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head security finding on 97e0a4d875166ad733e78c1d3f213454ee615f01: the new lifecycle request is non-caller-constructible, but its port ownership is still self-asserted through public DisposableContextPortId::new(u64) plus DisposableContextPort::port_id(). Two distinct adapter instances can both report port_id=101; after A is bound, Browser Session's equality check will accept B as the same port. More strongly, A can relay the borrowed aggregate-issued create/destroy request to B, and B can satisfy the same scalar equality and reach remote lifecycle I/O even though B was never the aggregate-approved adapter instance. The current hostile test only uses 101 vs 102, so it proves mismatch rejection but not non-forgeable adapter ownership.
Required RED before this prerequisite can be GREEN: bind adapter A with id 101, then use distinct adapter B also claiming id 101; B must be rejected before create/destroy I/O and must not be able to consume/replay A's request. Do not repair this by documenting port-id uniqueness or randomizing a public scalar. The binding itself needs a non-caller-constructible Browser Session-approved/linear port capability or equivalent identity that a second adapter cannot self-select or replay. Keep remote BiDi identifiers outside Browser Session domain truth.
Separately, current hosted CI 34520503299 is still RED at rustfmt before tests/Clippy/rustdoc, and coverage measurement also fails; fix that operational RED without weakening gates after the authority model is corrected.
|
Exact-head RED evidence for Coverage job
So a distinct adapter B that self-reports the already-bound scalar port id can both reach create I/O and reach destroy I/O. This falsifies the current claim that aggregate-issued request + Repository status on this exact head is independently RED at |
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head security finding on 9caf9bbe4228c443b7d5a4279831765a6a38765a: the current port-binding preflight still cannot guarantee the stated “reject before lifecycle I/O” boundary because bind_lifecycle_port / require_bound_lifecycle_port call the public trait method DisposableContextPort::port_id(&self) before issuing or validating the aggregate request. Rust &self is not a purity guarantee: an implementation can use interior mutability, a socket/client handle, atomics, or delegated state and perform remote side effects from port_id(). The rustdoc requirement that port_id be side-effect-free is therefore a convention, not an enforced security invariant. The same problem remains even after replacing the scalar id with a harder-to-forge value if Browser Session must first invoke an arbitrary adapter callback to discover/validate that value.
Required RED: use a hostile port whose identity/preflight callback records a lifecycle side effect, then exercise (a) a first create attempt and (b) a mismatched destroy/create attempt that Browser Session is expected to reject. Both must show zero adapter/browser mutation before an aggregate-issued authority exists and is validated. Do not satisfy this by documenting port_id() purity or by testing only the reviewed WebDriver adapter implementation.
Causal direction: remove authority-sensitive identity discovery from an arbitrary preflight callback. Establish the lifecycle-port binding through a non-caller-constructible composition/binding capability (or a session-owned/linear port wrapper) before lifecycle mutation, then invoke the adapter only under the already validated create/destroy request. If DisposableContextPort remains externally implementable, the contract must not rely on &self implying side-effect freedom. Preserve the current same-id/different-adapter hostile RED and the requirement that requests cannot be replayed into a different bound adapter. Keep remote BiDi identifiers outside Browser Session domain truth.
|
@coderabbitai review Please review exact head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '== Exact revision and PR diff summary =='
git rev-parse HEAD
git status --short
git diff --stat 6d87dff5dc572fbd74d06309d574a998f23cf02f..HEAD
git diff --name-only 6d87dff5dc572fbd74d06309d574a998f23cf02f..HEAD
printf '%s\n' '== Candidate lifecycle files =='
fd -t f -e rs . | while IFS= read -r file; do
if rg -q -i 'BrowserSession|BoundBrowserSession|DisposableContextPort|bind_lifecycle_port|DisposableContext(Create|Destroy)Request' "$file"; then
printf '%s\n' "$file"
fi
done
printf '%s\n' '== Rust API outline for candidate files =='
fd -t f -e rs . | while IFS= read -r file; do
if rg -q -i 'BrowserSession|BoundBrowserSession|DisposableContextPort|bind_lifecycle_port|DisposableContext(Create|Destroy)Request' "$file"; then
ast-grep outline "$file" --items all --type struct,trait,impl,function,method --match 'BrowserSession|BoundBrowserSession|DisposableContextPort|bind_lifecycle_port|DisposableContextCreateRequest|DisposableContextDestroyRequest' || true
fi
done
printf '%s\n' '== Lifecycle declarations, calls, and port-ID remnants =='
rg -n -C 4 --type rust \
'(\bbind_lifecycle_port\s*\(|\b(create|destroy)_(disposable_)?context\s*\(|\bDisposableContextPort\b|\bBoundBrowserSession\b|\bDisposableContextPortId\b|\bport_id\s*\()' \
.
printf '%s\n' '== Browser Session BiDi/protocol-specific references =='
rg -n -i -C 3 --type rust \
'(bidi|webdriver|pending|quarantin|remote.liveness|remote_liveness|accepted)' \
. \
-g '!**/test/**' -g '!**/tests/**' || true
printf '%s\n' '== Documentation and external fixtures relevant to binding =='
fd -t f . | while IFS= read -r file; do
if rg -q -i 'BoundBrowserSession|bind_lifecycle_port|DisposableContextPortId|distinct.adapter|hostile|linear.lifecycle|ADR.?.?0114' "$file"; then
printf '%s\n' "$file"
fi
doneLength of output: 147304 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '== Browser Session public types and lifecycle implementation =='
sed -n '1,720p' crates/originweave-browser-session/src/lib.rs
printf '%s\n' '== Focused external hostile tests =='
for file in \
crates/originweave-browser-session/tests/lifecycle_port_authority.rs \
crates/originweave-browser-session/tests/lifecycle_port_preflight_side_effect.rs \
crates/originweave-browser-session/tests/lifecycle_port_same_id_spoof.rs \
crates/originweave-browser-session/tests/sequential_incarnation_reuse.rs \
crates/originweave-browser-session/tests/destroy_failure_requires_recovery.rs \
tests/test_browser_session_lifecycle_contract.py
do
printf '\n== %s ==\n' "$file"
cat -n "$file"
done
printf '%s\n' '== Browser Session dependency boundary =='
browser_manifest="$(fd -t f '^Cargo\.toml$' crates/originweave-browser-session | head -n 1)"
printf '%s\n' "$browser_manifest"
cat -n "$browser_manifest"
printf '%s\n' '== Direct Browser Session crate imports =='
rg -n '^\s*use\s+originweave_[A-Za-z0-9_]+' crates/originweave-browser-session/src crates/originweave-browser-session/tests
printf '%s\n' '== Public mutable extraction and lifecycle helper reachability =='
rg -n -C 3 --type rust \
'(pub(\([^)]*\))?\s+(fn|struct|enum|trait)|fn\s+(create|destroy)_disposable_context_with_port|fn\s+(into_|as_mut|lifecycle_port_mut)|&mut\s+(self|P)|->\s*&mut)' \
crates/originweave-browser-session/src/lib.rs
printf '%s\n' '== Focused PR diff for implementation and contracts =='
git diff --find-renames --find-copies --unified=50 \
6d87dff5dc572fbd74d06309d574a998f23cf02f..HEAD -- \
crates/originweave-browser-session/src/lib.rs \
tests/test_browser_session_lifecycle_contract.py \
docs/adr/0114-browser-session-disposable-context-authority.md \
docs/traceability/browser-session-lifecycle-authority.md \
docs/uml/browser-session-lifecycle-authority.md
printf '%s\n' '== Changed documents =='
for file in \
docs/adr/0114-browser-session-disposable-context-authority.md \
docs/traceability/browser-session-lifecycle-authority.md \
docs/uml/browser-session-lifecycle-authority.md
do
printf '\n== %s ==\n' "$file"
cat -n "$file"
doneLength of output: 215228 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '== Repository-wide lifecycle API references =='
rg -n -C 3 --type rust \
'(\bcreate_disposable_context\s*\(|\bdestroy_disposable_context\s*\(|\bbind_lifecycle_port\s*\(|\blifecycle_port_mut\b|\binto_(inner|port|browser_session)\b|\bDisposableContextPortId\b|\bport_id\s*\()' \
.
printf '%s\n' '== Public methods on BrowserSession and BoundBrowserSession =='
rg -n -U --type rust \
'impl(?:<[^>]+>)?\s+(?:BrowserSession|BoundBrowserSession(?:<[^>]+>)?)\s*\{(?s:.*?)^\}' \
crates/originweave-browser-session/src/lib.rs || true
rg -n -C 2 --type rust \
'pub\s+(?:const\s+)?fn\s+(?:create_disposable_context|destroy_disposable_context|bind_lifecycle_port|lifecycle_port|browser_session|into_[A-Za-z_]+|[A-Za-z_]+_mut)\s*\(' \
crates/originweave-browser-session/src/lib.rs
printf '%s\n' '== Trait implementation locations =='
rg -n -C 2 --type rust 'impl\s+(?:[A-Za-z0-9_<>:]+\s+for\s+)?DisposableContextPort\b' .
printf '%s\n' '== Patch whitespace integrity =='
git diff --check 6d87dff5dc572fbd74d06309d574a998f23cf02f..HEAD
printf '%s\n' '== Exact-head PR checks and review state =='
gh pr view 317 --repo ContextualWisdomLab/OriginWeave \
--json number,headRefOid,isDraft,mergeStateStatus,reviewDecision,statusCheckRollup \
--jq '{number, headRefOid, isDraft, mergeStateStatus, reviewDecision, checks: [.statusCheckRollup[] | {name, status, conclusion, workflowName}]}'Length of output: 68284
I found no code-level issue in the requested security/API change.
The structural binding prevents ordinary callers from substituting or replaying a different adapter after binding. It cannot prevent a malicious trusted Exact-head verification is not complete. GitHub reports ✅ Action performedReview finished.
|
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Merge-blocking integration finding on exact 9cde981899950b900698a17e7fa739af59f6bb4f: the linear BoundBrowserSession<P> closes caller-side port substitution, but the lifecycle contract still has no aggregate→adapter completion phase for creation, and DisposableContextCreateRequest is not unique per create attempt. Every create request in one Browser Session incarnation exposes only the same (BrowserSessionId, BrowserSessionIncarnation) pair, even though BrowserSession already reserves a distinct context epoch before remote I/O. After port.create_disposable_context(&request) returns, the aggregate may accept the handle or reject it as duplicate/ambiguous and enter RecoveryRequired, but DisposableContextPort receives no accept/reject completion carrying a per-attempt identity. A BiDi adapter implementing #316 therefore cannot deterministically move the exact remote tuple from pending→accepted or pending→quarantine without inventing adapter-local correlation/authority, keying only by the same session/incarnation, relying on call order, or prematurely authorizing before Browser Session validation.
Required RED before adoption: one bound port produces at least two creation candidates in the same Browser Session incarnation, retains them as separate pending protocol tuples, and Browser Session accepts one while rejecting the other (e.g. duplicate domain context/isolation). The adapter must promote only the accepted candidate and quarantine exactly the rejected candidate; neither candidate may collide/overwrite because their create requests are indistinguishable. No remote tuple may become authorizing before aggregate acceptance.
Causal repair should extend the Browser Session-owned transaction boundary, not move BiDi ids into this domain: mint a non-caller-constructible per-attempt lifecycle identity/capability (the already-reserved epoch is a natural candidate if its semantics fit), pass it in the create request, and provide an aggregate-issued accept/reject completion that the exact bound port consumes. The adapter keeps protocol tuples pending until that completion. Preserve structural port binding and the existing no-preflight/same-id hostile tests. #316 remains the owner of remote BiDi pending/accepted/quarantine data; #317 should only provide the domain transaction identity/completion contract it needs.
|
@coderabbitai review Please review exact head
Verify these against |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
expected='77819f1236d295f9cca2c5eced72cac9b812b78f'
actual="$(git rev-parse HEAD)"
printf '== revision ==\nexpected=%s\nactual=%s\n' "$expected" "$actual"
git status --short
printf '\n== exact-head changed files ==\n'
git diff --name-status "${expected}^" "$expected"
printf '\n== focused file outlines ==\n'
for file in \
crates/originweave-browser-session/src/recovery.rs \
crates/originweave-browser-session/src/browser_session.rs \
crates/originweave-browser-session/tests/recovery_owner_handoff.rs \
crates/originweave-browser-session/tests/proven_destroy_releases_hot_ownership.rs
do
printf '\n== %s ==\n' "$file"
ast-grep outline "$file" --items all --type struct,enum,trait,impl,function,method \
--match 'BoundBrowserSessionRecovery|BoundBrowserSession|BrowserSession|Recovery|destroy|context|authority|end' || true
done
printf '\n== exact documentation and contract patch ==\n'
git diff --find-renames --unified=80 "${expected}^" "$expected" -- \
docs/adr/0115-browser-session-recovery-custody-and-hot-ownership.md \
docs/adr/README.md \
docs/traceability/browser-session-lifecycle-authority.md \
docs/uml/browser-session-lifecycle-authority.md \
tests/test_browser_session_lifecycle_contract.py
printf '\n== requested documentation and repository contract ==\n'
for file in \
docs/adr/0115-browser-session-recovery-custody-and-hot-ownership.md \
docs/adr/README.md \
docs/traceability/browser-session-lifecycle-authority.md \
docs/uml/browser-session-lifecycle-authority.md \
tests/test_browser_session_lifecycle_contract.py
do
printf '\n== %s ==\n' "$file"
cat -n "$file"
done
printf '\n== recovery custody implementation and authority escape search ==\n'
rg -n -C 14 --type rust \
'(struct\s+BoundBrowserSessionRecovery|impl.*BoundBrowserSessionRecovery|fn\s+into_recovery|pub(\([^)]*\))?\s+(?:const\s+)?fn|browser_session|lifecycle_port|into_(inner|port|browser_session)|Deref(Mut)?|From<.*BoundBrowserSessionRecovery|Into<.*BoundBrowserSession|create_disposable_context|destroy_disposable_context|advance_context_epoch|presentation_authority|finish)' \
crates/originweave-browser-session/src/recovery.rs \
crates/originweave-browser-session/src/browser_session.rs \
crates/originweave-browser-session/src/lib.rs
printf '\n== destruction, hot ownership, uncertainty, abandonment, and reuse implementation ==\n'
rg -n -C 14 --type rust \
'(enum\s+OwnedContextState|struct\s+OwnedContextRecord|fn\s+(destroy_disposable_context_with_port|context_for_authority_mut|end|has_unresolved_remote_ownership|record_transport_loss)|contexts\.(remove|get_mut|insert|is_empty)|UnprovenDestruction|RecoveryRequiredOwnedHandle|TransportLossOwnedHandle|next_epoch|context_epoch|port\.destroy_disposable_context\s*\()' \
crates/originweave-browser-session/src/browser_session.rs \
crates/originweave-browser-session/src/recovery.rs
printf '\n== focused hostile tests ==\n'
for file in \
crates/originweave-browser-session/tests/recovery_owner_handoff.rs \
crates/originweave-browser-session/tests/proven_destroy_releases_hot_ownership.rs
do
printf '\n== %s ==\n' "$file"
cat -n "$file"
done
printf '\n== protocol ownership and `#316` references ==\n'
rg -n -C 8 '(`#316`|BiDi|WebDriver|protocol-specific|protocol specific|remote liveness|recovery)' \
docs/adr/0115-browser-session-recovery-custody-and-hot-ownership.md \
docs/traceability/browser-session-lifecycle-authority.md \
docs/uml/browser-session-lifecycle-authority.md \
crates/originweave-browser-session/src
printf '\n== contract coverage of docs, public surface, and hostile tests ==\n'
rg -n -C 12 \
'(0115|recovery_owner_handoff|proven_destroy_releases_hot_ownership|BoundBrowserSessionRecovery|into_recovery|browser_session|create_attempt_recovery_evidence|recovery_evidence|hot ownership|same.raw|same-raw|`#316`)' \
tests/test_browser_session_lifecycle_contract.py
printf '\n== whitespace integrity ==\n'
git diff --check "${expected}^" "$expected"Length of output: 204516 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '== ADR index entries for 0114 and 0115 =='
rg -n -C 8 '(0114|0115|Browser Session)' docs/adr/README.md
printf '%s\n' '== ADR 0115 exact decision, evidence, and adoption claims =='
sed -n '1,155p' docs/adr/0115-browser-session-recovery-custody-and-hot-ownership.md | nl -ba -v 1
printf '%s\n' '== Traceability recovery custody, hot ownership, and status claims =='
sed -n '70,170p' docs/traceability/browser-session-lifecycle-authority.md | nl -ba -v 70
printf '%s\n' '== UML recovery custody and same-raw-identity sections =='
sed -n '100,245p' docs/uml/browser-session-lifecycle-authority.md | nl -ba -v 100
printf '%s\n' '== Python repository contract complete architecture-document method =='
sed -n '264,430p' tests/test_browser_session_lifecycle_contract.py | nl -ba -v 264
printf '%s\n' '== Public recovery surface with all public impls =='
sed -n '1,150p' crates/originweave-browser-session/src/recovery.rs | nl -ba -v 1
rg -n -U --type rust 'impl(?:<[^>]+>)?\s+BoundBrowserSessionRecovery(?:<[^>]+>)?\s*\{(?s:.*?)^\}' crates/originweave-browser-session/src
printf '%s\n' '== Destroy path and completion/end/abandonment classification =='
sed -n '585,615p' crates/originweave-browser-session/src/browser_session.rs | nl -ba -v 585
sed -n '875,1010p' crates/originweave-browser-session/src/browser_session.rs | nl -ba -v 875
sed -n '1030,1125p' crates/originweave-browser-session/src/browser_session.rs | nl -ba -v 1030
printf '%s\n' '== exact-head docs contract diff integrity =='
git diff --check 77819f1236d295f9cca2c5eced72cac9b812b78f^ 77819f1236d295f9cca2c5eced72cac9b812b78fLength of output: 69765
I found no implementation, documentation, or repository-contract defect in the requested slice.
The hot-ownership documentation matches the implementation.
The protocol boundary remains clear.
The active-PR maturity boundary is also correct.
This was a static review. It is not test, CI, or protected-main GREEN evidence. You are interacting with an AI system. |
|
@coderabbitai review Please review exact head
Verify there is no remaining 0115 collision in #317, that ADR 0116 is Proposed and code-current with |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
expected='cde661784345b9489f2a98f4ccc4d261b6c8da3d'
actual="$(git rev-parse HEAD)"
printf '== revision ==\nexpected=%s\nactual=%s\n' "$expected" "$actual"
git status --short
printf '\n== changed files at exact head ==\n'
git diff --name-status "${expected}^" "$expected"
printf '\n== ADR filename inventory ==\n'
fd -t f '^011[56].*\.md$' docs/adr -x sh -c 'printf "%s\n" "$1"' sh {}
printf '\n== all ADR 0115/0116 references in changed PR material ==\n'
rg -n -C 8 '(ADR[ -]?0115|ADR[ -]?0116|0115-browser-session|0116-browser-session|`#316`|`#317`)' \
docs tests crates/originweave-browser-session
printf '\n== exact documentation and repository-contract patch ==\n'
git diff --find-renames --unified=90 "${expected}^" "$expected" -- \
docs/adr/0116-browser-session-recovery-custody-and-hot-ownership.md \
docs/adr/README.md \
docs/traceability/browser-session-lifecycle-authority.md \
docs/uml/browser-session-lifecycle-authority.md \
tests/test_browser_session_lifecycle_contract.py
printf '\n== requested documentation and contract ==\n'
for file in \
docs/adr/0116-browser-session-recovery-custody-and-hot-ownership.md \
docs/adr/README.md \
docs/traceability/browser-session-lifecycle-authority.md \
docs/uml/browser-session-lifecycle-authority.md \
tests/test_browser_session_lifecycle_contract.py
do
printf '\n== %s ==\n' "$file"
cat -n "$file"
done
printf '\n== focused source and tests ==\n'
for file in \
crates/originweave-browser-session/src/recovery.rs \
crates/originweave-browser-session/src/browser_session.rs \
crates/originweave-browser-session/tests/recovery_owner_handoff.rs \
crates/originweave-browser-session/tests/proven_destroy_releases_hot_ownership.rs \
crates/originweave-browser-session/tests/destroy_failure_requires_recovery.rs
do
printf '\n== %s ==\n' "$file"
ast-grep outline "$file" --items all --type struct,enum,trait,impl,function,method \
--match 'BoundBrowserSessionRecovery|BoundBrowserSession|BrowserSession|Recovery|destroy|context|authority|end' || true
cat -n "$file"
done
printf '\n== public recovery surface and authority escape paths ==\n'
rg -n -C 10 --type rust \
'(struct\s+BoundBrowserSessionRecovery|impl.*BoundBrowserSessionRecovery|fn\s+into_recovery|pub(\([^)]*\))?\s+(?:const\s+)?fn|browser_session|lifecycle_port|into_(inner|port|browser_session)|Deref(Mut)?|From<.*BoundBrowserSessionRecovery|Into<.*BoundBrowserSession|create_disposable_context|destroy_disposable_context|advance_context_epoch|presentation_authority|finish)' \
crates/originweave-browser-session/src
printf '\n== hot ownership, failure, and abandonment implementation ==\n'
rg -n -C 14 --type rust \
'(enum\s+OwnedContextState|struct\s+OwnedContextRecord|fn\s+(destroy_disposable_context_with_port|context_for_authority_mut|end|has_unresolved_remote_ownership|record_transport_loss)|contexts\.(remove|get_mut|insert|is_empty)|UnprovenDestruction|RecoveryRequiredOwnedHandle|TransportLossOwnedHandle|next_epoch|context_epoch|port\.destroy_disposable_context\s*\()' \
crates/originweave-browser-session/src
printf '\n== contract coverage for ADR number, custody, and hot ownership ==\n'
rg -n -C 14 \
'(0115|0116|recovery_owner_handoff|proven_destroy_releases_hot_ownership|BoundBrowserSessionRecovery|into_recovery|browser_session|compile_fail|hot ownership|same.raw|same-raw|protected.main|protected-main|IMPLEMENTED_ON_ACTIVE_PR|Proposed)' \
tests/test_browser_session_lifecycle_contract.py
printf '\n== whitespace integrity ==\n'
git diff --check "${expected}^" "$expected"
printf '\n== exact-head GitHub status ==\n'
gh pr view 317 --repo ContextualWisdomLab/OriginWeave \
--json headRefOid,mergeStateStatus,reviewDecision,statusCheckRollup \
--jq '{headRefOid, mergeStateStatus, reviewDecision, checks: [.statusCheckRollup[] | {name, status, conclusion, workflowName}]}' || trueLength of output: 204446 🏁 Script executed: #!/bin/bash
set -euo pipefail
expected='cde661784345b9489f2a98f4ccc4d261b6c8da3d'
printf '== revision ==\nexpected=%s\nactual=%s\n' "$expected" "$(git rev-parse HEAD)"
printf '\n== direct-parent changed ADR and documentation paths ==\n'
git diff --name-status "${expected}^" "$expected" -- docs/adr docs/traceability docs/uml tests/test_browser_session_lifecycle_contract.py
printf '\n== ADR 0115 and ADR 0116 files ==\n'
fd -t f '0115|0116' docs/adr -x sh -c 'printf "\n== %s ==\n"; cat -n "$1"' sh {}
printf '\n== direct-parent ADR collision diff ==\n'
git diff --unified=30 "${expected}^" "$expected" -- docs/adr docs/traceability/browser-session-lifecycle-authority.md docs/uml/browser-session-lifecycle-authority.md tests/test_browser_session_lifecycle_contract.py
printf '\n== `#317` changed-file references to 0115/0116 ==\n'
git diff --name-only 6d87dff5dc572fbd74d06309d574a998f23cf02f.."$expected" | while IFS= read -r file; do
case "$file" in
docs/*|tests/*|crates/originweave-browser-session/*)
if rg -n -i '(ADR[ -]?0115|ADR[ -]?0116|0115-browser-session|0116-browser-session)' "$file"; then
printf '%s\n' "-- file: $file"
fi
;;
esac
done
printf '\n== requested documentation, focused sections ==\n'
sed -n '1,240p' docs/adr/0116-browser-session-recovery-custody-and-hot-ownership.md | nl -ba -v 1
sed -n '1,220p' docs/adr/README.md | nl -ba -v 1
sed -n '1,230p' docs/traceability/browser-session-lifecycle-authority.md | nl -ba -v 1
sed -n '1,300p' docs/uml/browser-session-lifecycle-authority.md | nl -ba -v 1
printf '\n== recovery owner implementation ==\n'
cat -n crates/originweave-browser-session/src/recovery.rs
printf '\n== handoff implementation and Drop ==\n'
sed -n '560,650p' crates/originweave-browser-session/src/browser_session.rs | nl -ba -v 560
sed -n '990,1135p' crates/originweave-browser-session/src/browser_session.rs | nl -ba -v 990
printf '\n== destroy and recovery implementation ==\n'
sed -n '875,1005p' crates/originweave-browser-session/src/browser_session.rs | nl -ba -v 875
printf '\n== recovery ownership hostile tests ==\n'
cat -n crates/originweave-browser-session/tests/recovery_owner_handoff.rs
cat -n crates/originweave-browser-session/tests/proven_destroy_releases_hot_ownership.rs
printf '\n== contract architecture section ==\n'
sed -n '264,430p' tests/test_browser_session_lifecycle_contract.py | nl -ba -v 264
printf '\n== public recovery conversion and raw port escape search ==\n'
rg -n -C 4 --type rust \
'(pub\s+(?:const\s+)?fn\s+|impl.*(Deref|From|Into)|struct\s+BoundBrowserSessionRecovery|fn\s+(browser_session|lifecycle_port|into_(inner|port|browser_session)|create_disposable_context|destroy_disposable_context|advance_context_epoch|presentation_authority|finish))' \
crates/originweave-browser-session/src/recovery.rs crates/originweave-browser-session/src/browser_session.rsLength of output: 191174
I found no issue in the requested ADR collision repair or Browser Session documentation and contract slice.
The documentation is code-current with the reviewed implementation.
The hot-ownership statements also match the source.
This is a static review only. You are interacting with an AI system. |
|
@coderabbitai review Please review current exact head |
|
@coderabbitai review Exact head Fresh source verification found that |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
expected='9d48b3e0f81dfadd20140fb5145794a5c2df1292'
actual="$(git rev-parse HEAD)"
printf '== revision ==\nexpected=%s\nactual=%s\n' "$expected" "$actual"
git status --short
printf '\n== direct-parent changes ==\n'
git diff --name-status "${expected}^" "$expected"
printf '\n== focused file outlines ==\n'
for file in \
crates/originweave-browser-session/src/browser_session.rs \
crates/originweave-browser-session/tests/user_context_identity_length.rs
do
printf '\n== %s ==\n' "$file"
ast-grep outline "$file" --items all --type struct,enum,impl,function,method \
--match 'DisposableIsolationId|parse|identity|context|user' || true
done
printf '\n== parser declaration, error handling, and all call sites ==\n'
rg -n -C 12 --type rust \
'(struct\s+DisposableIsolationId|enum\s+DisposableIsolationId|impl\s+DisposableIsolationId|fn\s+parse\s*\(|DisposableIsolationId::parse\s*\(|is_empty|trim|control|is_control|chars\(\)|len\(\)|4096)' \
crates/originweave-browser-session/src crates/originweave-browser-session/tests
printf '\n== focused source and hostile test ==\n'
sed -n '1,260p' crates/originweave-browser-session/src/browser_session.rs | nl -ba -v 1
printf '\n== crates/originweave-browser-session/tests/user_context_identity_length.rs ==\n'
cat -n crates/originweave-browser-session/tests/user_context_identity_length.rs
printf '\n== identity representation contracts and documentation ==\n'
rg -n -C 8 \
'(DisposableIsolationId|browser\.UserContext|user context|UserContext|4096|whitespace|control character|identity)' \
docs tests crates/originweave-browser-session
printf '\n== exact identity-related patch ==\n'
git diff --find-renames --unified=60 "${expected}^" "$expected" -- \
crates/originweave-browser-session/src/browser_session.rs \
crates/originweave-browser-session/tests/user_context_identity_length.rs \
tests/test_browser_session_lifecycle_contract.py \
docs
printf '\n== public type surface and construction sites ==\n'
rg -n -C 6 --type rust \
'(pub\s+(?:struct|enum)\s+DisposableIsolationId|pub\s+(?:const\s+)?fn\s+(parse|as_str)|DisposableIsolationId\s*\{|DisposableIsolationId\()' \
crates/originweave-browser-session/src
printf '\n== whitespace integrity ==\n'
git diff --check "${expected}^" "$expected"Length of output: 202724 🤖 web_search executed:
Length of output: 2337
P1 — Remove the local
The W3C WebDriver BiDi definition is: The inspected specification result defines no non-empty rule, whitespace rule, control-character rule, or maximum-length rule for this type. A local parser must not narrow this protocol representation without versioned primary evidence for that narrower constraint. Make This change affects protocol addressability and recovery evidence only. It must not change lifecycle mutation authority. The prior 4097-byte case remains necessary, but it is insufficient by itself. Keep all four cases in The retrieved W3C publication guidance also remains applicable: the dated 9 September 2026 publication is standards provenance, not runtime qualification. The separately qualified runtime revision must not change from this representation repair alone. This is a static review. I did not run tests or CI. The earlier learning that requires non-empty, non-control, non-surrounding-whitespace values now conflicts with this review. Please confirm that it is obsolete before I remove it. ✏️ Learnings added
🧠 Learnings usedYou are interacting with an AI system. |
|
@coderabbitai review Please review exact head |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
expected='9a886b4bb8feeddef746b5bad2646f5c414d4189'
actual="$(git rev-parse HEAD)"
printf '== revision ==\nexpected=%s\nactual=%s\n' "$expected" "$actual"
git status --short
printf '\n== direct-parent changed files ==\n'
git diff --name-status "${expected}^" "$expected"
printf '\n== relevant module and test outlines ==\n'
for file in \
crates/originweave-browser-session/src/lib.rs \
crates/originweave-browser-session/src/browser_session.rs \
crates/originweave-browser-session/src/recovery.rs \
crates/originweave-browser-session/tests/user_context_identity_length.rs \
crates/originweave-browser-session/tests/recovery_owner_handoff.rs
do
if test -f "$file"; then
printf '\n== %s ==\n' "$file"
ast-grep outline "$file" --items all --type struct,enum,trait,impl,function,method \
--match 'DisposableIsolationId|BrowserSession|BoundBrowserSession|BoundBrowserSessionRecovery|PresentationMutationAuthority|parse|presentation_authority|recovery' || true
fi
done
printf '\n== parser declaration, implementation, and all construction/call sites ==\n'
rg -n -C 12 --type rust \
'(struct\s+DisposableIsolationId|enum\s+DisposableIsolationIdError|impl\s+DisposableIsolationId|fn\s+parse\s*\(|DisposableIsolationId::parse\s*\(|is_empty|trim|is_control|chars\(\)|len\(\)|4096|as_str\s*\()' \
crates/originweave-browser-session/src crates/originweave-browser-session/tests tests
printf '\n== public Browser Session projections and capability-minting paths ==\n'
rg -n -C 12 --type rust \
'(struct\s+(BrowserSession|BoundBrowserSession|BoundBrowserSessionRecovery|PresentationMutationAuthority)|impl.*(BrowserSession|BoundBrowserSession|BoundBrowserSessionRecovery)|pub(\([^)]*\))?\s+(?:const\s+)?fn\s+|fn\s+presentation_authority\s*\(|PresentationMutationAuthority\s*\{|presentation.*authorit|browser_session\(\)|Deref(Mut)?|From<.*BoundBrowserSessionRecovery|Into<.*BoundBrowserSession)' \
crates/originweave-browser-session/src
printf '\n== focused source sections ==\n'
sed -n '1,280p' crates/originweave-browser-session/src/browser_session.rs | nl -ba -v 1
sed -n '520,1135p' crates/originweave-browser-session/src/browser_session.rs | nl -ba -v 520
if test -f crates/originweave-browser-session/src/recovery.rs; then
cat -n crates/originweave-browser-session/src/recovery.rs
fi
cat -n crates/originweave-browser-session/src/lib.rs
printf '\n== requested hostile tests and relevant contracts ==\n'
for file in \
crates/originweave-browser-session/tests/user_context_identity_length.rs \
crates/originweave-browser-session/tests/recovery_owner_handoff.rs \
tests/test_browser_session_lifecycle_contract.py
do
if test -f "$file"; then
printf '\n== %s ==\n' "$file"
cat -n "$file"
fi
done
printf '\n== identity and read-only projection references in documentation/contracts ==\n'
rg -n -C 8 \
'(DisposableIsolationId|browser\.UserContext|CDDL\s+`?text|4096|whitespace|control.character|lossless|presentation.authority|read-only|recovery custody|BoundBrowserSessionRecovery)' \
docs tests crates/originweave-browser-session
printf '\n== compile-fail contracts and projection escape checks ==\n'
rg -n -U -C 10 \
'(compile_fail|BoundBrowserSessionRecovery|browser_session\(\)|presentation_authority|PresentationMutationAuthority|Deref|into_(inner|port|browser_session)|lifecycle_port)' \
crates/originweave-browser-session/src tests
printf '\n== exact patch and whitespace integrity ==\n'
git diff --find-renames --unified=70 "${expected}^" "$expected" -- \
crates/originweave-browser-session/src \
crates/originweave-browser-session/tests/user_context_identity_length.rs \
tests/test_browser_session_lifecycle_contract.py \
docs
git diff --check "${expected}^" "$expected"
printf '\n== exact-head GitHub checks ==\n'
gh pr view 317 --repo ContextualWisdomLab/OriginWeave \
--json headRefOid,mergeStateStatus,reviewDecision,statusCheckRollup \
--jq '{headRefOid, mergeStateStatus, reviewDecision, checks: [.statusCheckRollup[] | {name, status, conclusion, workflowName}]}' || trueLength of output: 204616
I used the existing The
I found one authority-surface issue. P1 — Remove indirect presentation-authority minting from the read-only projection.
Therefore, code that has only a read-only bound-session reference can mint a current let authority = bound
.browser_session()
.presentation_authority(browsing_context)?;The authority has private fields and no public constructor. However, this public lookup still mints it from a raw Remove
🧠 Learnings usedYou are interacting with an AI system. |
|
@coderabbitai review Please review exact head |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
expected='12c2d03b875c5a12fd8ba4225dc51c8a064d1d6b'
actual="$(git rev-parse HEAD)"
printf '== revision ==\nexpected=%s\nactual=%s\n' "$expected" "$actual"
git status --short
printf '\n== direct-parent changed files ==\n'
git diff --name-status "${expected}^" "$expected"
printf '\n== focused module outlines ==\n'
for file in \
crates/originweave-browser-session/src/lib.rs \
crates/originweave-browser-session/src/browser_session.rs \
crates/originweave-browser-session/src/recovery.rs \
crates/originweave-browser-session/tests/user_context_identity_length.rs \
tests/test_browser_session_lifecycle_contract.py
do
if test -f "$file"; then
printf '\n== %s ==\n' "$file"
ast-grep outline "$file" --items all --type struct,enum,trait,impl,function,method \
--match 'BrowserSession|BoundBrowserSession|BoundBrowserSessionRecovery|PresentationMutationAuthority|DisposableIsolationId|parse|presentation_authority|browser_session' || true
fi
done
printf '\n== exact focused patch ==\n'
git diff --find-renames --unified=80 "${expected}^" "$expected" -- \
crates/originweave-browser-session/src \
crates/originweave-browser-session/tests/user_context_identity_length.rs \
tests/test_browser_session_lifecycle_contract.py \
docs
printf '\n== authority declarations, visibility, construction, and external surface ==\n'
rg -n -C 12 --type rust \
'(struct\s+(BrowserSession|BoundBrowserSession|BoundBrowserSessionRecovery|PresentationMutationAuthority)|impl.*(BrowserSession|BoundBrowserSession|BoundBrowserSessionRecovery)|pub(\([^)]*\))?\s+(?:const\s+)?fn\s+(browser_session|presentation_authority)|pub\(crate\]\s+(?:const\s+)?fn\s+presentation_authority|fn\s+presentation_authority\s*\(|PresentationMutationAuthority\s*\{|browser_session\(\)|Deref(Mut)?|From<.*BoundBrowserSessionRecovery|Into<.*BoundBrowserSession)' \
crates/originweave-browser-session/src
printf '\n== authority call sites and compile-fail contracts ==\n'
rg -n -U -C 10 --type rust \
'(presentation_authority\s*\(|browser_session\(\)|```compile_fail|BoundBrowserSessionRecovery)' \
crates/originweave-browser-session/src crates/originweave-browser-session/tests
printf '\n== identity parser and call sites ==\n'
rg -n -C 12 --type rust \
'(struct\s+DisposableIsolationId|enum\s+DisposableIsolationIdError|impl\s+DisposableIsolationId|fn\s+parse\s*\(|DisposableIsolationId::parse\s*\(|is_empty|trim|is_control|chars\(\)|len\(\)|4096|as_str\s*\()' \
crates/originweave-browser-session/src crates/originweave-browser-session/tests tests
printf '\n== selected full source and contract sections ==\n'
sed -n '1,250p' crates/originweave-browser-session/src/browser_session.rs | nl -ba -v 1
sed -n '600,735p' crates/originweave-browser-session/src/browser_session.rs | nl -ba -v 600
sed -n '990,1080p' crates/originweave-browser-session/src/browser_session.rs | nl -ba -v 990
cat -n crates/originweave-browser-session/tests/user_context_identity_length.rs
rg -n -C 16 '(presentation_authority|browser_session\(\)|DisposableIsolationId|user_context_identity_length|compile_fail)' tests/test_browser_session_lifecycle_contract.py
printf '\n== raw-port and owner escape search ==\n'
rg -n -C 6 --type rust \
'(lifecycle_port|into_(inner|port|browser_session)|as_mut|Deref(Mut)?|From<.*BoundBrowserSession|Into<.*BoundBrowserSession|->\s*&(?:mut\s+)?P|&mut\s+P)' \
crates/originweave-browser-session/src
printf '\n== whitespace integrity ==\n'
git diff --check "${expected}^" "$expected"
printf '\n== exact-head GitHub status ==\n'
gh pr view 317 --repo ContextualWisdomLab/OriginWeave \
--json headRefOid,mergeStateStatus,reviewDecision,statusCheckRollup \
--jq '{headRefOid, mergeStateStatus, reviewDecision, checks: [.statusCheckRollup[] | {name, status, conclusion, workflowName}]}' || trueLength of output: 204384 🏁 Script executed: #!/bin/bash
set -euo pipefail
expected='12c2d03b875c5a12fd8ba4225dc51c8a064d1d6b'
printf 'expected=%s\nactual=%s\n' "$expected" "$(git rev-parse HEAD)"
printf '\n== direct parent patch around authority visibility and contracts ==\n'
git diff --unified=45 "${expected}^" "$expected" -- \
crates/originweave-browser-session/src/browser_session.rs \
crates/originweave-browser-session/src/lib.rs \
tests/test_browser_session_lifecycle_contract.py
printf '\n== compile-fail blocks and surrounding docs ==\n'
rg -n -U -C 14 --type rust \
'```compile_fail[\s\S]{0,1500}(browser_session\(\)|presentation_authority|BoundBrowserSession)' \
crates/originweave-browser-session/src
printf '\n== repository-contract assertions for the owner/read-model authority surface ==\n'
rg -n -C 20 \
'(presentation_authority|browser_session\(\)|module-private|pub\(crate\)|compile_fail|BoundBrowserSession)' \
tests/test_browser_session_lifecycle_contract.py
printf '\n== all public BrowserSession immutable methods and authority methods ==\n'
rg -n -C 3 --type rust \
'(pub\s+(?:const\s+)?fn\s+(id|incarnation|state|transport_is_lost|recovery_evidence|create_attempt_recovery_evidence|presentation_authority)|^\s*fn\s+presentation_authority)' \
crates/originweave-browser-session/src/browser_session.rs
printf '\n== external integration uses of BrowserSession::presentation_authority ==\n'
rg -n -C 5 --type rust \
'\bBrowserSession\b.*presentation_authority|\.presentation_authority\s*\(' \
crates/originweave-browser-session/tests \
-g '!crates/originweave-browser-session/src/browser_session.rs'
printf '\n== parser direct patch and test ==\n'
git diff --unified=30 "${expected}^" "$expected" -- \
crates/originweave-browser-session/src/browser_session.rs \
crates/originweave-browser-session/tests/user_context_identity_length.rs
sed -n '84,122p' crates/originweave-browser-session/src/browser_session.rs | nl -ba -v 84
cat -n crates/originweave-browser-session/tests/user_context_identity_length.rsLength of output: 39983
The prior P1 is repaired.
The
This is a static source review. I did not run tests or CI. GitHub shows queued and skipped Actions, and PR 🧠 Learnings usedYou are interacting with an AI system. |
1 participant
Prerequisite Browser Session lifecycle-authority repair for #312 and #314/#316, stacked on #229 exact
6d87dff5dc572fbd74d06309d574a998f23cf02f.Current exact head:
12c2d03b875c5a12fd8ba4225dc51c8a064d1d6b. The PR is open / Draft / mergeable. A prior Ready probe created exact-head CI but never received a runner; the PR has since returned to Draft. This is not a merge-ready claim.Repaired on this lineage
BrowserSession::bind_lifecycle_portconsumes one concrete lifecycle adapter intoBoundBrowserSession<P>; no public raw-port accessor or alternate-port lifecycle path exists.DisposableContextCreateRecoveryEvidencepreserves aggregate-issued createattempt_epoch, disposition and complete candidate identity without collapsing a previously accepted same-valued owner into later candidate evidence.BrowserContextEpoch; stale authority fails before adapter I/O.finish(&mut self)retains the same owner;Dropperforms no browser I/O.BoundBrowserSession::into_recovery(self)performs one-way same-adapter custody only fromRecoveryRequired|TransportLost.BoundBrowserSessionRecovery<P>exposes state and exact non-authorizing evidence but no raw adapter, inner Browser Session, create, presentation-authority, epoch-advance, destroy, authorized-operation or ordinary finish path; negative capabilities are pinned by rustdoccompile_failcontracts.Uncertainwith exactUnprovenDestruction { context, context_epoch }. A 258-generation same-handle hostile fixture requires monotonic epochs and predecessor rejection before lifecycle I/O.DisposableIsolationIdno longer invents a Browser Session lexical grammar for browser-issued WebDriver BiDibrowser.UserContext. Protocol text is preserved exactly for empty, surrounding-whitespace, control-containing, ordinary and >4096-byte strings. The legacy result/error shape remains source-compatible but does not normalize or reject protocol text at this domain boundary.BrowserSession::presentation_authorityis module-private, whileBoundBrowserSession::presentation_authorityremains the explicit lifecycle-owner capability path. A crate-levelcompile_failcontract proves external code holding onlybound.browser_session()cannot mint authority from a rawBrowsingContextId.Standards boundary
The immutable W3C WebDriver BiDi Working Draft dated 9 September 2026 defines
browser.UserContext = textand uses that value as the input tobrowser.removeUserContext. Browser Session therefore preserves the browser-issued lifecycle address instead of adding trim/control/length grammar. Runtime-specific qualification remains a versioned adapter/deployment concern and does not mutate the protocol identity after remote creation.#316 owns ADR 0115 (
docs/adr/0115-browser-session-webdriver-bidi-lifecycle-acl.md). #317 owns Proposed ADR 0116 (docs/adr/0116-browser-session-recovery-custody-and-hot-ownership.md); #316 source and ADR 0115 remain untouched.Review and verification state
Fresh CodeRabbit focused review completed against exact
12c2d03b875c5a12fd8ba4225dc51c8a064d1d6b. It verified that the prior P1 is repaired: the aggregate authority lookup is module-private, the explicit bound-owner lookup remains public, the read-only projection cannot call the private method, the crate-levelcompile_failpins that negative capability, and no public conversion/raw-port/recovery path reopens it. It also verified thebrowser.UserContextlossless representation repair remains intact andgit diff --checkis clean. This is static source review only, not execution evidence.Exact-head CI run
34897432671is now terminal cancelled. Rust contracts104154928312and Production coverage104154928552both ended cancelled withsteps=null; neither ever executed. Later Draft-policy CI34900948264is skipped. The runner-admission pattern remains handed to canonical ownerContextualWisdomLab/.github#712. OriginWeave is not weakeningruns-on, synthesizing success, blind-rerunning, or bypassing gates.No runner-backed repository-contract/rustfmt/locked-test/strict-Clippy/rustdoc/API-doc/production-coverage/Chromium-E2E GREEN is claimed for this exact head.
Remaining owner path
No force-push, destructive rebase, self-approval, bypass, gate weakening, workflow/ruleset/secret change, protected-main merge, PR close, tag, publish or release was performed.