Add setup guides for GitHub OAuth and local CLI usage #415
Conversation
patniko
commented
Feb 9, 2026
patniko
commented
- Introduced a comprehensive guide for setting up GitHub OAuth authentication, detailing the flow, architecture, and implementation steps.
- Created a new setup guide for using the Copilot SDK with a locally signed-in CLI, emphasizing ease of use for personal projects and development.
- Added a scaling and multi-tenancy guide to help developers design deployments that support multiple users and concurrent sessions.
- Included configuration options, session management strategies, and production best practices across the new guides.
- Introduced a comprehensive guide for setting up GitHub OAuth authentication, detailing the flow, architecture, and implementation steps. - Created a new setup guide for using the Copilot SDK with a locally signed-in CLI, emphasizing ease of use for personal projects and development. - Added a scaling and multi-tenancy guide to help developers design deployments that support multiple users and concurrent sessions. - Included configuration options, session management strategies, and production best practices across the new guides.
There was a problem hiding this comment.
Pull request overview
Adds a set of setup guides covering local development, multi-user GitHub OAuth auth, server-side/headless CLI deployments, BYOK provider configuration, and scaling/multi-tenancy patterns for production use.
Changes:
- Introduces new setup guide index and persona-based navigation for choosing an integration pattern.
- Adds dedicated guides for Local CLI, Bundled CLI, Backend Services, GitHub OAuth, BYOK, and Scaling/Multi-Tenancy.
- Documents session isolation, persistence strategies, and operational checklists for production deployments.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/guides/setup/index.md | Adds a setup guide landing page with decision matrix and architecture overview. |
| docs/guides/setup/local-cli.md | Documents using the SDK with an already-signed-in local Copilot CLI (stdio). |
| docs/guides/setup/bundled-cli.md | Documents packaging a CLI binary with an app and pointing the SDK at it. |
| docs/guides/setup/backend-services.md | Documents running the CLI in headless server mode and connecting over TCP. |
| docs/guides/setup/github-oauth.md | Documents multi-user GitHub OAuth flow and passing per-user tokens to the SDK. |
| docs/guides/setup/byok.md | Documents configuring external model providers (BYOK) and identity patterns. |
| docs/guides/setup/scaling.md | Documents isolation patterns, horizontal scaling topologies, and multi-tenancy guidance. |
| "cli-2:4321", | ||
| "cli-3:4321", |
There was a problem hiding this comment.
The server list uses the same port for all three entries, but the preceding diagram and text describe a pool with distinct ports (e.g., :4321, :4322, :4323). Either update the list to match the documented topology (distinct ports), or adjust the diagram/text to reflect that all CLI servers listen on the same port and are differentiated by hostname/service.
| "cli-2:4321", | |
| "cli-3:4321", | |
| "cli-2:4322", | |
| "cli-3:4323", |
| // Enforce session isolation through naming conventions | ||
| function getSessionId(userId: string, purpose: string): string { | ||
| return `${userId}-${purpose}-${Date.now()}`; | ||
| } | ||
|
|
||
| // Access control: ensure users can only access their own sessions | ||
| async function resumeSessionWithAuth( | ||
| sessionId: string, | ||
| currentUserId: string | ||
| ): Promise<Session> { | ||
| const [sessionUserId] = sessionId.split("-"); | ||
| if (sessionUserId !== currentUserId) { | ||
| throw new Error("Access denied: session belongs to another user"); | ||
| } | ||
| return sharedClient.resumeSession(sessionId); | ||
| } |
There was a problem hiding this comment.
This authorization check is unreliable: sessionId.split("-") will break if userId contains -, and the scheme is forgeable (an attacker can craft a sessionId with a victim prefix). For documentation promoting “session isolation”, recommend enforcing ownership via a trusted mapping (sessionId → userId stored server-side) or using a signed/validated token (e.g., HMAC of {userId, purpose, nonce}) rather than parsing user identity out of the sessionId string.
| async function withSessionLock<T>( | ||
| sessionId: string, | ||
| fn: () => Promise<T>, | ||
| timeoutSec = 300 | ||
| ): Promise<T> { | ||
| const lockKey = `session-lock:${sessionId}`; | ||
| const lockId = crypto.randomUUID(); | ||
|
|
||
| // Acquire lock | ||
| const acquired = await redis.set(lockKey, lockId, "NX", "EX", timeoutSec); | ||
| if (!acquired) { | ||
| throw new Error("Session is in use by another user"); | ||
| } | ||
|
|
||
| try { | ||
| return await fn(); | ||
| } finally { | ||
| // Release lock (only if we still own it) | ||
| const currentLock = await redis.get(lockKey); | ||
| if (currentLock === lockId) { | ||
| await redis.del(lockKey); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Two concrete issues in this locking example: (1) crypto isn’t imported in the snippet, so it won’t run as-is; (2) the unlock path is not atomic (GET then DEL), which can release another process’s lock if the key expires and is re-acquired between the operations. For correctness, recommend an atomic compare-and-delete (Lua script / Redis EVAL) and include the required import for crypto (or show another lockId generator).
| }), | ||
| }); | ||
|
|
||
| const data = await response.json(); |
There was a problem hiding this comment.
This example returns data.access_token without checking response.ok or whether GitHub returned an error payload (e.g., error, error_description). As written, it can return undefined and push failures downstream in a confusing way. Update the snippet to validate the HTTP status and token presence, and surface a clear error when the exchange fails.
| const data = await response.json(); | |
| const data = await response.json(); | |
| if (!response.ok) { | |
| const description = | |
| data.error_description || | |
| data.error || | |
| data.message || | |
| `HTTP ${response.status}`; | |
| throw new Error(`GitHub token exchange failed: ${description}`); | |
| } | |
| if (typeof data.access_token !== "string" || !data.access_token) { | |
| const description = | |
| data.error_description || | |
| data.error || | |
| "No access token in response."; | |
| throw new Error(`GitHub token exchange failed: ${description}`); | |
| } |
| ``` | ||
|
|
||
| For production, run it as a system service or in a container: | ||
|
|
There was a problem hiding this comment.
The systemd unit content is included inside a bash code fence, which makes it easy to copy/paste incorrectly and suggests it’s shell syntax. Consider splitting this into two fenced blocks (one bash for Docker, one ini for systemd) or clearly labeling the systemd section with its own appropriate code fence.
|
|
||
| <details> | ||
| <summary><strong>Go</strong></summary> | ||
|
|
There was a problem hiding this comment.
The Go snippet discards errors from CreateSession and SendAndWait, and then dereferences response.Data.Content unconditionally. Since this is a setup guide, it’s better to demonstrate idiomatic error handling to avoid readers copying a pattern that can panic or silently fail; handle the returned errors and only print the content when the response is present.
1 participant
