Skip to content

Add session.setModel() for mid-session model switching#621

Merged
SteveSandersonMS merged 4 commits intomainfrom
add-session-set-model
Mar 3, 2026
Merged

Add session.setModel() for mid-session model switching#621
SteveSandersonMS merged 4 commits intomainfrom
add-session-set-model

Conversation

@patniko
Copy link
Contributor

@patniko patniko commented Mar 2, 2026

  • Fix ESLint error: params_params in Node.js test
  • Fix .NET test: replace message-send approach with SessionModelChangeEvent check (no snapshot needed, shows model was changed)
  • Add skipped Go E2E test for SetModel in rpc_test.go using existing infrastructure
  • All builds pass (TypeScript, Go, .NET)

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Allows changing the model mid-session without destroying it.
The new model takes effect for the next message while preserving
conversation history.

Node.js:  session.setModel(model: string): Promise<void>
Python:   session.set_model(model: str) -> None
Go:       session.SetModel(ctx, model string) error
.NET:     session.SetModelAsync(model, cancellationToken): Task

All send the 'session.setModel' JSON-RPC method with
{ sessionId, model } params.

Tests added for all 4 languages:
- Node.js: mocked sendRequest verifies correct RPC params (25/25 pass)
- Python:  mocked request verifies sessionId + model (unit test)
- Go:      JSON marshaling test for request type (pass)
- .NET:    e2e test creating session, calling SetModelAsync

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@patniko patniko requested a review from a team as a code owner March 2, 2026 02:49
Copilot AI review requested due to automatic review settings March 2, 2026 02:49
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a cross-language SDK API for switching the selected AI model mid-session without destroying the session, by forwarding a new session.setModel JSON-RPC call.

Changes:

  • Node.js: add CopilotSession.setModel(model) and a unit test validating the JSON-RPC request.
  • Python: add CopilotSession.set_model(model) and a unit test validating request forwarding.
  • Go / .NET: add SetModel / SetModelAsync methods plus request types and basic tests.

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/test_client.py Adds a unit test asserting session.setModel is sent with sessionId + model.
python/copilot/session.py Adds set_model() that forwards session.setModel JSON-RPC with {sessionId, model}.
nodejs/test/client.test.ts Adds a unit test verifying session.setModel RPC method/params.
nodejs/src/session.ts Adds setModel() to CopilotSession that calls connection.sendRequest("session.setModel", ...).
go/types.go Adds sessionSetModelRequest struct for marshaling session.setModel params.
go/session.go Adds Session.SetModel() that sends session.setModel request and wraps errors.
go/client_test.go Adds JSON marshaling test for sessionSetModelRequest.
dotnet/test/SessionTests.cs Adds E2E test exercising SetModelAsync then sending a message afterward.
dotnet/src/Session.cs Adds SetModelAsync() + SetModelRequest record and registers it for source-gen JSON serialization.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 2, 2026

✅ Cross-SDK Consistency Review

I've reviewed this PR for consistency across all four SDK implementations (Node.js/TypeScript, Python, Go, and .NET). Excellent work! This PR maintains strong feature parity and follows language-specific conventions appropriately.

Summary of Consistency

All four SDKs implement the setModel() functionality with:

  • Consistent behavior: Changes model mid-session, preserves conversation history
  • Same JSON-RPC method: session.setModel
  • Same parameters: { sessionId, model }
  • Appropriate naming conventions:
    • Node.js: session.setModel(model) ✅ (camelCase)
    • Python: session.set_model(model) ✅ (snake_case)
    • Go: session.SetModel(ctx, model) ✅ (PascalCase for exported method)
    • .NET: session.SetModelAsync(model, cancellationToken) ✅ (PascalCase + Async suffix)

Consistency Highlights

✅ API Signatures

  • Each language follows its standard async patterns (Promise, async/await, context.Context, Task)
  • .NET correctly includes optional CancellationToken parameter (consistent with other async methods)
  • Go correctly includes context.Context first parameter (idiomatic Go)

✅ Documentation

  • All implementations include:
    • Clear description of behavior
    • Example usage showing "gpt-4.1" as sample model
    • Consistent messaging about conversation history preservation

✅ Error Handling

  • Node.js/Python: Promise rejection on failure
  • Go: Returns error with wrapped context (fmt.Errorf)
  • .NET: Throws exception on failure
  • All consistent with their respective SDK patterns

✅ Type Safety

  • Go: Proper struct type sessionSetModelRequest with JSON tags
  • .NET: Proper record type SetModelRequest with JSON source generation
  • Node.js/Python: TypeScript types and runtime dict validation

✅ Testing Coverage

  • Node.js: Mocks sendRequest, verifies method name and params ✅
  • Python: Mocks request, verifies sessionId and model forwarding ✅
  • Go: JSON marshaling test for request type ✅
  • .NET: E2E test verifying session remains usable after model change ✅

Placement Consistency

All SDKs place setModel() immediately after abort() in the Session class, maintaining a logical grouping of session control methods.

No Issues Found

This PR successfully maintains cross-SDK consistency while respecting language-specific idioms. No changes are needed from a consistency perspective. 🎉

AI generated by SDK Consistency Review Agent

@SteveSandersonMS
Copy link
Contributor

@copilot Can you update this to be a thin wrapper around session.Rpc.Model.SwitchToAsync etc?

// Session should still be usable after model change
await session.SendAsync(new MessageOptions { Prompt = "What is 1+1?" });
var assistantMessage = await TestHelper.GetFinalAssistantMessageAsync(session);
Assert.NotNull(assistantMessage);
Copy link
Contributor

Choose a reason for hiding this comment

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

@copilot Can you find some way to make this test show the model was changed?

Copy link
Contributor

Choose a reason for hiding this comment

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

Done in b91f69c. The test now subscribes for SessionModelChangeEvent before calling SetModelAsync, then asserts modelChanged.Data.NewModel == "gpt-4.1". This uses the existing TestHelper.GetNextEventOfTypeAsync infrastructure.

Note: the test will still fail if the CLI doesn't emit session.model_change in response to session.model.switchTo. The CLI-side handler is a pre-existing gap (same reason RpcTests.Should_Call_Session_Rpc_Model_SwitchTo is skipped). Without a working server-side handler, there's no way to verify the model was changed using existing test infrastructure.

…To()

Co-authored-by: SteveSandersonMS <1101362+SteveSandersonMS@users.noreply.github.com>
Copy link
Contributor

Copilot AI commented Mar 2, 2026

@copilot Can you update this to be a thin wrapper around session.Rpc.Model.SwitchToAsync etc?

Done in 6936cce. All four implementations now delegate to the generated RPC layer:

  • Node.js: this.rpc.model.switchTo({ modelId: model })
  • Python: self.rpc.model.switch_to(SessionModelSwitchToParams(model_id=model))
  • Go: s.RPC.Model.SwitchTo(ctx, &rpc.SessionModelSwitchToParams{ModelID: model})
  • .NET: await Rpc.Model.SwitchToAsync(model, cancellationToken)

Also removed the now-unused sessionSetModelRequest (Go) and SetModelRequest (.NET) types that were previously hand-rolling the same request.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • https://api.github.com/copilot_internal/user
    • Triggering command: /opt/hostedtoolcache/node/22.22.0/x64/bin/node /opt/hostedtoolcache/node/22.22.0/x64/bin/node /home/REDACTED/work/copilot-sdk/copilot-sdk/nodejs/node_modules/@github/copilot/index.js --headless --no-auto-update --log-level debug --stdio bash (http block)
    • Triggering command: /opt/hostedtoolcache/node/22.22.0/x64/bin/node /opt/hostedtoolcache/node/22.22.0/x64/bin/node /home/REDACTED/work/copilot-sdk/copilot-sdk/nodejs/node_modules/@github/copilot/index.js --headless --no-auto-update --log-level debug --stdio n-dir/bash user.name (http block)
    • Triggering command: /opt/hostedtoolcache/node/22.22.0/x64/bin/node /opt/hostedtoolcache/node/22.22.0/x64/bin/node /home/REDACTED/work/copilot-sdk/copilot-sdk/nodejs/node_modules/@github/copilot/index.js --headless --no-auto-update --log-level debug --stdio 0/x64/bin/bash (http block)

If you need me to access, download, or install something from one of these locations, you can either:

}

return nil
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Test coverage gap: The Go SDK is missing a test for SetModel(), while Node.js, Python, and .NET all have tests for this new method.

Consider adding a test in go/client_test.go (for unit testing with mocks) or go/internal/e2e/session_test.go (for E2E testing) to match the test coverage in the other SDKs.

Example pattern (based on Python/Node.js tests):

func TestSession_SetModel(t *testing.T) {
    // Mock the RPC call and verify session.model.switchTo is called
    // with correct sessionId and modelId parameters
}

This would ensure cross-SDK test parity and prevent regressions.

AI generated by SDK Consistency Review Agent for #621

@github-actions
Copy link
Contributor

github-actions bot commented Mar 2, 2026

Cross-SDK Consistency Review ✅

I've reviewed PR #621 for consistency across all four SDK implementations (Node.js, Python, Go, .NET).

✅ Implementation Consistency

All four SDKs correctly implement the new model switching feature:

  • Node.js: session.setModel(model)rpc.model.switchTo({ modelId })
  • Python: session.set_model(model)rpc.model.switch_to(SessionModelSwitchToParams(model_id))
  • Go: session.SetModel(ctx, model)RPC.Model.SwitchTo(ctx, &rpc.SessionModelSwitchToParams{ModelID})
  • .NET: session.SetModelAsync(model, cancellationToken)Rpc.Model.SwitchToAsync(model, cancellationToken)

✅ API Design Consistency

  • Naming follows language conventions (camelCase, snake_case, PascalCase) ✓
  • All call the same underlying RPC method: session.model.switchTo
  • Documentation is consistent across all SDKs ✓
  • Parameter types are equivalent (string model ID) ✓

⚠️ Test Coverage Gap

One minor inconsistency: The Go SDK is missing a test for SetModel() while the other three SDKs all have tests:

  • ✅ Node.js: nodejs/test/client.test.ts - mocked RPC test
  • ✅ Python: python/test_client.py - mocked RPC test
  • ✅ .NET: dotnet/test/SessionTests.cs - E2E test
  • ❌ Go: No test added

I've left an inline comment on the Go implementation suggesting adding a test for feature parity.


Overall assessment: This PR does an excellent job maintaining cross-SDK consistency. The implementations are parallel, well-documented, and follow language idioms correctly. The only suggestion is to add a Go test to match the coverage in other SDKs.

AI generated by SDK Consistency Review Agent

@SteveSandersonMS
Copy link
Contributor

@copilot Fix:

[xUnit.net 00:01:16.46] --- End of stack trace from previous location ---
Failed GitHub.Copilot.SDK.Test.SessionTests.Should_Set_Model_On_Existing_Session [1 m]
Error Message:
System.TimeoutException : Timeout waiting for assistant message
Stack Trace:
at GitHub.Copilot.SDK.Test.Harness.TestHelper.GetFinalAssistantMessageAsync(CopilotSession session, Nullable`1 timeout) in /home/runner/work/copilot-sdk/copilot-sdk/dotnet/test/Harness/TestHelper.cs:line 39
at GitHub.Copilot.SDK.Test.SessionTests.Should_Set_Model_On_Existing_Session() in /home/runner/work/copilot-sdk/copilot-sdk/dotnet/test/SessionTests.cs:line 455
--- End of stack trace from previous location ---

--

Run npm run lint

@github/copilot-sdk@0.1.8 lint
eslint "src//*.ts" "test//*.ts"

/Users/runner/work/copilot-sdk/copilot-sdk/nodejs/test/client.test.ts
Error: 92:56 error 'params' is defined but never used. Allowed unused args must match /^_/u @typescript-eslint/no-unused-vars

--

The Go SDK is missing a test for SetModel() while the other three SDKs all have tests:

…skipped Go test

Co-authored-by: SteveSandersonMS <1101362+SteveSandersonMS@users.noreply.github.com>
Copy link
Contributor

Copilot AI commented Mar 2, 2026

@copilot Fix:

[xUnit.net 00:01:16.46] --- End of stack trace from previous location ---
...

Fixed in b91f69c:

  • ESLint: renamed params to _params in the mock lambda
  • .NET timeout: restructured the test to check for SessionModelChangeEvent instead of sending a message — removes the CAPI dependency that was causing the timeout
  • Go test: added should call session.SetModel to go/internal/e2e/rpc_test.go using the existing skipped-test pattern (same reason as the adjacent SwitchTo test)

@github-actions
Copy link
Contributor

github-actions bot commented Mar 2, 2026

✅ Cross-SDK Consistency Review

This PR maintains excellent consistency across all four SDK implementations (Node.js, Python, Go, and .NET).

Summary

The setModel() / SetModel() / set_model() method is implemented uniformly with:

  • Proper naming conventions for each language (camelCase/snake_case/PascalCase)
  • Consistent parameters (single model string with appropriate async context)
  • Identical behavior (delegates to session.model.switchTo RPC)
  • Clear documentation with examples in all four SDKs
  • Comprehensive tests verifying RPC calls and parameters

No consistency issues found. Great work maintaining feature parity! 🎉

AI generated by SDK Consistency Review Agent

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2026

✅ Cross-SDK Consistency Review: Excellent

This PR adds the setModel() / SetModel() / set_model() feature across all four SDK implementations with excellent consistency.

Consistent Implementation Across SDKs

API Surface:

  • Node.js/TypeScript: async setModel(model: string): Promise(void)
  • Python: async def set_model(self, model: str) -> None
  • Go: func (s *Session) SetModel(ctx context.Context, model string) error
  • .NET: public async Task SetModelAsync(string model, CancellationToken cancellationToken = default)

All naming follows proper language conventions (camelCase, snake_case, PascalCase) while maintaining semantic equivalence.

Documentation:

  • ✅ All implementations include clear docstrings/comments explaining the feature
  • ✅ Consistent messaging: "The new model takes effect for the next message. Conversation history is preserved."
  • ✅ All include example usage with "gpt-4.1" as the example model

Test Coverage:

  • Node.js: Unit test mocking the RPC call and verifying parameters
  • Python: Unit test mocking the RPC call and verifying parameters
  • Go: E2E test added (currently skipped with note about CLI implementation)
  • .NET: Integration test verifying SessionModelChangeEvent is emitted

Underlying RPC:
All SDKs correctly call session.model.switchTo with sessionId and modelId parameters.

Summary

This is a textbook example of how to add a feature across a multi-language SDK repository. The implementation maintains complete feature parity while respecting each language's idioms and conventions. No consistency issues found. 🎉

AI generated by SDK Consistency Review Agent

@SteveSandersonMS SteveSandersonMS merged commit bd98e3a into main Mar 3, 2026
34 of 35 checks passed
@SteveSandersonMS SteveSandersonMS deleted the add-session-set-model branch March 3, 2026 15:14
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.

4 participants