feat: Add GitHub Copilot as an AI provider with device flow authentication#2881
Open
sykuang wants to merge 2 commits intowavetermdev:mainfrom
Open
feat: Add GitHub Copilot as an AI provider with device flow authentication#2881sykuang wants to merge 2 commits intowavetermdev:mainfrom
sykuang wants to merge 2 commits intowavetermdev:mainfrom
Conversation
…ation - Added GitHub Copilot backend to handle chat requests and token exchanges. - Implemented device flow for GitHub OAuth, allowing users to authenticate via a device code. - Created functions to manage Copilot API tokens, including caching and refreshing. - Updated existing structures to support GitHub Copilot as a new AI provider. - Enhanced the chat request handling to include Copilot-specific headers. - Added commands for device login start, poll, status, and logout in the RPC server. - Updated configuration schema to include GitHub Copilot as a valid provider and API type. - Ensured that Copilot modes are created in the waveai.json configuration file upon successful login.
Contributor
WalkthroughThis pull request introduces GitHub Copilot integration into Wave Terminal across frontend and backend layers. Changes include documentation additions for the new provider, a frontend UI login flow component for device-code OAuth authentication, TypeScript type definitions for RPC commands, a Go backend package implementing Copilot token exchange and chat API integration, RPC command handlers for device flow management, and schema updates recognizing Copilot as a valid AI provider and API type. The implementation includes token caching, model discovery, automatic mode configuration, and persistent state tracking for the device login flow. Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Fix all issues with AI agents
In `@docs/docs/waveai-modes.mdx`:
- Line 528: The table row for the ai:apitype field is missing the
`github-copilot` option; update the `ai:apitype` field reference in
docs/waveai-modes.mdx (the table entry containing "`ai:apitype` | No | API type:
...") to include `github-copilot` alongside `openai-chat`, `openai-responses`,
and `google-gemini` so it matches the schema and the earlier mention of
supported API types.
In `@frontend/app/aipanel/aimode.tsx`:
- Around line 196-199: The 500ms setTimeout that calls
model.setAIMode(pollResult.modename) is brittle and can race with the config
watcher; remove the fixed delay and instead wait until the new mode is actually
registered before switching — e.g., subscribe to the config/modes update event
(or poll model.hasMode / availableModes) and only call
model.setAIMode(pollResult.modename) once the registry reports the mode exists,
with a reasonable fallback timeout and error/logging path if it never appears;
replace the setTimeout block with this event-driven or existence-checked flow so
model.setAIMode is called reliably.
- Around line 161-170: The useEffect starting with fireAndForget that calls
RpcApi.CopilotDeviceLoginStatusCommand/TabRpcClient and then calls
setCopilotLoggedIn leaks work on unmount; modify it to create an AbortController
(or an isCancelled flag) inside the effect, pass its signal to any cancellable
RPC call (RpcApi.CopilotDeviceLoginStatusCommand) if supported, store any
setTimeout IDs used later (the timers referenced around lines 197/201/207/212)
and clear them in the effect cleanup, and in async callbacks check
abortController.signal.aborted (or isCancelled) before calling
setCopilotLoggedIn so state is never set after unmount. Ensure the cleanup
function calls abortController.abort() and clears all pending timeouts.
- Around line 172-215: The long-running poll started in handleCopilotLogin calls
RpcApi.CopilotDeviceLoginPollCommand with a 15-minute timeout but does not
actually cancel server-side when the AbortController is triggered; implement
cancellation by adding a client cancel call and backend handler (e.g., introduce
RpcApi.CopilotDeviceLoginCancelCommand / backend CopilotDeviceLoginCancel
endpoint) that signals the server poll to stop, wire the UI cancel path to call
that cancel RPC when copilotLoginAbortRef.current.abort() is invoked, and update
CopilotDeviceLoginPollCommand server logic to listen for that cancel signal (or
a shared cancellation token) so the server stops the in-flight poll immediately
instead of continuing for the full timeout; alternatively, if you prefer
RPC-layer cancellation, extend the RpcOpts shape (used by
CopilotDeviceLoginPollCommand) to carry an abort/cancellation token and thread
that through the RPC transport and server poll loop so the server can abort on
demand.
In `@pkg/aiusechat/githubcopilot/deviceflow.go`:
- Around line 80-110: PollForAccessToken currently calls tryExchangeDeviceCode
which uses context.Background(), so cancellations aren’t honored; change
tryExchangeDeviceCode to accept a context parameter (e.g.,
tryExchangeDeviceCode(ctx context.Context, deviceCode string) ) and in
PollForAccessToken derive a per-request context from the parent using
context.WithTimeout(ctx, 30*time.Second) (and cancel it after use) before
calling tryExchangeDeviceCode; update all other call sites (including the later
block at 113-166) to pass the derived context so in-flight HTTP requests are
cancelled when the parent ctx is cancelled.
- Around line 41-74: The RequestDeviceCode function uses http.DefaultClient (no
timeout); create a dedicated http.Client with an explicit Timeout (e.g. client
:= &http.Client{Timeout: 10 * time.Second}) and use client.Do(req) instead of
http.DefaultClient.Do(req); ensure you import time and keep the existing context
usage (NewRequestWithContext) so requests still honor context cancellation while
preventing indefinite hangs.
- Around line 43-46: Update the device-flow scope string used when building the
OAuth form in deviceflow.go: replace the existing form.Set("scope", "read:user")
call with a scope value that includes all required Copilot permissions
(space-separated) such as "repo read:org gist" so the device flow requests repo,
read:org and gist scopes alongside the existing client_id usage (look for the
form variable and GitHubCopilotClientID in this file).
In `@pkg/aiusechat/openaichat/openaichat-convertmessage.go`:
- Around line 170-176: The loop currently sets "Copilot-Vision-Request" whenever
any message has ContentParts, which flags text-only multimodal messages; update
the check in the code that builds the request (the loop iterating finalMessages
and calling req.Header.Set("Copilot-Vision-Request", "true")) to instead scan
each msg.ContentParts for actual image parts (e.g., check each part's MIME/type
field such as part.MimeType starting with "image/" or part.Type == "image" or an
IsImage/hasImageAttachment flag if present) and only set the header when at
least one content part is identified as an image. Ensure you reference
finalMessages, msg.ContentParts and the existing req.Header.Set call when making
the change.
In `@pkg/wshrpc/wshserver/wshserver.go`:
- Around line 68-74: The pending device-flow state (pendingDeviceFlowMu,
pendingDeviceCode, pendingDeviceInterval, pendingDeviceExpiry) is global and can
be clobber multiple clients; change this to a map protected by the mutex keyed
by client/route ID (e.g., RPC route ID or client ID) that maps to a small struct
{code, interval, expiry} so each session has its own pending state; update all
uses of pendingDeviceCode/Interval/Expiry to look up the entry by the current
client ID, create/delete entries as flows start/complete, and keep
pendingDeviceFlowMu to guard the map operations to avoid races.
- Around line 1673-1694: The CopilotDeviceLoginStatusCommand currently treats a
present but empty secret as logged in; update the LoggedIn return to require a
non-empty token (i.e., set LoggedIn to exists && token != "") so an empty string
does not report true; keep the existing goroutine that calls
ensureCopilotAIMode(token) only when token is non-empty.
- Around line 1608-1671: The function ensureCopilotAIMode currently ignores read
errors from wconfig.ReadWaveHomeConfigFile and reinitializes m, which can
overwrite/erase existing waveai.json; change the logic to detect and surface
read errors instead of clobbering the file: call
wconfig.ReadWaveHomeConfigFile("waveai.json"), if cerrs is non-empty or if an
error result is returned, log/return the read error(s) (or merge safely) and do
not proceed to write with an empty map; only if the file was successfully read
(m != nil and cerrs empty) should you add missing copilot/* entries to m and
then call wconfig.WriteWaveHomeConfigFile; reference ensureCopilotAIMode,
ReadWaveHomeConfigFile, WriteWaveHomeConfigFile, and the m/waveai.json variables
when making the change.
🧹 Nitpick comments (1)
pkg/aiusechat/openaichat/openaichat-convertmessage.go (1)
154-160: Hardcoded Copilot version strings will become stale.The version strings
"GitHubCopilotChat/0.35.0"and"CopilotChat/0.35.0"are hardcoded inline. Consider extracting these as constants so they can be updated in one place. Also,Copilot-Integration-Idis set to"vscode-chat"— is this intentional for a WaveTerm integration, or should it use a Wave-specific identifier?,
| | `display:description` | No | Full description of the mode | | ||
| | `ai:provider` | No | Provider preset: `openai`, `openrouter`, `google`, `azure`, `azure-legacy`, `custom` | | ||
| | `ai:provider` | No | Provider preset: `openai`, `openrouter`, `google`, `azure`, `azure-legacy`, `github-copilot`, `custom` | | ||
| | `ai:apitype` | No | API type: `openai-chat`, `openai-responses`, or `google-gemini` (defaults to `openai-chat` if not specified) | |
Contributor
There was a problem hiding this comment.
ai:apitype field reference is missing github-copilot.
Line 528 lists valid API types as openai-chat, openai-responses, or google-gemini, but github-copilot was added to the supported API types list at line 50 and to the schema. Update the field reference to include it for consistency.
🤖 Prompt for AI Agents
In `@docs/docs/waveai-modes.mdx` at line 528, The table row for the ai:apitype
field is missing the `github-copilot` option; update the `ai:apitype` field
reference in docs/waveai-modes.mdx (the table entry containing "`ai:apitype` |
No | API type: ...") to include `github-copilot` alongside `openai-chat`,
`openai-responses`, and `google-gemini` so it matches the schema and the earlier
mention of supported API types.
Comment on lines
+161
to
+170
| useEffect(() => { | ||
| fireAndForget(async () => { | ||
| try { | ||
| const status = await RpcApi.CopilotDeviceLoginStatusCommand(TabRpcClient); | ||
| setCopilotLoggedIn(status.loggedin); | ||
| } catch { | ||
| // ignore errors | ||
| } | ||
| }); | ||
| }, [isOpen]); |
Contributor
There was a problem hiding this comment.
Missing cleanup on unmount — leaked RPC poll and timers.
If the component unmounts (e.g., tab switch) while a Copilot login poll is in progress, the fireAndForget async chain continues running, and setTimeout callbacks (lines 197, 201, 207, 212) will attempt to set state on an unmounted component. The abort controller should be triggered on unmount.
🛡️ Proposed fix — add cleanup effect
// Check Copilot login status on mount and when dropdown opens
useEffect(() => {
fireAndForget(async () => {
try {
const status = await RpcApi.CopilotDeviceLoginStatusCommand(TabRpcClient);
setCopilotLoggedIn(status.loggedin);
} catch {
// ignore errors
}
});
+ return () => {
+ if (copilotLoginAbortRef.current) {
+ copilotLoginAbortRef.current.abort();
+ copilotLoginAbortRef.current = null;
+ }
+ };
}, [isOpen]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| useEffect(() => { | |
| fireAndForget(async () => { | |
| try { | |
| const status = await RpcApi.CopilotDeviceLoginStatusCommand(TabRpcClient); | |
| setCopilotLoggedIn(status.loggedin); | |
| } catch { | |
| // ignore errors | |
| } | |
| }); | |
| }, [isOpen]); | |
| useEffect(() => { | |
| fireAndForget(async () => { | |
| try { | |
| const status = await RpcApi.CopilotDeviceLoginStatusCommand(TabRpcClient); | |
| setCopilotLoggedIn(status.loggedin); | |
| } catch { | |
| // ignore errors | |
| } | |
| }); | |
| return () => { | |
| if (copilotLoginAbortRef.current) { | |
| copilotLoginAbortRef.current.abort(); | |
| copilotLoginAbortRef.current = null; | |
| } | |
| }; | |
| }, [isOpen]); |
🤖 Prompt for AI Agents
In `@frontend/app/aipanel/aimode.tsx` around lines 161 - 170, The useEffect
starting with fireAndForget that calls
RpcApi.CopilotDeviceLoginStatusCommand/TabRpcClient and then calls
setCopilotLoggedIn leaks work on unmount; modify it to create an AbortController
(or an isCancelled flag) inside the effect, pass its signal to any cancellable
RPC call (RpcApi.CopilotDeviceLoginStatusCommand) if supported, store any
setTimeout IDs used later (the timers referenced around lines 197/201/207/212)
and clear them in the effect cleanup, and in async callbacks check
abortController.signal.aborted (or isCancelled) before calling
setCopilotLoggedIn so state is never set after unmount. Ensure the cleanup
function calls abortController.abort() and clears all pending timeouts.
Comment on lines
+172
to
+215
| const handleCopilotLogin = useCallback(() => { | ||
| const abortController = new AbortController(); | ||
| copilotLoginAbortRef.current = abortController; | ||
| fireAndForget(async () => { | ||
| setCopilotLoginState({ status: "polling" }); | ||
| try { | ||
| const startData = await RpcApi.CopilotDeviceLoginStartCommand(TabRpcClient); | ||
| if (abortController.signal.aborted) return; | ||
| setCopilotLoginState({ | ||
| status: "waiting", | ||
| userCode: startData.usercode, | ||
| verificationUri: startData.verificationuri, | ||
| }); | ||
| // Copy code to clipboard and open browser | ||
| await navigator.clipboard.writeText(startData.usercode); | ||
| getApi().openExternal(startData.verificationuri); | ||
| // Now poll for completion | ||
| const pollResult = await RpcApi.CopilotDeviceLoginPollCommand(TabRpcClient, {}, { timeout: 15 * 60 * 1000 }); | ||
| if (abortController.signal.aborted) return; | ||
| if (pollResult.status === "complete") { | ||
| setCopilotLoginState({ status: "complete" }); | ||
| setCopilotLoggedIn(true); | ||
| // Auto-switch to the copilot mode if one was created | ||
| if (pollResult.modename) { | ||
| // Wait briefly for config watcher to pick up the new mode | ||
| setTimeout(() => { | ||
| model.setAIMode(pollResult.modename); | ||
| }, 500); | ||
| } | ||
| setTimeout(() => setCopilotLoginState({ status: "idle" }), 3000); | ||
| } else { | ||
| setCopilotLoginState({ | ||
| status: "error", | ||
| message: pollResult.error || "Login failed", | ||
| }); | ||
| setTimeout(() => setCopilotLoginState({ status: "idle" }), 5000); | ||
| } | ||
| } catch (e: any) { | ||
| if (abortController.signal.aborted) return; | ||
| setCopilotLoginState({ status: "error", message: e.message || "Login failed" }); | ||
| setTimeout(() => setCopilotLoginState({ status: "idle" }), 5000); | ||
| } | ||
| }); | ||
| }, []); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for CopilotDeviceLoginPollCommand definition
rg -n "CopilotDeviceLoginPollCommand" --type ts --type=go -B 2 -A 5Repository: wavetermdev/waveterm
Length of output: 4072
🏁 Script executed:
# Search for RpcOpts definition and how it's used
rg -n "RpcOpts" -A 10 --type ts | head -80Repository: wavetermdev/waveterm
Length of output: 6446
🏁 Script executed:
# Search for RpcOpts type/interface definition
rg -n "interface RpcOpts|type RpcOpts" --type ts -A 10Repository: wavetermdev/waveterm
Length of output: 650
🏁 Script executed:
# Get the full implementation of CopilotDeviceLoginPollCommand in Go
sed -n '1549,1600p' pkg/wshrpc/wshserver/wshserver.goRepository: wavetermdev/waveterm
Length of output: 1698
🏁 Script executed:
# Search for how timeout is handled on the server side in the RPC processing
rg -n "timeout" pkg/wshrpc/wshutil/ --type go -B 2 -A 2 | head -100Repository: wavetermdev/waveterm
Length of output: 123
🏁 Script executed:
# Find the correct directory structure
fd . pkg/wshrpc --type d | head -20Repository: wavetermdev/waveterm
Length of output: 129
🏁 Script executed:
# Search for timeout handling in the wshrpc package
rg -n "timeout" pkg/wshrpc/ --type go -B 2 -A 2 | head -150Repository: wavetermdev/waveterm
Length of output: 3058
🏁 Script executed:
# Search for RPC message dispatch and handler invocation
rg -n "timeout" pkg/wshrpc/wshserver/ --type go -B 3 -A 3 | head -150Repository: wavetermdev/waveterm
Length of output: 46
🏁 Script executed:
# Search for how RPC commands are dispatched/handled
rg -n "handleRequest|dispatch|invokeCommand" pkg/wshrpc/ --type go -B 2 -A 5 | head -150Repository: wavetermdev/waveterm
Length of output: 46
🏁 Script executed:
# Search for message handling and command routing
rg -n "command.*RpcMessage|func.*command" pkg/wshrpc/wshserver/ --type go -B 1 -A 3 | head -150Repository: wavetermdev/waveterm
Length of output: 46
🏁 Script executed:
# List files in wshserver
fd . pkg/wshrpc/wshserver/ --type f
# Also search for where commands are actually invoked
rg -n "case.*:" pkg/wshrpc/wshserver/ --type go -B 1 -A 3 | head -100Repository: wavetermdev/waveterm
Length of output: 3537
🏁 Script executed:
# Look for how RPC commands are dispatched from client messages to server handlers
rg -n "sendRpcRequest|wshRpcCall" pkg/wshrpc/wshclient/ --type go -B 2 -A 10 | head -150Repository: wavetermdev/waveterm
Length of output: 11983
🏁 Script executed:
# Search for WshRpc implementation and how it processes opts/timeout
rg -n "SendCommand|SendComplexRequest" --type go -B 2 -A 10 | head -200Repository: wavetermdev/waveterm
Length of output: 6716
🏁 Script executed:
# Search for RPC message dispatch on server side
rg -n "invokeHandler|processRpc|dispatchCommand|handleMessage" pkg/wshrpc/ --type go -B 2 -A 10 | head -200Repository: wavetermdev/waveterm
Length of output: 46
🏁 Script executed:
# Look at wshserver.go structure - search for message handling
rg -n "func.*message|processMsg|handleMsg" pkg/wshrpc/wshserver/wshserver.go --type go -B 1 -A 5Repository: wavetermdev/waveterm
Length of output: 46
🏁 Script executed:
# Look at wshserver.go line counts and search for dispatch/invoke patterns more broadly
wc -l pkg/wshrpc/wshserver/wshserver.go
# Search for where commands are actually invoked with the message
rg -n "reflect|invokeMethod" pkg/wshrpc/wshserver/ --type go -B 2 -A 5 | head -100Repository: wavetermdev/waveterm
Length of output: 102
AbortController doesn't cancel the backend RPC poll.
abortController.signal.aborted is only checked between async steps (lines 179, 190, 210), but the long-running CopilotDeviceLoginPollCommand (line 189, 15-minute timeout) isn't passed the abort signal and will continue executing server-side even after "Cancel" is clicked. The cancel is cosmetic — the backend poll continues consuming resources for up to 15 minutes.
The RpcOpts interface only supports timeout, noresponse, and route fields — there is no mechanism to pass an abort/cancellation signal through the RPC layer. Consider either:
- Implementing a cancellation endpoint the client can call to interrupt in-flight polls
- Or at minimum, documenting that cancel is UI-only
🤖 Prompt for AI Agents
In `@frontend/app/aipanel/aimode.tsx` around lines 172 - 215, The long-running
poll started in handleCopilotLogin calls RpcApi.CopilotDeviceLoginPollCommand
with a 15-minute timeout but does not actually cancel server-side when the
AbortController is triggered; implement cancellation by adding a client cancel
call and backend handler (e.g., introduce RpcApi.CopilotDeviceLoginCancelCommand
/ backend CopilotDeviceLoginCancel endpoint) that signals the server poll to
stop, wire the UI cancel path to call that cancel RPC when
copilotLoginAbortRef.current.abort() is invoked, and update
CopilotDeviceLoginPollCommand server logic to listen for that cancel signal (or
a shared cancellation token) so the server stops the in-flight poll immediately
instead of continuing for the full timeout; alternatively, if you prefer
RPC-layer cancellation, extend the RpcOpts shape (used by
CopilotDeviceLoginPollCommand) to carry an abort/cancellation token and thread
that through the RPC transport and server poll loop so the server can abort on
demand.
Comment on lines
+196
to
+199
| // Wait briefly for config watcher to pick up the new mode | ||
| setTimeout(() => { | ||
| model.setAIMode(pollResult.modename); | ||
| }, 500); |
Contributor
There was a problem hiding this comment.
Race condition: 500ms setTimeout to auto-switch mode is fragile.
The delay assumes the config watcher will have processed the new Copilot modes within 500ms, but this depends on file I/O, watcher debounce, and event propagation timing. If the mode hasn't been registered yet, model.setAIMode will silently fail or set an invalid mode.
Consider subscribing to config updates and switching when the mode actually becomes available, rather than relying on a fixed delay.
🤖 Prompt for AI Agents
In `@frontend/app/aipanel/aimode.tsx` around lines 196 - 199, The 500ms setTimeout
that calls model.setAIMode(pollResult.modename) is brittle and can race with the
config watcher; remove the fixed delay and instead wait until the new mode is
actually registered before switching — e.g., subscribe to the config/modes
update event (or poll model.hasMode / availableModes) and only call
model.setAIMode(pollResult.modename) once the registry reports the mode exists,
with a reasonable fallback timeout and error/logging path if it never appears;
replace the setTimeout block with this event-driven or existence-checked flow so
model.setAIMode is called reliably.
Comment on lines
+41
to
+74
| // RequestDeviceCode initiates the OAuth device code flow by requesting a device code from GitHub. | ||
| func RequestDeviceCode(ctx context.Context) (*DeviceCodeResponse, error) { | ||
| form := url.Values{} | ||
| form.Set("client_id", GitHubCopilotClientID) | ||
| form.Set("scope", "read:user") | ||
|
|
||
| req, err := http.NewRequestWithContext(ctx, "POST", GitHubDeviceCodeURL, strings.NewReader(form.Encode())) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("creating device code request: %w", err) | ||
| } | ||
| req.Header.Set("Content-Type", "application/x-www-form-urlencoded") | ||
| req.Header.Set("Accept", "application/json") | ||
|
|
||
| resp, err := http.DefaultClient.Do(req) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("requesting device code: %w", err) | ||
| } | ||
| defer resp.Body.Close() | ||
|
|
||
| body, err := io.ReadAll(resp.Body) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("reading device code response: %w", err) | ||
| } | ||
|
|
||
| if resp.StatusCode != http.StatusOK { | ||
| return nil, fmt.Errorf("device code request failed (status %d): %s", resp.StatusCode, string(body)) | ||
| } | ||
|
|
||
| var dcResp DeviceCodeResponse | ||
| if err := json.Unmarshal(body, &dcResp); err != nil { | ||
| return nil, fmt.Errorf("parsing device code response: %w", err) | ||
| } | ||
|
|
||
| return &dcResp, nil |
Contributor
There was a problem hiding this comment.
Add an explicit HTTP timeout for device-code requests.
http.DefaultClient has no timeout, so a stalled network can hang the RPC indefinitely if the context has no deadline.
🔧 Suggested fix (explicit timeout)
- resp, err := http.DefaultClient.Do(req)
+ client := &http.Client{Timeout: 15 * time.Second}
+ resp, err := client.Do(req)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // RequestDeviceCode initiates the OAuth device code flow by requesting a device code from GitHub. | |
| func RequestDeviceCode(ctx context.Context) (*DeviceCodeResponse, error) { | |
| form := url.Values{} | |
| form.Set("client_id", GitHubCopilotClientID) | |
| form.Set("scope", "read:user") | |
| req, err := http.NewRequestWithContext(ctx, "POST", GitHubDeviceCodeURL, strings.NewReader(form.Encode())) | |
| if err != nil { | |
| return nil, fmt.Errorf("creating device code request: %w", err) | |
| } | |
| req.Header.Set("Content-Type", "application/x-www-form-urlencoded") | |
| req.Header.Set("Accept", "application/json") | |
| resp, err := http.DefaultClient.Do(req) | |
| if err != nil { | |
| return nil, fmt.Errorf("requesting device code: %w", err) | |
| } | |
| defer resp.Body.Close() | |
| body, err := io.ReadAll(resp.Body) | |
| if err != nil { | |
| return nil, fmt.Errorf("reading device code response: %w", err) | |
| } | |
| if resp.StatusCode != http.StatusOK { | |
| return nil, fmt.Errorf("device code request failed (status %d): %s", resp.StatusCode, string(body)) | |
| } | |
| var dcResp DeviceCodeResponse | |
| if err := json.Unmarshal(body, &dcResp); err != nil { | |
| return nil, fmt.Errorf("parsing device code response: %w", err) | |
| } | |
| return &dcResp, nil | |
| // RequestDeviceCode initiates the OAuth device code flow by requesting a device code from GitHub. | |
| func RequestDeviceCode(ctx context.Context) (*DeviceCodeResponse, error) { | |
| form := url.Values{} | |
| form.Set("client_id", GitHubCopilotClientID) | |
| form.Set("scope", "read:user") | |
| req, err := http.NewRequestWithContext(ctx, "POST", GitHubDeviceCodeURL, strings.NewReader(form.Encode())) | |
| if err != nil { | |
| return nil, fmt.Errorf("creating device code request: %w", err) | |
| } | |
| req.Header.Set("Content-Type", "application/x-www-form-urlencoded") | |
| req.Header.Set("Accept", "application/json") | |
| client := &http.Client{Timeout: 15 * time.Second} | |
| resp, err := client.Do(req) | |
| if err != nil { | |
| return nil, fmt.Errorf("requesting device code: %w", err) | |
| } | |
| defer resp.Body.Close() | |
| body, err := io.ReadAll(resp.Body) | |
| if err != nil { | |
| return nil, fmt.Errorf("reading device code response: %w", err) | |
| } | |
| if resp.StatusCode != http.StatusOK { | |
| return nil, fmt.Errorf("device code request failed (status %d): %s", resp.StatusCode, string(body)) | |
| } | |
| var dcResp DeviceCodeResponse | |
| if err := json.Unmarshal(body, &dcResp); err != nil { | |
| return nil, fmt.Errorf("parsing device code response: %w", err) | |
| } | |
| return &dcResp, nil |
🤖 Prompt for AI Agents
In `@pkg/aiusechat/githubcopilot/deviceflow.go` around lines 41 - 74, The
RequestDeviceCode function uses http.DefaultClient (no timeout); create a
dedicated http.Client with an explicit Timeout (e.g. client :=
&http.Client{Timeout: 10 * time.Second}) and use client.Do(req) instead of
http.DefaultClient.Do(req); ensure you import time and keep the existing context
usage (NewRequestWithContext) so requests still honor context cancellation while
preventing indefinite hangs.
Comment on lines
+80
to
+110
| func PollForAccessToken(ctx context.Context, deviceCode string, interval int, expiresIn int) (string, error) { | ||
| if interval < 1 { | ||
| interval = 5 | ||
| } | ||
|
|
||
| deadline := time.Now().Add(time.Duration(expiresIn) * time.Second) | ||
|
|
||
| for { | ||
| if time.Now().After(deadline) { | ||
| return "", fmt.Errorf("device code expired, please try again") | ||
| } | ||
|
|
||
| // Use a simple timer for the interval wait, but still respect cancellation | ||
| timer := time.NewTimer(time.Duration(interval) * time.Second) | ||
| select { | ||
| case <-ctx.Done(): | ||
| timer.Stop() | ||
| return "", ctx.Err() | ||
| case <-timer.C: | ||
| } | ||
|
|
||
| // Each HTTP request gets its own 30-second timeout, independent of the | ||
| // parent context, so that the RPC deadline doesn't kill poll requests. | ||
| token, done, err := tryExchangeDeviceCode(deviceCode) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| if done { | ||
| return token, nil | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Propagate cancellation into token-exchange requests.
tryExchangeDeviceCode uses context.Background(), so in-flight HTTP calls ignore cancellation and may linger for up to 30s after the user cancels. Consider deriving the per-request timeout from the parent context.
🔧 Suggested fix (context-aware exchange)
-func PollForAccessToken(ctx context.Context, deviceCode string, interval int, expiresIn int) (string, error) {
+func PollForAccessToken(ctx context.Context, deviceCode string, interval int, expiresIn int) (string, error) {
@@
- token, done, err := tryExchangeDeviceCode(deviceCode)
+ token, done, err := tryExchangeDeviceCode(ctx, deviceCode)
@@
-}
+}
-func tryExchangeDeviceCode(deviceCode string) (string, bool, error) {
- reqCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+func tryExchangeDeviceCode(ctx context.Context, deviceCode string) (string, bool, error) {
+ reqCtx, cancel := context.WithTimeout(ctx, 30*time.Second)
defer cancel()Also applies to: 113-166
🤖 Prompt for AI Agents
In `@pkg/aiusechat/githubcopilot/deviceflow.go` around lines 80 - 110,
PollForAccessToken currently calls tryExchangeDeviceCode which uses
context.Background(), so cancellations aren’t honored; change
tryExchangeDeviceCode to accept a context parameter (e.g.,
tryExchangeDeviceCode(ctx context.Context, deviceCode string) ) and in
PollForAccessToken derive a per-request context from the parent using
context.WithTimeout(ctx, 30*time.Second) (and cancel it after use) before
calling tryExchangeDeviceCode; update all other call sites (including the later
block at 113-166) to pass the derived context so in-flight HTTP requests are
cancelled when the parent ctx is cancelled.
Comment on lines
+170
to
+176
| // Vision header — only when images are present | ||
| for _, msg := range finalMessages { | ||
| if len(msg.ContentParts) > 0 { | ||
| req.Header.Set("Copilot-Vision-Request", "true") | ||
| break | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Copilot-Vision-Request is set for non-image multimodal messages.
ContentParts can contain text-only parts (e.g., when a user message has multiple text segments). This check triggers Copilot-Vision-Request: true even when no images are present. You should check for actual image content parts.
🐛 Proposed fix
// Vision header — only when images are present
- for _, msg := range finalMessages {
- if len(msg.ContentParts) > 0 {
- req.Header.Set("Copilot-Vision-Request", "true")
- break
+ hasImages := false
+ for _, msg := range finalMessages {
+ for _, part := range msg.ContentParts {
+ if part.Type == "image_url" {
+ hasImages = true
+ break
+ }
+ }
+ if hasImages {
+ break
}
}
+ if hasImages {
+ req.Header.Set("Copilot-Vision-Request", "true")
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Vision header — only when images are present | |
| for _, msg := range finalMessages { | |
| if len(msg.ContentParts) > 0 { | |
| req.Header.Set("Copilot-Vision-Request", "true") | |
| break | |
| } | |
| } | |
| // Vision header — only when images are present | |
| hasImages := false | |
| for _, msg := range finalMessages { | |
| for _, part := range msg.ContentParts { | |
| if part.Type == "image_url" { | |
| hasImages = true | |
| break | |
| } | |
| } | |
| if hasImages { | |
| break | |
| } | |
| } | |
| if hasImages { | |
| req.Header.Set("Copilot-Vision-Request", "true") | |
| } |
🤖 Prompt for AI Agents
In `@pkg/aiusechat/openaichat/openaichat-convertmessage.go` around lines 170 -
176, The loop currently sets "Copilot-Vision-Request" whenever any message has
ContentParts, which flags text-only multimodal messages; update the check in the
code that builds the request (the loop iterating finalMessages and calling
req.Header.Set("Copilot-Vision-Request", "true")) to instead scan each
msg.ContentParts for actual image parts (e.g., check each part's MIME/type field
such as part.MimeType starting with "image/" or part.Type == "image" or an
IsImage/hasImageAttachment flag if present) and only set the header when at
least one content part is identified as an image. Ensure you reference
finalMessages, msg.ContentParts and the existing req.Header.Set call when making
the change.
Comment on lines
+68
to
+74
| // pendingDeviceFlow tracks the in-progress GitHub Copilot device code flow | ||
| var ( | ||
| pendingDeviceFlowMu sync.Mutex | ||
| pendingDeviceCode string | ||
| pendingDeviceInterval int | ||
| pendingDeviceExpiry int | ||
| ) |
Contributor
There was a problem hiding this comment.
Pending device-flow state is global and can be clobbered across clients.
If multiple windows/clients start a device login simultaneously, they will overwrite each other’s device codes and intervals. Consider storing pending state keyed by client/route (e.g., RPC route ID or client ID) to avoid cross-session collisions.
🤖 Prompt for AI Agents
In `@pkg/wshrpc/wshserver/wshserver.go` around lines 68 - 74, The pending
device-flow state (pendingDeviceFlowMu, pendingDeviceCode,
pendingDeviceInterval, pendingDeviceExpiry) is global and can be clobber
multiple clients; change this to a map protected by the mutex keyed by
client/route ID (e.g., RPC route ID or client ID) that maps to a small struct
{code, interval, expiry} so each session has its own pending state; update all
uses of pendingDeviceCode/Interval/Expiry to look up the entry by the current
client ID, create/delete entries as flows start/complete, and keep
pendingDeviceFlowMu to guard the map operations to avoid races.
Comment on lines
+1608
to
+1671
| // ensureCopilotAIMode creates copilot modes in waveai.json for each available model. | ||
| // It queries the Copilot API to discover which models the user's subscription supports. | ||
| // Returns the default mode name that should be selected. | ||
| func ensureCopilotAIMode(githubToken string) (string, error) { | ||
| // Check if any copilot modes already exist | ||
| fullConfig := wconfig.GetWatcher().GetFullConfig() | ||
| for name, cfg := range fullConfig.WaveAIModes { | ||
| if cfg.Provider == "github-copilot" { | ||
| return name, nil | ||
| } | ||
| } | ||
|
|
||
| // Discover available models from the Copilot API | ||
| models, err := githubcopilot.FetchAvailableModels(githubToken) | ||
| if err != nil { | ||
| log.Printf("githubcopilot: model discovery failed, using defaults: %v\n", err) | ||
| // Fall back to default models | ||
| for _, id := range githubcopilot.GetDefaultCopilotModels() { | ||
| models = append(models, githubcopilot.CopilotModelInfo{ID: id}) | ||
| } | ||
| } | ||
|
|
||
| if len(models) == 0 { | ||
| // Absolute fallback | ||
| models = []githubcopilot.CopilotModelInfo{{ID: "gpt-4o"}} | ||
| } | ||
|
|
||
| // Read existing waveai.json | ||
| m, cerrs := wconfig.ReadWaveHomeConfigFile("waveai.json") | ||
| if len(cerrs) > 0 { | ||
| m = make(waveobj.MetaMapType) | ||
| } | ||
| if m == nil { | ||
| m = make(waveobj.MetaMapType) | ||
| } | ||
|
|
||
| defaultModeName := "copilot/" + models[0].ID | ||
|
|
||
| for i, model := range models { | ||
| modeName := "copilot/" + model.ID | ||
| if _, exists := m[modeName]; exists { | ||
| continue | ||
| } | ||
| displayName := "Copilot " + model.ID | ||
| modeConfig := map[string]interface{}{ | ||
| "display:name": displayName, | ||
| "display:order": 10 + i, | ||
| "display:icon": "github", | ||
| "display:description": fmt.Sprintf("GitHub Copilot (%s)", model.ID), | ||
| "ai:provider": "github-copilot", | ||
| "ai:model": model.ID, | ||
| "ai:capabilities": []string{"tools", "images"}, | ||
| } | ||
| m[modeName] = modeConfig | ||
| } | ||
|
|
||
| err = wconfig.WriteWaveHomeConfigFile("waveai.json", m) | ||
| if err != nil { | ||
| return "", fmt.Errorf("writing waveai.json: %w", err) | ||
| } | ||
|
|
||
| log.Printf("githubcopilot: created %d copilot modes in waveai.json\n", len(models)) | ||
| return defaultModeName, nil | ||
| } |
Contributor
There was a problem hiding this comment.
Avoid overwriting waveai.json on read errors.
If ReadWaveHomeConfigFile returns errors, this code reinitializes the map and writes it back, which can discard existing user configuration. Prefer failing early (or at least logging and returning the read errors) rather than overwriting.
🔧 Suggested fix (preserve existing config on read error)
- m, cerrs := wconfig.ReadWaveHomeConfigFile("waveai.json")
- if len(cerrs) > 0 {
- m = make(waveobj.MetaMapType)
- }
- if m == nil {
- m = make(waveobj.MetaMapType)
- }
+ m, cerrs := wconfig.ReadWaveHomeConfigFile("waveai.json")
+ if len(cerrs) > 0 {
+ return "", fmt.Errorf("reading waveai.json: %v", cerrs)
+ }
+ if m == nil {
+ m = make(waveobj.MetaMapType)
+ }🤖 Prompt for AI Agents
In `@pkg/wshrpc/wshserver/wshserver.go` around lines 1608 - 1671, The function
ensureCopilotAIMode currently ignores read errors from
wconfig.ReadWaveHomeConfigFile and reinitializes m, which can overwrite/erase
existing waveai.json; change the logic to detect and surface read errors instead
of clobbering the file: call wconfig.ReadWaveHomeConfigFile("waveai.json"), if
cerrs is non-empty or if an error result is returned, log/return the read
error(s) (or merge safely) and do not proceed to write with an empty map; only
if the file was successfully read (m != nil and cerrs empty) should you add
missing copilot/* entries to m and then call wconfig.WriteWaveHomeConfigFile;
reference ensureCopilotAIMode, ReadWaveHomeConfigFile, WriteWaveHomeConfigFile,
and the m/waveai.json variables when making the change.
Comment on lines
+1673
to
+1694
| // CopilotDeviceLoginStatusCommand checks whether there is a stored GitHub Copilot token. | ||
| // If the user is logged in but no copilot modes exist yet, it auto-creates them. | ||
| func (ws *WshServer) CopilotDeviceLoginStatusCommand(ctx context.Context) (*wshrpc.CopilotDeviceLoginStatusRtnData, error) { | ||
| token, exists, err := secretstore.GetSecret("GITHUB_COPILOT_TOKEN") | ||
| if err != nil { | ||
| return &wshrpc.CopilotDeviceLoginStatusRtnData{ | ||
| LoggedIn: false, | ||
| Error: err.Error(), | ||
| }, nil | ||
| } | ||
| if exists && token != "" { | ||
| // Ensure copilot modes exist in waveai.json (no-ops if they already do) | ||
| go func() { | ||
| if _, err := ensureCopilotAIMode(token); err != nil { | ||
| log.Printf("githubcopilot: failed to ensure copilot modes: %v\n", err) | ||
| } | ||
| }() | ||
| } | ||
| return &wshrpc.CopilotDeviceLoginStatusRtnData{ | ||
| LoggedIn: exists, | ||
| }, nil | ||
| } |
Contributor
There was a problem hiding this comment.
LoggedIn should require a non-empty token.
If the secret exists but is empty, the current code still reports LoggedIn: true.
🔧 Suggested fix
- return &wshrpc.CopilotDeviceLoginStatusRtnData{
- LoggedIn: exists,
- }, nil
+ return &wshrpc.CopilotDeviceLoginStatusRtnData{
+ LoggedIn: exists && token != "",
+ }, nil🤖 Prompt for AI Agents
In `@pkg/wshrpc/wshserver/wshserver.go` around lines 1673 - 1694, The
CopilotDeviceLoginStatusCommand currently treats a present but empty secret as
logged in; update the LoggedIn return to require a non-empty token (i.e., set
LoggedIn to exists && token != "") so an empty string does not report true; keep
the existing goroutine that calls ensureCopilotAIMode(token) only when token is
non-empty.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds GitHub Copilot as a first-class AI provider in Wave Terminal. Users with a GitHub Copilot subscription can authenticate via GitHub's device code flow directly from the AI mode dropdown — no manual API key configuration needed.
Changes
Backend (Go)
githubcopilotpackage (pkg/aiusechat/githubcopilot/)copilotbackend.go—RunChatStep()exchanges GitHub token for a short-lived Copilot API token, sets the endpoint, and delegates to the OpenAI-compatible chat completions backendcopilottoken.go—CopilotTokenManagerwith caching/refresh,FetchAvailableModels()for dynamic model discovery viaGET /modelsdeviceflow.go— OAuth device code flow:RequestDeviceCode()andPollForAccessToken()against GitHub's device auth endpointspkg/wshrpc/)CopilotDeviceLoginStartCommand— initiates device code flowCopilotDeviceLoginPollCommand— polls for authorization, stores token, auto-creates modesCopilotDeviceLoginStatusCommand— checks if user is logged in, auto-creates modes if token exists but modes are missingCopilotDeviceLogoutCommand— deletes stored tokengithub-copilotprovider with defaults inusechat-backend.goandusechat-mode.goUser-Agent,Editor-Version,Editor-Plugin-Version,Copilot-Integration-Id,Openai-Intent,X-Initiator,Copilot-Vision-Requestadded to requests when provider isgithub-copilotFrontend (TypeScript/React)
frontend/app/aipanel/aimode.tsx)Documentation
docs/docs/waveai-modes.mdxwith:github-copilotin supported providers and API types listsHow It Works
waveai.jsonFiles Changed (16 files, +1140 / -17)
pkg/aiusechat/githubcopilot/{copilotbackend,copilottoken,deviceflow}.gouctypes.go,usechat-backend.go,usechat-mode.go,settingsconfig.gowshrpctypes.go,wshserver.go,wshclient.goaimode.tsx,wshclientapi.ts,gotypes.d.tsschema/waveai.jsondocs/docs/waveai-modes.mdx