Commit 0dacbcd
[Rust] Add PreparedSession for loss-free startup event subscription (#2319)
* [Rust] Add PreparedSession for loss-free startup event subscription
`Session::subscribe()` can only be called once the session handle exists,
so every event the runtime broadcast during `session.create` /
`session.resume` had no receiver installed and was dropped. Ephemeral
events like `session.idle` are never written to the session log, so
`get_messages` cannot recover them afterwards either.
`Client::prepare_session` / `prepare_resume_session` return a
`PreparedSession` that owns the session's broadcast channel up front:
subscribe first, then `start()`. `prepare_*` is synchronous and inert —
it validates the event buffer capacity, allocates a local channel and
cancellation token, and performs no router registration, task spawn, or
wire activity until `start()` is first polled. `start(self)` consumes the
handle and the type is deliberately not `Clone`, so a prepared session
can never produce two event loops.
`create_session` / `resume_session` become wrappers over
`prepare_*(config)?.start().await`, preserving their RPC sequences and
error kinds. Their bodies moved into private start paths that take the
sender and token by injection rather than allocating their own.
Both configs gain a runtime-only `event_buffer_capacity` (default 512,
`Some(0)` rejected as `InvalidConfig`, never clamped). The buffer is
finite, so slow subscribers observe `Lagged` instead of applying
backpressure.
Cancellation cleanup is now symmetric. `PendingSessionRegistration` grew
a deferred variant that resolves the session ID from the inline-response
stash, so the create path — including the cloud server-assigned-ID path,
which previously had no RAII guard at all — unregisters and cancels when
the startup future is dropped or fails. Registration and stashing now
happen under one lock hold to close the window where a concurrent drop
would miss a just-registered session. The mcp-auth-interest error path on
both create and resume now cancels and awaits the event loop instead of
returning through `?`.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* [Rust] Close two session-registration cancellation races
Follow-up to the `PreparedSession` change. Two cancellation races
remained in session registration, both reachable from a caller simply
dropping a `start()` future.
**Deferred cloud-create registration.** For a cloud session with no
caller-pinned ID the CLI assigns the ID, so the SDK can only register on
the notification router from the inline `session.create` response
callback. The read loop removes the pending-response entry *before*
invoking that callback, so a startup future dropped in that window found
an empty stash, cleaned up nothing, and the callback then registered a
session with no owner — a permanent router leak.
Registration state now lives in a shared `DeferredRegistration` slot
(`Pending` / `Registered` / `Cancelled` / `Claimed`) that the callback,
the startup path, and the cancellation guard all arbitrate through. The
callback registers *under the slot lock*, so registering and publishing
ownership are atomic with respect to cancellation: a concurrent guard
either wins and marks the slot `Cancelled`, in which case the callback
registers nothing, or it loses and finds a `Registered` slot to tear
down. Never both, and never neither. The pinned-ID path uses the same
slot, pre-populated, so create has one cleanup mechanism instead of two.
**Stale cleanup versus a same-ID retry.** Unregistering by session ID
alone removed whichever registration happened to hold the ID. Because
cleanup of an abandoned startup is signalled rather than awaited, a
caller that aborted a startup and immediately retried with the same
pinned ID could have the retry's registration evicted by the dead
attempt, silently stranding the live session with no event routing. The
same applied to a `Session` dropped after being superseded.
Registrations now carry a `RegistrationToken` identity and removal is a
compare-and-remove: an owner removes only the exact registration it
registered. Applied to create, resume, `Session::disconnect`, and
`Session::drop`. `Client::stop` and `cleanup_sessions_for_test` keep
removing unconditionally — they tear down every session and the runtime
regardless of owner.
Tests gate both windows deterministically rather than by timing. The
slot state machine is driven directly at the exact interleaving the read
loop creates, in both orders, and the router's compare-and-remove is
covered on its own. End to end: a cancelled cloud create leaves no
registration, no subscription, and no task behind whether cancellation
lands before or after the callback registered, and a same-ID retry still
succeeds; and create, resume, and `Session` drop each survive a stale
owner's cleanup running after a retry has taken over the ID. Each test
was confirmed to fail against a mutated implementation.
No public API change.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0f9d5ac7-9999-4f37-82b3-a5533bfbc1f6
* [Rust] Gate registered_session_ids to test configurations
`Client::registered_session_ids` has no caller in a default-feature
build: the in-crate unit tests reach it under `cfg(test)`, and the
public `registered_session_ids_for_test` wrapper is gated on
`feature = "test-support"`. A plain `cargo build` or `cargo clippy`
therefore warned `dead_code` for it.
Gate the method on `any(test, feature = "test-support")`, matching the
convention already used for the other test-only helpers in this file.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0f9d5ac7-9999-4f37-82b3-a5533bfbc1f6
* [Rust] Narrow the PreparedSession guarantee to routed events
`Client::prepare_session` promised consumers would observe "every event a
session emits". That is broader than the implementation for cloud creates
with a server-assigned ID: the SDK cannot register the session on its
notification router until the `session.create` response arrives, so
notifications emitted before that point are not routable to any session
and never reach a subscriber.
Qualify the primary API documentation and the changelog as *routed*
events, and point callers at pinning `SessionConfig::session_id` for
complete pre-response coverage. `PreparedSession`'s type-level docs,
`rust/README.md`, and `docs/features/streaming-events.md` already
documented this limitation; the entry-point docs now match them.
Documentation only: no API, behavior, or wire change.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0f9d5ac7-9999-4f37-82b3-a5533bfbc1f6
* [Docs] Unwrap the lone Rust tab in the streaming-events article
The "Subscribing before a session starts" section has only a Rust
example, and the docs normalization pipeline converts a `<details>` group
into a tabbed language switcher only when two or more consecutive blocks
are present. A single block renders as raw collapsible HTML on
docs.github.com.
Drop the `<details>`/`<summary>` wrapper and leave the code fence
directly in the article, matching the repository docs style guide.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0f9d5ac7-9999-4f37-82b3-a5533bfbc1f6
* [Rust] Keep session IDs out of prepared-session test diagnostics
Two polling helpers formatted session identifiers into their failure
messages: `await_no_registrations` rendered the router's registered ID
list with `{:?}`, and `await_registered` interpolated the awaited ID.
A downstream consumer that vendors this crate has CodeQL rules flagging
identifiers reaching formatted output, so both were reported there even
though the SDK's own analysis was clean.
Report an outstanding-registration count and a static expectation
message instead. Both helpers keep their exact predicates and deadline
behavior: `await_no_registrations` still returns only when the router
holds zero registrations, and `await_registered` still blocks on the
exact ID it was given, so no assertion is weakened.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0f9d5ac7-9999-4f37-82b3-a5533bfbc1f6
* [Rust] Cover the MCP-auth interest failure path on create and resume
`session.eventLog.registerInterest` is the last fallible step of startup
when an MCP-auth handler is installed, and its failure branch was
untested: the existing MCP-auth tests only return successful interest
responses, and the prepared-session failure tests covered create RPC
errors and ID mismatches only.
Add a failing-interest test for each of create and resume, asserting the
same contract the sibling failure tests assert: the original error kind
reaches the caller, the router registration is gone, and a subscription
installed before `start()` is closed. The resume test also asserts the
best-effort `session.skills.reload` is never issued, since interest
registration runs ahead of it.
Verified by mutation that both tests execute the branch. Removing the
`registration.cleanup(event_loop)` call does not turn them red, because
`PendingSessionRegistration::drop` cancels and releases the same
registration synchronously — the explicit cleanup is defense in depth on
this path, and the tests assert the observable contract rather than which
of the two mechanisms performed it.
Test-only change: no API, behavior, or wire impact.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0f9d5ac7-9999-4f37-82b3-a5533bfbc1f6
* [Rust] Reconcile the subscribe docs with the buffering contract
`PreparedSession::subscribe` promised that "every subscriber receives
every event", which contradicts the paragraph directly above it: the
broadcast buffer is finite, so a subscriber that falls behind the
configured capacity observes `Lagged` and skips events instead of
applying backpressure. Say that explicitly and link the `Lagged` type.
Also replace "should" with "must" where the streaming-events article and
the changelog describe what a consumer needing lossless startup delivery
has to do. The docs style guide reserves ambiguous modals for optional
actions, and `rust/README.md` already phrased this as "must".
Documentation only: no API, behavior, or wire change.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0f9d5ac7-9999-4f37-82b3-a5533bfbc1f6
* [Rust] Reconcile PreparedSession with current main
Preserve current router registration ownership and feature configuration while rebasing the focused startup subscription change.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* [Rust] Format rebased session startup code
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* [Rust] Avoid exposing session IDs in diagnostics
Use a count-only test helper for the polling diagnostic so CodeQL does not treat session identifiers as logged data.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* [Rust] Keep session IDs out of test diagnostics
CodeQL flagged the registration polling helper because the count was
derived from a Vec<SessionId>. Count registrations directly on the
router instead, and assert on registration identity without formatting
IDs into failure messages.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Steve Sanderson <SteveSandersonMS@users.noreply.github.com>
Copilot-Session: 0f9d5ac7-9999-4f37-82b3-a5533bfbc1f61 parent 8f14e9a commit 0dacbcd
9 files changed
Lines changed: 1996 additions & 62 deletions
File tree
- docs/features
- rust
- src
- tests
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
44 | 44 | | |
45 | 45 | | |
46 | 46 | | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
47 | 69 | | |
48 | 70 | | |
49 | 71 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
218 | 218 | | |
219 | 219 | | |
220 | 220 | | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
221 | 263 | | |
222 | 264 | | |
223 | 265 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
118 | 118 | | |
119 | 119 | | |
120 | 120 | | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
121 | 124 | | |
122 | 125 | | |
123 | 126 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
717 | 717 | | |
718 | 718 | | |
719 | 719 | | |
| 720 | + | |
| 721 | + | |
| 722 | + | |
| 723 | + | |
| 724 | + | |
| 725 | + | |
| 726 | + | |
| 727 | + | |
| 728 | + | |
| 729 | + | |
| 730 | + | |
| 731 | + | |
| 732 | + | |
| 733 | + | |
| 734 | + | |
| 735 | + | |
| 736 | + | |
| 737 | + | |
| 738 | + | |
| 739 | + | |
| 740 | + | |
| 741 | + | |
| 742 | + | |
| 743 | + | |
| 744 | + | |
| 745 | + | |
| 746 | + | |
| 747 | + | |
| 748 | + | |
| 749 | + | |
720 | 750 | | |
721 | 751 | | |
722 | 752 | | |
| |||
920 | 950 | | |
921 | 951 | | |
922 | 952 | | |
| 953 | + | |
| 954 | + | |
| 955 | + | |
| 956 | + | |
| 957 | + | |
| 958 | + | |
923 | 959 | | |
924 | 960 | | |
925 | 961 | | |
926 | 962 | | |
927 | 963 | | |
928 | 964 | | |
929 | 965 | | |
930 | | - | |
| 966 | + | |
931 | 967 | | |
932 | 968 | | |
933 | 969 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2324 | 2324 | | |
2325 | 2325 | | |
2326 | 2326 | | |
2327 | | - | |
2328 | | - | |
2329 | | - | |
2330 | | - | |
2331 | | - | |
| 2327 | + | |
| 2328 | + | |
| 2329 | + | |
| 2330 | + | |
| 2331 | + | |
| 2332 | + | |
| 2333 | + | |
| 2334 | + | |
2332 | 2335 | | |
2333 | 2336 | | |
2334 | 2337 | | |
2335 | | - | |
| 2338 | + | |
2336 | 2339 | | |
2337 | 2340 | | |
2338 | 2341 | | |
| |||
2344 | 2347 | | |
2345 | 2348 | | |
2346 | 2349 | | |
2347 | | - | |
2348 | | - | |
2349 | | - | |
| 2350 | + | |
| 2351 | + | |
| 2352 | + | |
| 2353 | + | |
| 2354 | + | |
| 2355 | + | |
| 2356 | + | |
| 2357 | + | |
| 2358 | + | |
| 2359 | + | |
| 2360 | + | |
| 2361 | + | |
| 2362 | + | |
| 2363 | + | |
| 2364 | + | |
| 2365 | + | |
| 2366 | + | |
| 2367 | + | |
| 2368 | + | |
| 2369 | + | |
| 2370 | + | |
| 2371 | + | |
| 2372 | + | |
| 2373 | + | |
2350 | 2374 | | |
2351 | 2375 | | |
2352 | 2376 | | |
| |||
2596 | 2620 | | |
2597 | 2621 | | |
2598 | 2622 | | |
| 2623 | + | |
| 2624 | + | |
| 2625 | + | |
| 2626 | + | |
| 2627 | + | |
| 2628 | + | |
| 2629 | + | |
| 2630 | + | |
| 2631 | + | |
| 2632 | + | |
| 2633 | + | |
| 2634 | + | |
| 2635 | + | |
| 2636 | + | |
| 2637 | + | |
| 2638 | + | |
| 2639 | + | |
| 2640 | + | |
2599 | 2641 | | |
2600 | 2642 | | |
2601 | 2643 | | |
| |||
0 commit comments