Skip to content

fix(rust): route messages to current session owner - #2792

Closed
gimenete wants to merge 1 commit into
mainfrom
gimenete-audit-sdk-jsonrpc-lifecycle
Closed

gimenete wants to merge 1 commit into
mainfrom
gimenete-audit-sdk-jsonrpc-lifecycle

Conversation

@gimenete

Copy link
Copy Markdown
Contributor

Summary

  • Keep per-session JSON-RPC request and session.event enqueue atomic with router registration lookup, preventing delivery through a stale sender after a session ID is replaced.
  • Preserve session-ID reuse, unknown-session dropping, and the existing wire protocol; no child-process lifecycle changes were needed.

Fixes #2791

Validation

  • cargo test --no-default-features --features test-support --lib router::tests
  • cargo clippy --no-default-features --features test-support --lib -- -D warnings
  • cargo +nightly-2026-04-14 fmt --all -- --config-path .rustfmt.nightly.toml --check
  • git diff --check

Fixes #2791

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@gimenete
gimenete requested a review from a team as a code owner September 29, 2026 11:41
Copilot AI balanced review requested due to automatic review settings September 29, 2026 11:41
@gimenete

Copy link
Copy Markdown
Contributor Author

Closing after further review: this is not an observable bug. The router task is the sole producer on each per-session channel and unbounded sends never block, so sending through a sender cloned before a concurrent re-registration is indistinguishable from linearizing the send at lookup time (before the replacement). Holding the lock across enqueue changes no reachable outcome, and the added test passes against the pre-change semantics, so it is not a regression test. No change needed.

@gimenete gimenete closed this Sep 29, 2026
@gimenete
gimenete deleted the gimenete-audit-sdk-jsonrpc-lifecycle branch September 29, 2026 11:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The test does not reproduce the concurrency race the change is intended to prevent.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

This PR fixes stale Rust session routing by making sender lookup and message enqueue atomic.

Changes:

  • Routes requests and notifications while holding the session registry lock.
  • Adds coverage for reused session IDs.
File Description
rust/​src/​router.rs Adds atomic routing helpers and related tests.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread rust/src/router.rs
}

#[test]
fn routed_requests_and_notifications_reach_the_current_registration() {
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rust router can send requests to a replaced session registration

2 participants