feat(accessibility): Improves screen reader accessibility for indexing status#1
feat(accessibility): Improves screen reader accessibility for indexing status#1
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughSummary by CodeRabbit
WalkthroughReworked accessibility for the indexing badge: ChatTextArea now uses Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
webview-ui/src/components/chat/IndexingStatusBadge.tsx (1)
144-147: Reduce screen reader “chatter” during frequent progress updates (optional)If the backend sends rapid progress messages, even “polite” live regions can get noisy. Consider announcing only on meaningful deltas (e.g., every 5% or when the phase changes).
I can sketch a small throttle (useRef + timestamp or last-announced percentage) if you want to keep this lean and deterministic.
webview-ui/src/components/chat/ChatTextArea.tsx (1)
1406-1408: Make the sr-only control visible when keyboard-focused (avoid “invisible focus” for sighted users)An interactive element that’s sr-only remains in the tab order but is visually hidden—confusing for keyboard-only users who don’t use a screen reader. A pragmatic pattern is to reveal it when it receives focus.
Apply this diff within the selected lines:
- className={cn({ - "sr-only": containerWidth < 235, // kilocode_change: use sr-only instead of hidden for screen reader accessibility - })} + className={cn({ + // Keep it readable by screen readers, but reveal it when focused for keyboard users. + "sr-only focus:not-sr-only": containerWidth < 235, + })}Note: Tailwind’s not-sr-only utility supports state variants. If your config doesn’t, we can fall back to a small CSS helper class that cancels sr-only on :focus-visible.
Please sanity-check with:
- Narrow pane < 235px
- Keyboard-only navigation (Tab/Shift+Tab) without a screen reader
- NVDA/VoiceOver to confirm live announcements still occur and focus behavior is sane
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
webview-ui/src/components/chat/ChatTextArea.tsx(1 hunks)webview-ui/src/components/chat/IndexingStatusBadge.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
webview-ui/src/components/chat/IndexingStatusBadge.tsx (2)
apps/storybook/src/lib/utils.ts (1)
cn(4-6)webview-ui/src/components/chat/CodeIndexPopover.tsx (1)
CodeIndexPopover(144-1287)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: test-webview (windows-latest)
- GitHub Check: test-webview (ubuntu-latest)
- GitHub Check: storybook-playwright-snapshot
- GitHub Check: compile
- GitHub Check: test-extension (windows-latest)
- GitHub Check: test-extension (ubuntu-latest)
🔇 Additional comments (2)
webview-ui/src/components/chat/IndexingStatusBadge.tsx (1)
140-141: Decorative dot correctly hidden from assistive techNice touch. Marking the status dot aria-hidden prevents redundant narration. Carry on.
webview-ui/src/components/chat/ChatTextArea.tsx (1)
1405-1409: LGTM: sr-only instead of hidden achieves the accessibility goalSwitching to sr-only keeps the badge present for assistive tech when space gets tight. This aligns with the PR objective without altering behavior elsewhere.
| aria-label={getTooltipText()} | ||
| aria-describedby="indexing-status-description"> | ||
| {" "} | ||
| {/* kilocode_change: add describedby for better context */} |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Avoid duplicate announcements; use a stable aria-label and a unique describedby id
Right now the button’s accessible name (aria-label) and its description (aria-describedby) are the same string. Many screen readers will read both, resulting in repetitive output. Also, the hard-coded id risks collisions if this badge renders more than once.
Proposed fix: keep a short, stable label (e.g., “Indexing status”) and move all dynamic status text to the describedby target, using a per-instance id.
Apply this diff within the selected lines:
- aria-label={getTooltipText()}
- aria-describedby="indexing-status-description">
+ aria-label={t("chat:indexingStatus.status")}
+ aria-describedby={descriptionId}>And add (outside the selected range) at the top-level imports and inside the component:
// add to imports at Line 1
import React, { useState, useEffect, useMemo, useId } from "react"
// add inside IndexingStatusBadge component body, near other hooks
const descriptionId = useId()To confirm there aren’t any other places still using a hard-coded id for this description, run:
🏁 Script executed:
#!/bin/bash
# Find any hard-coded uses of the old id
rg -n 'id="indexing-status-description"|aria-describedby="indexing-status-description"' -A 2
# Find all usages of IndexingStatusBadge in the repo to estimate potential multiple instances
rg -n '<IndexingStatusBadge' -A 2Length of output: 1961
Use React’s useId for dynamic aria-describedby in IndexingStatusBadge
We’ve confirmed there are two hard-coded occurrences of "indexing-status-description" in IndexingStatusBadge.tsx (the button’s aria-describedby and the hidden <div id=...>). To avoid duplicate screen-reader announcements and ID collisions, let’s:
• Replace the hard-coded aria-describedby="indexing-status-description" on the badge button with a dynamic descriptionId.
• Change the hidden <div id="indexing-status-description" to use that same dynamic descriptionId.
• Import and invoke React’s useId hook to generate a per-instance ID.
• Update the test in __tests__/IndexingStatusBadge.spec.tsx to no longer expect the old static ID (adjust snapshots or queries to use the new dynamic ID or test for the relationship between button and description container).
Suggested diff:
--- a/webview-ui/src/components/chat/IndexingStatusBadge.tsx
+++ b/webview-ui/src/components/chat/IndexingStatusBadge.tsx
@@ top of file
-import React, { useState, useEffect, useMemo } from "react"
+import React, { useState, useEffect, useMemo, useId } from "react"
@@ inside IndexingStatusBadge component
- const { t } = useAppTranslation()
+ const { t } = useAppTranslation()
+ const descriptionId = useId()
@@ Lines 128-131 (badge button)
- aria-label={getTooltipText()}
- aria-describedby="indexing-status-description">
+ aria-label={t("chat:indexingStatus.status")}
+ aria-describedby={descriptionId}>
@@ Lines 145-147 (visually hidden status text)
- <div id="indexing-status-description" aria-live="polite" aria-atomic="true" className="sr-only">
+ <div id={descriptionId} aria-live="polite" aria-atomic="true" className="sr-only">
{getTooltipText()}
</div>• Don’t forget to update your test (IndexingStatusBadge.spec.tsx) so it no longer looks for "indexing-status-description" but instead confirms the button’s aria-describedby matches the generated ID and that the live region contains the dynamic text.
🤖 Prompt for AI Agents
In webview-ui/src/components/chat/IndexingStatusBadge.tsx around lines 128 to
131, the component uses a hard-coded "indexing-status-description" id for both
the button's aria-describedby and the hidden description div which can cause
duplicate IDs and screen-reader collisions; import React's useId, call it to
create a per-instance descriptionId, replace the static aria-describedby value
with that descriptionId on the badge button and set the hidden div's id to
descriptionId, and then update __tests__/IndexingStatusBadge.spec.tsx to stop
asserting the static string and instead assert that the button's
aria-describedby equals the generated id and that the live region/hidden div
contains the expected dynamic text (update snapshots/queries accordingly).
| {/* kilocode_change start: Add live region for status announcements */} | ||
| <div id="indexing-status-description" aria-live="polite" aria-atomic="true" className="sr-only"> | ||
| {getTooltipText()} | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make the announced text a proper status live region and tie it to the unique id
Marking the description container as role="status" gives us the right semantics for polite announcements. Also, wire it to the unique id to avoid collisions and ensure the button’s describedby points to this element.
Apply this diff within the selected lines:
- <div id="indexing-status-description" aria-live="polite" aria-atomic="true" className="sr-only">
+ <div id={descriptionId} role="status" aria-live="polite" aria-atomic="true" className="sr-only">
{getTooltipText()}
</div>📝 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.
| {/* kilocode_change start: Add live region for status announcements */} | |
| <div id="indexing-status-description" aria-live="polite" aria-atomic="true" className="sr-only"> | |
| {getTooltipText()} | |
| </div> | |
| {/* kilocode_change start: Add live region for status announcements */} | |
| <div id={descriptionId} role="status" aria-live="polite" aria-atomic="true" className="sr-only"> | |
| {getTooltipText()} | |
| </div> |
🤖 Prompt for AI Agents
In webview-ui/src/components/chat/IndexingStatusBadge.tsx around lines 144-147,
the live region currently lacks status semantics and a unique id tie-in; change
the div to include role="status" and give it a unique id (e.g.
`${uniqueId}-indexing-status-description`) and update the button's
aria-describedby to reference that same unique id so announcements are correctly
scoped and non-colliding.
There was a problem hiding this comment.
Pull Request Overview
This PR improves screen reader accessibility for the indexing status badge. Previously, when the screen width was narrow (< 235px), the badge was completely hidden using the hidden class, making it inaccessible to screen readers. The changes ensure the badge remains available to assistive technologies while maintaining the visual behavior.
- Replaces
hiddenclass withsr-onlyto maintain screen reader access in narrow containers - Adds aria-describedby attribute and live region for better status announcements
- Marks decorative status indicator as aria-hidden to prevent redundant content
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
webview-ui/src/components/chat/ChatTextArea.tsx |
Changes visibility class from hidden to sr-only for narrow containers |
webview-ui/src/components/chat/IndexingStatusBadge.tsx |
Adds aria-describedby, live region, and aria-hidden for decorative elements |
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| aria-label={getTooltipText()}> | ||
| aria-label={getTooltipText()} | ||
| aria-describedby="indexing-status-description"> | ||
| {" "} |
There was a problem hiding this comment.
The whitespace string {" "} appears to be unnecessary and adds no semantic value. Consider removing this line.
| {" "} |
| </button> | ||
| </CodeIndexPopover> | ||
| {/* kilocode_change start: Add live region for status announcements */} | ||
| <div id="indexing-status-description" aria-live="polite" aria-atomic="true" className="sr-only"> |
There was a problem hiding this comment.
The live region will announce status changes whenever getTooltipText() returns a different value, but this could result in frequent announcements during indexing progress updates. Consider using aria-live="assertive" for critical status changes like errors, or debouncing the updates to reduce announcement frequency.
…g status Replaces hidden class with sr-only to maintain screen reader access when container is narrow. Adds aria-describedby attribute and live region to provide better context and status announcements for assistive technologies. Marks decorative status indicator as aria-hidden to prevent redundant screen reader content.
adamhill
force-pushed
the
adamhill/fix-code-indexing-vis-for-screen-readers
branch
from
198bdda to
dac1da2
Compare
Adds an optional API key setting for the Ollama provider. The API key is included as an 'Authorization' header in requests to the Ollama API. This change updates the provider settings schema, modifies the Ollama client to pass the key, and enhances the fetchWithTimeout utility to support persistent headers. The UI now includes an input field for the API key with a visibility toggle and corresponding internationalization. Note that the key is passed as 'Authorization: <key>', not as a Bearer token. This is intentional, and matches the docs for ollama-js.
* changeset version bump * Updating CHANGELOG.md format --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: kilocode-bot <github-actions@github.com>
* fix: skip turbo cache on vsix publish ci workflow * refactor: remove changeset * refactor: remove mcp config
* Make providers list searchable * Sort providers alphabetically and add Kilo Code to top * Add changeset
* changeset version bump * Updating CHANGELOG.md format --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: kilocode-bot <github-actions@github.com>
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/i18n/locales/vi/common.json (1)
217-217: Stray non‑Vietnamese characters ("θα") in user-facing text.There’s a likely paste artifact in this sentence (“cũng θα xóa…”). A small fix prevents bewildering readers like a star blinking out of phase.
Apply this fix:
- "description": "Bạn có chắc chắn muốn xóa chế độ {{scope}} này không? Thao tác này cũng θα xóa thư mục quy tắc liên quan tại {{rulesFolderPath}}", + "description": "Bạn có chắc chắn muốn xóa chế độ {{scope}} này không? Thao tác này cũng sẽ xóa thư mục quy tắc liên quan tại {{rulesFolderPath}}",webview-ui/src/i18n/locales/uk/settings.json (1)
61-75: Duplicate/near-duplicate keys will shadow each other (JSON) and confuse consumersThere are repeated variants for several OpenAI-compatible keys (camelCase vs lowerCamel with lowercase “openai”), plus duplicate descriptions. In JSON, later keys silently override earlier ones—an invisible banana peel for future maintainers and translators.
Let’s normalize on the established camelCase prefix “openAiCompatible*” and remove the duplicates (and the extra description). Does this match what the UI expects?
Apply this minimal cleanup (adjust trailing commas as needed):
- "openaiKeyLabel": "Ключ OpenAI:", - "openaiCompatibleBaseUrlLabel": "Базовий URL:", - "openaiCompatibleApiKeyLabel": "Ключ API:", - "openaiCompatibleModelDimensionLabel": "Розмірність вбудовування:", - "openaiCompatibleModelDimensionPlaceholder": "наприклад, 1536", - "openaiCompatibleModelDimensionDescription": "Розмірність вбудовування (розмір виводу) для вашої моделі. Перевірте документацію вашого постачальника для цього значення. Загальні значення: 384, 768, 1536, 3072.",And keep only one description variant (this one is already present):
- "openAiCompatibleModelDimensionDescription": "Розмірність вбудовування (розмір виводу) для вашої моделі. Перевірте документацію вашого постачальника для цього значення. Загальні значення: 384, 768, 1536, 3072.", + "openAiCompatibleModelDimensionDescription": "Розмірність вбудовування (розмір виводу) для вашої моделі. Перевірте документацію вашого постачальника для цього значення. Загальні значення: 384, 768, 1536, 3072.",webview-ui/src/components/kilocode/common/OrganizationSelector.tsx (1)
44-65: Gate message handling to trusted messages and typesA gentle guard keeps stray window messages from confusing our little dropdown universe.
Apply this diff:
const handleMessage = (event: MessageEvent<WebviewMessage>) => { - const message = event.data + const message = event.data + // Guard: ensure structured message with string type + if (!message || typeof message !== "object" || typeof (message as any).type !== "string") return + // Optional: if you want to be extra safe in VS Code webviews, you can gate by origin prefix: + // if (!event.origin?.startsWith("vscode-webview://")) return
🧹 Nitpick comments (36)
src/core/webview/ClineProvider.ts (1)
2407-2407: Nice conditional enrichment of telemetry — keep it modularSpreading the helper’s result keeps the payload tidy and avoids dangling empty keys. If you later move getOpenRouter to a class method, this line stays just as clean. No monsters under the bed here.
If you expect multiple such conditional blocks in the future (fastApply, memory, openRouter, etc.), consider a small composeTelemetryProps helper to collect and merge them in one place.
package.json (1)
19-21: Consider aligningbuildwithvsix:productionfor consistency.If CI now packages with
vsix:production, havingbuilddo something different may surprise local devs. Aligning them keeps the mental model simple.- "build": "pnpm vsix", + "build": "pnpm vsix:production",src/i18n/locales/ar/common.json (1)
113-115: LGTM; tiny wording tweak for consistency with geminiCli?To keep the voice of the cosmos consistent across sibling keys, consider aligning “مصادقة” with the existing “تسجيل دخول” phrasing used in geminiCli.oauthLoadFailed.
Apply this minimal diff:
- "oauthLoadFailed": "فشل تحميل بيانات OAuth. لازم تسوي مصادقة أول: {{error}}" + "oauthLoadFailed": "فشل تحميل بيانات OAuth. لازم تسوي تسجيل دخول أول: {{error}}"webview-ui/src/i18n/locales/id/settings.json (1)
374-377: Align Indonesian phrasing for API key placeholder with the rest of the localeA tiny linguistic wobble in an otherwise elegant cosmos: elsewhere in this file you use “kunci API…”, but here the placeholder says “API key.” For clarity and consistency, let’s harmonize the phrasing. Shall we make the stars line up?
Apply this minimal diff:
- "apiKeyPlaceholder": "Masukkan API key Anda", + "apiKeyPlaceholder": "Masukkan kunci API Anda",src/i18n/locales/es/common.json (1)
122-124: Add a tiny comma for natural Spanish flowA microscopic edit that makes the sentence orbit more smoothly: add a comma after “Por favor”.
- "oauthLoadFailed": "Error al cargar credenciales OAuth. Por favor autentícate primero: {{error}}" + "oauthLoadFailed": "Error al cargar credenciales OAuth. Por favor, autentícate primero: {{error}}"webview-ui/src/i18n/locales/fr/settings.json (1)
374-377: Clarify header name and improve idiomatic FrenchLet’s say it like a native and preserve the literal header name so screen-readers and devs hear the same music.
- "apiKeyInfo": "La clé API sera envoyée comme en-tête d'autorisation" + "apiKeyInfo": "La clé API sera envoyée dans l'en-tête Authorization"webview-ui/src/i18n/locales/pl/settings.json (1)
374-377: Polish grammar touch-up for a smoother readA small nudge toward idiomatic Polish and clarity on the literal header name.
- "apiKeyInfo": "Klucz API będzie wysłany jako nagłówek Authorization" + "apiKeyInfo": "Klucz API zostanie wysłany w nagłówku Authorization"webview-ui/src/i18n/locales/ca/settings.json (1)
374-376: LGTM on new Ollama API key strings; consider literal header name for clarityTranslations look good and consistent with other locales. One tiny nit: elsewhere we often keep the literal HTTP header name “Authorization” in English to reduce ambiguity in technical UIs. Want to align to “capçalera Authorization” or “capçalera «Authorization»” for precision?
Like all good experiments, consistency breeds fewer surprises—worth a thought?
- "apiKeyInfo": "La clau API s'enviarà com a capçalera d'autorització", + "apiKeyInfo": "La clau API s'enviarà com a capçalera «Authorization»",src/api/providers/kilocode/fetchWithTimeout.ts (2)
3-3: Signature looks good; optional rename for intent clarityMinor naming nit: consider
persistentHeadersordefaultHeadersto make the lifetime/precedence crystal clear to future readers.-export function fetchWithTimeout(timeoutMs: number, headers?: Record<string, string>): typeof fetch { +export function fetchWithTimeout(timeoutMs: number, persistentHeaders?: Record<string, string>): typeof fetch {And update references within accordingly.
10-19: Optional: enforce true request timeouts with AbortSignalThe Agent’s
headersTimeout/bodyTimeouthelp, but pairing withAbortSignal.timeout(timeoutMs)guarantees cancellation from the caller’s perspective (and plays nice if the server stalls before headers).Example tweak:
- const response = (await undici.fetch( + const response = (await undici.fetch( input as undici.RequestInfo, { ...init, - headers: mergedHeaders, + headers: mergedHeaders, + signal: (init?.signal as AbortSignal) ?? AbortSignal.timeout(timeoutMs), dispatcher: agent, } as undici.RequestInit, )) as unknown as ResponseQuestion to ponder, fellow traveler: do you want the operation to be observable as “aborted” upstream, or merely to time out at the transport layer?
webview-ui/src/i18n/locales/es/settings.json (1)
374-377: Use the literal header name “Authorization” to avoid ambiguityHeader names are identifiers, not prose. Using “Authorization” verbatim avoids confusion and aligns with other locales.
- "apiKeyInfo": "La clave API se enviará como encabezado de autorización", + "apiKeyInfo": "La clave API se enviará como encabezado Authorization",A small phrasing tweak, but it keeps the cosmos of translations in elegant harmony.
src/api/providers/kilocode-ollama.ts (2)
141-152: Header injection works; trim the key and avoid double-injecting headers.You pass Authorization in two places: as default headers in fetchWithTimeout and again via the Ollama constructor headers. That’s redundant and mildly confusing to readers. Also, trimming user input avoids subtle whitespace bugs.
Suggested minimal change within this block:
- const headers = this.options.ollamaApiKey - ? { Authorization: this.options.ollamaApiKey } //Yes, this is weird, its not a Bearer token - : undefined + const apiKey = this.options.ollamaApiKey?.trim() + const headers = apiKey + ? { Authorization: apiKey } // Intentionally not forcing "Bearer " prefix + : undefined this.client = new Ollama({ host: this.options.ollamaBaseUrl || "http://localhost:11434", - fetch: fetchWithTimeout(OLLAMA_TIMEOUT_MS, headers), - headers: headers, + // Avoid duplicate injection — let the client own its headers + fetch: fetchWithTimeout(OLLAMA_TIMEOUT_MS), + headers, })Additionally, given the “not Bearer” nuance is atypical, consider making the scheme explicit in settings later (e.g., a toggle for raw vs Bearer) or documenting that users can paste “Bearer ” as the value if their endpoint requires it.
153-155: Type-safe catch for clearer error reporting.Accessing error.message on unknown will trip strict TS configs and can drop important context. Guard once and preserve message.
Outside the selected range, apply:
} catch (err) { const message = err instanceof Error ? err.message : String(err) throw new Error(`Error creating Ollama client: ${message}`) }webview-ui/src/components/settings/constants.ts (1)
86-89: Avoid mutating the exported PROVIDERS array at module load.unshift after export is a small side effect that can surprise readers and complicate future refactors. Prefer composing the final array with “Kilo Code” pinned first without mutation.
Here’s a refactor that keeps Kilo Code first and sorts the rest, without runtime mutation:
// 1) Rename your current array to a base list const BASE_PROVIDERS = [ { value: "openrouter", label: "OpenRouter" }, // ...all others except kilocode ] as const // 2) Export the final composed list export const PROVIDERS = [ { value: "kilocode", label: "Kilo Code" }, ...[...BASE_PROVIDERS].sort((a, b) => a.label.localeCompare(b.label)), ] as constwebview-ui/src/components/settings/ApiOptions.tsx (1)
446-455: SearchableSelect: consider an explicit accessible name.Right now the trigger Button has role="combobox" and aria-expanded, but it doesn’t get an accessible name from the surrounding label element. Small UX boost: let SearchableSelect accept aria-label or aria-labelledby and pass it to the Button.
Follow-up change in SearchableSelect (webview-ui/src/components/ui/searchable-select.tsx):
- Add optional ariaLabel?: string and ariaLabelledBy?: string props
- Forward them to the Button as aria-label/aria-labelledby
Then here:
<SearchableSelect ariaLabel={t("settings:providers.apiProvider")} // ...existing props />Cosmic bonus: this aligns with the broader accessibility theme of the PR without changing behavior.
CHANGELOG.md (1)
3-10: Changelog entry reads well; consider clarifying Ollama auth header semantics.Nice, concise notes. Since the Ollama API key is sent in Authorization without forcing “Bearer”, a short parenthetical could prevent head-scratching:
-- Add API key support to the Ollama provider, enabling usage of Ollama Turbo +- Add API key support to the Ollama provider, enabling usage of Ollama Turbo + (Authorization header is sent as provided; include 'Bearer ' prefix yourself if your endpoint requires it)Also, markdownlint flagged unordered list indentation elsewhere in this file (MD007). Not introduced here, but worth a quick sweep when convenient.
webview-ui/src/components/kilocode/profile/ProfileView.tsx (1)
162-162: Spacing addition is fine; consider exposing a visible label for a11y parity.Given the PR theme is accessibility, you might also pass showLabel here for a clearer accessible name in contexts where the surrounding copy isn’t read by screen readers. The internal button text does provide a name, so this is a nice-to-have, not a must.
webview-ui/src/components/kilocode/settings/providers/KiloCode.tsx (1)
94-95: Nice: labeled organization selector improves discoverability.This aligns with the goal of clearer semantics in settings. If you want a touch more separation from ModelPicker, consider adding className="mb-4" here.
src/api/providers/kilocode-openrouter.ts (1)
89-95: Thread Organization into Default-Model Cache Key
- Verified that getOpenRouterModels is invoked with
X-KiloCode-OrganizationIdbut isn’t cached—each call hits the API, so no cross-org collision there.- Inspected getKilocodeDefaultModel.ts: it keys its in-memory cache solely on
kilocodeToken(cache.get(kilocodeToken ?? "")), ignoringkilocodeOrganizationId.- If your default model varies per organization, you’ll want to refactor:
• ChangegetKilocodeDefaultModel(token)→getKilocodeDefaultModel(token, organizationId)in kilocode-openrouter.ts
• Update the function signature in getKilocodeDefaultModel.ts to acceptkilocodeOrganizationIdand include it in the cache key (e.g.cache.get(\${token}|${orgId}`)`).src/api/providers/fetchers/openrouter.ts (4)
99-108: Header plumbing and signature change are correct; consider mirroring for endpoints.
- Merging headers with user-provided values (user overrides last) is the right precedence.
- The options signature now carrying headers is pragmatic.
Optional follow-through: getOpenRouterModelEndpoints doesn’t accept/merge headers. If some endpoints ever require auth or org scoping, you may want to parallel the same pattern there to avoid divergent behavior.
Apply this diff if you want parity in endpoints:
export async function getOpenRouterModelEndpoints( modelId: string, - options?: ApiHandlerOptions, + options?: ApiHandlerOptions & { headers?: RawAxiosRequestHeaders }, ): Promise<Record<string, ModelInfo>> { @@ - const response = await axios.get<OpenRouterModelEndpointsResponse>(`${baseURL}/models/${modelId}/endpoints`) + const response = await axios.get<OpenRouterModelEndpointsResponse>(`${baseURL}/models/${modelId}/endpoints`, { + headers: { ...DEFAULT_HEADERS, ...(options?.headers ?? {}) }, + })
113-114: Zod error formatting will stringify to “[object Object]”. Use JSON for useful diagnostics.Error.details from zod’s format() is an object; concatenation coalesces it poorly. Prefer JSON.stringify for debuggable telemetry.
- throw new Error("OpenRouter models response is invalid: " + result.error.format()) + throw new Error( + "OpenRouter models response is invalid: " + + JSON.stringify(result.error.format(), null, 2) + )
160-160: Key selection by tag with provider_name fallback is reasonable.One gentle question: are tags guaranteed unique per (modelId, provider)? If not, you might collide. If uniqueness isn’t guaranteed upstream, consider composing a key like
${provider_name}:${tag ?? "default"}.
131-132: Ensure getOpenRouterModels rejections are caught upstreamIn our cosmic dance of promises, we’ve spotted direct
await getOpenRouterModels(...)calls that, if they throw, will streak up to the UI unimpeded:
- src/api/providers/fetchers/modelCache.ts
- line 60 (case
"openrouter")- line 83 (case
"kilocode-openrouter")Tests also
awaiton line 24 of__tests__/openrouter.spec.ts, but failures there belong to the test universe. In production code, wrap these calls intry/catchor employPromise.allSettledso a meteor-sized error doesn’t crater your UI:- models = await getOpenRouterModels({...}) + try { + models = await getOpenRouterModels({...}) + } catch (err) { + logger.warn('OpenRouter fetch failed, falling back to empty list', err) + models = [] + }webview-ui/src/components/chat/ChatView.tsx (1)
1993-1995: Absolute positioning is fine; add z-index to avoid accidental overlap.On small panes, other elements (tooltips, banners) can layer above or trap clicks. A small z-10 goes a long way.
- <OrganizationSelector className="absolute top-2 right-3" /> + <OrganizationSelector className="absolute top-2 right-3 z-10" />Would you also want pointer-events-auto if nested in any translucent overlay?
webview-ui/src/components/settings/providers/Ollama.tsx (1)
90-123: Tighten label and improve accessibility for the Ollama API key field
- Replace the duplicated
apiKeyInfolabel with the specificapiKeylabel, and keep the descriptive text only in the helper below.- Add accessible attributes to the eye-toggle button so screen readers announce both its purpose and current state:
•aria-label={showApiKey ? t("common:hide") : t("common:show")}
•aria-pressed={showApiKey}
•title={showApiKey ? t("common:hide") : t("common:show")}- No action needed on storage: all provider API keys (including
ollamaApiKey) are already persisted securely via ProviderSettingsManager’s use of VS Code secret storage.- <label className="block font-medium mb-1"> - {t("settings:providers.ollama.apiKeyInfo")}{" "} + <label className="block font-medium mb-1"> + {t("settings:providers.ollama.apiKey")}{" "} <span className="text-vscode-descriptionForeground font-normal"> ({t("settings:optional")}) </span> </label> @@ - <button + <button type="button" onClick={() => setShowApiKey(!showApiKey)} className="absolute right-3 top-8 text-vscode-foreground hover:text-vscode-descriptionForeground focus:outline-none" + aria-label={showApiKey ? t("common:hide") : t("common:show")} + aria-pressed={showApiKey} + title={showApiKey ? t("common:hide") : t("common:show")} >webview-ui/src/i18n/locales/hi/settings.json (1)
374-377: New Ollama keys added; consistent with other locales.The phrasing reads clearly. One suggestion: ensure the UI uses apiKey for the label (not apiKeyInfo) to avoid duplication with the helper text.
src/core/webview/webviewMessageHandler.ts (2)
581-584: Passing kilocodeOrganizationId into model fetch is the right propagation.This keeps org-scoped model catalog queries aligned with the selector. Curious: do you also thread this through any caching layers to avoid cross-org pollution?
2158-2161: Minor hardening: avoid sending kilocodeToken back to the webview if not needed.If the webview doesn’t need the token for UI-only profile rendering, drop it from payload to reduce blast radius.
- payload: { success: true, data: { kilocodeToken, ...response.data } }, + payload: { success: true, data: response.data },Would anything in OrganizationSelector or account UI break if the token isn’t echoed here?
webview-ui/src/components/kilocode/common/OrganizationSelector.tsx (6)
15-34: Close-on-escape/outside-click handlers: consider pointer events and scoping to the open stateYou’re listening globally even when the menu is closed, and mousedown won’t catch touch. Use pointerdown (with passive) and attach listeners only while open. It’s a small tweak that makes the component feel more “of the cosmos” across input types.
Apply this diff:
useEffect(() => { - const onKeyDown = (e: KeyboardEvent) => { - if (e.key === "Escape") setIsOpen(false) - } - window.addEventListener("keydown", onKeyDown) - - const onMouseDown = (e: MouseEvent) => { - if (!containerRef.current) return - if (!containerRef.current.contains(e.target as Node)) { - setIsOpen(false) - } - } - - window.addEventListener("mousedown", onMouseDown) + if (!isOpen) return + + const onKeyDown = (e: KeyboardEvent) => { + if (e.key === "Escape") setIsOpen(false) + } + window.addEventListener("keydown", onKeyDown) + + const onPointerDown = (e: PointerEvent) => { + if (!containerRef.current) return + if (!containerRef.current.contains(e.target as Node)) { + setIsOpen(false) + } + } + window.addEventListener("pointerdown", onPointerDown, { passive: true }) return () => { window.removeEventListener("keydown", onKeyDown) - window.removeEventListener("mousedown", onMouseDown) + window.removeEventListener("pointerdown", onPointerDown) } -}, []) +}, [isOpen])
67-96: Balance fetch immediately after upsert: verify ordering assumptionsTwo postMessage calls fire back-to-back; in most cases VS Code delivers in order, but the extension may read “balance” with stale config if it resolves too eagerly. If the host already triggers a balance refresh after handling upsert, consider removing the extra fetch here. Otherwise, confirm ordering guarantees.
Would you like me to adjust the flow so the extension emits a balance refresh after persisting the new configuration (and the webview listens for it), ensuring causal ordering?
99-101: Do we want “Personal” available even with zero orgs?Returning null hides the control entirely. If the intent is “always selectable personal,” render the trigger with just the Personal option. If not, ignore this.
Apply this diff to always render and rely on the Personal item:
-if (!organizations.length) return null +// Keep rendering so users can switch back to Personal even with zero orgs +// (If undesired, revert this change.)
109-121: Wire up control relationships for assistive tech:aria-controls+ stable IDsLet’s give screen readers a clear thread to follow between the trigger and the listbox.
Apply this diff:
- <button + <button type="button" onClick={() => setIsOpen((o) => !o)} aria-haspopup="listbox" aria-expanded={isOpen} + aria-controls="org-selector-listbox"- <div role="listbox" aria-label={t("kilocode:profile.organization")}> + <div id="org-selector-listbox" role="listbox" aria-label={t("kilocode:profile.organization")}>Also applies to: 145-146
145-175: Focus management and basic keyboard support for the listboxOpening the menu should move focus into the listbox so Escape/Arrow keys feel natural. A small ref and focus-on-open do wonders; arrow navigation can be next.
Apply this diff inside the listbox:
- <div id="org-selector-listbox" role="listbox" aria-label={t("kilocode:profile.organization")}> + <div + id="org-selector-listbox" + role="listbox" + aria-label={t("kilocode:profile.organization")} + tabIndex={-1} + ref={listboxRef} + >And add these minimal bits outside the shown ranges:
// near other refs const listboxRef = useRef<HTMLDivElement>(null) // focus the list on open useEffect(() => { if (isOpen) listboxRef.current?.focus() }, [isOpen])If you want to go further, I can add roving tabindex and ArrowUp/ArrowDown handling next.
127-139: Theme-friendly icon colorThe chevron’s hard-coded stroke color ignores theme changes. Use a VS Code theme var to let it sing in every cosmic palette.
Apply this diff:
- stroke="rgb(156 163 175)" + stroke="var(--vscode-icon-foreground)"src/api/providers/openrouter.ts (2)
99-108: Guard endpoint metadata when computing ignored providersIf an endpoint is missing contextWindow (or it’s not a number), the comparison can mislead. A tiny guard makes the filter sturdier.
Apply this diff:
- const endpoints = Object.entries(this.endpoints) - const ignoredProviders = endpoints - .filter((endpoint) => endpoint[1].contextWindow < model.contextWindow) - .map((endpoint) => endpoint[0]) + const endpoints = Object.entries(this.endpoints) + const ignoredProviders = endpoints + .filter(([, info]) => typeof info?.contextWindow === "number" && typeof model?.contextWindow === "number" && (info.contextWindow as number) < (model.contextWindow as number)) + .map(([provider]) => provider)
110-135: Provider param shaping: sensible defaults and conditional populationNice: prefer a specific provider when configured; otherwise, only send
providerwhen there’s something meaningful to say. One question: if all endpoints would be ignored, you intentionally send noignoreto let OpenRouter pick—smart.Optional polish: avoid including undefineds at construction.
return { provider: { - ignore: ignoredProviders, + ...(ignoredProviders && { ignore: ignoredProviders }), data_collection: this.options.openRouterProviderDataCollection, sort: this.options.openRouterProviderSort, }, }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (70)
.changeset/famous-crabs-think.md(1 hunks).changeset/social-times-march.md(0 hunks).github/workflows/marketplace-publish.yml(1 hunks)CHANGELOG.md(1 hunks)package.json(1 hunks)packages/types/src/provider-settings.ts(1 hunks)packages/types/src/providers/openrouter.ts(1 hunks)src/api/providers/fetchers/__tests__/openrouter.spec.ts(1 hunks)src/api/providers/fetchers/modelCache.ts(1 hunks)src/api/providers/fetchers/openrouter.ts(2 hunks)src/api/providers/kilocode-ollama.ts(2 hunks)src/api/providers/kilocode-openrouter.ts(1 hunks)src/api/providers/kilocode/fetchWithTimeout.ts(1 hunks)src/api/providers/openrouter.ts(7 hunks)src/core/webview/ClineProvider.ts(2 hunks)src/core/webview/webviewMessageHandler.ts(3 hunks)src/i18n/locales/ar/common.json(1 hunks)src/i18n/locales/ca/common.json(1 hunks)src/i18n/locales/cs/common.json(1 hunks)src/i18n/locales/de/common.json(1 hunks)src/i18n/locales/en/common.json(1 hunks)src/i18n/locales/es/common.json(1 hunks)src/i18n/locales/fr/common.json(1 hunks)src/i18n/locales/hi/common.json(1 hunks)src/i18n/locales/id/common.json(1 hunks)src/i18n/locales/it/common.json(1 hunks)src/i18n/locales/ja/common.json(1 hunks)src/i18n/locales/ko/common.json(1 hunks)src/i18n/locales/nl/common.json(1 hunks)src/i18n/locales/pl/common.json(1 hunks)src/i18n/locales/pt-BR/common.json(1 hunks)src/i18n/locales/ru/common.json(1 hunks)src/i18n/locales/th/common.json(1 hunks)src/i18n/locales/tr/common.json(1 hunks)src/i18n/locales/uk/common.json(1 hunks)src/i18n/locales/vi/common.json(1 hunks)src/i18n/locales/zh-CN/common.json(1 hunks)src/i18n/locales/zh-TW/common.json(1 hunks)src/package.json(3 hunks)src/shared/api.ts(1 hunks)webview-ui/src/components/chat/ChatView.tsx(3 hunks)webview-ui/src/components/kilocode/common/OrganizationSelector.tsx(3 hunks)webview-ui/src/components/kilocode/profile/ProfileView.tsx(1 hunks)webview-ui/src/components/kilocode/settings/providers/KiloCode.tsx(2 hunks)webview-ui/src/components/settings/ApiOptions.tsx(3 hunks)webview-ui/src/components/settings/constants.ts(1 hunks)webview-ui/src/components/settings/providers/Ollama.tsx(2 hunks)webview-ui/src/components/settings/providers/QwenCode.tsx(1 hunks)webview-ui/src/i18n/locales/ar/settings.json(1 hunks)webview-ui/src/i18n/locales/ca/settings.json(1 hunks)webview-ui/src/i18n/locales/cs/settings.json(1 hunks)webview-ui/src/i18n/locales/de/settings.json(1 hunks)webview-ui/src/i18n/locales/en/settings.json(1 hunks)webview-ui/src/i18n/locales/es/settings.json(1 hunks)webview-ui/src/i18n/locales/fr/settings.json(1 hunks)webview-ui/src/i18n/locales/hi/settings.json(1 hunks)webview-ui/src/i18n/locales/id/settings.json(1 hunks)webview-ui/src/i18n/locales/it/settings.json(1 hunks)webview-ui/src/i18n/locales/ja/settings.json(1 hunks)webview-ui/src/i18n/locales/ko/settings.json(1 hunks)webview-ui/src/i18n/locales/nl/settings.json(1 hunks)webview-ui/src/i18n/locales/pl/settings.json(1 hunks)webview-ui/src/i18n/locales/pt-BR/settings.json(1 hunks)webview-ui/src/i18n/locales/ru/settings.json(1 hunks)webview-ui/src/i18n/locales/th/settings.json(1 hunks)webview-ui/src/i18n/locales/tr/settings.json(1 hunks)webview-ui/src/i18n/locales/uk/settings.json(1 hunks)webview-ui/src/i18n/locales/vi/settings.json(1 hunks)webview-ui/src/i18n/locales/zh-CN/settings.json(1 hunks)webview-ui/src/i18n/locales/zh-TW/settings.json(1 hunks)
💤 Files with no reviewable changes (1)
- .changeset/social-times-march.md
✅ Files skipped from review due to trivial changes (11)
- src/i18n/locales/pt-BR/common.json
- .changeset/famous-crabs-think.md
- webview-ui/src/i18n/locales/th/settings.json
- webview-ui/src/i18n/locales/zh-TW/settings.json
- src/i18n/locales/ko/common.json
- webview-ui/src/i18n/locales/ko/settings.json
- src/i18n/locales/ca/common.json
- webview-ui/src/i18n/locales/vi/settings.json
- webview-ui/src/i18n/locales/cs/settings.json
- webview-ui/src/i18n/locales/pt-BR/settings.json
- src/i18n/locales/hi/common.json
🧰 Additional context used
🧬 Code Graph Analysis (10)
webview-ui/src/components/kilocode/settings/providers/KiloCode.tsx (1)
webview-ui/src/components/kilocode/common/OrganizationSelector.tsx (1)
OrganizationSelector(7-181)
webview-ui/src/components/kilocode/profile/ProfileView.tsx (1)
webview-ui/src/components/kilocode/common/OrganizationSelector.tsx (1)
OrganizationSelector(7-181)
src/api/providers/kilocode-ollama.ts (2)
webview-ui/src/components/settings/providers/Ollama.tsx (1)
Ollama(20-162)src/api/providers/kilocode/fetchWithTimeout.ts (1)
fetchWithTimeout(3-20)
src/api/providers/kilocode/fetchWithTimeout.ts (1)
packages/cloud/src/TelemetryClient.ts (1)
fetch(25-49)
webview-ui/src/components/chat/ChatView.tsx (2)
webview-ui/src/components/kilocode/common/OrganizationSelector.tsx (1)
OrganizationSelector(7-181)webview-ui/src/components/kilocode/KilocodeNotifications.tsx (1)
KilocodeNotifications(20-127)
src/api/providers/fetchers/openrouter.ts (1)
src/api/providers/constants.ts (1)
DEFAULT_HEADERS(3-8)
src/core/webview/webviewMessageHandler.ts (2)
src/shared/WebviewMessage.ts (1)
ProfileData(355-364)src/shared/kilocode/token.ts (1)
getKiloBaseUriFromToken(1-13)
webview-ui/src/components/settings/ApiOptions.tsx (3)
webview-ui/src/components/settings/constants.ts (1)
PROVIDERS(51-86)webview-ui/src/components/ui/searchable-select.tsx (1)
SearchableSelect(37-180)packages/types/src/provider-settings.ts (1)
ProviderName(59-59)
src/api/providers/openrouter.ts (1)
packages/types/src/model.ts (1)
ModelInfo(76-76)
webview-ui/src/components/kilocode/common/OrganizationSelector.tsx (4)
src/shared/WebviewMessage.ts (1)
UserOrganizationWithApiKey(347-353)webview-ui/src/context/ExtensionStateContext.tsx (1)
useExtensionState(614-622)webview-ui/src/i18n/TranslationContext.tsx (1)
useAppTranslation(58-58)webview-ui/src/utils/vscode.ts (1)
vscode(80-80)
🪛 markdownlint-cli2 (0.17.2)
CHANGELOG.md
15-15: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
16-16: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
17-17: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
18-18: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
19-19: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
20-20: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
21-21: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
22-22: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
23-23: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
24-24: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
25-25: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
26-26: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
27-27: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
28-28: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
29-29: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
30-30: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
31-31: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
32-32: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
33-33: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
34-34: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
35-35: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
36-36: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
37-37: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
38-38: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
39-39: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
40-40: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
41-41: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
42-42: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: test-webview (windows-latest)
- GitHub Check: test-webview (ubuntu-latest)
- GitHub Check: test-extension (windows-latest)
- GitHub Check: test-extension (ubuntu-latest)
- GitHub Check: compile
- GitHub Check: storybook-playwright-snapshot
🔇 Additional comments (53)
webview-ui/src/components/settings/providers/QwenCode.tsx (1)
53-57: Cosmic Link Check Passed
The QwenLM README link returns HTTP 200—no event horizon here. Safe to approve and merge!.github/workflows/marketplace-publish.yml (2)
49-49: Workflow switch tovsix:productionis consistent with the new script.The stars line up: this matches the root script and should produce the intended production artifact.
45-66: Confirm.envInclusion and PostHog Key SensitivityI see that your root
.vscodeignoreexplicitly un-ignores.env(!.env), so the CI step that greps for “.env” will package it into every VSIX. This raises the question: Is yourKILOCODE_POSTHOG_API_KEYtruly a public‐facing client key, or is it a secret that shouldn’t be distributed?Please verify:
• Is the PostHog key safe to ship in a published VSIX?
• If not, exclude.envfrom the artifact by:
– Removing the!.envline from.vscodeignore
– Dropping thegrep -q ".env"check in.github/workflows/marketplace-publish.yml
– Injecting the key at build time (e.g., via a DefinePlugin or env‐replacement step)Minimal diffs:
--- .vscodeignore - # Include .env file for telemetry - !.env --- .github/workflows/marketplace-publish.yml - grep -q ".env" /tmp/kilo-code-vsix-contents.txt || exit 1src/i18n/locales/cs/common.json (1)
113-115: All locales containerrors.qwenCode.oauthLoadFailed– cosmic symmetry achieved!Verified that every
src/i18n/locales/*/common.jsonincludes the newerrors.qwenCode.oauthLoadFailedentry. No missing translations detected. Good to merge.src/i18n/locales/it/common.json (1)
121-124: Italian translation added in the expected path.Nice—and consistent with the Czech structure under
errors.qwenCode. Onward through the cosmos of consistency.src/i18n/locales/th/common.json (1)
127-129: Thai translation addition looks solid and consistent.Matches geminiCli wording and placeholder usage. Nicely orbiting in the same semantic plane.
src/i18n/locales/vi/common.json (1)
122-124: Vietnamese qwenCode addition reads well and mirrors geminiCli.Placeholder and tone aligned. Clean drop-in.
packages/types/src/providers/openrouter.ts (1)
43-46: Confirm OpenRouter semantics for google/gemini-2.5-pro and google/gemini-2.5-pro-previewQuick summary: repo changes add both IDs to the prompt-caching set, mark both as reasoning-budget-capable, and mark only "google/gemini-2.5-pro" as requiredReasoningBudget. A few code spots and tests need your confirmation against OpenRouter's actual API semantics (prompt-cache pricing fields + whether pro-preview must require the reasoning budget).
Findings (repo evidence)
- packages/types/src/providers/openrouter.ts
- OPEN_ROUTER_PROMPT_CACHING_MODELS now includes:
- "google/gemini-2.5-pro"
- "google/gemini-2.5-pro-preview"
- OPEN_ROUTER_REQUIRED_REASONING_BUDGET_MODELS includes:
- "google/gemini-2.5-pro"
- OPEN_ROUTER_REASONING_BUDGET_MODELS includes both pro and pro-preview
- src/api/providers/openrouter.ts
- Uses OPEN_ROUTER_PROMPT_CACHING_MODELS to gate cache-breakpoint logic for Google models.
- Special-case: if modelId === pro or pro-preview and reasoning is undefined, reasoning = { exclude: true } (so default handling is being applied for both).
- src/api/providers/fetchers/openrouter.ts
- Sets modelInfo.supportsReasoningBudget and modelInfo.requiredReasoningBudget from the two sets above.
- webview-ui/src/components/ui/hooks/useOpenRouterModelProviders.ts
- UI detects prompt-cache support from endpoint.pricing?.input_cache_read / input_cache_write (i.e., runtime relies on OpenRouter endpoint metadata, not just the static set).
- Tests/fixtures
- src/api/providers/fetchers/tests/openrouter.spec.ts currently excludes these models from some mocked responses (e.g., excludedModels includes both pro & pro-preview; excludedRequiredReasoningBudgetModels excludes "google/gemini-2.5-pro"), and fixtures reference pro-preview endpoint.
Action items / questions to resolve (please confirm)
- Does OpenRouter actually advertise input_cache_read / input_cache_write for both pro and pro-preview endpoints today? If not, the UI will not surface prompt-cache options even if these IDs are in OPEN_ROUTER_PROMPT_CACHING_MODELS.
- Should "google/gemini-2.5-pro-preview" be listed in OPEN_ROUTER_REQUIRED_REASONING_BUDGET_MODELS (i.e., made required) or is it correct to leave it only in OPEN_ROUTER_REASONING_BUDGET_MODELS so reasoning can be toggled?
- Reconcile the server-side special-case that sets reasoning = { exclude: true } for both pro and pro-preview with the requiredReasoningBudget entry for pro (if pro truly requires the budget enabled, the default/exclude logic may be inverted).
- Update fetcher tests/fixtures once you confirm API behavior (remove the explicit exclusions or add endpoints/pricing to the fixtures).
Snippet (where the new IDs were added)
- packages/types/src/providers/openrouter.ts
// kilocode_change start "google/gemini-2.5-pro", "google/gemini-2.5-pro-preview", // kilocode_change endIf you want I can:
- Look up current OpenRouter model endpoint responses (input_cache_read / input_cache_write presence) for these models (requires external API/docs access), or
- Prepare a small test/fixture update patch once you confirm the desired semantics.
src/i18n/locales/uk/common.json (1)
127-129: Ukrainian qwenCode addition looks correct and consistent.Natural tone, consistent pronoun usage with the rest of the locale, and correct placeholder.
src/i18n/locales/pl/common.json (1)
121-124: LGTM: Added qwenCode OAuth error mirrors Gemini CLI phrasing and placeholdersConsistent tone and placeholder usage with existing
geminiCli.oauthLoadFailed. No collisions; JSON structure remains valid.src/i18n/locales/tr/common.json (1)
108-110: LGTM: Turkish qwenCode OAuth error matches existing Gemini CLI wordingClear, idiomatic, and consistent with
geminiCli.oauthLoadFailed. JSON commas and grouping are correct.webview-ui/src/i18n/locales/ru/settings.json (1)
374-377: LGTM: Ollama API key strings read naturally and match UI intent“API-ключ” phrasing, placeholder, and Authorization header note are clear and consistent with surrounding RU strings.
src/package.json (1)
372-372: Context menu regrouping to numeric groups: sensible for ordering; confirm intended UX positionSwitching submenu groups from “navigation” to numeric (“1”/“2”) will push your submenu earlier in the menu ordering—exactly the kind of subtle nudge that changes how users discover features. Is this the position you want across all platforms and themes?
- Check that the submenu now appears where you expect in both editor and terminal contexts.
- Ensure it doesn’t unintentionally precede critical built-in items in crowded contexts.
If this was the goal (as per your changeset), then stardust aligned—no action needed.
Also applies to: 395-395
src/i18n/locales/fr/common.json (1)
108-110: LGTM — accurate and consistent phrasingThe French reads clean and matches the established pattern for OAuth load failures. Nothing to tune here.
src/i18n/locales/id/common.json (1)
108-110: LGTM — faithful, concise translationThe Indonesian string preserves placeholders and matches tone/structure of sibling entries.
webview-ui/src/i18n/locales/ca/settings.json (1)
371-379: Scope check: these i18n changes don’t relate to the PR’s accessibility objectiveThis file adds Ollama API key strings, while the PR title/summary focuses on screen reader improvements for indexing status. Is the PR intentionally bundling orthogonal work? If not, consider splitting for crisper review and rollback boundaries.
webview-ui/src/i18n/locales/it/settings.json (1)
374-376: Ben fatto: Italian Ollama API key strings are consistent and clearLabel, placeholder, and info text read naturally and match other locales’ semantics.
webview-ui/src/i18n/locales/de/settings.json (1)
374-376: LGTM: German Ollama API key entries are accurate and idiomatic“Authorization-Header” keeps the literal header name, which is crisp for a technical audience.
webview-ui/src/i18n/locales/zh-CN/settings.json (1)
375-377: LGTM: zh-CN keys added; terminology consistent with existing “标头” usageThe trio integrates cleanly. Nice touch keeping “Authorization” literal in the info string.
webview-ui/src/i18n/locales/uk/settings.json (2)
377-381: LGTM: Ukrainian Ollama API key entries land wellClear, consistent with the pattern across locales. No blockers here.
1-915: All locales include the three Ollama keys – no action required.
Our cosmic audit confirms everysettings.jsoninwebview-ui/src/i18n/localesdefinesproviders.ollama.apiKey,apiKeyPlaceholder, andapiKeyInfo. Delightful harmony across languages!packages/types/src/provider-settings.ts (1)
181-185: Ollama API key wiring is consistent with existing provider patternsAdding
ollamaApiKeymirrors how other providers store secrets and flows cleanly into the unifiedproviderSettingsSchema. No type or validation mismatches spotted.webview-ui/src/i18n/locales/ar/settings.json (1)
389-392: Arabic Ollama API key strings: clear and consistentTranslations read naturally and match the pattern used in other locales. Keeping “Authorization” unlocalized is the right call for a header name.
webview-ui/src/i18n/locales/ja/settings.json (1)
374-377: Japanese Ollama API key strings: spot onConcise, accurate, and consistent with the rest of the UI copy. Nothing to tune here.
webview-ui/src/i18n/locales/nl/settings.json (1)
374-377: New Ollama API key strings look good; verify UI uses label vs. info correctly.The keys are consistent and Dutch reads well. One pragmatic check: in Ollama.tsx, the field label currently renders apiKeyInfo (helper text) instead of apiKey (the label). That risks the label reading like a sentence. If that’s unintentional, swap to:
- label: t("settings:providers.ollama.apiKey")
- helper text below: t("settings:providers.ollama.apiKeyInfo")
If helpful, I can push a patch to Ollama.tsx to wire the correct key into the label and show apiKeyInfo as the helper text.
src/api/providers/kilocode-ollama.ts (1)
125-129: Interface extension is sound.Adding ollamaApiKey?: string keeps backward compatibility and cleanly threads auth through the handler.
webview-ui/src/components/settings/ApiOptions.tsx (1)
422-431: Provider options mapping: straightforward and correct.Using PROVIDERS to build stable options via useMemo is clean. Given PROVIDERS is static, empty deps are fine.
src/api/providers/fetchers/modelCache.ts (1)
85-87: Confirm auth header requirements for kilocode-openrouter.You’re forwarding only X-KiloCode-OrganizationId. If the endpoint requires auth (e.g., Bearer kilocodeToken or apiKey), we should include it here or ensure getOpenRouterModels merges it in. Otherwise, you’ll get mysterious 401s depending on environment.
Would you verify the expected auth for this path?
webview-ui/src/components/kilocode/settings/providers/KiloCode.tsx (1)
13-13: Import looks good and consistent with shared selector usage.All good on path and ownership.
src/api/providers/fetchers/__tests__/openrouter.spec.ts (1)
33-33: Test adjustment is aligned with current fixtures.Excluding google/gemini-2.5-pro from expectations makes sense given it’s not in the locked mock response. Keeps the test sky serene rather than stormy.
src/i18n/locales/ja/common.json (1)
121-124: JA translation addition LGTM.Key is consistent with the existing pattern (errors.qwenCode.oauthLoadFailed), placeholder syntax matches other entries, and punctuation reads naturally in Japanese.
src/i18n/locales/zh-TW/common.json (1)
107-109: All locales confirmed: errors.qwenCode.oauthLoadFailed present
Our cosmic probe scanned everysrc/i18n/locales/**/common.jsonand each beacon returned “OK” – the new OAuth error string shines across all 23 locales.
✅ Approving code changes.src/i18n/locales/zh-CN/common.json (1)
126-129: Good addition: errors.qwenCode.oauthLoadFailed mirrors other localesString reads naturally, placeholder preserved, and it’s placed under errors like the others. A small change, but it keeps the constellation of i18n keys aligned.
src/i18n/locales/de/common.json (1)
121-124: Deutsch entry looks clean and consistenterrors.qwenCode.oauthLoadFailed is added with proper phrasing and formatting. All set.
src/shared/api.ts (1)
246-246: All “kilocode-openrouter” GetModelsOptions calls now includekilocodeOrganizationIdI’ve scoured the remaining call sites:
- src/core/webview/webviewMessageHandler.ts (lines 578–584): options now pass both
kilocodeTokenandkilocodeOrganizationId- src/api/providers/kilocode-openrouter.ts (lines 91–94): you’re already threading through
kilocodeOrganizationIdNo stray constructors assuming the old shape—everything’s in sync. Great work!
src/i18n/locales/nl/common.json (1)
108-110: Dutch translation reads naturally and mirrors geminiCli — steady as she goesMessage structure and placeholder usage are aligned with other locales. No action needed.
src/i18n/locales/ru/common.json (1)
113-115: RU qwenCode OAuth error added — consistent nesting and placeholderMatches the existing error taxonomy and keeps the {{error}} token intact. Nicely done.
webview-ui/src/i18n/locales/tr/settings.json (1)
374-376: All Ollama API key strings present in every locale—stellar work!
I’ve confirmed that eachsettings.jsonunderwebview-ui/src/i18n/localesincludesproviders.ollama.apiKey,apiKeyPlaceholder, andapiKeyInfo.Next, please verify that your UI components actually reference these keys—e.g.:
providers.ollama.apiKeyproviders.ollama.apiKeyPlaceholderproviders.ollama.apiKeyInfoThis will guard against any silent fallbacks.
webview-ui/src/i18n/locales/en/settings.json (1)
373-375: EN Ollama API key strings — crisp and on-messageKeys are well-named and match expected UI usage. No notes.
src/api/providers/fetchers/openrouter.ts (2)
15-15: Good call: default headers centralization.Importing and reusing DEFAULT_HEADERS keeps the OpenRouter surface consistent and easier to reason about. The stars like a common reference frame.
38-40: Schema extensions look sound.
- preferredIndex on the base model and tag on endpoints are clean, optional extensions that won’t break existing parses.
- Tag-based keying prepares you for multiple endpoints per provider. Nice.
Also applies to: 61-64
webview-ui/src/components/chat/ChatView.tsx (2)
46-46: Importing OrganizationSelector here is a solid UX move.This positions org-scoped model selection where users actually work. Simple and effective.
2011-2014: Spacing wrapper around notifications: looks good.Conditionally adding mt-10 when there are no tasks keeps the visual rhythm without code gymnastics.
webview-ui/src/components/settings/providers/Ollama.tsx (1)
24-24: Local show/hide state: simple and fits the need.This keeps the intent clear, without dragging in a more complex form state just for visibility.
src/core/webview/webviewMessageHandler.ts (2)
10-10: Typing import for ProfileData is a nice touch.Using Omit<ProfileData, "kilocodeToken"> later keeps the API boundary honest.
2135-2144: Switch to /api/profile with typed Axios is solid.Clean headers, typed payload, and you keep kilocodeToken off the network call itself. Elegant.
src/api/providers/openrouter.ts (7)
56-72: Usage struct changes look goodThe cost composition via
upstream_inference_cost + costmakes the ledger clearer, and keepingis_byokis future-friendly.
191-194: Model-aware provider params in streaming calls: LGTMThreading
model.infointogetProviderParamskeeps the router constraints in lockstep with the selected model.
214-227: Reasoning stream handling covers both “reasoning” and “reasoning_content”Good catch on OpenRouter’s dual paths; also guarding delta before probing keeps things from going supernova on nulls.
292-309: Non-streaming path parity: LGTMParameterization mirrors the streaming path, including provider constraints and reasoning. Consistency is the name of the game.
324-338: Readable rate-limit errorsThis makes 429/418 kinder to humans. Consider logging the raw payload at debug level for post-mortems, but this flow is crisp as-is.
254-266: Ignore org-scoping suggestion for OpenRouter providerI dove into the cosmos of our codebase and confirmed that the plain
openrouterprovider (src/api/providers/openrouter.ts) is a public, unscoped endpoint—it isn’t meant to carry akilocodeOrganizationId. All organization‐aware plumbing lives in the KiloCode‐specific provider (src/api/providers/kilocode-openrouter.ts), which already passes the org ID togetModelsandgetModelEndpoints. No changes are needed here.Likely an incorrect or invalid review comment.
34-41: Provider params: “ignore” is officially correctPer the OpenRouter API documentation, the field to exclude specific providers is indeed named “ignore.” No changes required here—stellar work!
| headers: options.kilocodeOrganizationId | ||
| ? { "X-KiloCode-OrganizationId": options.kilocodeOrganizationId } | ||
| : undefined, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Models cache can bleed across organizations — key the memory cache by organization.
You’re now varying the response via X-KiloCode-OrganizationId, but the memory cache key is still just provider. If a user switches orgs within the TTL, they’ll see the prior org’s models. That’s the kind of subtle bug that makes people think the cosmos is conspiring.
Recommend deriving a cacheKey that includes org context (and optionally baseUrl), e.g.:
// inside getModels(...)
const { provider, kilocodeOrganizationId, baseUrl } = options
const cacheKey =
provider === "kilocode-openrouter"
? `${provider}:${kilicodeOrganizationId ?? "personal"}:${baseUrl ?? ""}`
: `${provider}:${baseUrl ?? ""}`
let models = memoryCache.get<ModelRecord>(cacheKey)
if (models) return models
// ...
memoryCache.set(cacheKey, models)If you want to keep the existing exports unchanged, implement the cacheKey locally in getModels and keep the public getModelsFromCache/flushModels untouched for now.
🤖 Prompt for AI Agents
In src/api/providers/fetchers/modelCache.ts around lines 85-87, the memory cache
key currently uses only the provider, allowing models to bleed across
organizations when X-KiloCode-OrganizationId varies; derive a local cacheKey
inside getModels that includes provider plus organization context
(kilocodeOrganizationId, using a fallback like "personal") and optionally
baseUrl for uniqueness, then use memoryCache.get(cacheKey) and
memoryCache.set(cacheKey, models) instead of the provider-only key; keep
existing exported helpers (getModelsFromCache/flushModels) unchanged if you want
to preserve public API.
| const mergedHeaders = { | ||
| ...headers, // Persistent headers from function parameter | ||
| ...(init?.headers || {}), // Request-specific headers (can override persistent ones) | ||
| } | ||
| const response = (await undici.fetch( | ||
| input as undici.RequestInfo, | ||
| { | ||
| ...init, | ||
| headers: mergedHeaders, | ||
| dispatcher: agent, |
There was a problem hiding this comment.
Fix header merging to avoid runtime crash and preserve per-request headers
Spreading headers when it's undefined will throw, and spreading init.headers loses values for Headers and tuple-array forms. Let’s merge via Headers so we support all HeadersInit shapes and keep per-request precedence.
Apply this diff:
- const mergedHeaders = {
- ...headers, // Persistent headers from function parameter
- ...(init?.headers || {}), // Request-specific headers (can override persistent ones)
- }
+ // Build a Headers object to support all HeadersInit shapes and avoid spreading undefined.
+ const mergedHeaders = new undici.Headers(headers ?? {})
+ if (init?.headers) {
+ const reqHeaders = new undici.Headers(init.headers as undici.HeadersInit)
+ reqHeaders.forEach((value, key) => {
+ // Per-request headers override persistent headers
+ mergedHeaders.set(key, value)
+ })
+ }
@@
- headers: mergedHeaders,
+ headers: mergedHeaders,📝 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.
| const mergedHeaders = { | |
| ...headers, // Persistent headers from function parameter | |
| ...(init?.headers || {}), // Request-specific headers (can override persistent ones) | |
| } | |
| const response = (await undici.fetch( | |
| input as undici.RequestInfo, | |
| { | |
| ...init, | |
| headers: mergedHeaders, | |
| dispatcher: agent, | |
| // Build a Headers object to support all HeadersInit shapes and avoid spreading undefined. | |
| const mergedHeaders = new undici.Headers(headers ?? {}) | |
| if (init?.headers) { | |
| const reqHeaders = new undici.Headers(init.headers as undici.HeadersInit) | |
| reqHeaders.forEach((value, key) => { | |
| // Per-request headers override persistent headers | |
| mergedHeaders.set(key, value) | |
| }) | |
| } | |
| const response = (await undici.fetch( | |
| input as undici.RequestInfo, | |
| { | |
| ...init, | |
| headers: mergedHeaders, | |
| dispatcher: agent, |
🤖 Prompt for AI Agents
In src/api/providers/kilocode/fetchWithTimeout.ts around lines 6 to 15, the
current merging uses object spread which crashes if headers is undefined and
loses data when init.headers is a Headers instance or header tuples; replace the
spread merge with a Headers-based merge: create a new Headers from the
persistent headers (if any), then iterate over init.headers (using new
Headers(init.headers) or appropriate iteration) and set each header to ensure
per-request headers override persistent ones, and pass that Headers instance as
the request headers to undici.fetch.
| function getOpenRouter() { | ||
| if ( | ||
| apiConfiguration && | ||
| (apiConfiguration.apiProvider === "openrouter" || apiConfiguration.apiProvider === "kilocode") | ||
| ) { | ||
| return { | ||
| openRouter: { | ||
| sort: apiConfiguration.openRouterProviderSort, | ||
| dataCollection: apiConfiguration.openRouterProviderDataCollection, | ||
| specificProvider: apiConfiguration.openRouterSpecificProvider, | ||
| }, | ||
| } | ||
| } | ||
| return {} | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Clarify return typing and boolean coercion in getOpenRouter for safer telemetry payloads
Like tuning a radio telescope, a tiny bit of type clarity helps us hear the signal without the noise. Two small improvements here:
- Add an explicit return type so this helper’s shape is crystal clear to the compiler and future readers.
- Coerce dataCollection to boolean to avoid tri-state surprises in telemetry.
Apply within this hunk:
- function getOpenRouter() {
- if (
- apiConfiguration &&
- (apiConfiguration.apiProvider === "openrouter" || apiConfiguration.apiProvider === "kilocode")
- ) {
- return {
- openRouter: {
- sort: apiConfiguration.openRouterProviderSort,
- dataCollection: apiConfiguration.openRouterProviderDataCollection,
- specificProvider: apiConfiguration.openRouterSpecificProvider,
- },
- }
- }
- return {}
- }
+ function getOpenRouter(): Partial<{
+ openRouter: {
+ sort?: string
+ dataCollection?: boolean
+ specificProvider?: string
+ }
+ }> {
+ const provider = apiConfiguration?.apiProvider
+ if (provider === "openrouter" || provider === "kilocode") {
+ return {
+ openRouter: {
+ sort: apiConfiguration.openRouterProviderSort,
+ dataCollection: Boolean(apiConfiguration.openRouterProviderDataCollection),
+ specificProvider: apiConfiguration.openRouterSpecificProvider,
+ },
+ }
+ }
+ return {}
+ }And a thought experiment: since this helper is likely useful for webview state as well, consider hoisting it to a private method on the class (e.g., buildOpenRouterProps()) to avoid repeating logic and to make intent discoverable. It’s like putting the important instruments on the same control panel.
Run this quick scan to verify the field names exist and that TelemetryProperties tolerates an optional openRouter object:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Search ProviderSettings fields used in telemetry..."
rg -n -C3 --type=ts '\bopenRouterProvider(Sort|DataCollection|SpecificProvider)\b'
echo
echo "Locate TelemetryProperties definition and usages of 'openRouter' within it..."
rg -n -C3 --type=ts '\b(type|interface)\s+TelemetryProperties\b'
rg -n -C3 --type=ts '\bopenRouter\s*:' || true
echo
echo "Callsites that spread an 'openRouter' blob into telemetry payloads..."
rg -n -C3 --type=ts '\.\.\.\s*getOpenRouter\s*\('Length of output: 8557
I’d like to double-check whether openRouter is actually defined as an optional field in the telemetry schema. Let’s inspect the telemetryPropertiesSchema in packages/types/src/telemetry.ts to see if it includes an openRouter entry:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate telemetryPropertiesSchema definition:"
rg -n 'telemetryPropertiesSchema\s*=\s*z\.object' -C3 packages/types/src/telemetry.ts
echo
echo "Show lines around telemetryPropertiesSchema:"
rg -n 'telemetryPropertiesSchema\s*=' -C10 packages/types/src/telemetry.ts
echo
echo "Search for any 'openRouter' keys in the schema:"
rg -n 'openRouter' -C3 packages/types/src/telemetry.ts || trueLength of output: 1377
Add openRouter to the telemetry schema and refine getOpenRouter signature
Our little expedition into the depths of telemetryPropertiesSchema revealed that there is no openRouter field defined—Zod will strip or reject it, and our carefully booleans-coerced payload will vanish into the void. Let’s chart this coordinate before moving on:
• In packages/types/src/telemetry.ts, extend the schema to accept an optional openRouter object:
export const telemetryPropertiesSchema = z.object({
...appPropertiesSchema.shape,
...taskPropertiesSchema.shape,
...gitPropertiesSchema.shape,
+ openRouter: z
+ .object({
+ sort: z.string().optional(),
+ dataCollection: z.boolean().optional(),
+ specificProvider: z.string().optional(),
+ })
+ .optional(),
})• This ensures TelemetryProperties (inferred from that schema) tolerates the payload produced by our helper.
• In src/core/webview/ClineProvider.ts, keep your explicit return type and Boolean coercion:
function getOpenRouter(): Partial<{
openRouter: {
sort?: string
dataCollection?: boolean
specificProvider?: string
}
}> {
const provider = apiConfiguration?.apiProvider
if (provider === "openrouter" || provider === "kilocode") {
return {
openRouter: {
sort: apiConfiguration.openRouterProviderSort,
dataCollection: Boolean(apiConfiguration.openRouterProviderDataCollection),
specificProvider: apiConfiguration.openRouterSpecificProvider,
},
}
}
return {}
}Optional refactor thought: hoist getOpenRouter() into a private class method (e.g. buildOpenRouterProps()) to make your control panel neater and avoid logic duplication.
🤖 Prompt for AI Agents
In src/core/webview/ClineProvider.ts around lines 2338-2352 and
packages/types/src/telemetry.ts: add an optional openRouter object to the
telemetryPropertiesSchema in packages/types/src/telemetry.ts so
TelemetryProperties accepts { openRouter?: { sort?: string; dataCollection?:
boolean; specificProvider?: string } }, and update ClineProvider.getOpenRouter
to keep its explicit return type (Partial<{ openRouter: { sort?: string;
dataCollection?: boolean; specificProvider?: string } }>) and coerce
apiConfiguration.openRouterProviderDataCollection with Boolean(...); optionally
move the helper into a private class method (e.g., buildOpenRouterProps()) to
avoid duplication.
| if ( | ||
| apiConfiguration?.kilocodeOrganizationId && | ||
| !(response.data.organizations ?? []).some( | ||
| ({ id }) => id === apiConfiguration?.kilocodeOrganizationId, | ||
| ) | ||
| ) { | ||
| provider.upsertProviderProfile(provider.providerSettingsManager.activateProfile.name, { | ||
| ...apiConfiguration, | ||
| kilocodeOrganizationId: undefined, | ||
| }) | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
BUG: upserting with provider.providerSettingsManager.activateProfile.name is wrong target; use active config name.
activateProfile is a function; .name would be the function’s identifier, not the active profile’s name. This will clobber the wrong config (or no-op). Also, await the upsert to avoid races.
- provider.upsertProviderProfile(provider.providerSettingsManager.activateProfile.name, {
- ...apiConfiguration,
- kilocodeOrganizationId: undefined,
- })
+ const currentProfileName = getGlobalState("currentApiConfigName")
+ if (currentProfileName) {
+ await provider.upsertProviderProfile(currentProfileName, {
+ ...apiConfiguration,
+ kilocodeOrganizationId: undefined,
+ })
+}To confirm the bug surface area, search for other places using provider.providerSettingsManager.activateProfile as a property:
🏁 Script executed:
#!/bin/bash
rg -n -C2 --type=ts 'providerSettingsManager\.activateProfile(\.|$)'Length of output: 481
🏁 Script executed:
#!/bin/bash
echo "Searching for upsertProviderProfile definitions/usages"
rg -n 'upsertProviderProfile' -C3 --type=ts
echo -e "\nSearching for ProviderSettingsManager class declaration"
rg -n 'class ProviderSettingsManager' -C5 --type=ts
echo -e "\nSearching for activateProfile method definition"
rg -n 'activateProfile' -C3 --type=ts
echo -e "\nSearching for getActiveProfileName or activeProfileName usages"
rg -n 'getActiveProfileName' -C3 --type=ts
rg -n 'activeProfileName' -C3 --type=tsLength of output: 19775
🏁 Script executed:
#!/bin/bash
sed -n '2120,2200p' src/core/webview/webviewMessageHandler.tsLength of output: 2763
Use the actual profile name and await the upsert call
The code is currently peeling off .activateProfile.name—which is just the string "activateProfile"—instead of the real, user-chosen profile name. That mis-targets (or no-ops) the upsert, and because it isn’t awaited we also open ourselves to race conditions. Let’s grab currentApiConfigName from state and call:
• File: src/core/webview/webviewMessageHandler.ts (around lines 2149–2155)
• Change the state destructure:
- const { apiConfiguration } = await provider.getState()
+ const { apiConfiguration, currentApiConfigName } = await provider.getState()• Replace the mis-targeted, fire-and-forget upsert:
- if (
- apiConfiguration?.kilocodeOrganizationId &&
- !(response.data.organizations ?? []).some(({ id }) => id === apiConfiguration?.kilocodeOrganizationId)
- ) {
- provider.upsertProviderProfile(provider.providerSettingsManager.activateProfile.name, {
- ...apiConfiguration,
- kilocodeOrganizationId: undefined,
- })
- }
+ if (
+ apiConfiguration?.kilocodeOrganizationId &&
+ !(response.data.organizations ?? []).some(({ id }) => id === apiConfiguration?.kilocodeOrganizationId)
+ ) {
+ if (currentApiConfigName) {
+ await provider.upsertProviderProfile(currentApiConfigName, {
+ ...apiConfiguration,
+ kilocodeOrganizationId: undefined,
+ })
+ }
+ }This ensures we’re updating the user’s active profile and waiting for the operation to complete.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/core/webview/webviewMessageHandler.ts around lines 2149–2155, the code
uses provider.providerSettingsManager.activateProfile.name (the literal
"activateProfile") and fires an unawaited upsert, so the wrong profile is
updated and a race is possible; change the state destructure to pull
currentApiConfigName (or equivalent actual active profile name) and call await
provider.upsertProviderProfile(currentApiConfigName, { ...apiConfiguration,
kilocodeOrganizationId: undefined }) so the correct user-selected profile is
updated and the upsert is awaited.
| "qwenCode": { | ||
| "oauthLoadFailed": "Failed to load OAuth credentials. Please authenticate first: {{error}}" | ||
| }, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
qwenCode OAuth error string added — looks consistent; let’s sanity-check cross-locale shape
String matches the pattern used for geminiCli (same {{error}} placeholder, punctuation, and nesting under errors). Good data-only change.
To avoid runtime i18n fallbacks or undefined paths, verify all locales define errors.qwenCode.oauthLoadFailed (not top-level):
🏁 Script executed:
#!/bin/bash
set -euo pipefail
missing=0
while IFS= read -r -d '' f; do
if ! jq -e 'has("errors") and .errors|has("qwenCode") and .errors.qwenCode|has("oauthLoadFailed")' "$f" >/dev/null; then
echo "Missing errors.qwenCode.oauthLoadFailed in: $f"
missing=$((missing+1))
continue
fi
if ! jq -er '.errors.qwenCode.oauthLoadFailed' "$f" | grep -q '{{\s*error\s*}}'; then
echo "Placeholder {{error}} missing in: $f"
missing=$((missing+1))
fi
done < <(fd -a 'common.json' src/i18n/locales -0)
if [ "$missing" -gt 0 ]; then
echo "Found $missing issue(s)."
exit 1
else
echo "All locales include errors.qwenCode.oauthLoadFailed with {{error}} placeholder."
fiLength of output: 182
Let’s sidestep the /dev/fd conundrum by using a simple find-driven loop instead of process substitution:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
missing=0
# Iterate over every common.json under src/i18n/locales
for f in $(find src/i18n/locales -type f -name 'common.json'); do
# Check for errors.qwenCode.oauthLoadFailed key
if ! jq -e 'has("errors") and .errors|has("qwenCode") and .errors.qwenCode|has("oauthLoadFailed")' "$f" >/dev/null; then
echo "Missing errors.qwenCode.oauthLoadFailed in: $f"
missing=$((missing + 1))
continue
fi
# Ensure the placeholder is present
if ! jq -er '.errors.qwenCode.oauthLoadFailed' "$f" | grep -Eq '{{[[:space:]]*error[[:space:]]*}}'; then
echo "Placeholder {{error}} missing in: $f"
missing=$((missing + 1))
fi
done
if [ "$missing" -gt 0 ]; then
echo "Found $missing issue(s)."
exit 1
else
echo "All locales include errors.qwenCode.oauthLoadFailed with {{error}} placeholder."
fiLength of output: 10634
Add missing errors.qwenCode.oauthLoadFailed to all locale files
Our sweep of src/i18n/locales/*/common.json uncovered 22 locales lacking the new key (or mis-scoped it under a non-object). To prevent runtime fallbacks, please insert the following under the top-level "errors" object in every common.json:
"errors": {
"qwenCode": {
"oauthLoadFailed": "Failed to load OAuth credentials. Please authenticate first: {{error}}"
}
}• Affects all files matching src/i18n/locales/*/common.json
• Ensure errors is an object (not a boolean) before nesting qwenCode
• Verify the {{error}} placeholder is present
🤖 Prompt for AI Agents
In src/i18n/locales/en/common.json around lines 108-110, the new translation key
is currently placed at top-level instead of under the top-level "errors" object;
move or insert the qwenCode.oauthLoadFailed entry under "errors" so the file
contains an "errors" object (not a boolean) with a nested "qwenCode" object that
includes the "oauthLoadFailed" string and the {{error}} placeholder; repeat this
change for every file matching src/i18n/locales/*/common.json, ensuring "errors"
is an object before adding "qwenCode" and that the placeholder text is present
exactly as {{error}}.
| "description": "%extension.description%", | ||
| "publisher": "kilocode", | ||
| "version": "4.81.0", | ||
| "version": "4.82.1", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Version bump looks fine; verify release artifacts reference 4.82.1
Trust, but verify, as any good experimentalist would. Ensure the version is reflected in changelog/changeset and publish workflows.
Run this quick check to confirm version coherency:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Checking for 4.82.1 references…"
rg -n -C2 -F "4.82.1" CHANGELOG.md .changeset || true
echo
echo "Confirming a changeset exists for this bump…"
fd .changeset -t f | xargs -I{} sh -c 'echo "--- {} ---"; cat "{}"' | rg -n -C2 -F "4.82.1" || true
echo
echo "Verifying marketplace workflow uses the intended packaging script…"
rg -n -C2 -F "vsix:production" .github/workflows || trueLength of output: 1574
Missing changeset for v4.82.1 – please add one
I see the changelog has a “## [v4.82.1]” entry, and the marketplace-publish workflow correctly picks up vsix:production and reads the version from src/package.json. However, I couldn’t find any .changeset/* file referencing 4.82.1, which you’ll need to drive the automated release steps.
• No .changeset/*.md contains “4.82.1”
• CHANGELOG.md entry present ✅
• .github/workflows/marketplace-publish.yml invokes pnpm vsix:production and reads the bump ✅
Please add a .changeset file for this bump so the release pipeline can process the version correctly.
🤖 Prompt for AI Agents
In src/package.json around line 6 the version is "4.82.1" but there is no
corresponding .changeset entry for that bump; create a new .changeset/*.md file
(unique name) that specifies the package(s) and the release type
(patch/minor/major) matching the CHANGELOG entry and includes a short summary
referencing v4.82.1 so the release tooling can pick it up, then add and commit
the new .changeset file alongside the package.json change.
… tools (#7649) Fixes issue where isWriteProtected was incorrectly passed as the partial flag to task.ask(), causing "Current ask promise was ignored (#1)" error. Both tools now use the askApproval wrapper function which correctly handles the parameter ordering, passing undefined for progressStatus and isWriteProtected as the fourth parameter. Fixes #7648 Co-authored-by: Roo Code <roomote@roocode.com>
Content
Replaces hidden class with sr-only to maintain screen reader access when container is narrow.
Adds aria-describedby attribute and live region to provide better context and status announcements for assistive technologies.
Marks decorative status indicator as aria-hidden to prevent redundant screen reader content.
Previously, if the screenWidth was less than 235, it would sit the visibility to hidden:, which would hide it to screen readers. We already have sr-only from Tailwind in the app.
Reported by the community
Screenshots
No difference. Icons should still hide themselves.
How to Test
Get in Touch
@GodelEscherSpock on Discord. @adamhill on Github
Fixes: Kilo-Org#1935