docs: clarify session destroy vs delete semantics by patniko · Pull Request #599 · github/copilot-sdk

patniko · 2026-02-27T05:16:03Z

Summary

Clarifies the distinction between destroy() and deleteSession() across all four SDK languages and the session persistence guide.

Fixes #526

Problem

As reported in #526, the API documentation around session disposal is counterintuitive:

destroy() says it "destroys this session and releases all associated resources" but then says the session can be resumed — contradicting what "destroy" implies
deleteSession() is the truly permanent operation, but there's no clear guidance on when to use one vs the other
stop() says it "destroys all active sessions" but actually leaves session data on disk, which isn't mentioned

Changes

All four SDK languages (Go, Node.js, Python, .NET) — updated doc comments for:

destroy() / DisposeAsync(): Now explicitly states it releases in-memory resources only, preserves disk state, and points to deleteSession() for permanent removal
stop() / StopAsync(): Clarifies it closes sessions (not "destroys") and notes that disk data is preserved — users wanting permanent removal must call deleteSession() first
deleteSession() / DeleteSessionAsync(): Now explicitly contrasts with destroy() to make the difference clear

Session persistence guide (docs/guides/session-persistence.md):

Renamed "Explicit Session Destruction" to "Closing a Session" and "Permanently Deleting a Session" — two separate subsections
Added a callout box summarizing destroy() vs deleteSession()
Updated the summary table with clarifying descriptions

Clarify the distinction between destroy() (closes session, releases in-memory resources, preserves disk state for resumption) and deleteSession() (permanently removes all data from disk). Update doc comments across all four SDK languages (Go, Node.js, Python, .NET) and the session persistence guide to make the behavioral difference explicit and help users choose the right method. Fixes #526 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

This PR clarifies the semantic distinction between destroy() and deleteSession() methods across all four SDK languages (Go, Node.js, Python, .NET) and updates the session persistence guide. The changes address issue #526, which reported that the API documentation around session disposal was counterintuitive and contradictory.

Changes:

Updated destroy() / DisposeAsync() documentation to explicitly state it releases in-memory resources only, preserves disk state, and can be resumed
Updated stop() / StopAsync() documentation to clarify it closes (not "destroys") sessions and preserves disk data
Updated deleteSession() / DeleteSessionAsync() documentation to explicitly contrast with destroy() and emphasize permanent deletion
Restructured session persistence guide with separate sections for "Closing a Session" vs "Permanently Deleting a Session" and added a callout box

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
python/copilot/session.py	Updated `destroy()` docstring to clarify it closes session and releases in-memory resources while preserving disk state
python/copilot/client.py	Updated `stop()` to clarify it closes (not destroys) sessions; updated `delete_session()` to contrast with `destroy()`
nodejs/src/session.ts	Updated `destroy()` JSDoc to clarify it closes session and releases in-memory resources while preserving disk state
nodejs/src/client.ts	Updated `stop()` to clarify it closes sessions; updated `deleteSession()` to contrast with `destroy()`
go/session.go	Updated `Destroy()` doc comment to clarify it closes session and releases in-memory resources while preserving disk state
go/client.go	Updated `Stop()` to clarify it closes sessions; updated `DeleteSession()` to contrast with `Destroy()`
dotnet/src/Session.cs	Updated `DisposeAsync()` XML doc to clarify it closes session and releases in-memory resources while preserving disk state
dotnet/src/Client.cs	Updated `StopAsync()` to clarify it closes sessions; updated `DeleteSessionAsync()` to contrast with `DisposeAsync()`
docs/guides/session-persistence.md	Restructured with separate sections for closing vs deleting; added callout box and updated summary table

patniko · 2026-02-27T05:18:15Z

@SteveSandersonMS would be great to chat about this

github-actions · 2026-02-27T05:18:44Z

✅ Cross-SDK Consistency Review: APPROVED

This PR demonstrates excellent cross-language consistency in clarifying the destroy() vs deleteSession() semantics. All four SDK implementations have been updated with parallel documentation changes:

Consistent Changes Across All Languages

1. Session destroy() / DisposeAsync() method:

✅ Node.js, Python, Go, and .NET all now clearly state it "closes" (not "destroys") the session
✅ All specify that it releases "in-memory resources" only
✅ All mention that session data on disk is preserved
✅ All point to deleteSession() for permanent removal
✅ All note the session object cannot be reused after calling this method
✅ Examples updated to clarify "session can still be resumed later"

2. Client stop() / StopAsync() method:

✅ All four languages changed "destroys" to "closes" active sessions
✅ All clarify that in-memory resources are released
✅ All note that disk data is preserved
✅ All suggest calling deleteSession() first for permanent removal

3. Client deleteSession() / DeleteSessionAsync() method:

✅ All four languages now explicitly describe this as "permanently deletes"
✅ All mention it removes "conversation history, planning state, and artifacts"
✅ All contrast with destroy() to highlight the difference
✅ All state that deletion is "irreversible"

4. Documentation guide:

✅ docs/guides/session-persistence.md updated with new subsections
✅ Added callout box summarizing destroy() vs deleteSession()
✅ Summary table updated with clarifying descriptions

Language-Specific Adaptations (Appropriate)

The following differences respect language conventions and are not consistency issues:

Documentation style:
- TypeScript: JSDoc with {@link}
- Python: reStructuredText with :meth:
- Go: godoc with [Client.ResumeSession] reference style
- .NET: XML doc comments with (see cref=""/)
Method naming:
- Node/Python: destroy(), deleteSession()
- Go: Destroy(), DeleteSession() (exported functions use PascalCase)
- .NET: DisposeAsync() (IAsyncDisposable pattern), DeleteSessionAsync()

These variations follow each language's idiomatic documentation and naming conventions.

Conclusion

This PR maintains complete feature parity and semantic consistency across all four SDK implementations. The documentation now provides clear, consistent guidance on session lifecycle management regardless of which SDK language developers are using.

Great work addressing issue #526! 🎉

AI generated by SDK Consistency Review Agent

patniko · 2026-02-27T16:11:20Z

We will move session.destroy to session.disconnect to provide more clarity. In the future we'll allow for more explicit disconnect preferences (destroy completely, leave idle, leave other clients running, etc). We will add the new disconnect method shim and mark destroy obsolete before removing it.

patniko · 2026-02-27T16:13:11Z

We need to define and document the IDisposable behavior in .NET and check other languages for similar types of changes.

Add disconnect() as the preferred method for closing sessions across all four SDK languages, marking destroy() as deprecated: - Node.js: disconnect() + Symbol.asyncDispose support, destroy() delegates - Python: disconnect() + __aenter__/__aexit__ context manager, destroy() emits DeprecationWarning - Go: Disconnect() method, Destroy() marked with Deprecated godoc tag - .NET: DisconnectAsync() method, DisposeAsync() delegates to it Update all samples, READMEs, and documentation guides to use the new disconnect() terminology. Internal stop() methods now call disconnect(). Resolves PR #599 comments: - Rename destroy → disconnect for clarity - Define IDisposable behavior in .NET (DisposeAsync delegates to DisconnectAsync) - Add idiomatic cleanup patterns (async context managers, Symbol.asyncDispose) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Migrate all test scenarios, e2e tests, READMEs, and documentation references from destroy()/Destroy() to disconnect()/Disconnect(). - 90 test scenario files across Go/Python/TypeScript/C# - 15 Node.js e2e test files - 8 Python e2e test files - 3 Go e2e test files - 1 .NET test file - READMEs and compatibility docs updated with new API reference - Agent docs updated with new method names - Reconnect scenario log messages updated to 'disconnected' Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Resolve merge conflicts from main: - dotnet/src/Session.cs: Integrate SetModelAsync, simplified permission cleanup - python/copilot/client.py: Adopt ExceptionGroup-based stop() error handling - python/e2e/test_client.py: Update to ExceptionGroup test pattern Document IDisposable/cleanup patterns across all SDKs: - .NET: Class-level XML docs explain IAsyncDisposable contract, README documents DisconnectAsync + DisposeAsync with await using examples - Python: README documents async with context manager pattern - Node.js: README documents Symbol.asyncDispose / await using pattern - Session persistence guide: Add cross-SDK cleanup pattern comparison table Update new files from main (tool-overrides scenario, streaming fidelity tests) to use disconnect()/Disconnect() instead of destroy()/Destroy(). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

github-actions · 2026-03-05T03:47:04Z

Cross-SDK Consistency Review ✅

I've reviewed this PR for consistency across all four SDK implementations (Node.js, Python, Go, .NET). The changes maintain excellent cross-language consistency with appropriate adaptations for each language's idioms.

Summary of Changes

All four SDKs consistently:

✅ Renamed the primary cleanup method to disconnect()/Disconnect()/DisconnectAsync()
✅ Updated documentation to clarify that these methods release in-memory resources only while preserving disk state
✅ Point users to deleteSession()/DeleteSession()/DeleteSessionAsync() for permanent deletion
✅ Provide backward compatibility for the old destroy() method name
✅ Updated all cross-references in client stop() methods and documentation

Language-Specific Adaptations (Correct) ✅

The PR properly accounts for language conventions:

Node.js:

disconnect() as new primary method
destroy() marked @deprecated
Added Symbol.asyncDispose support for await using

Python:

disconnect() as new primary method
destroy() marked deprecated with DeprecationWarning
Added __aenter__/__aexit__ for async with context manager

Go:

Disconnect() as new primary method (PascalCase for exported)
Destroy() marked with // Deprecated: godoc tag

.NET:

DisconnectAsync() as new explicit method
DisposeAsync() kept as the standard IAsyncDisposable implementation (not deprecated—this is correct)
Properly documents that DisposeAsync() calls DisconnectAsync() internally

Minor Observation (Not a blocker)

The .NET implementation differs slightly from other languages in that DisposeAsync() is not marked deprecated. This is actually correct because:

DisposeAsync() is part of the IAsyncDisposable interface contract
In C#, the await using pattern relies on DisposeAsync()
You cannot truly deprecate an interface implementation in .NET

However, the documentation could be slightly clearer about when to use DisconnectAsync() vs DisposeAsync(). Consider adding a sentence like:

Note: While DisposeAsync() and DisconnectAsync() perform the same cleanup, prefer calling DisconnectAsync() explicitly when you need programmatic control, and use DisposeAsync() (via await using) for automatic resource management.

But this is a very minor point and not a consistency issue—just a documentation enhancement opportunity.

Conclusion

No blocking consistency issues found. The PR successfully maintains feature parity and semantic consistency across all four SDK implementations while respecting each language's idioms and best practices. Great work! 🎉

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

The hooks_extended test 'should invoke onSessionEnd hook when session is destroyed' was renamed to '...disconnected', but the snapshot YAML file wasn't renamed to match, causing CI to fail with 'No cached response'. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

github-actions · 2026-03-05T04:26:27Z

✅ Cross-SDK Consistency Review: PASSED

I've reviewed PR #599 for consistency across all four SDK implementations (Node.js/TypeScript, Python, Go, and .NET), and I'm pleased to report that this PR demonstrates excellent cross-language consistency.

Summary of Changes

This PR clarifies the distinction between destroy()/disconnect() and deleteSession() across all SDKs. The changes are consistently applied to all four languages:

1. New `disconnect()` method (replaces `destroy()`)

All four SDKs now introduce a disconnect() method with consistent semantics:

SDK	Method Name	Return Type	Notes
Node.js	`disconnect()`	`Promise(void)`	Also adds `Symbol.asyncDispose` support
Python	`disconnect()`	`None` (async)	Also adds `__aenter__`/`__aexit__` context manager
Go	`Disconnect()`	`error`	Follows Go error handling conventions
.NET	`DisconnectAsync()`	`Task`	Also updates `DisposeAsync()` to call it

✅ Consistent behavior: All methods release in-memory resources while preserving disk state.

2. Deprecation of `destroy()`

All four SDKs properly deprecate the old destroy() method while maintaining backward compatibility:

Node.js: @deprecated JSDoc tag pointing to disconnect()
Python: warnings.warn() with DeprecationWarning
Go: // Deprecated: comment with reference to Disconnect()
.NET: N/A — DisposeAsync() calls DisconnectAsync() internally (no breaking change)

✅ Consistent deprecation strategy across languages.

3. Updated documentation for `deleteSession()`

All SDKs consistently clarify that deleteSession() is permanent and irreversible:

Node.js: client.deleteSession() - ✅ Updated
Python: client.delete_session() - ✅ Updated
Go: client.DeleteSession() - ✅ Updated
.NET: client.DeleteSessionAsync() - ✅ Updated

All docs now explicitly contrast with disconnect() and note irreversibility.

4. Updated `stop()` documentation

All client stop() methods now clarify they preserve disk state:

Node.js: client.stop() - ✅ Updated
Python: client.stop() - ✅ Updated
Go: client.Stop() - ✅ Updated
.NET: client.StopAsync() - ✅ Updated

5. Language-idiomatic disposal patterns

Each SDK properly implements its language's idiomatic cleanup pattern:

Node.js: Symbol.asyncDispose for await using (TypeScript 5.2+)
Python: __aenter__/__aexit__ for async with
Go: Updated examples to use defer session.Disconnect()
.NET: DisconnectAsync() called by existing DisposeAsync() for await using

✅ Excellent adaptation to language conventions.

6. Comprehensive test updates

All E2E tests updated across all languages:

Node.js: destroy() → disconnect() in all test files ✅
Python: destroy() → disconnect() in all test files ✅
Go: Destroy() → Disconnect() in all test files ✅
.NET: Test method renamed to ShouldCreateAndDisconnectSessions() ✅

7. Unified documentation guide

The docs/guides/session-persistence.md guide now includes:

Clear distinction between "Disconnecting" vs "Permanently Deleting"
Cross-language table showing idiomatic disposal patterns
Deprecation notice for destroy()

✅ Central documentation updated to reflect all SDKs.

Conclusion

This PR maintains excellent feature parity and consistent API design across all four SDK implementations. The terminology, behavior, and migration path are uniform. No consistency issues found.

Recommendation: ✅ Approve from a cross-SDK consistency perspective.

Great work on maintaining consistency across this complex multi-language refactoring! 🎉

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

patniko added the documentation Improvements or additions to documentation label Feb 27, 2026

patniko requested a review from a team as a code owner February 27, 2026 05:16

Copilot AI review requested due to automatic review settings February 27, 2026 05:16

Copilot started reviewing on behalf of patniko February 27, 2026 05:16 View session

Copilot AI reviewed Feb 27, 2026

View reviewed changes

patniko self-assigned this Feb 27, 2026

patniko and others added 3 commits March 4, 2026 19:23

patniko enabled auto-merge March 5, 2026 04:28

patniko requested a review from SteveSandersonMS March 5, 2026 04:30

patniko removed the documentation Improvements or additions to documentation label Mar 5, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: clarify session destroy vs delete semantics#599

docs: clarify session destroy vs delete semantics#599
patniko wants to merge 5 commits intomainfrom
agent/20260226-210955-session-d9d7fb87

patniko commented Feb 27, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

patniko commented Feb 27, 2026

Uh oh!

github-actions bot commented Feb 27, 2026

Uh oh!

patniko commented Feb 27, 2026

Uh oh!

patniko commented Feb 27, 2026

Uh oh!

github-actions bot commented Mar 5, 2026

Uh oh!

github-actions bot commented Mar 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

patniko commented Feb 27, 2026

Summary

Problem

Changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

patniko commented Feb 27, 2026

Uh oh!

github-actions bot commented Feb 27, 2026

✅ Cross-SDK Consistency Review: APPROVED

Consistent Changes Across All Languages

Language-Specific Adaptations (Appropriate)

Conclusion

Uh oh!

patniko commented Feb 27, 2026

Uh oh!

patniko commented Feb 27, 2026

Uh oh!

github-actions bot commented Mar 5, 2026

Cross-SDK Consistency Review ✅

Summary of Changes

Language-Specific Adaptations (Correct) ✅

Minor Observation (Not a blocker)

Conclusion

Uh oh!

github-actions bot commented Mar 5, 2026

✅ Cross-SDK Consistency Review: PASSED

Summary of Changes

1. New disconnect() method (replaces destroy())

2. Deprecation of destroy()

3. Updated documentation for deleteSession()

4. Updated stop() documentation

5. Language-idiomatic disposal patterns

6. Comprehensive test updates

7. Unified documentation guide

Conclusion

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

1. New `disconnect()` method (replaces `destroy()`)

2. Deprecation of `destroy()`

3. Updated documentation for `deleteSession()`

4. Updated `stop()` documentation