Skip to content

Add shared session watch APIs - #2415

Open
toliaqat wants to merge 8 commits into
mainfrom
toliaqat-expose-shared-watch-api
Open

Add shared session watch APIs#2415
toliaqat wants to merge 8 commits into
mainfrom
toliaqat-expose-shared-watch-api

Conversation

@toliaqat

@toliaqat toliaqat commented Aug 26, 2026

Copy link
Copy Markdown

Summary

  • generate the additive Node and Rust sessions.watch({ sessionId }) RPC surface from Runtime schema commit 7c34f30aa270d2a6214d1aa0274fc1afd4c9790d (SHA-256 dec3e7be93b7a3ca023eff9f6e90e9858bb6305d15f547d0e599283828a9b310)
  • add passive SharedSessionWatch handles with immutable read-only metadata, ordered replay/live session.event delivery, idempotent sessions.close, and no interactive or lane-credential surface
  • propagate the exact connection-scoped { type: "session.disconnected", sessionId } terminal lifecycle signal and atomically clean up watch routing after preceding events
  • make client shutdown classify watch entries in the router so concurrent disconnect cleanup cannot select session.destroy
  • document and test the generated payload, replay registration, passive surface, terminal propagation, close cleanup, and shutdown cleanup

Validation

  • Node focused watch/lifecycle tests (3 passed)
  • Node typecheck
  • Node lint (existing unrelated warnings only)
  • Rust focused watch/shutdown tests (2 passed)
  • Rust generated watch payload test
  • Rust lifecycle-union test
  • Rust clippy with all features/targets
  • Rust nightly format check

Runtime and publication dependency

The generated API is pinned to the main-based schema-only Runtime commit above. Execution and normal package-backed regeneration remain blocked until a new @github/copilot publication carries Runtime PR github/copilot-agent-runtime#17282 (0179092d8c319b60b4b8cb7df0b0829c095adb4f) together with sessions.watch and the hand-written session.disconnected lifecycle union. The lifecycle variant is intentionally absent from generated/api.schema.json.

Expose passive shared-session watch handles for Node and Rust, with generated RPC types, ordered event routing, lifecycle cleanup, and tests.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings August 26, 2026 19:40
@toliaqat
toliaqat requested a review from a team as a code owner August 26, 2026 19:40
Comment thread nodejs/test/client.test.ts Fixed
@github-actions

This comment has been minimized.

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.

Pull request overview

Adds passive shared-session watch APIs for Node.js and Rust, including event replay, lifecycle handling, cleanup, documentation, and generated RPC types.

Changes:

  • Adds read-only shared-session watch handles and sessions.watch integration.
  • Propagates session.disconnected lifecycle events and cleans up routing.
  • Updates generated schemas, tests, documentation, and line-ending rules.
Show a summary per file
File Description
.gitattributes Standardizes Node.js and Rust line endings.
nodejs/README.md Documents shared-session watching.
nodejs/src/client.ts Implements watch creation and cleanup.
nodejs/src/generated/rpc.ts Adds generated watch RPC types.
nodejs/src/generated/session-events.ts Refreshes generated event types.
nodejs/src/index.ts Exports watch and lifecycle APIs.
nodejs/src/session.ts Adds the passive watch handle.
nodejs/src/types.ts Adds disconnected lifecycle typing.
nodejs/test/client.test.ts Tests watch behavior and payloads.
rust/README.md Documents Rust watch usage.
rust/src/generated/api_types.rs Refreshes generated RPC payload types.
rust/src/generated/rpc.rs Adds the generated watch RPC method.
rust/src/generated/session_events.rs Refreshes generated event types.
rust/src/handler.rs Updates permission-context example.
rust/src/lib.rs Tracks watches and lifecycle cleanup.
rust/src/router.rs Makes the session router cloneable.
rust/src/session.rs Updates permission-context fixtures.
rust/src/types.rs Adds disconnected lifecycle support.
rust/src/watch.rs Implements Rust shared-session watches.
rust/tests/api_types_test.rs Tests credential-free payload shapes.
rust/tests/session_test.rs Tests replay, disconnect, and close behavior.

Review details

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 16/21 changed files
  • Comments generated: 3
  • Review effort level: Balanced

Comment thread rust/src/lib.rs Outdated
Comment thread rust/tests/session_test.rs Outdated
Comment thread nodejs/src/session.ts
toliaqat and others added 2 commits August 26, 2026 12:50
Guard the hand-authored Node and Rust lifecycle unions against silent drift from the Runtime notification contract.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Regenerate the watch RPC from the accepted contract while preserving the SDK's published CLI schema surface and removing unrelated newer Runtime event/type drift.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: be0a255d-04f8-4830-888a-59f54ad1e607
@github-actions

This comment has been minimized.

Keep the generated watch API additive against the SDK's pinned CLI schema and leave repository-wide line-ending policy unchanged.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: be0a255d-04f8-4830-888a-59f54ad1e607
@github-actions

This comment has been minimized.

toliaqat and others added 2 commits August 26, 2026 14:07
Route disconnect cleanup in the ordered session notification task and store watch classification with router entries so shutdown always uses sessions.close without racing terminal cleanup.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: be0a255d-04f8-4830-888a-59f54ad1e607
Resolve conflicts preserving both the shared session watch API surface and
main's GitHub token provider work:

- nodejs/src/client.ts: keep both sharedSessionWatches.clear() and
  githubTokenProviders.clear() in stop() and forceStop().
- rust/src/lib.rs: keep register/unregister_watch_session alongside main's
  register/retire_github_token_provider, and pass main's new required
  github_token_registry argument to router.ensure_started().
- rust/src/lib.rs: keep main's github_token_registry.clear() in force_stop().
- rust/src/types.rs: union both test import lists
  (SessionLifecycleEventType + PermissionResponseCapability).
- rust/tests/session_test.rs: keep both new test sets.

Also address PR review feedback:

- nodejs/test/client.test.ts: replace the unreachable if (false) block
  (CodeQL) with a reachable compile-time PassiveWatch type assertion that
  still fails to compile if send/abort appear on SharedSessionWatch.
- nodejs/src/session.ts: wrap replay dispatch in the same try/catch used for
  live delivery so a throwing handler cannot drop remaining replay events or
  prevent on() from returning its unsubscribe function.
- rust/tests/session_test.rs: bound the final watch.events().recv() with
  timeout(TIMEOUT, ...) so a cleanup regression fails instead of hanging.
- nodejs/src/client.ts: wrap an over-length if condition to satisfy prettier.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Comment thread rust/src/router.rs Fixed
@github-actions

This comment has been minimized.

Apply the Runtime-authored sessions.watch fragment to the Node and Rust generators until the CLI package publishes it. Fail closed on contract drift and preserve canonical schema ordering so repository codegen remains reproducible.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: be0a255d-04f8-4830-888a-59f54ad1e607
@github-actions

This comment has been minimized.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: be0a255d-04f8-4830-888a-59f54ad1e607
@github-actions

Copy link
Copy Markdown
Contributor

Cross-SDK Consistency Review

This PR adds a watchSharedSession / watch_shared_session API to Node.js and Rust that allows passively watching a session shared by another user. The new public API surface is:

Language Method Status
Node.js client.watchSharedSession(sessionId)SharedSessionWatch ✅ Added in this PR
Rust client.watch_shared_session(session_id)SharedSessionWatch ✅ Added in this PR
Go client.WatchSharedSession(ctx, sessionID) ❌ Not yet implemented
.NET client.WatchSharedSessionAsync(sessionId) ❌ Not yet implemented
Python client.watch_shared_session(session_id) ❌ Not yet implemented
Java client.watchSharedSession(sessionId) ❌ Not yet implemented

Assessment

The PR body explicitly acknowledges that this is a partial rollout pinned to a pending Runtime publication. That context makes the intentional split understandable. However, for tracking purposes, Go, .NET, Python, and Java do not expose sessions.watch nor a SharedSessionWatch equivalent. All four SDKs expose a fully parallel session API (create/resume/list/delete), so this feature would naturally fit each once the Runtime dependency ships.

Suggested follow-ups (not blockers for this PR)

  1. Go – add func (c *Client) WatchSharedSession(ctx context.Context, sessionID string) (*SharedSessionWatch, error) in go/client.go, returning a struct with Metadata(), Events() (channel-based), and Close() — consistent with Go's existing On/OnEventType pattern.
  2. .NET – add Task<SharedSessionWatch> WatchSharedSessionAsync(string sessionId, CancellationToken cancellationToken = default) in dotnet/src/Client.cs consistent with the OnLifecycle<T> pattern.
  3. Python – add async def watch_shared_session(self, session_id: str) -> SharedSessionWatch with an async-generator or callback pattern consistent with the existing create_session / resume_session API.
  4. Java – add CompletableFuture<SharedSessionWatch> watchSharedSession(String sessionId) in CopilotClient.java with a reactive or callback-based events surface consistent with onLifecycle.

Consider opening a follow-up issue to track feature parity across the four remaining SDKs once the Runtime PR ships.


This review was generated by the cross-SDK consistency agent. It is informational — no changes are required in this PR.

Generated by SDK Consistency Review Agent for #2415 · sonnet46 32.7 AIC · ⌖ 8.38 AIC · ⊞ 6.6K ·

注册 for free to join this conversation on GitHub. Already have an account? 登录 to comment

标签

None yet

项目

None yet

Development

Successfully merging this pull request may close these issues.

3 participants