improvement: forward status of copilot to footer icon #7312
Conversation
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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR enhances the GitHub Copilot integration by implementing real-time status forwarding from the Copilot language server to the footer UI icon. The implementation listens to LSP notifications (status updates and log messages) and displays them to users through visual indicators and tooltips. A key improvement is the fix to LazyTransport to properly forward subscriptions to the underlying WebSocket delegate, which enables the notification system to work correctly.
- Added notification handling for Copilot status updates (
statusNotificationanddidChangeStatus) - Implemented log message forwarding from Copilot LSP to browser console with appropriate log levels
- Enhanced footer icon to display status through color changes (yellow for warnings/errors) and dynamic labels
- Fixed subscription forwarding in
LazyWebsocketTransportto ensure notifications are properly registered on the delegate
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/core/codemirror/copilot/types.ts | Added GitHubCopilotStatusNotificationParams interface to define the structure of status notifications from the Copilot LSP |
| frontend/src/core/codemirror/copilot/transport.ts | New file implementing LazyWebsocketTransport with proper subscription forwarding to delegate, extracted from inline class in client.ts |
| frontend/src/core/codemirror/copilot/state.ts | Added copilotStatusState atom to store and reactively update Copilot status across the application |
| frontend/src/core/codemirror/copilot/language-server.ts | Implemented handleNotification method to process incoming LSP notifications and update state/logs accordingly |
| frontend/src/core/codemirror/copilot/client.ts | Refactored to use extracted LazyWebsocketTransport class with dependency injection pattern for better testability |
| frontend/src/core/codemirror/copilot/tests/transport.test.ts | Comprehensive test suite covering subscription handling, reconnection logic, and error scenarios for the transport layer |
| frontend/src/components/editor/chrome/wrapper/footer-items/copilot-status.tsx | Enhanced footer icon with dynamic status display based on busy, kind, and message fields from status notifications |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
frontend/src/components/editor/chrome/wrapper/footer-items/copilot-status.tsx
Show resolved
Hide resolved
Comment on lines
+280
to
+324
| /** | ||
| * Handle notifications from the Copilot language server. | ||
| * Uses onNotification to listen for statusNotification, didChangeStatus, and window/logMessage. | ||
| */ | ||
| private handleNotification: Parameters< | ||
| LanguageServerClient["onNotification"] | ||
| >[0] = (notif) => { | ||
| if (!notif.params) { | ||
| return; | ||
| } | ||
|
|
||
| const notification = notif as unknown as EnhancedNotification; | ||
|
|
||
| // Handle statusNotification | ||
| if (notification.method === "statusNotification") { | ||
| store.set(copilotStatusState, notification.params); | ||
| } | ||
|
|
||
| // Handle didChangeStatus | ||
| if (notification.method === "didChangeStatus") { | ||
| store.set(copilotStatusState, notification.params); | ||
| } | ||
|
|
||
| // Handle window/logMessage | ||
| if (notification.method === "window/logMessage") { | ||
| const params = notification.params as { type: number; message: string }; | ||
| const { type, message } = params; | ||
| // Map LSP log types to console methods | ||
| // type: 1 = Error, 2 = Warning, 3 = Info, 4 = Log | ||
| switch (type) { | ||
| case 1: // Error | ||
| logger.error("[GitHub Copilot]", message); | ||
| break; | ||
| case 2: // Warning | ||
| logger.warn("[GitHub Copilot]", message); | ||
| break; | ||
| case 3: // Info | ||
| logger.debug("[GitHub Copilot]", message); | ||
| break; | ||
| default: // Log (type 4 and others) | ||
| logger.log("[GitHub Copilot]", message); | ||
| break; | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
Missing test coverage for the new notification handling functionality. The handleNotification method handles three different notification types (statusNotification, didChangeStatus, and window/logMessage) but there are no tests for this behavior. Consider adding tests to verify that:
- Status notifications properly update the
copilotStatusState - Log messages are correctly routed to the appropriate logger methods based on type
- Notifications without params are ignored
Light2Dark
approved these changes
Dec 1, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This listens on notifications GitHub copilot status and logs and forwards them to the UI and browser console, respectively.
This also fixes an issue with the
LazyTransportto forward subscriptions to get the notifications wired up.