feat(vscode): add Kilo notifications banner to VS Code extension#437
Open
catrielmuller wants to merge 3 commits intodevfrom
Open
feat(vscode): add Kilo notifications banner to VS Code extension#437catrielmuller wants to merge 3 commits intodevfrom
catrielmuller wants to merge 3 commits intodevfrom
Conversation
Ports the CLI's Kilo news/notifications feature to the VS Code extension. Shows a dismissable carousel banner at the top of the chat view when no session is active, fetching notifications from the kilo-gateway API with dismissed IDs persisted in extension globalState.
![kiloconnect[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| async getNotifications(): Promise<KilocodeNotification[]> { | ||
| try { | ||
| return await this.request<KilocodeNotification[]>("GET", "/kilo/notifications") | ||
| } catch { |
Contributor
There was a problem hiding this comment.
[SUGGESTION]: Empty catch block silently swallows errors
Per the project's AGENTS.md style guide: "Never leave a catch block empty." While this follows the existing getProfile() pattern, consider at minimum logging the error so failures are visible:
} catch (err) {
console.warn("[Kilo] Failed to fetch notifications:", err)
return []
}
Collaborator
There was a problem hiding this comment.
I agree, why no logging?
Contributor
Code Review SummaryStatus: 4 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (9 files)
|
| if (!this.extensionContext) return | ||
| const existing = this.extensionContext.globalState.get<string[]>("kilo.dismissedNotificationIds", []) | ||
| if (!existing.includes(notificationId)) { | ||
| await this.extensionContext.globalState.update("kilo.dismissedNotificationIds", [...existing, notificationId]) |
Contributor
There was a problem hiding this comment.
[SUGGESTION]: Dismissed notification IDs accumulate in globalState indefinitely
The kilo.dismissedNotificationIds array grows without bound as users dismiss notifications over time. Consider pruning stale IDs by intersecting with the current notification list, e.g.:
const existing = this.extensionContext.globalState.get<string[]>("kilo.dismissedNotificationIds", [])
const validIds = existing.filter(id => notifications.some(n => n.id === id))
const updated = [...validIds, notificationId]
await this.extensionContext.globalState.update("kilo.dismissedNotificationIds", updated)This keeps the persisted list from growing with IDs for notifications that no longer exist.
This comment has been minimized.
This comment has been minimized.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| async getNotifications(): Promise<KilocodeNotification[]> { | ||
| try { | ||
| return await this.request<KilocodeNotification[]>("GET", "/kilo/notifications") | ||
| } catch { |
Collaborator
There was a problem hiding this comment.
I agree, why no logging?
| const items = filteredNotifications | ||
| const total = () => items().length | ||
| const safeIndex = () => Math.min(index(), Math.max(0, total() - 1)) | ||
| const current = createMemo(() => items()[safeIndex()]) |
Collaborator
There was a problem hiding this comment.
Is 0 a safe index? this will just return undefined or not?
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
2 participants
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.
Context
Resolve: #320
Ports the Kilo news/notifications feature from the CLI (
dialog-kilo-notifications.tsx) to the VS Code extension webview. Users can see Kilo news and announcements without needing to open the CLI.Implementation
The implementation follows the established extension ↔ webview message-passing pattern used for providers, agents, and config:
Extension side (
packages/kilo-vscode/src/):types.ts— addedKilocodeNotification/KilocodeNotificationActioninterfacesindex.ts— exported the new typeshttp-client.ts— addedgetNotifications()callingGET /kilo/notifications(already exists in kilo-gateway)KiloProvider.ts— addedfetchAndSendNotifications()with the standard cached-message pattern;handleDismissNotification()persists dismissed IDs toextensionContext.globalStateso they survive webview reloads; constructor now accepts optionalExtensionContextextension.ts— passescontexttoKiloProviderWebview side (
packages/kilo-vscode/webview-ui/src/):types/messages.ts— addedNotificationsLoadedMessage,RequestNotificationsMessage,DismissNotificationMessageand wired into unionscontext/notifications.tsx—NotificationsProvider+useNotifications()context; subscribes tonotificationsLoadedmessages, exposesfilteredNotifications()(excludes dismissed),dismiss(id)components/chat/KiloNotifications.tsx— carousel card using kilo-uiButton,IconButton,Icon; pagination (arrow-left/right) on the left, action button on the right in a footer rowcomponents/chat/ChatView.tsx— renders<KiloNotifications />betweenTaskHeaderand messages, wrapped in<Show when={!id()}>so it only appears when no session is activeApp.tsx— addedNotificationsProviderto the provider chainstyles/chat.css— added notification card styles using VS Code CSS variablesKey design decisions:
extensionContext.globalStateso they survive webview disposal and VS Code restartscreateMemo+ direct signal reads (not keyedShowaccessor) to ensuretotal() > 1re-evaluates reactively for the pagination displayScreenshots