Add session shutdown method across all SDKs

[stephentoub]

Problem

When a session is destroyed, the CLI sends a session.shutdown notification after responding to the session.destroy RPC request. However, all four SDKs clear every event handler immediately upon receiving the session.destroy response. By the time the session.shutdown notification arrives, there are no handlers left to receive it. The event is silently dropped.

This means session.shutdown — which is defined in the generated types and documented as part of the session lifecycle — is effectively unreceivable by any SDK consumer today.

Alternatives considered

Remove handler clearing from destroy entirely. After destroy, the CLI sends no further events (shutdown is the last one), so clearing is not strictly necessary. However, all four SDKs document that destroy() / DisposeAsync() clears handlers, and in .NET, DisposeAsync carries the expectation that all work has quiesced upon return. Removing the clearing would break that contract as the handler for shutdown could run at any point after DisposeAsync. DisposeAsync could stall waiting for it, but then that's introducing potentially long waits into an operation that's supposed to be relatively quick.

Add a timeout-based wait inside destroy. We could wait a short window (e.g. 500ms) for the shutdown event before clearing handlers. This is fragile — the delay is arbitrary, adds latency to every destroy call, and still provides no guarantee the event arrives in time.

Solution

Introduce a new shutdown method in each SDK:

Language	Method
Node.js	session.shutdown()
Python	await session.shutdown()
Go	session.Shutdown(ctx)
.NET	await session.ShutdownAsync()

shutdown() sends the session.destroy RPC to the CLI but does not clear event handlers. This gives the caller an explicit window to observe the session.shutdown notification before they choose to finalize cleanup.

destroy() / DisposeAsync() now calls shutdown() internally, then clears handlers as before. Callers who never call shutdown() directly see no behavior change — full backward compatibility is preserved.

Both methods are idempotent: calling shutdown() twice is a no-op on the second call, and destroy() safely handles the case where shutdown() was already called.

Typical usage pattern

// Register handler before shutdown
session.on('session.shutdown', (event) => {
  console.log('Session shut down:', event);
});

// Send the destroy RPC, keeping handlers alive
await session.shutdown();

// ... wait for / process the shutdown event ...

// Clean up handlers and finalize
await session.destroy();

What's included

• 4 SDK source files: New shutdown method + refactored destroy in Node.js, Python, Go, and .NET
• 4 E2E test files: Each SDK's session event test now calls shutdown, asserts the session.shutdown event is received, then calls destroy
• 5 doc files: API reference updates in all language READMEs, docs/compatibility.md, and the docs-maintenance agent

[Copilot]

Pull request overview

This PR adds a shutdown() method (in language-appropriate naming) to all four SDKs — Node.js, Python, Go, and .NET — that sends the session.destroy JSON-RPC request to the server without clearing local event handlers. This enables callers to observe the server-sent session.shutdown notification before finalizing cleanup. destroy()/DisposeAsync() now delegates to shutdown() first, maintaining full backward compatibility for existing callers.

Changes:

• New shutdown() / Shutdown() / ShutdownAsync() method in each SDK with idempotency guards.
• Refactored destroy() to call shutdown() internally then clear handlers as before.
• E2E tests updated to call shutdown(), await the session.shutdown event, then call destroy(). Documentation updated in all language READMEs, docs/compatibility.md, and the docs-maintenance agent.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
nodejs/src/session.ts	Adds shutdown() with _isShutdown bool guard; destroy() delegates to it
nodejs/test/e2e/session.test.ts	E2E test awaits session.shutdown event before calling destroy()
python/copilot/session.py	Adds shutdown() with _is_shutdown bool flag; destroy() delegates to it
python/e2e/test_session.py	E2E test awaits shutdown_event before calling destroy()
go/session.go	Adds Shutdown() using atomic.Bool; Destroy() delegates to it (error message changed)
go/internal/e2e/session_test.go	E2E test awaits session.shutdown event via channel before calling Destroy()
dotnet/src/Session.cs	Adds ShutdownAsync() with Interlocked.Exchange guard; DisposeAsync() delegates to it
dotnet/test/SessionTests.cs	E2E test awaits shutdownReceived task before calling DisposeAsync()
nodejs/README.md	Documents new shutdown() method and updated destroy() description
go/README.md	Documents new Shutdown() method
dotnet/README.md	Documents new ShutdownAsync() method and updated DisposeAsync() description
docs/compatibility.md	Adds "Shutdown session" entry to the compatibility table
.github/agents/docs-maintenance.agent.md	Updates all four SDK method lists to include the new shutdown method

[stephentoub]

Actually, I'm pretty sure the CLI isn't ever actually sending the shutdown event as part of session destroy. In which case the changes here are necessary but insufficient.

[github-actions]

✅ Cross-SDK Consistency Review

I've reviewed this PR for consistency across all four SDK implementations, and I'm pleased to report excellent consistency throughout. The shutdown() method has been implemented with strong feature parity across all languages.

Summary of Changes

This PR adds a new shutdown() method to all four SDKs (Node.js, Python, Go, and .NET) to address the race condition where the session.shutdown notification is silently dropped because handlers are cleared before it arrives.

Consistency Analysis

✅ Method Naming - Perfectly Consistent

All SDKs follow proper language conventions:

• Node.js: session.shutdown() (camelCase)
• Python: session.shutdown() (snake_case)
• Go: session.Shutdown() (PascalCase for exported method)
• .NET: session.ShutdownAsync() (PascalCase + Async suffix)

✅ Core Behavior - Identical Across All SDKs

1. Idempotency: All implementations use proper guards to make shutdown() a no-op on repeated calls:

   • Node.js: _isShutdown boolean flag
   • Python: _is_shutdown boolean flag
   • Go: isShutdown atomic.Bool
   • .NET: _isShutdown with Interlocked.Exchange

2. RPC Call: All send the same session.destroy RPC request without clearing handlers

3. Integration with destroy(): All SDKs now call shutdown() first in their destroy()/DisposeAsync() methods, maintaining backward compatibility

✅ Documentation - Consistent Structure

All four implementations include:

• Clear XML/docstring comments explaining the purpose
• Usage examples showing the pattern: register handler → shutdown → wait → destroy
• Warnings about the relationship with destroy()/DisposeAsync()
• Error/exception documentation

✅ README Updates - Complete Coverage

• Node.js README: Added shutdown() method documentation ✅
• Python README: No detailed method reference section (existing pattern) ⚠️
• Go README: Added Shutdown() method documentation ✅
• .NET README: Added ShutdownAsync() method documentation ✅

Note: Python README follows a different documentation pattern (minimal API reference) compared to other SDKs. This is pre-existing and not introduced by this PR.

✅ E2E Tests - Parallel Implementation

All four E2E test suites now include the same pattern:

// Call shutdown explicitly before destroy
await session.shutdown();  // or Shutdown() or ShutdownAsync()
await session.destroy();    // or Destroy() or DisposeAsync()

✅ Compatibility Documentation - Updated

The docs/compatibility.md table now includes the new shutdown method.

✅ Docs-Maintenance Agent - Updated

The validation checklist in .github/agents/docs-maintenance.agent.md has been updated to include shutdown() in all four SDK method lists.

Recommendations

No changes needed - This PR demonstrates exemplary cross-SDK consistency. The implementation:

• Maintains feature parity across all languages
• Respects language-specific idioms and conventions
• Provides consistent documentation and examples
• Includes comprehensive test coverage
• Updates all relevant documentation

The only minor observation is that Python's README doesn't have the same detailed method reference as the other SDKs, but this is a pre-existing pattern difference, not something introduced by this PR.

Great work maintaining consistency across this multi-language SDK! 🎉

Generated by SDK Consistency Review Agent for issue #682 · ◷

[github-actions]

✅ Cross-SDK Consistency Review: PASSED

This PR demonstrates excellent cross-SDK consistency. The shutdown() method has been uniformly implemented across all four SDKs while respecting language-specific idioms.

Verified Consistency

1. Method naming follows language conventions ✅

• Node.js: shutdown() / disconnect()
• Python: shutdown() / disconnect()
• Go: Shutdown() / Disconnect()
• .NET: ShutdownAsync() / DisposeAsync()

2. Idempotency using appropriate patterns ✅

• Node.js & Python: boolean flags (_isShutdown, _is_shutdown)
• Go: atomic.Bool with Swap()
• .NET: Interlocked.Exchange(ref _isShutdown, 1)

3. API behavior is identical ✅

All SDKs implement:

• shutdown() sends session.destroy RPC without clearing handlers
• disconnect()/DisposeAsync() calls shutdown() then clears handlers
• Both methods are idempotent
• Full backward compatibility (calling disconnect directly still works)

4. Documentation is comprehensive and parallel ✅

• All README files updated with new method
• Consistent JSDoc/docstrings/XML comments explaining the use case
• Example code shows the two-phase pattern: shutdown() → wait for event → disconnect()

5. E2E tests verify identical behavior ✅

All language tests follow the same pattern:

1. Register handler for session.shutdown event
2. Call shutdown()
3. Wait for and assert event is received
4. Call cleanup method

Language-Specific Appropriateness ✅

• Go: Correctly omits context.Context parameter (underlying client.Request() is synchronous, unlike Send/GetMessages)
• Go: E2E test uses session.Destroy() which is a valid alias for Disconnect()
• .NET: Accepts CancellationToken parameter (following .NET async patterns)
• Node.js/Python: Both async, returning Promise(void) / None

Conclusion

No consistency issues found. This PR maintains excellent feature parity across all SDK implementations. The changes are well-architected and appropriately adapted to each language's conventions. 🎉

Generated by SDK Consistency Review Agent for issue #682 · ◷

[MattKotsenas]

Fixes #535

[SteveSandersonMS]

@stephentoub Thanks for this. I'm on the fence about adding another shutdown-type API, as we already have both:

• Dispose (.NET) / disconnect (others)
• Delete

Adding a third one makes it quite intimidating and definitely will raise questions about whether you're supposed to shut down before disposing or deleting or similar.

Ideally I'd rather ensure that the event behavior makes sense across all usage patterns. For example:

• If some other client or the server causes the session to get shut down while you're connected, you definitely need to receive this event so you can update your app state. This is the most important case.
  • From the description in this PR it sounds like that would already work, because your client hasn't removed the event handlers.
• If you call dispose/disposeasync, I think the ideal would be if this async call doesn't complete until the server has acknowledged the request, and if the session is going to shut down (because there's no other connected client) then you should also receive the session.shutdown event during this period. If you don't receive session.shutdown during this period, then it implies the session isn't shutting down (likely because there's another client).
  • From the description in the PR it sounds like this case definitely doesn't already work. I think we could make it work.

What do you think?

I guess the multi-client scenario is possibly another argument in favour of having a separate "shutdown" API that implies you want the session to stop even if other clients are also connected. That seems like a drastic and unusual situation but maybe the use case will emerge at some point.

[stephentoub]

If you call dispose/disposeasync, I think the ideal would be if this async call doesn't complete until the server has acknowledged the request, and if the session is going to shut down (because there's no other connected client) then you should also receive the session.shutdown event during this period. If you don't receive session.shutdown during this period, then it implies the session isn't shutting down (likely because there's another client).

How long would you wait? There's (currently) no separate registration specifically for shutdown, so we have no way of knowing whether the consumer is interested in session.shutdown or not. That means we need to wait indefinitely or for some specific timeout until we receive a session.shutdown, regardless of whether the consumer cares, which could delay DisposeAsync and friends unnecessarily.

From the description in this PR it sounds like that would already work, because your client hasn't removed the event handlers.

I think there's still a race condition in the runtime where a session.shutdown isn't reliably being delivered in all cases. That needs to be investigated.

[SteveSandersonMS]

How long would you wait?

With it being an async dispose/destroy, I imagine it's reasonable for the process to continue for as long as it takes to get an ack from the server, which is generally just the network round-trip time. And we expect the server to issue this shutdown event before it resolves the call, if it's going to.

If the caller doesn't want to wait for any ack from the server, they can always choose not to await the dispose/destroy call. Yes it is possible that the call can take arbitrarily log, but that's also true for every other RPC call.

Does this disagree with the semantics you have in mind?

[stephentoub]

If the caller doesn't want to wait for any ack from the server, they can always choose not to await the dispose/destroy call.

It doesn't sit well with me that we'd encourage fire-and-forget DisposeAsync calls.

Does this disagree with the semantics you have in mind?

It's not just about the roundtrip. There's an expectation that all work associated with the relevant object has quiesced by the time Dispose (or the task returned from DisposeAsync) completes; that way you can reliably know the state of any resources they may have been using. If DisposeAsync needs to wait for session.shutdown, then it also needs to wait for the session event handler to process that event, as otherwise it violates the rule about all work quiescing. And that means DisposeAsync needs to delay completion arbitrarily long while waiting for all On handlers to complete.

[SteveSandersonMS]

Right now we're at a halfway point in migrating to multi-client support. Today, when a client calls session.dispose/disconnect, this fires the "session.destroy" RPC call, and the runtime responds by actually terminating the session. It does that termination pretty quickly as far as I can tell - it doesn't appear to wait for any in-flight activity to complete other than flushing partly-written session events out to disk.

What we will get to soon is complete multi-client support such that when one client calls session.disconnect/DisposeAsync, the thing it's disposing/disconnecting is that single connection to the session, not the session itself. Arguably the .NET name DisposeAsync is a bit of a mismatch here, or arguably for .NET only we should call it SessionConnection instead of Session.

If DisposeAsync needs to wait for session.shutdown

So getting back to that point, conceptually DisposeAsync shouldn't need to wait for session.shutdown. It should just tell the server that it's disconnecting, and the server should decide whether it wants to shut down the session or not at that point. If it does, the act of shutting it down and raising the session.shutdown event should be almost immediate (it only needs to wait until it's flushed any session log writes to disk - it doesn't wait for any other pending work like ongoing agent activity). If the server chooses not to shut down the session, or chooses to do it more slowly, the client will still get a fast response to the DisposeAsync call but there may not be a session.shutdown event prior to its completion.

On one hand we could argue there's no need for a separate "request shutdown" API since as long as each client disconnects, the server can make its own choice about when to shut down a session (e.g., when it's idle or when there are no remaining connections). The "session.shutdown" event is still relevant to receive because that's how the app knows if the server chose to do this while it's still connected.

But on the other hand, the request to make "session.shutdown" get delivered even to the client that triggers the shutdown was so that its properties (usage data) could be received. Maybe that's the wrong way to get the data, given that the server might choose not to shut down the session at all if there's another client. Maybe we need a "request usage data" API instead.