Add githubToken and useLoggedInUser options to all SDK clients#237
Add githubToken and useLoggedInUser options to all SDK clients#237
Conversation
…ients Enable SDK clients to customize authentication when spawning the CLI server. Node.js: - Add githubToken and useLoggedInUser options to CopilotClientOptions - Set COPILOT_SDK_AUTH_TOKEN env var and pass --auth-token-env flag - Pass --no-auto-login when useLoggedInUser is false - Default useLoggedInUser to false when githubToken is provided Python: - Add github_token and use_logged_in_user options - Same behavior as Node.js SDK Go: - Add GithubToken and UseLoggedInUser fields to ClientOptions - Same behavior as Node.js SDK .NET: - Add GithubToken and UseLoggedInUser properties to CopilotClientOptions - Same behavior as Node.js SDK All SDKs include validation to prevent use with cliUrl (external server) and tests for the new options.
✅ Cross-SDK Consistency ReviewThis PR demonstrates excellent consistency across all four SDK implementations. The authentication feature has been implemented uniformly with proper attention to language-specific conventions. Consistency Verified ✓API Naming (properly adapted to language conventions):
Behavior consistency across all SDKs:
Test coverage - all SDKs include equivalent tests:
Documentation - consistent comments/docstrings across all SDKs explaining:
SummaryThis PR maintains excellent feature parity and API design consistency. The implementation correctly accounts for each language's idioms while keeping the semantic behavior identical. No inconsistencies found—great work! 🎉
|
… expression' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds authentication customization options to all SDK clients (Node.js, Python, Go, and .NET), enabling them to specify a GitHub token and control whether to use the logged-in user when spawning the CLI server.
Changes:
- Add
githubToken/github_token/GithubTokenoption to pass authentication tokens to the CLI via environment variable - Add
useLoggedInUser/use_logged_in_user/UseLoggedInUseroption to control whether the CLI uses stored OAuth tokens - Default
useLoggedInUserto false whengithubTokenis provided, but allow explicit override - Add validation to prevent using these auth options with external servers (cliUrl)
- Add comprehensive tests for the new authentication options across all SDKs
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| python/copilot/types.py | Add type definitions for github_token and use_logged_in_user options |
| python/copilot/client.py | Implement auth option handling, validation, and CLI flag generation for Python SDK |
| python/test_client.py | Add comprehensive tests for auth options in Python SDK |
| nodejs/src/types.ts | Add type definitions for githubToken and useLoggedInUser options |
| nodejs/src/client.ts | Implement auth option handling, validation, and CLI flag generation for Node.js SDK |
| nodejs/test/client.test.ts | Add comprehensive tests for auth options in Node.js SDK |
| go/types.go | Add type definitions for GithubToken and UseLoggedInUser options |
| go/client.go | Implement auth option handling, validation, and CLI flag generation for Go SDK |
| go/client_test.go | Add tests for auth options in Go SDK (missing one test case) |
| dotnet/src/Types.cs | Add type definitions for GithubToken and UseLoggedInUser options |
| dotnet/src/Client.cs | Implement auth option handling, validation, and CLI flag generation for .NET SDK |
| dotnet/test/ClientTests.cs | Add comprehensive tests for auth options in .NET SDK |
Comments suppressed due to low confidence (3)
dotnet/src/Client.cs:788
- Disposable 'TcpClient' is created but not disposed.
tcpClient = new();
python/copilot/client.py:175
- This import of module re is redundant, as it was previously imported on line 18.
import re
python/copilot/client.py:989
- 'except' clause does nothing but pass and there is no explanatory comment.
except OSError:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| t.Run("should default UseLoggedInUser to nil when no GithubToken", func(t *testing.T) { | ||
| client := NewClient(&ClientOptions{}) | ||
|
|
||
| if client.options.UseLoggedInUser != nil { | ||
| t.Errorf("Expected UseLoggedInUser to be nil, got %v", client.options.UseLoggedInUser) | ||
| } | ||
| }) |
There was a problem hiding this comment.
Missing test case for default UseLoggedInUser behavior when GithubToken is provided. All other SDK implementations (Python, Node.js, and .NET) include a test that verifies UseLoggedInUser defaults to false when a GithubToken is provided. This test case should be added to ensure consistent behavior and test coverage across all SDKs.
| startInfo.Environment["COPILOT_SDK_AUTH_TOKEN"] = options.GithubToken; | ||
| } | ||
|
|
||
| var cliProcess = new Process { StartInfo = startInfo }; |
There was a problem hiding this comment.
Disposable 'Process' is created but not disposed.
Update all SDK documentation with new features from PRs #237 and #269: - Add githubToken and useLoggedInUser client options for authentication - Add onUserInputRequest handler for enabling ask_user tool - Add hooks configuration for session lifecycle events - Add User Input Requests section with examples - Add Session Hooks section documenting all available hooks: - onPreToolUse, onPostToolUse, onUserPromptSubmitted - onSessionStart, onSessionEnd, onErrorOccurred
Update all SDK documentation with new features from PRs #237 and #269: - Add githubToken and useLoggedInUser client options for authentication - Add onUserInputRequest handler for enabling ask_user tool - Add hooks configuration for session lifecycle events - Add User Input Requests section with examples - Add Session Hooks section documenting all available hooks: - onPreToolUse, onPostToolUse, onUserPromptSubmitted - onSessionStart, onSessionEnd, onErrorOccurred
Draft PR until we land the upstream changes in the CLI.
--
Enable SDK clients to customize authentication when spawning the CLI server.
Node.js:
Python:
Go:
.NET:
All SDKs include validation to prevent use with cliUrl (external server) and tests for the new options.