Refactored Settings UI #65
Conversation
|
Claude finished @olesho's task —— View job PR Review: Refactored Settings UII've completed a comprehensive review of this Settings UI refactor. This is an impressive modularization effort that transforms a monolithic 3788-line dialog into a well-structured, maintainable architecture. Review Tasks:
✅ Architectural ImprovementsExcellent modularization: The refactor breaks down a massive monolithic file into a clean, organized structure: Strong inheritance pattern: Proper separation of concerns: Clean separation between UI rendering, data persistence, validation, and business logic. ✅ Code QualityTypeScript usage: Excellent type safety with comprehensive interfaces in Error handling: Good defensive programming in Consistent patterns: All providers follow the same architectural pattern, making the codebase predictable and maintainable. Clean abstractions: The storage utilities in ✅ Security AssessmentAPI key handling: Proper security practices:
Input validation: Good validation utilities for API keys and endpoints, though basic. No obvious vulnerabilities: No signs of XSS risks, SQL injection, or credential leakage.
|
WalkthroughAdds a complete AI chat settings UI: new build entries plus many TypeScript modules for settings UI, provider-specific settings, advanced features, utilities (storage/validation/styles), i18n, and type definitions. Several components introduce UI controls, async model/key flows, and periodic status updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant SettingsDialog
participant ProviderSettings
participant Storage
participant LLMClient
participant OAuthModule
Note over SettingsDialog,ProviderSettings: Open settings dialog
User->>SettingsDialog: open()
SettingsDialog->>Storage: load saved configs & keys
SettingsDialog->>ProviderSettings: render(container)
Note over ProviderSettings: Model fetch / OAuth flows
User->>ProviderSettings: click "Fetch Models"
ProviderSettings->>LLMClient: fetchModels(endpoint/apiKey)
LLMClient-->>ProviderSettings: models
ProviderSettings->>Storage: cache models + timestamp
ProviderSettings->>ProviderSettings: updateModelSelectors()
User->>ProviderSettings: choose mini/nano
ProviderSettings->>Storage: persist selection
alt OpenRouter OAuth flow
User->>ProviderSettings: click "Login with OpenRouter"
ProviderSettings->>OAuthModule: startPKCE()
OAuthModule-->>ProviderSettings: onSuccess(tokens)
ProviderSettings->>Storage: store auth / refresh UI
ProviderSettings->>SettingsDialog: maybe close on success
end
User->>SettingsDialog: click Save
SettingsDialog->>ProviderSettings: save()
ProviderSettings->>Storage: persist final config
SettingsDialog->>User: show save status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–90 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 23
🧹 Nitpick comments (59)
front_end/panels/ai_chat/BUILD.gn (2)
229-248: Keep _ai_chat_sources in sync with module sources.The same note applies here; after renaming i18n strings file, ensure this list mirrors sources. Consider factoring a shared array to avoid maintenance overhead.
43-62: Remove conditional guidance about ModuleUIStrings rename; reframe consolidation as preventative refactoring.The i18n file remains named "ui/settings/i18n-strings.ts" (no rename to ModuleUIStrings.ts is in progress). Both settings file lists are currently identical with no drift. Consider consolidating the repeated list into a shared variable to prevent future maintenance issues.
front_end/panels/ai_chat/ui/settings/components/SettingsHeader.ts (2)
10-12: Fix JSDoc hyphen formatting.Remove hyphens before @param descriptions to satisfy jsdoc rule.
- * @param container - Parent element to append the header to - * @param onClose - Callback function when close button is clicked + * @param container Parent element to append the header to + * @param onClose Callback invoked when the close button is clicked
29-31: Localize the close button label.Add a UIStrings key (e.g.,
closeSettingsAriaLabel) and use it here.- closeButton.setAttribute('aria-label', 'Close settings'); + closeButton.setAttribute('aria-label', i18nString(UIStrings.closeSettingsAriaLabel));front_end/panels/ai_chat/ui/settings/utils/styles.ts (1)
509-513: Guard against duplicate style injection.If dialog opens multiple times, this can append multiple identical <style> tags. Add an id and check before injecting.
-export function applySettingsStyles(dialogElement: HTMLElement): void { - const styleElement = document.createElement('style'); - styleElement.textContent = getSettingsStyles(); - dialogElement.appendChild(styleElement); -} +export function applySettingsStyles(dialogElement: HTMLElement): void { + const STYLE_ID = 'ai-chat-settings-styles'; + if (!dialogElement.querySelector(`#${STYLE_ID}`)) { + const styleElement = document.createElement('style'); + styleElement.id = STYLE_ID; + styleElement.textContent = getSettingsStyles(); + dialogElement.appendChild(styleElement); + } +}front_end/panels/ai_chat/ui/settings/components/AdvancedToggle.ts (4)
5-7: Import UIStrings for localization.Use shared ModuleUIStrings to localize label/hint.
-import { ADVANCED_SETTINGS_ENABLED_KEY } from '../constants.js'; -import { getStorageBoolean, setStorageBoolean } from '../utils/storage.js'; +import { ADVANCED_SETTINGS_ENABLED_KEY } from '../constants.js'; +import { getStorageBoolean, setStorageBoolean } from '../utils/storage.js'; +import { UIStrings, i18nString } from '../ModuleUIStrings.js';
20-23: Fix JSDoc hyphens.Remove hyphens before @param descriptions.
- * @param container - Parent element to append the toggle to - * @param onChange - Callback function when toggle state changes + * @param container Parent element to append the toggle to + * @param onChange Callback invoked when toggle state changes
43-53: Localize label/hint and add ARIA wiring.
- Localize textContent via UIStrings (add keys: advancedSettingsLabel, advancedSettingsHint).
- Use a unique id and connect ARIA attributes; expose expanded state.
- advancedToggleCheckbox.id = 'advanced-settings-toggle'; + const toggleId = 'advanced-settings-toggle'; + advancedToggleCheckbox.id = toggleId; advancedToggleCheckbox.className = 'advanced-settings-checkbox'; advancedToggleCheckbox.checked = getStorageBoolean(ADVANCED_SETTINGS_ENABLED_KEY, false); + advancedToggleCheckbox.setAttribute('aria-expanded', String(advancedToggleCheckbox.checked)); advancedToggleContainer.appendChild(advancedToggleCheckbox); const advancedToggleLabel = document.createElement('label'); - advancedToggleLabel.htmlFor = 'advanced-settings-toggle'; + advancedToggleLabel.htmlFor = toggleId; advancedToggleLabel.className = 'advanced-settings-label'; - advancedToggleLabel.textContent = '⚙️ Advanced Settings'; + advancedToggleLabel.textContent = i18nString(UIStrings.advancedSettingsLabel); advancedToggleContainer.appendChild(advancedToggleLabel); const advancedToggleHint = document.createElement('div'); advancedToggleHint.className = 'settings-hint'; - advancedToggleHint.textContent = - 'Show advanced configuration options (Browsing History, Vector DB, Tracing, Evaluation)'; + advancedToggleHint.textContent = i18nString(UIStrings.advancedSettingsHint); advancedToggleSection.appendChild(advancedToggleHint);
55-60: Keep ARIA in sync with state.Update aria-expanded on change.
advancedToggleCheckbox.addEventListener('change', () => { const isEnabled = advancedToggleCheckbox.checked; + advancedToggleCheckbox.setAttribute('aria-expanded', String(isEnabled)); setStorageBoolean(ADVANCED_SETTINGS_ENABLED_KEY, isEnabled); onChange(isEnabled); });front_end/panels/ai_chat/ui/settings/advanced/BrowsingHistorySettings.ts (2)
41-46: Announce status to assistive tech and use UIStrings.- this.statusMessage = document.createElement('div'); + this.statusMessage = document.createElement('div'); this.statusMessage.className = 'settings-status history-status'; this.statusMessage.style.display = 'none'; - this.statusMessage.textContent = i18nString('historyCleared'); + this.statusMessage.setAttribute('aria-live', 'polite'); + this.statusMessage.textContent = i18nString(UIStrings.historyCleared);
48-75: Add confirm dialog and disable button during async.Avoid accidental data loss and prevent double clicks. Localize button label.
- clearHistoryButton.textContent = i18nString('clearHistoryButton'); + clearHistoryButton.textContent = i18nString(UIStrings.clearHistoryButton); @@ - clearHistoryButton.addEventListener('click', async () => { + clearHistoryButton.addEventListener('click', async () => { + // Confirm destructive action + // Add UIStrings.clearHistoryConfirm: 'Clear all stored browsing history? This cannot be undone.' + if (!window.confirm(i18nString(UIStrings.clearHistoryConfirm))) { + return; + } + clearHistoryButton.disabled = true; try { @@ - if (this.statusMessage) { + if (this.statusMessage) { this.statusMessage.style.display = 'block'; @@ - } + } } catch (error) { logger.error('Error clearing browsing history:', error); } + clearHistoryButton.disabled = false; });front_end/panels/ai_chat/ui/settings/advanced/EvaluationSettings.ts (2)
123-177: Localize client ID labels and hints; avoid hard-coded placeholders.Add UIStrings for clientIdLabel, clientIdHint, and clientIdAutoGenerated.
- clientIdLabel.textContent = 'Client ID'; + clientIdLabel.textContent = i18nString(UIStrings.clientIdLabel); @@ - clientIdHint.textContent = 'Unique identifier for this DevTools instance'; + clientIdHint.textContent = i18nString(UIStrings.clientIdHint); @@ - clientIdInput.value = currentEvaluationConfig.clientId || 'Auto-generated on first connection'; + clientIdInput.value = currentEvaluationConfig.clientId || i18nString(UIStrings.clientIdAutoGenerated); @@ - evaluationEndpointLabel.textContent = i18nString('evaluationEndpoint'); + evaluationEndpointLabel.textContent = i18nString(UIStrings.evaluationEndpoint); @@ - evaluationEndpointHint.textContent = i18nString('evaluationEndpointHint'); + evaluationEndpointHint.textContent = i18nString(UIStrings.evaluationEndpointHint); @@ - evaluationSecretKeyLabel.textContent = i18nString('evaluationSecretKey'); + evaluationSecretKeyLabel.textContent = i18nString(UIStrings.evaluationSecretKey); @@ - evaluationSecretKeyHint.textContent = i18nString('evaluationSecretKeyHint'); + evaluationSecretKeyHint.textContent = i18nString(UIStrings.evaluationSecretKeyHint);
36-39: Note on innerHTML lint warning.
this.container.innerHTML = ''is safe here (constant empty string). If the repo enforces the rule strictly, replace with a loop removing children.- this.container.innerHTML = ''; + while (this.container.firstChild) { + this.container.removeChild(this.container.firstChild); + }front_end/panels/ai_chat/ui/settings/advanced/TracingSettings.ts (3)
26-28: Avoid innerHTML; use safer DOM APIs.Replace with replaceChildren() to eliminate XSS lint and avoid layout thrash.
- this.container.innerHTML = ''; + this.container.replaceChildren();As per static analysis hints.
142-145: Localize user-visible strings.Replace hard-coded English with i18n keys to pass l10n lint and for translation.
- testTracingStatus.textContent = 'Testing connection...'; + testTracingStatus.textContent = i18nString(UIStrings.testingTracingConnection); @@ - throw new Error('All fields are required for testing'); + throw new Error(i18nString(UIStrings.tracingAllFieldsRequired)); @@ - testTracingStatus.textContent = '✓ Connection successful'; + testTracingStatus.textContent = i18nString(UIStrings.tracingConnectionSuccess); @@ - throw new Error(`HTTP ${response.status}: ${errorText}`); + throw new Error(i18nString(UIStrings.tracingHttpError, { PH1: String(response.status), PH2: errorText })); @@ - testTracingStatus.textContent = `✗ ${error instanceof Error ? error.message : 'Connection failed'}`; + testTracingStatus.textContent = error instanceof Error + ? error.message + : i18nString(UIStrings.tracingConnectionFailed);Note: add the missing UIStrings in i18n-strings.ts.
Also applies to: 151-153, 178-185, 187-189
199-220: Validate on save; avoid silent no-op and stale config.If enabled but incomplete, either disable tracing or surface error. Minimal option: disable.
save(): void { if (!this.tracingEnabledCheckbox || !this.endpointInput || !this.publicKeyInput || !this.secretKeyInput) { return; } if (this.tracingEnabledCheckbox.checked) { const endpoint = this.endpointInput.value.trim(); const publicKey = this.publicKeyInput.value.trim(); const secretKey = this.secretKeyInput.value.trim(); - if (endpoint && publicKey && secretKey) { + if (endpoint && publicKey && secretKey) { setTracingConfig({ provider: 'langfuse', endpoint, publicKey, secretKey }); - } + } else { + // Incomplete config; disable to avoid half-enabled state + setTracingConfig({ provider: 'disabled' }); + } } else { setTracingConfig({ provider: 'disabled' }); } }Alternatively, bubble a validation error to the footer and keep previous config.
front_end/panels/ai_chat/ui/settings/advanced/VectorDBSettings.ts (2)
35-37: Avoid innerHTML; use replaceChildren().Safer and passes XSS lint.
- this.container.innerHTML = ''; + this.container.replaceChildren();As per static analysis hints.
236-242: Localize literals and avoidanyin catch.Use i18n for messages and prefer
unknownwith narrowing.- vectorDBTestStatus.textContent = 'Please enter an endpoint URL'; + vectorDBTestStatus.textContent = i18nString(UIStrings.vectorDBEnterEndpoint); @@ - vectorDBTestStatus.textContent = i18nString('testingVectorDBConnection'); + vectorDBTestStatus.textContent = i18nString(UIStrings.testingVectorDBConnection); @@ - if (testResult.success) { - vectorDBTestStatus.textContent = i18nString('vectorDBConnectionSuccess'); + if (testResult.success) { + vectorDBTestStatus.textContent = i18nString(UIStrings.vectorDBConnectionSuccess); vectorDBTestStatus.style.color = 'var(--color-accent-green)'; } else { - vectorDBTestStatus.textContent = `${i18nString('vectorDBConnectionFailed')}: ${testResult.error}`; + vectorDBTestStatus.textContent = `${i18nString(UIStrings.vectorDBConnectionFailed)}: ${testResult.error}`; vectorDBTestStatus.style.color = 'var(--color-accent-red)'; } - } catch (error: any) { - vectorDBTestStatus.textContent = `${i18nString('vectorDBConnectionFailed')}: ${error.message}`; + } catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error); + vectorDBTestStatus.textContent = `${i18nString(UIStrings.vectorDBConnectionFailed)}: ${msg}`; vectorDBTestStatus.style.color = 'var(--color-accent-red)'; } finally {As per static analysis hints.
Also applies to: 246-249, 263-273
front_end/panels/ai_chat/ui/settings/components/SettingsFooter.ts (3)
18-21: Fix JSDoc formatting (no hyphen before descriptions).Conforms to jsdoc/require-hyphen-before-param-description rule.
- * @param container - Parent element to append the footer to - * @param onCancel - Callback function when cancel button is clicked - * @param onSave - Callback function when save button is clicked + * @param container Parent element to append the footer to + * @param onCancel Callback function when cancel button is clicked + * @param onSave Callback function when save button is clicked @@ - * @param statusElement - The status message element - * @param message - Message to display - * @param type - Type of message (info, success, error) - * @param duration - How long to show the message (ms), 0 = don't auto-hide + * @param statusElement The status message element + * @param message Message to display + * @param type Type of message (info, success, error) + * @param duration How long to show the message (ms), 0 = don't auto-hideAs per static analysis hints.
Also applies to: 66-70
41-53: Localize button labels.Replace literals with i18n strings.
- cancelButton.textContent = 'Cancel'; + cancelButton.textContent = i18nString(UIStrings.cancel); @@ - saveButton.textContent = 'Save'; + saveButton.textContent = i18nString(UIStrings.save);Note: import { i18nString, UIStrings } from '../i18n-strings.js'.
74-99: Remove redundant type annotation and cleanup overlapping timers.Simplify parameter type and clear any previous hide timer to avoid premature hide.
-export function showFooterStatus( +export function showFooterStatus( statusElement: HTMLElement, message: string, type: 'info' | 'success' | 'error' = 'info', - duration: number = 3000, + duration = 3000, ): void { statusElement.textContent = message; statusElement.style.display = 'block'; @@ - if (duration > 0) { - setTimeout(() => { - statusElement.style.display = 'none'; - }, duration); - } + if (duration > 0) { + // @ts-expect-error attach timer id on element + if ((statusElement as any).__footerTimerId) { + clearTimeout((statusElement as any).__footerTimerId); + } + // @ts-expect-error store timer id on element + (statusElement as any).__footerTimerId = setTimeout(() => { + statusElement.style.display = 'none'; + // @ts-expect-error cleanup + (statusElement as any).__footerTimerId = undefined; + }, duration); + }As per static analysis hints.
front_end/panels/ai_chat/ui/settings/types.ts (1)
36-43: Unify option shape with ModelOption for consistency.If consumers can handle it, prefer ModelOption[] to keep type parity across UI.
- options: Array<{value: string, label: string}>; + options: ModelOption[];If not feasible now, add a TODO to migrate later.
front_end/panels/ai_chat/ui/settings/providers/GroqSettings.ts (3)
5-13: Fix import order and groupings to satisfy lint.Place utility/type imports before provider class import, and separate groups with a blank line.
-import { BaseProviderSettings } from './BaseProviderSettings.js'; -import { createModelSelector, refreshModelSelectOptions } from '../components/ModelSelectorFactory.js'; -import { i18nString } from '../i18n-strings.js'; -import { getValidModelForProvider } from '../utils/validation.js'; -import { getStorageItem, setStorageItem } from '../utils/storage.js'; -import { GROQ_API_KEY_STORAGE_KEY, MINI_MODEL_STORAGE_KEY, NANO_MODEL_STORAGE_KEY } from '../constants.js'; -import type { UpdateModelOptionsFunction, GetModelOptionsFunction, AddCustomModelOptionFunction, RemoveCustomModelOptionFunction, ModelOption } from '../types.js'; -import { LLMClient } from '../../../LLM/LLMClient.js'; -import { createLogger } from '../../../core/Logger.js'; +import { createModelSelector, refreshModelSelectOptions } from '../components/ModelSelectorFactory.js'; +import { i18nString, UIStrings } from '../i18n-strings.js'; +import { getValidModelForProvider } from '../utils/validation.js'; +import { getStorageItem, setStorageItem } from '../utils/storage.js'; +import { GROQ_API_KEY_STORAGE_KEY, MINI_MODEL_STORAGE_KEY, NANO_MODEL_STORAGE_KEY } from '../constants.js'; +import type { UpdateModelOptionsFunction, GetModelOptionsFunction, AddCustomModelOptionFunction, RemoveCustomModelOptionFunction, ModelOption } from '../types.js'; +import { LLMClient } from '../../../LLM/LLMClient.js'; +import { createLogger } from '../../../core/Logger.js'; + +import { BaseProviderSettings } from './BaseProviderSettings.js';
51-58: Use UIStrings with i18nString; avoid raw string keys and hardcoded text.Switch to i18nString(UIStrings.key) and externalize “Model Size Selection” and placeholders.
- groqApiKeyLabel.textContent = i18nString('groqApiKeyLabel'); - groqApiKeyHint.textContent = i18nString('groqApiKeyHint'); + groqApiKeyLabel.textContent = i18nString(UIStrings.groqApiKeyLabel); + groqApiKeyHint.textContent = i18nString(UIStrings.groqApiKeyHint); - this.fetchModelsButton.textContent = i18nString('fetchGroqModelsButton'); + this.fetchModelsButton.textContent = i18nString(UIStrings.fetchGroqModelsButton); - this.fetchModelsStatus.textContent = i18nString('fetchingModels'); + this.fetchModelsStatus.textContent = i18nString(UIStrings.fetchingModels); - refreshModelSelectOptions(this.miniModelSelector as any, allGroqModels, miniModel, i18nString('defaultMiniOption')); + refreshModelSelectOptions(this.miniModelSelector as unknown as ModelSelectorElement, allGroqModels, miniModel, i18nString(UIStrings.defaultMiniOption)); - refreshModelSelectOptions(this.nanoModelSelector as any, allGroqModels, nanoModel, i18nString('defaultNanoOption')); + refreshModelSelectOptions(this.nanoModelSelector as unknown as ModelSelectorElement, allGroqModels, nanoModel, i18nString(UIStrings.defaultNanoOption)); - this.fetchModelsStatus.textContent = i18nString('fetchedModels', {PH1: actualModelCount}); + this.fetchModelsStatus.textContent = i18nString(UIStrings.fetchedModels, {PH1: actualModelCount}); - groqModelSectionTitle.textContent = 'Model Size Selection'; + // TODO: add UIStrings.modelSizeSelectionTitle in i18n-strings.ts + groqModelSectionTitle.textContent = i18nString(UIStrings.modelSizeSelectionTitle); - i18nString('miniModelLabel'), i18nString('miniModelDescription'), i18nString('defaultMiniOption'), + i18nString(UIStrings.miniModelLabel), i18nString(UIStrings.miniModelDescription), i18nString(UIStrings.defaultMiniOption), - i18nString('nanoModelLabel'), i18nString('nanoModelDescription'), i18nString('defaultNanoOption'), + i18nString(UIStrings.nanoModelLabel), i18nString(UIStrings.nanoModelDescription), i18nString(UIStrings.defaultNanoOption),Please confirm UIStrings include all referenced keys or share the list so I can generate a patch to add the missing ones.
Also applies to: 75-76, 96-101, 127-131, 133-136, 197-204, 211-218
92-99: Wrap single-line if statements with braces.Satisfies curly rule and improves readability.
- if (!this.fetchModelsButton || !this.fetchModelsStatus || !this.apiKeyInput) return; + if (!this.fetchModelsButton || !this.fetchModelsStatus || !this.apiKeyInput) { + return; + } - if (!this.container) return; + if (!this.container) { + return; + }Also applies to: 161-166
front_end/panels/ai_chat/ui/settings/components/ModelSelectorFactory.ts (2)
11-19: Fix JSDoc: remove hyphens before @param descriptions.Unblocks jsdoc lint.
- * @param container - Parent element to append the selector to - * @param labelText - Label text for the selector - * @param description - Description text below the label - * @param selectorType - Semantic identifier for the selector - * @param modelOptions - Available model options - * @param selectedModel - Currently selected model value - * @param defaultOptionText - Text for the default option - * @param onFocus - Optional callback for when the selector is opened/focused + * @param container Parent element to append the selector to + * @param labelText Label text for the selector + * @param description Description text below the label + * @param selectorType Semantic identifier for the selector + * @param modelOptions Available model options + * @param selectedModel Currently selected model value + * @param defaultOptionText Text for the default option + * @param onFocus Optional callback for when the selector is opened/focused @@ - * @param select - The model selector element - * @param models - New model options - * @param currentValue - Current selected value - * @param defaultLabel - Label for the default option + * @param select The model selector element + * @param models New model options + * @param currentValue Current selected value + * @param defaultLabel Label for the default optionAlso applies to: 74-82
94-101: Minor style: drop unnecessary parens around single arrow params.Keeps style consistent with lint.
- if (previousValue && opts.some((o) => o.value === previousValue)) { + if (previousValue && opts.some(o => o.value === previousValue)) { @@ - } else if (currentValue && opts.some((o) => o.value === currentValue)) { + } else if (currentValue && opts.some(o => o.value === currentValue)) { @@ - if (previousValue && Array.from(nativeSelect.options).some((opt) => opt.value === previousValue)) { + if (previousValue && Array.from(nativeSelect.options).some(opt => opt.value === previousValue)) { - } else if (currentValue && Array.from(nativeSelect.options).some((opt) => opt.value === currentValue)) { + } else if (currentValue && Array.from(nativeSelect.options).some(opt => opt.value === currentValue)) {Also applies to: 116-120
front_end/panels/ai_chat/ui/settings/providers/LiteLLMSettings.ts (8)
5-14: Fix import order; remove unused ModelOption; add UIStrings/type for selector.Aligns with lint and prepares to eliminate any-casts.
-import { BaseProviderSettings } from './BaseProviderSettings.js'; -import { createModelSelector, refreshModelSelectOptions } from '../components/ModelSelectorFactory.js'; -import { i18nString } from '../i18n-strings.js'; +import { createModelSelector, refreshModelSelectOptions } from '../components/ModelSelectorFactory.js'; +import { i18nString, UIStrings } from '../i18n-strings.js'; import { getValidModelForProvider } from '../utils/validation.js'; import { getStorageItem, setStorageItem } from '../utils/storage.js'; import { LITELLM_ENDPOINT_KEY, LITELLM_API_KEY_STORAGE_KEY, MINI_MODEL_STORAGE_KEY, NANO_MODEL_STORAGE_KEY } from '../constants.js'; -import type { UpdateModelOptionsFunction, GetModelOptionsFunction, AddCustomModelOptionFunction, RemoveCustomModelOptionFunction, FetchLiteLLMModelsFunction, ModelOption } from '../types.js'; +import type { UpdateModelOptionsFunction, GetModelOptionsFunction, AddCustomModelOptionFunction, RemoveCustomModelOptionFunction, FetchLiteLLMModelsFunction, ModelSelectorElement } from '../types.js'; import { LLMClient } from '../../../LLM/LLMClient.js'; import { createLogger } from '../../../core/Logger.js'; + +import { BaseProviderSettings } from './BaseProviderSettings.js';
30-33: Drop trivial type annotation.Let TS infer boolean.
-private testPassed: boolean = false; +private testPassed = false;
47-57: Use UIStrings with i18nString; externalize hardcoded strings.Replace string keys and literal messages with i18nString(UIStrings.*).
- litellmEndpointLabel.textContent = i18nString('litellmEndpointLabel'); - litellmEndpointHint.textContent = i18nString('litellmEndpointHint'); + litellmEndpointLabel.textContent = i18nString(UIStrings.litellmEndpointLabel); + litellmEndpointHint.textContent = i18nString(UIStrings.litellmEndpointHint); @@ - this.fetchModelsButton.textContent = i18nString('fetchModelsButton'); + this.fetchModelsButton.textContent = i18nString(UIStrings.fetchModelsButton); @@ - this.fetchModelsStatus.textContent = i18nString('fetchingModels'); + this.fetchModelsStatus.textContent = i18nString(UIStrings.fetchingModels); @@ - refreshModelSelectOptions(this.miniModelSelector as any, allLiteLLMModels, miniModel, i18nString('defaultMiniOption')); + refreshModelSelectOptions(this.miniModelSelector as unknown as ModelSelectorElement, allLiteLLMModels, miniModel, i18nString(UIStrings.defaultMiniOption)); @@ - refreshModelSelectOptions(this.nanoModelSelector as any, allLiteLLMModels, nanoModel, i18nString('defaultNanoOption')); + refreshModelSelectOptions(this.nanoModelSelector as unknown as ModelSelectorElement, allLiteLLMModels, nanoModel, i18nString(UIStrings.defaultNanoOption)); @@ - this.fetchModelsStatus.textContent = i18nString('wildcardModelsOnly'); + this.fetchModelsStatus.textContent = i18nString(UIStrings.wildcardModelsOnly); @@ - this.fetchModelsStatus.textContent = i18nString('wildcardAndCustomModels'); + this.fetchModelsStatus.textContent = i18nString(UIStrings.wildcardAndCustomModels); @@ - this.fetchModelsStatus.textContent = i18nString('wildcardAndOtherModels', {PH1: actualModelCount}); + this.fetchModelsStatus.textContent = i18nString(UIStrings.wildcardAndOtherModels, {PH1: actualModelCount}); @@ - this.fetchModelsStatus.textContent = i18nString('fetchedModels', {PH1: actualModelCount}); + this.fetchModelsStatus.textContent = i18nString(UIStrings.fetchedModels, {PH1: actualModelCount}); @@ - customModelsLabel.textContent = i18nString('customModelsLabel'); - customModelsHint.textContent = i18nString('customModelsHint'); + customModelsLabel.textContent = i18nString(UIStrings.customModelsLabel); + customModelsHint.textContent = i18nString(UIStrings.customModelsHint); @@ - addModelButton.textContent = i18nString('addButton'); + addModelButton.textContent = i18nString(UIStrings.addButton); @@ - testButton.setAttribute('aria-label', i18nString('testButton')); + testButton.setAttribute('aria-label', i18nString(UIStrings.testButton)); @@ - removeButton.setAttribute('aria-label', i18nString('removeButton')); + removeButton.setAttribute('aria-label', i18nString(UIStrings.removeButton)); @@ - throw new Error(i18nString('endpointRequired')); + throw new Error(i18nString(UIStrings.endpointRequired)); @@ - this.modelTestStatus.textContent = 'Please enter a model name'; + this.modelTestStatus.textContent = i18nString(UIStrings.enterModelNamePrompt); @@ - this.modelTestStatus.textContent = 'Testing model...'; + this.modelTestStatus.textContent = i18nString(UIStrings.testingModel); @@ - this.modelTestStatus.textContent = `Test passed: ${result.message}`; + this.modelTestStatus.textContent = i18nString(UIStrings.testPassedMessage, {PH1: result.message}); @@ - this.modelTestStatus.textContent = `Test failed: ${result.message}`; + this.modelTestStatus.textContent = i18nString(UIStrings.testFailedMessage, {PH1: result.message}); @@ - i18nString('miniModelLabel'), i18nString('miniModelDescription'), i18nString('defaultMiniOption'), + i18nString(UIStrings.miniModelLabel), i18nString(UIStrings.miniModelDescription), i18nString(UIStrings.defaultMiniOption), @@ - i18nString('nanoModelLabel'), i18nString('nanoModelDescription'), i18nString('defaultNanoOption'), + i18nString(UIStrings.nanoModelLabel), i18nString(UIStrings.nanoModelDescription), i18nString(UIStrings.defaultNanoOption),Please confirm UIStrings includes the referenced keys or share the file so I can add missing entries.
Also applies to: 76-85, 107-113, 123-131, 147-151, 154-172, 197-205, 228-235, 334-336, 349-355, 419-426, 429-433, 439-444, 466-473, 517-526, 531-540, 545-557
94-101: Add braces to single-line if/return and annotate function return types.Unblocks curly and explicit return type rules.
- const updateFetchButtonState = () => { + const updateFetchButtonState = (): void => { @@ - if (!this.fetchModelsButton || !this.fetchModelsStatus || !this.endpointInput || !this.apiKeyInput) return; + if (!this.fetchModelsButton || !this.fetchModelsStatus || !this.endpointInput || !this.apiKeyInput) { + return; + } @@ - if (!this.customModelInput) return; + if (!this.customModelInput) { + return; + } @@ - try { + try { @@ - } finally { + } finally { @@ - private async testModelConnection(modelName: string): Promise<boolean> { - if (!this.modelTestStatus) return false; + private async testModelConnection(modelName: string): Promise<boolean> { + if (!this.modelTestStatus) { + return false; + } @@ - if (!endpoint) { - throw new Error(i18nString('endpointRequired')); - } + if (!endpoint) { + throw new Error(i18nString(UIStrings.endpointRequired)); + } @@ - updateModelSelectors(): void { - if (!this.container) return; + updateModelSelectors(): void { + if (!this.container) { + return; + }Also applies to: 119-122, 236-243, 369-376, 398-402, 418-426, 434-441, 466-473
497-515: Annotate async focus handler return type.Satisfies explicit return type rule.
- const onLiteLLMSelectorFocus = async () => { + const onLiteLLMSelectorFocus = async (): Promise<void> => {
303-317: Avoid non-null assertion on DOM element.Use a local const after null-check.
- // Clear existing list - this.customModelsList.innerHTML = ''; + // Clear existing list + const list = this.customModelsList; + if (!list) return; + list.innerHTML = ''; @@ - this.customModelsList!.appendChild(modelRow); + list.appendChild(modelRow);
338-346: Replace innerHTML SVG injection with createElementNS to silence unsafe-html rules.Static strings are relatively safe, but using DOM APIs avoids the rule entirely.
- const checkIcon = document.createElement('span'); - checkIcon.className = 'check-icon'; - checkIcon.innerHTML = ` - <svg width="16" height="16" viewBox="0 0 16 16" fill="none" xmlns="http://www.w3.org/2000/svg"> - <path d="M12 5L6.5 10.5L4 8" stroke="currentColor" stroke-width="1.5" stroke-linecap="round" stroke-linejoin="round"/> - <circle cx="8" cy="8" r="6.25" stroke="currentColor" stroke-width="1.5"/> - </svg> - `; + const checkIcon = document.createElement('span'); + checkIcon.className = 'check-icon'; + const svg = document.createElementNS('http://www.w3.org/2000/svg', 'svg'); + svg.setAttribute('width', '16'); svg.setAttribute('height', '16'); svg.setAttribute('viewBox', '0 0 16 16'); + const p = document.createElementNS('http://www.w3.org/2000/svg', 'path'); + p.setAttribute('d','M12 5L6.5 10.5L4 8'); p.setAttribute('stroke','currentColor'); p.setAttribute('stroke-width','1.5'); p.setAttribute('stroke-linecap','round'); p.setAttribute('stroke-linejoin','round'); + const c = document.createElementNS('http://www.w3.org/2000/svg', 'circle'); + c.setAttribute('cx','8'); c.setAttribute('cy','8'); c.setAttribute('r','6.25'); c.setAttribute('stroke','currentColor'); c.setAttribute('stroke-width','1.5'); + svg.appendChild(p); svg.appendChild(c); checkIcon.appendChild(svg); @@ - const trashIcon = document.createElement('span'); - trashIcon.className = 'trash-icon'; - trashIcon.innerHTML = ` - <svg width="16" height="16" viewBox="0 0 16 16" fill="none" xmlns="http://www.w3.org/2000/svg"> - <path d="M6 2.5V2C6 1.44772 6.44772 1 7 1H9C9.55228 1 10 1.44772 10 2V2.5M2 2.5H14M12.5 2.5V13C12.5 13.5523 12.0523 14 11.5 14H4.5C3.94772 14 3.5 13.5523 3.5 13V2.5M5.5 5.5V11M8 5.5V11M10.5 5.5V11" - stroke="currentColor" stroke-width="1.2" stroke-linecap="round" stroke-linejoin="round"/> - </svg> - `; + const trashIcon = document.createElement('span'); + trashIcon.className = 'trash-icon'; + // Build minimal trash icon or reuse existing icon component here...Also applies to: 357-365
444-456: Simplify no-else-after-return in testModelConnection.Conforms to lint and reduces nesting.
- if (result.success) { - this.modelTestStatus.textContent = `Test passed: ${result.message}`; - this.modelTestStatus.style.backgroundColor = 'var(--color-accent-green-background)'; - this.modelTestStatus.style.color = 'var(--color-accent-green)'; - this.testPassed = true; - return true; - } else { - this.modelTestStatus.textContent = `Test failed: ${result.message}`; - this.modelTestStatus.style.backgroundColor = 'var(--color-accent-red-background)'; - this.modelTestStatus.style.color = 'var(--color-accent-red)'; - this.testPassed = false; - return false; - } + if (result.success) { + this.modelTestStatus.textContent = i18nString(UIStrings.testPassedMessage, {PH1: result.message}); + this.modelTestStatus.style.backgroundColor = 'var(--color-accent-green-background)'; + this.modelTestStatus.style.color = 'var(--color-accent-green)'; + this.testPassed = true; + return true; + } + this.modelTestStatus.textContent = i18nString(UIStrings.testFailedMessage, {PH1: result.message}); + this.modelTestStatus.style.backgroundColor = 'var(--color-accent-red-background)'; + this.modelTestStatus.style.color = 'var(--color-accent-red)'; + this.testPassed = false; + return false;front_end/panels/ai_chat/ui/settings/utils/validation.ts (1)
59-69: Optional: accept schemeless endpoints.Many users type host:port; consider auto-prefixing http:// for valid URL() parsing.
-export function validateEndpoint(endpoint: string): boolean { +export function validateEndpoint(endpoint: string): boolean { if (!endpoint.trim()) { return false; } try { - new URL(endpoint); + const candidate = /^https?:\/\//i.test(endpoint) ? endpoint : `http://${endpoint}`; + new URL(candidate); return true; } catch { return false; } }front_end/panels/ai_chat/ui/settings/providers/OpenAISettings.ts (3)
5-12: Fix import order and add UIStrings import.-import { BaseProviderSettings } from './BaseProviderSettings.js'; -import { createModelSelector } from '../components/ModelSelectorFactory.js'; -import { i18nString } from '../i18n-strings.js'; +import { createModelSelector } from '../components/ModelSelectorFactory.js'; +import { i18nString, UIStrings } from '../i18n-strings.js'; import { getValidModelForProvider } from '../utils/validation.js'; import { getStorageItem, setStorageItem } from '../utils/storage.js'; import { OPENAI_API_KEY_STORAGE_KEY, MINI_MODEL_STORAGE_KEY, NANO_MODEL_STORAGE_KEY } from '../constants.js'; import type { GetModelOptionsFunction, AddCustomModelOptionFunction, RemoveCustomModelOptionFunction } from '../types.js'; + +import { BaseProviderSettings } from './BaseProviderSettings.js';
41-50: Use UIStrings and externalize title.- apiKeyLabel.textContent = i18nString('apiKeyLabel'); - apiKeyHint.textContent = i18nString('apiKeyHint'); + apiKeyLabel.textContent = i18nString(UIStrings.apiKeyLabel); + apiKeyHint.textContent = i18nString(UIStrings.apiKeyHint); @@ - this.apiKeyInput.placeholder = 'Enter your OpenAI API key'; + // TODO: add UIStrings.openAiApiKeyPlaceholder + this.apiKeyInput.placeholder = i18nString(UIStrings.openAiApiKeyPlaceholder); @@ - openaiModelSectionTitle.textContent = 'Model Size Selection'; + // TODO: add UIStrings.modelSizeSelectionTitle + openaiModelSectionTitle.textContent = i18nString(UIStrings.modelSizeSelectionTitle); @@ - i18nString('miniModelLabel'), i18nString('miniModelDescription'), i18nString('defaultMiniOption'), + i18nString(UIStrings.miniModelLabel), i18nString(UIStrings.miniModelDescription), i18nString(UIStrings.defaultMiniOption), @@ - i18nString('nanoModelLabel'), i18nString('nanoModelDescription'), i18nString('defaultNanoOption'), + i18nString(UIStrings.nanoModelLabel), i18nString(UIStrings.nanoModelDescription), i18nString(UIStrings.defaultNanoOption),Also applies to: 55-57, 91-95, 99-108, 111-120
68-76: Wrap single-line if with braces.- if (!this.container) return; + if (!this.container) { + return; + }front_end/panels/ai_chat/ui/settings/utils/storage.ts (3)
8-10: Remove trivially inferrable type on defaulted parameter to satisfy ESLint.-export function getStorageItem(key: string, defaultValue: string = ''): string { +export function getStorageItem(key: string, defaultValue = ''): string { return localStorage.getItem(key) || defaultValue; }
26-32: Same here: drop redundant boolean annotation on default param.-export function getStorageBoolean(key: string, defaultValue: boolean = false): boolean { +export function getStorageBoolean(key: string, defaultValue = false): boolean { const value = localStorage.getItem(key); if (value === null) { return defaultValue; } return value === 'true'; }
15-21: Harden writes to handle quota/security exceptions; avoid breaking UI.export function setStorageItem(key: string, value: string): void { - if (value.trim()) { - localStorage.setItem(key, value); - } else { - localStorage.removeItem(key); - } + try { + if (value.trim()) { + localStorage.setItem(key, value); + } else { + localStorage.removeItem(key); + } + } catch { + // noop: storage unavailable or quota exceeded + } } ... export function setStorageJSON<T>(key: string, value: T): void { - localStorage.setItem(key, JSON.stringify(value)); + try { + localStorage.setItem(key, JSON.stringify(value)); + } catch { + // noop: storage unavailable or quota exceeded + } }Also applies to: 59-61
front_end/panels/ai_chat/ui/settings/providers/OpenRouterSettings.ts (8)
5-13: Fix import order per lint (place BaseProviderSettings after sibling imports).-import { BaseProviderSettings } from './BaseProviderSettings.js'; -import { createModelSelector } from '../components/ModelSelectorFactory.js'; +import { createModelSelector } from '../components/ModelSelectorFactory.js'; import { i18nString } from '../i18n-strings.js'; import { getValidModelForProvider } from '../utils/validation.js'; import { getStorageItem, setStorageItem } from '../utils/storage.js'; import { OPENROUTER_API_KEY_STORAGE_KEY, MINI_MODEL_STORAGE_KEY, NANO_MODEL_STORAGE_KEY, OPENROUTER_MODELS_CACHE_DURATION_MS } from '../constants.js'; import type { UpdateModelOptionsFunction, GetModelOptionsFunction, AddCustomModelOptionFunction, RemoveCustomModelOptionFunction, ModelOption } from '../types.js'; import { LLMClient } from '../../../LLM/LLMClient.js'; import { createLogger } from '../../../core/Logger.js'; +import { BaseProviderSettings } from './BaseProviderSettings.js';
68-71: Replace innerHTML clearing with safe child removal to satisfy XSS/static checks.- this.container.innerHTML = ''; + while (this.container.firstChild) { + this.container.removeChild(this.container.firstChild); + }
118-164: Prevent duplicating injected styles across renders; add id and guard.- const oauthStyles = document.createElement('style'); - oauthStyles.textContent = ` + if (!document.getElementById('openrouter-oauth-styles')) { + const oauthStyles = document.createElement('style'); + oauthStyles.id = 'openrouter-oauth-styles'; + oauthStyles.textContent = ` .settings-divider { text-align: center; margin: 15px 0; color: var(--color-text-secondary); font-size: 12px; font-weight: bold; } .oauth-button-container { margin-bottom: 10px; } .oauth-button { background: linear-gradient(135deg, #667eea 0%, #764ba2 100%); color: white; border: none; padding: 10px 20px; border-radius: 6px; cursor: pointer; font-weight: 500; transition: all 0.3s ease; width: 100%; margin-bottom: 8px; } .oauth-button:hover { transform: translateY(-1px); box-shadow: 0 4px 12px rgba(102, 126, 234, 0.3); } .oauth-button:disabled { opacity: 0.6; cursor: not-allowed; transform: none; box-shadow: none; } .oauth-button.disconnect { background: linear-gradient(135deg, #f093fb 0%, #f5576c 100%); } .oauth-status { font-size: 12px; margin-top: 5px; padding: 5px 8px; border-radius: 4px; background: var(--color-background-highlight); } - `; - document.head.appendChild(oauthStyles); + `; + document.head.appendChild(oauthStyles); + }
191-199: Comply with curly rule; avoid single-line early-returns and if-statements without braces.- this.oauthButton.addEventListener('click', async () => { - if (!this.oauthButton) return; + this.oauthButton.addEventListener('click', async () => { + if (!this.oauthButton) { + return; + } ... - } finally { - this.oauthButton.disabled = false; - if (!await OAuth.isOAuthAuthenticated()) { + } finally { + this.oauthButton.disabled = false; + if (!await OAuth.isOAuthAuthenticated()) { this.oauthButton.textContent = 'Connect with OpenRouter'; if (this.oauthStatus) { this.oauthStatus.style.display = 'none'; } } } }); ... - this.apiKeyInput.addEventListener('input', async () => { - if (!this.apiKeyInput) return; + this.apiKeyInput.addEventListener('input', async () => { + if (!this.apiKeyInput) { + return; + } ... - if (this.fetchModelsButton) { - this.fetchModelsButton.disabled = !this.apiKeyInput.value.trim(); - } + if (this.fetchModelsButton) { + this.fetchModelsButton.disabled = !this.apiKeyInput.value.trim(); + } ... - this.fetchModelsButton.addEventListener('click', async () => { - if (!this.fetchModelsButton || !this.fetchModelsStatus || !this.apiKeyInput) return; + this.fetchModelsButton.addEventListener('click', async () => { + if (!this.fetchModelsButton || !this.fetchModelsStatus || !this.apiKeyInput) { + return; + }Also applies to: 225-232, 298-304, 312-315, 335-339, 456-458
245-249: Remove any by narrowing the queried element type; keep null-safe.- const chatPanel = document.querySelector('ai-chat-panel') as any; - if (chatPanel && typeof chatPanel.refreshCredentials === 'function') { + const chatPanel = document.querySelector('ai-chat-panel') as (HTMLElement & { refreshCredentials?: () => void }) | null; + if (chatPanel?.refreshCredentials) { chatPanel.refreshCredentials(); }Apply similarly at Lines 283-287.
Also applies to: 283-287
347-359: Consider OAuth path for model fetch/refresh; current logic requires an API key even when OAuth is connected.If OpenRouterOAuth exposes a token or LLMClient supports OAuth tokens, add a branch:
- If OAuth.isOAuthAuthenticated(): use OAuth token to fetch models.
- Else: use API key.
Please confirm the supported method on LLMClient or OpenRouter provider for OAuth-based fetches and I can propose a concrete patch.
Also applies to: 434-441
80-86: i18n: pass UIStrings members instead of raw string keys to satisfy l10n rule.- openrouterApiKeyLabel.textContent = i18nString('openrouterApiKeyLabel'); + openrouterApiKeyLabel.textContent = i18nString(UIStrings.openrouterApiKeyLabel); ... - openrouterApiKeyHint.textContent = i18nString('openrouterApiKeyHint'); + openrouterApiKeyHint.textContent = i18nString(UIStrings.openrouterApiKeyHint); ... - this.fetchModelsButton.textContent = i18nString('fetchOpenRouterModelsButton'); + this.fetchModelsButton.textContent = i18nString(UIStrings.fetchOpenRouterModelsButton); - this.fetchModelsStatus.textContent = i18nString('fetchingModels'); + this.fetchModelsStatus.textContent = i18nString(UIStrings.fetchingModels); ... - this.fetchModelsStatus.textContent = i18nString('fetchedModels', {PH1: actualModelCount}); + this.fetchModelsStatus.textContent = i18nString(UIStrings.fetchedModels, {PH1: actualModelCount}); ... - i18nString('miniModelLabel'), - i18nString('miniModelDescription'), + i18nString(UIStrings.miniModelLabel), + i18nString(UIStrings.miniModelDescription), ... - i18nString('defaultMiniOption'), + i18nString(UIStrings.defaultMiniOption), ... - i18nString('nanoModelLabel'), - i18nString('nanoModelDescription'), + i18nString(UIStrings.nanoModelLabel), + i18nString(UIStrings.nanoModelDescription), ... - i18nString('defaultNanoOption'), + i18nString(UIStrings.defaultNanoOption),Add
import { UIStrings } from '../i18n-strings.js';at the top. Apply similarly for any other i18nString usage in this file. As per coding guidelines.Also applies to: 325-327, 339-343, 369-372, 489-506
361-364: Centralize cache/auth storage keys; avoid magic strings.Define in constants.ts:
+export const OPENROUTER_MODELS_CACHE_KEY = 'openrouter_models_cache'; +export const OPENROUTER_MODELS_CACHE_TS_KEY = 'openrouter_models_cache_timestamp'; +export const OPENROUTER_AUTH_METHOD_KEY = 'openrouter_auth_method';Then use:
- localStorage.setItem('openrouter_models_cache_timestamp', Date.now().toString()); + localStorage.setItem(OPENROUTER_MODELS_CACHE_TS_KEY, Date.now().toString()); ... - localStorage.setItem('openrouter_models_cache', JSON.stringify(modelOptions)); - localStorage.setItem('openrouter_models_cache_timestamp', Date.now().toString()); + localStorage.setItem(OPENROUTER_MODELS_CACHE_KEY, JSON.stringify(modelOptions)); + localStorage.setItem(OPENROUTER_MODELS_CACHE_TS_KEY, Date.now().toString());Also applies to: 443-449
front_end/panels/ai_chat/ui/settings/advanced/MCPSettings.ts (7)
56-66: Avoid innerHTML clearing; use safe child removal.- this.container.innerHTML = ''; + while (this.container.firstChild) { + this.container.removeChild(this.container.firstChild); + } this.container.className = 'settings-section mcp-section';
333-345: Add explicit return type to the arrow handler; keeps lint happy.- const updateBudgetControls = () => { + const updateBudgetControls: () => void = () => { const maxTools = Math.max(1, Math.min(100, parseInt(mcpMaxToolsInput.value, 10) || 20)); const maxMcp = Math.max(1, Math.min(50, parseInt(mcpMaxMcpInput.value, 10) || 8)); setMCPConfig({ maxToolsPerTurn: maxTools, maxMcpPerTurn: maxMcp, }); this.onSettingsSaved(); };
347-368: Conform to curly rule in formatMCPError; minor cleanup.- private formatMCPError(error: string, errorType?: string): {message: string, hint?: string} { - if (!errorType) return {message: error}; + private formatMCPError(error: string, errorType?: string): {message: string, hint?: string} { + if (!errorType) { + return {message: error}; + } switch (errorType) { case 'connection': return {message: `Connection failed: ${error}`, hint: 'Check if the MCP server is running and the endpoint URL is correct.'}; ... default: - return {message: error}; + return {message: error}; } }
438-449: no-lonely-if: collapse the else { if (...) } into else-if for readability.- } else { - if (serverAuthError) { + } else if (serverAuthError) { statusDot.style.backgroundColor = '#ef4444'; statusBadge.style.backgroundColor = '#fee2e2'; statusBadge.style.color = '#991b1b'; statusBadge.textContent = 'Auth Required'; - } else { + } else { statusDot.style.backgroundColor = '#9ca3af'; statusBadge.style.backgroundColor = '#f3f4f6'; statusBadge.style.color = '#6b7280'; statusBadge.textContent = 'Disconnected'; - } - } + }
491-504: Remove non-null assertions; guard once and reuse a local.- this.mcpStatusDetails!.appendChild(row); + const detailsEl = this.mcpStatusDetails; + if (detailsEl) { + detailsEl.appendChild(row); + } ... - this.mcpStatusDetails!.appendChild(errorDetails); + if (detailsEl) { + detailsEl.appendChild(errorDetails); + }
375-383: Minor perf: compute auth errors map once; avoids per-server scans.- const appendServerRow = (server: typeof status.servers[number], isConnected: boolean) => { + const authErrorsById = new Map(getStoredAuthErrors().map(e => [e.serverId, e])); + const appendServerRow = (server: typeof status.servers[number], isConnected: boolean) => { ... - const authErrors = getStoredAuthErrors(); - const serverAuthError = authErrors.find(error => error.serverId === server.id); + const serverAuthError = authErrorsById.get(server.id);Also applies to: 515-543, 575-592
190-231: i18n: ensure i18nString receives UIStrings members, not raw strings; update calls accordingly.Replace:
- i18nString('mcpConnectionsHeader') → i18nString(UIStrings.mcpConnectionsHeader)
- mcp...Hint/Labels/Button texts similarly.
Addimport { UIStrings } from '../i18n-strings.js';. As per coding guidelines.Also applies to: 255-274, 296-330
front_end/panels/ai_chat/ui/settings/constants.ts (2)
28-29: Complement OpenRouter cache/auth keys so other modules avoid magic strings.export const OPENROUTER_MODELS_CACHE_DURATION_MS = 60 * 60 * 1000; // 60 minutes +export const OPENROUTER_MODELS_CACHE_KEY = 'openrouter_models_cache'; +export const OPENROUTER_MODELS_CACHE_TS_KEY = 'openrouter_models_cache_timestamp'; +export const OPENROUTER_AUTH_METHOD_KEY = 'openrouter_auth_method';
19-24: Heads-up: API keys in localStorage persist and are exposed to any script running in the same origin; ensure threat model accepts this.If untrusted content can execute, prefer a more protected store (e.g., chrome.storage with extension-scoped isolation) or limit surface by scoping the origin. Document this in SECURITY.md.
Also applies to: 33-39
| import { i18nString } from '../i18n-strings.js'; | ||
| import * as Common from '../../../../../core/common/common.js'; | ||
|
|
There was a problem hiding this comment.
Import UIStrings and fix import order.
- Place Common import first per import/order.
- Import UIStrings and use it with i18nString.
-import { i18nString } from '../i18n-strings.js';
-import * as Common from '../../../../../core/common/common.js';
+import * as Common from '../../../../../core/common/common.js';
+import { UIStrings, i18nString } from '../ModuleUIStrings.js';Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 ESLint
[error] 6-6: ../../../../../core/common/common.js import should occur before import of ../i18n-strings.js
(import/order)
🤖 Prompt for AI Agents
In front_end/panels/ai_chat/ui/settings/advanced/BrowsingHistorySettings.ts
around lines 5 to 7, the import order should place the Common module before
local modules and you need to import UIStrings and use it with i18nString;
reorder imports so Common is first, add an import for UIStrings from the file
that defines the UI strings (e.g., import { UIStrings } from
'./BrowsingHistorySettings.i18n.js' or the correct path), then replace direct
string literals passed to i18nString with references into UIStrings (i.e.,
i18nString(UIStrings.someKey)) to comply with localization usage and
import/order rules.
front_end/panels/ai_chat/ui/settings/advanced/BrowsingHistorySettings.ts
Show resolved
Hide resolved
front_end/panels/ai_chat/ui/settings/advanced/EvaluationSettings.ts
Outdated
Show resolved
Hide resolved
| // Function to update connection status | ||
| const updateConnectionStatus = () => { | ||
| const isConnected = isEvaluationConnected(); | ||
|
|
||
| logger.debug('Updating connection status', { isConnected }); | ||
|
|
||
| if (isConnected) { | ||
| connectionStatusDot.style.backgroundColor = 'var(--color-accent-green)'; | ||
| connectionStatusText.textContent = 'Connected to evaluation server'; | ||
| connectionStatusText.style.color = 'var(--color-accent-green)'; | ||
| } else { | ||
| connectionStatusDot.style.backgroundColor = 'var(--color-text-disabled)'; | ||
| connectionStatusText.textContent = 'Not connected'; | ||
| connectionStatusText.style.color = 'var(--color-text-disabled)'; | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add explicit return type and localize connection status texts.
Also externalize the two status strings in ModuleUIStrings (e.g., evaluationConnectedStatus, evaluationDisconnectedStatus).
- const updateConnectionStatus = () => {
+ const updateConnectionStatus = (): void => {
const isConnected = isEvaluationConnected();
@@
- connectionStatusText.textContent = 'Connected to evaluation server';
+ connectionStatusText.textContent = i18nString(UIStrings.evaluationConnectedStatus);
@@
- connectionStatusText.textContent = 'Not connected';
+ connectionStatusText.textContent = i18nString(UIStrings.evaluationDisconnectedStatus);Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 ESLint
[error] 95-95: Missing return type on function.
(@typescript-eslint/explicit-function-return-type)
🤖 Prompt for AI Agents
In front_end/panels/ai_chat/ui/settings/advanced/EvaluationSettings.ts around
lines 94 to 109, add an explicit return type to updateConnectionStatus (e.g.,
updateConnectionStatus(): void) and ensure local typing for isConnected
(boolean); replace the hardcoded status strings 'Connected to evaluation server'
and 'Not connected' with localized values from ModuleUIStrings (e.g.,
ModuleUIStrings.evaluationConnectedStatus and
ModuleUIStrings.evaluationDisconnectedStatus), import ModuleUIStrings at the top
if not already imported, and use those localized strings when setting
connectionStatusText.textContent while keeping the same color and dot logic.
| // Update model selectors | ||
| this.updateModelSelectors(); | ||
|
|
||
| // Hide status after a delay | ||
| setTimeout(() => { | ||
| if (this.modelTestStatus) { | ||
| this.modelTestStatus.style.display = 'none'; | ||
| } | ||
| }, 3000); | ||
| } |
There was a problem hiding this comment.
Eliminate duplicate refresh + rebuild after fetch.
Avoid flicker by choosing one strategy (refresh or rebuild).
- // Update LiteLLM model selections
- this.updateModelSelectors();
+ // If you refreshed existing selectors above, do not rebuild here to prevent flicker.Also applies to: 174-176
| // Dynamically imported OAuth module | ||
| private OpenRouterOAuth: any = null; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Avoid any; type the OAuth surface explicitly and narrow the return type.
- private OpenRouterOAuth: any = null;
+ type OpenRouterOAuthAPI = {
+ isOAuthAuthenticated(): Promise<boolean>;
+ startAuthFlow(): Promise<void>;
+ revokeToken(): Promise<void>;
+ switchToManualApiKey(): void;
+ };
+ private OpenRouterOAuth: OpenRouterOAuthAPI | null = null;
...
- private async getOpenRouterOAuth(): Promise<any> {
+ private async getOpenRouterOAuth(): Promise<OpenRouterOAuthAPI> {
if (!this.OpenRouterOAuth) {
const module = await import('../../../auth/OpenRouterOAuth.js');
- this.OpenRouterOAuth = module.OpenRouterOAuth;
+ this.OpenRouterOAuth = module.OpenRouterOAuth as OpenRouterOAuthAPI;
}
return this.OpenRouterOAuth;
}Also applies to: 60-66
🧰 Tools
🪛 ESLint
[error] 43-43: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🤖 Prompt for AI Agents
In front_end/panels/ai_chat/ui/settings/providers/OpenRouterSettings.ts around
lines 42-44 and 60-66, the dynamically imported OAuth module and related
variables are typed as any; replace these with a concrete interface that
describes the OAuth surface (methods, properties, and return types you use,
e.g., init, authorize, getToken, revokeToken, config shape). Change the private
OpenRouterOAuth property to that interface or null, and change any dynamic
import return types to Promise<ThatOAuthInterface | null> (or the module
namespace type if you import a named export), updating all usages to match the
narrowed types so callers use the defined methods/props instead of any.
| const updateOAuthButton = async () => { | ||
| const OAuth = await this.getOpenRouterOAuth(); | ||
| if (await OAuth.isOAuthAuthenticated()) { | ||
| if (this.oauthButton) { | ||
| this.oauthButton.textContent = 'Disconnect OpenRouter'; | ||
| this.oauthButton.classList.add('disconnect'); | ||
| } | ||
| if (this.oauthStatus) { | ||
| this.oauthStatus.textContent = '✓ Connected via OpenRouter account'; | ||
| this.oauthStatus.style.color = 'var(--color-accent-green)'; | ||
| this.oauthStatus.style.display = 'block'; | ||
| } | ||
| } else { | ||
| if (this.oauthButton) { | ||
| this.oauthButton.textContent = 'Connect with OpenRouter'; | ||
| this.oauthButton.classList.remove('disconnect'); | ||
| } | ||
| if (this.oauthStatus) { | ||
| this.oauthStatus.style.display = 'none'; | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add explicit return type and enable fetch button when OAuth is connected (no API key).
- const updateOAuthButton = async () => {
+ const updateOAuthButton: () => Promise<void> = async () => {
const OAuth = await this.getOpenRouterOAuth();
if (await OAuth.isOAuthAuthenticated()) {
if (this.oauthButton) {
this.oauthButton.textContent = 'Disconnect OpenRouter';
this.oauthButton.classList.add('disconnect');
}
+ if (this.fetchModelsButton) {
+ this.fetchModelsButton.disabled = false;
+ }
if (this.oauthStatus) {
this.oauthStatus.textContent = '✓ Connected via OpenRouter account';
this.oauthStatus.style.color = 'var(--color-accent-green)';
this.oauthStatus.style.display = 'block';
}
} else {
if (this.oauthButton) {
this.oauthButton.textContent = 'Connect with OpenRouter';
this.oauthButton.classList.remove('disconnect');
}
+ if (this.fetchModelsButton && this.apiKeyInput) {
+ this.fetchModelsButton.disabled = !this.apiKeyInput.value.trim();
+ }
if (this.oauthStatus) {
this.oauthStatus.style.display = 'none';
}
}
};🧰 Tools
🪛 ESLint
[error] 166-166: Missing return type on function.
(@typescript-eslint/explicit-function-return-type)
| dataset: { | ||
| modelType?: string; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Satisfy member-delimiter-style: use commas in inline type.
- dataset: {
- modelType?: string;
- };
+ dataset: {
+ modelType?: string,
+ };As per static analysis hints.
📝 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.
| dataset: { | |
| modelType?: string; | |
| }; | |
| dataset: { | |
| modelType?: string, | |
| }; |
🧰 Tools
🪛 ESLint
[error] 40-40: Expected a comma.
(@stylistic/member-delimiter-style)
🤖 Prompt for AI Agents
In front_end/panels/ai_chat/ui/settings/types.ts around lines 39 to 41, the
inline type for dataset uses a semicolon as a member delimiter which violates
member-delimiter-style; replace the semicolon with a comma so the property is
comma-delimited (i.e., change "modelType?: string;" to "modelType?: string,") to
satisfy the lint rule.
| export function isVectorDBEnabled(): boolean { | ||
| return getStorageBoolean('ai_chat_vector_db_enabled', false); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Avoid hardcoded storage key; use the exported constant to prevent drift.
+import { VECTOR_DB_ENABLED_KEY } from '../constants.js';
...
export function isVectorDBEnabled(): boolean {
- return getStorageBoolean('ai_chat_vector_db_enabled', false);
+ return getStorageBoolean(VECTOR_DB_ENABLED_KEY, false);
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In front_end/panels/ai_chat/ui/settings/utils/storage.ts around lines 73 to 75,
the function isVectorDBEnabled() uses a hardcoded storage key string; replace
that string with the module-exported constant for the ai_chat vector DB key
(import it from its defining module if not already imported) and pass the same
default false value so the call becomes
getStorageBoolean(EXPORTED_CONSTANT_NAME, false). Ensure the import is
added/organized and tests/usage updated to reference the same constant to
prevent future drift.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (13)
front_end/panels/ai_chat/ui/settings/advanced/BrowsingHistorySettings.ts (1)
5-6: Fix import order to satisfy lint.
Common.Consoleis a core module and must be imported before local modules such as../i18n-strings.js; the current ordering still fails theimport/orderrule. Reorder the imports as below.-import { i18nString, UIStrings } from '../i18n-strings.js'; -import * as Common from '../../../../../core/common/common.js'; +import * as Common from '../../../../../core/common/common.js'; +import { i18nString, UIStrings } from '../i18n-strings.js';As per coding guidelines.
front_end/panels/ai_chat/ui/settings/advanced/TracingSettings.ts (1)
133-195: Remove non-null assertions and add a request timeout.The handlers still rely on
!(this.tracingEnabledCheckbox!,this.endpointInput!, etc.), which violates lint and risks runtime errors if render wiring ever changes. At the same time, the fetch lacks anAbortController, so the UI spins forever on a hung endpoint—this was called out earlier and remains unresolved.Capture the DOM nodes once, guard for null, and wire the event handlers without assertions while wrapping the fetch with a timeout, e.g.:
- this.tracingEnabledCheckbox.addEventListener('change', () => { - tracingConfigContainer.style.display = this.tracingEnabledCheckbox!.checked ? 'block' : 'none'; - }); + const enabledCheckbox = this.tracingEnabledCheckbox; + if (!enabledCheckbox) { + return; + } + enabledCheckbox.addEventListener('change', () => { + tracingConfigContainer.style.display = enabledCheckbox.checked ? 'block' : 'none'; + }); @@ - const endpoint = this.endpointInput!.value.trim(); - const publicKey = this.publicKeyInput!.value.trim(); - const secretKey = this.secretKeyInput!.value.trim(); + const endpoint = this.endpointInput?.value.trim() ?? ''; + const publicKey = this.publicKeyInput?.value.trim() ?? ''; + const secretKey = this.secretKeyInput?.value.trim() ?? ''; @@ - const response = await fetch(`${endpoint}/api/public/ingestion`, { + const controller = new AbortController(); + const timeoutId = window.setTimeout(() => controller.abort(), 10000); + const response = await fetch(`${endpoint}/api/public/ingestion`, { method: 'POST', @@ - body: JSON.stringify(testPayload) + body: JSON.stringify(testPayload), + signal: controller.signal, }); + clearTimeout(timeoutId);Based on coding guidelines.
front_end/panels/ai_chat/ui/settings/advanced/VectorDBSettings.ts (4)
5-13: Reorder imports to clear lint errors.
../constants.jsmust be loaded before the local i18n helper. The current ordering still triggersimport/order.-import { i18nString, UIStrings } from '../i18n-strings.js'; -import { +import { VECTOR_DB_ENABLED_KEY, @@ MILVUS_OPENAI_KEY } from '../constants.js'; +import { i18nString, UIStrings } from '../i18n-strings.js';As per coding guidelines.
132-159: Stop seeding secrets with baked-in defaults.Prefilling the Milvus password/token field with
'Milvus'(and defaulting the OpenAI key to non-empty values during test) reintroduces the insecure behavior previously flagged: it leaks a pretend password and encourages shipping with a real secret baked into the UI.- this.milvusPasswordInput.value = localStorage.getItem(MILVUS_PASSWORD_KEY) || 'Milvus'; + this.milvusPasswordInput.value = localStorage.getItem(MILVUS_PASSWORD_KEY) || ''; @@ - const vectorClient = new VectorDBClient({ - endpoint, - username: this.vectorDBApiKeyInput.value || 'root', - password: this.milvusPasswordInput.value || 'Milvus', - collection: this.vectorDBCollectionInput.value || 'bookmarks', - openaiApiKey: this.milvusOpenAIInput.value || undefined - }); + const vectorClient = new VectorDBClient({ + endpoint, + username: this.vectorDBApiKeyInput.value || 'root', + password: this.milvusPasswordInput.value || '', + collection: this.vectorDBCollectionInput.value || 'bookmarks', + openaiApiKey: this.milvusOpenAIInput.value || '' + });Ensure the client treats empty password/token as “no auth” and validate before connect.
205-218: Do not persist secrets in localStorage.
MILVUS_PASSWORD_KEYandMILVUS_OPENAI_KEYare still written directly tolocalStorageon every keystroke. LocalStorage is readable by any script on the origin and has no expiry, so this is a security regression. Please keep these secrets ephemeral (in-memory) or move them to a protected store (e.g., extension background page,chrome.storage.session, or encrypted storage) and only persist via an explicit secure channel.As per coding guidelines.
199-225: Guard DOM references and declare return types.
this.vectorDBEnabledCheckbox!.checkedand friends still use!, trippingno-non-null-assertion. Additionally,saveVectorDBSettingslacks an explicit return type. Capture the elements, guard for null, and annotate the helper:- this.vectorDBEnabledCheckbox.addEventListener('change', () => { - vectorDBConfigContainer.style.display = this.vectorDBEnabledCheckbox!.checked ? 'block' : 'none'; - localStorage.setItem(VECTOR_DB_ENABLED_KEY, this.vectorDBEnabledCheckbox!.checked.toString()); - }); + const enabledCheckbox = this.vectorDBEnabledCheckbox; + this.vectorDBEnabledCheckbox?.addEventListener('change', () => { + const checked = !!enabledCheckbox?.checked; + vectorDBConfigContainer.style.display = checked ? 'block' : 'none'; + localStorage.setItem(VECTOR_DB_ENABLED_KEY, String(checked)); + }); @@ - const saveVectorDBSettings = () => { - if (!this.vectorDBEnabledCheckbox || !this.vectorDBEndpointInput || !this.vectorDBApiKeyInput || - !this.milvusPasswordInput || !this.vectorDBCollectionInput || !this.milvusOpenAIInput) { + const saveVectorDBSettings = (): void => { + if (!this.vectorDBEnabledCheckbox || !this.vectorDBEndpointInput || !this.vectorDBApiKeyInput || + !this.milvusPasswordInput || !this.vectorDBCollectionInput || !this.milvusOpenAIInput) { return; }As per coding guidelines.
front_end/panels/ai_chat/ui/settings/i18n-strings.ts (1)
10-460: Move UIStrings into ModuleUIStrings and mark the objectas const.Exporting
UIStringsfromi18n-strings.tsviolates DevTools l10n policy (rulesdir/l10n-no-uistrings-export), and the object still lacks the requiredas const. Rename the file toModuleUIStrings.ts, update all imports accordingly, and add the const assertion:-export const UIStrings = { +export const UIStrings = { /* … */ -}; +} as const;After renaming, ensure callers import from
../ModuleUIStrings.js. As per coding guidelines.front_end/panels/ai_chat/ui/settings/providers/OpenRouterSettings.ts (2)
42-66: Replaceanywith a typed OAuth surface.
OpenRouterOAuthremains typed asany, so you lose type safety and lint still fails. Define the minimal interface and use it for both the property andgetOpenRouterOAuthreturn value.- private OpenRouterOAuth: any = null; + type OpenRouterOAuthAPI = { + isOAuthAuthenticated(): Promise<boolean>; + startAuthFlow(): Promise<void>; + revokeToken(): Promise<void>; + switchToManualApiKey(): void; + }; + + private OpenRouterOAuth: OpenRouterOAuthAPI | null = null; @@ - private async getOpenRouterOAuth(): Promise<any> { + private async getOpenRouterOAuth(): Promise<OpenRouterOAuthAPI> { if (!this.OpenRouterOAuth) { - const module = await import('../../../auth/OpenRouterOAuth.js'); - this.OpenRouterOAuth = module.OpenRouterOAuth; + const module = await import('../../../auth/OpenRouterOAuth.js'); + this.OpenRouterOAuth = module.OpenRouterOAuth as OpenRouterOAuthAPI; } return this.OpenRouterOAuth; }As per coding guidelines.
165-188: Return type + fetch button state still missing.
updateOAuthButtonstill lacks an explicit return type and never re-enables the Fetch button when OAuth is connected (the prior feedback). That leaves OAuth-only users unable to fetch models.- const updateOAuthButton = async () => { + const updateOAuthButton = async (): Promise<void> => { const OAuth = await this.getOpenRouterOAuth(); if (await OAuth.isOAuthAuthenticated()) { if (this.oauthButton) { this.oauthButton.textContent = 'Disconnect OpenRouter'; this.oauthButton.classList.add('disconnect'); } + if (this.fetchModelsButton) { + this.fetchModelsButton.disabled = false; + } @@ } else { if (this.oauthButton) { this.oauthButton.textContent = 'Connect with OpenRouter'; this.oauthButton.classList.remove('disconnect'); } + if (this.fetchModelsButton && this.apiKeyInput) { + this.fetchModelsButton.disabled = !this.apiKeyInput.value.trim(); + }This preserves existing API-key logic while letting OAuth users fetch models. As per coding guidelines.
front_end/panels/ai_chat/ui/settings/advanced/MCPSettings.ts (1)
508-512: Replace innerHTML with safe DOM construction.Using
innerHTMLwith template literals triggers XSS warnings. As previously noted, this should be replaced with safe DOM node construction.Apply this diff to replace innerHTML assignments with DOM construction:
if (!status.enabled) { this.mcpStatusDot.style.backgroundColor = 'var(--color-text-disabled)'; - this.mcpStatusText.innerHTML = `<span style="color: var(--color-text-disabled); font-weight: 500;">⚫ Disabled</span>`; + this.mcpStatusText.textContent = ''; + const span = document.createElement('span'); + span.style.color = 'var(--color-text-disabled)'; + span.style.fontWeight = '500'; + span.textContent = '⚫ Disabled'; + this.mcpStatusText.appendChild(span); this.mcpStatusDetails.textContent = ''; return; }Apply similar changes at lines 519-527 and 557-566 to replace all innerHTML assignments with DOM construction. See the past review comment for complete examples.
front_end/panels/ai_chat/ui/settings/providers/GroqSettings.ts (2)
125-139: Eliminate redundant rebuild after refresh to prevent flicker and focus loss.The code refreshes existing selectors (lines 125-131) and then immediately calls
updateModelSelectors()(line 138) which rebuilds them from scratch. This causes DOM replacement, resulting in flicker and lost focus.Apply this diff to remove the redundant rebuild:
// Refresh existing model selectors with new options if they exist if (this.miniModelSelector) { - refreshModelSelectOptions(this.miniModelSelector as any, allGroqModels, miniModel, i18nString(UIStrings.defaultMiniOption)); + refreshModelSelectOptions(this.miniModelSelector as unknown as ModelSelectorElement, allGroqModels, miniModel, i18nString(UIStrings.defaultMiniOption)); } if (this.nanoModelSelector) { - refreshModelSelectOptions(this.nanoModelSelector as any, allGroqModels, nanoModel, i18nString(UIStrings.defaultNanoOption)); + refreshModelSelectOptions(this.nanoModelSelector as unknown as ModelSelectorElement, allGroqModels, nanoModel, i18nString(UIStrings.defaultNanoOption)); } this.fetchModelsStatus.textContent = i18nString(UIStrings.fetchedModels, {PH1: actualModelCount}); this.fetchModelsStatus.style.backgroundColor = 'var(--color-accent-green-background)'; this.fetchModelsStatus.style.color = 'var(--color-accent-green)'; - // Update Groq model selections - this.updateModelSelectors(); + // Selectors already refreshed above; no need to rebuild
127-130: Replaceas anycasts with proper type assertion.Using
as anybypasses type safety. The cast should beas unknown as ModelSelectorElementto maintain type checking.Apply this diff:
if (this.miniModelSelector) { - refreshModelSelectOptions(this.miniModelSelector as any, allGroqModels, miniModel, i18nString(UIStrings.defaultMiniOption)); + refreshModelSelectOptions(this.miniModelSelector as unknown as ModelSelectorElement, allGroqModels, miniModel, i18nString(UIStrings.defaultMiniOption)); } if (this.nanoModelSelector) { - refreshModelSelectOptions(this.nanoModelSelector as any, allGroqModels, nanoModel, i18nString(UIStrings.defaultNanoOption)); + refreshModelSelectOptions(this.nanoModelSelector as unknown as ModelSelectorElement, allGroqModels, nanoModel, i18nString(UIStrings.defaultNanoOption)); }front_end/panels/ai_chat/ui/settings/providers/LiteLLMSettings.ts (1)
145-176: Remove redundant selector rebuild after refresh.The code refreshes existing selectors (lines 145-151) and then immediately rebuilds them via
updateModelSelectors()(line 175). This causes unnecessary DOM churn and focus loss.Apply this diff:
// Refresh existing model selectors with new options if they exist if (this.miniModelSelector) { - refreshModelSelectOptions(this.miniModelSelector as any, allLiteLLMModels, miniModel, i18nString(UIStrings.defaultMiniOption)); + refreshModelSelectOptions(this.miniModelSelector as unknown as ModelSelectorElement, allLiteLLMModels, miniModel, i18nString(UIStrings.defaultMiniOption)); } if (this.nanoModelSelector) { - refreshModelSelectOptions(this.nanoModelSelector as any, allLiteLLMModels, nanoModel, i18nString(UIStrings.defaultNanoOption)); + refreshModelSelectOptions(this.nanoModelSelector as unknown as ModelSelectorElement, allLiteLLMModels, nanoModel, i18nString(UIStrings.defaultNanoOption)); } if (hadWildcard && actualModelCount === 0 && !hasCustomModels) { this.fetchModelsStatus.textContent = i18nString(UIStrings.wildcardModelsOnly); this.fetchModelsStatus.style.backgroundColor = 'var(--color-accent-orange-background)'; this.fetchModelsStatus.style.color = 'var(--color-accent-orange)'; } else if (hadWildcard && actualModelCount === 0) { // Only wildcard was returned but we have custom models this.fetchModelsStatus.textContent = i18nString(UIStrings.wildcardAndCustomModels); this.fetchModelsStatus.style.backgroundColor = 'var(--color-accent-green-background)'; this.fetchModelsStatus.style.color = 'var(--color-accent-green)'; } else if (hadWildcard) { // Wildcard plus other models this.fetchModelsStatus.textContent = i18nString(UIStrings.wildcardAndOtherModels, {PH1: actualModelCount}); this.fetchModelsStatus.style.backgroundColor = 'var(--color-accent-green-background)'; this.fetchModelsStatus.style.color = 'var(--color-accent-green)'; } else { // No wildcard, just regular models this.fetchModelsStatus.textContent = i18nString(UIStrings.fetchedModels, {PH1: actualModelCount}); this.fetchModelsStatus.style.backgroundColor = 'var(--color-accent-green-background)'; this.fetchModelsStatus.style.color = 'var(--color-accent-green)'; } - // Update LiteLLM model selections - this.updateModelSelectors(); + // Selectors already refreshed above; no need to rebuild
🧹 Nitpick comments (5)
front_end/panels/ai_chat/ui/settings/advanced/EvaluationSettings.ts (4)
5-15: Reorder imports to satisfy linter convention.The i18n import should appear after the Logger import according to the project's import ordering rules.
Apply this diff to reorder the imports:
-import { i18nString, UIStrings } from '../i18n-strings.js'; import { getEvaluationConfig, setEvaluationConfig, @@ -12,6 +11,7 @@ isEvaluationConnected } from '../../../common/EvaluationConfig.js'; import { createLogger } from '../../../core/Logger.js'; +import { i18nString, UIStrings } from '../i18n-strings.js';
94-109: Add return type and localize connection status strings.The
updateConnectionStatusfunction is missing an explicit return type annotation, and the connection status strings are hard-coded instead of using localized UIStrings members.Apply these changes:
- const updateConnectionStatus = () => { + const updateConnectionStatus = (): void => { const isConnected = isEvaluationConnected(); logger.debug('Updating connection status', { isConnected }); if (isConnected) { connectionStatusDot.style.backgroundColor = 'var(--color-accent-green)'; - connectionStatusText.textContent = 'Connected to evaluation server'; + connectionStatusText.textContent = i18nString(UIStrings.evaluationConnectedStatus); connectionStatusText.style.color = 'var(--color-accent-green)'; } else { connectionStatusDot.style.backgroundColor = 'var(--color-text-disabled)'; - connectionStatusText.textContent = 'Not connected'; + connectionStatusText.textContent = i18nString(UIStrings.evaluationDisconnectedStatus); connectionStatusText.style.color = 'var(--color-text-disabled)'; } };Note: Ensure
UIStrings.evaluationConnectedStatusandUIStrings.evaluationDisconnectedStatusare defined in the i18n-strings module.
124-132: Localize Client ID label and hint text.The Client ID label and hint use hard-coded strings instead of localized UIStrings members, which is inconsistent with the rest of the settings fields.
Apply this diff to localize the strings:
const clientIdLabel = document.createElement('div'); clientIdLabel.className = 'settings-label'; - clientIdLabel.textContent = 'Client ID'; + clientIdLabel.textContent = i18nString(UIStrings.evaluationClientId); evaluationConfigContainer.appendChild(clientIdLabel); const clientIdHint = document.createElement('div'); clientIdHint.className = 'settings-hint'; - clientIdHint.textContent = 'Unique identifier for this DevTools instance'; + clientIdHint.textContent = i18nString(UIStrings.evaluationClientIdHint); evaluationConfigContainer.appendChild(clientIdHint);Note: Ensure
UIStrings.evaluationClientIdandUIStrings.evaluationClientIdHintare defined in the i18n-strings module.
185-266: Remove non-null assertions and localize transient status messages.The event handler uses multiple non-null assertions (
!) which violate the project's TypeScript rules. Additionally, all transient status messages are hard-coded instead of using localized strings.To address these issues:
- Capture DOM elements as locals at the beginning of the event handler to avoid non-null assertions:
this.evaluationEnabledCheckbox.addEventListener('change', async () => { - const isEnabled = this.evaluationEnabledCheckbox!.checked; + const checkboxEl = this.evaluationEnabledCheckbox; + const endpointEl = this.evaluationEndpointInput; + const secretEl = this.evaluationSecretKeyInput; + if (!checkboxEl || !endpointEl || !secretEl) { return; } + + const isEnabled = checkboxEl.checked; evaluationConfigContainer.style.display = isEnabled ? 'block' : 'none';
- Replace hard-coded strings with localized UIStrings members throughout the handler. For example:
- connectionStatusMessage.textContent = 'Connecting...'; + connectionStatusMessage.textContent = i18nString(UIStrings.evaluationConnecting);- const endpoint = this.evaluationEndpointInput!.value.trim() || 'ws://localhost:8080'; - const secretKey = this.evaluationSecretKeyInput!.value.trim(); + const endpoint = endpointEl.value.trim() || 'ws://localhost:8080'; + const secretKey = secretEl.value.trim();- connectionStatusMessage.textContent = '✓ Connected successfully'; + connectionStatusMessage.textContent = i18nString(UIStrings.evaluationConnected);- this.evaluationEnabledCheckbox!.checked = false; + checkboxEl.checked = false;Apply similar changes for disconnect messages (lines 235, 249, 256) and error messages (lines 225, 256).
Note: Ensure the following UIStrings members are defined:
evaluationConnecting,evaluationConnected,evaluationConnectionFailed,evaluationDisconnecting,evaluationDisconnected,evaluationDisconnectError.front_end/panels/ai_chat/ui/settings/providers/OpenAISettings.ts (1)
123-133: Simplify the save() method.The
elseblock is unnecessary sincesetStorageItemalready handles empty strings appropriately.Apply this diff:
save(): void { // Save OpenAI API key if (this.apiKeyInput) { const newApiKey = this.apiKeyInput.value.trim(); - if (newApiKey) { - setStorageItem(OPENAI_API_KEY_STORAGE_KEY, newApiKey); - } else { - setStorageItem(OPENAI_API_KEY_STORAGE_KEY, ''); - } + setStorageItem(OPENAI_API_KEY_STORAGE_KEY, newApiKey); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
front_end/panels/ai_chat/ui/settings/advanced/BrowsingHistorySettings.ts(1 hunks)front_end/panels/ai_chat/ui/settings/advanced/EvaluationSettings.ts(1 hunks)front_end/panels/ai_chat/ui/settings/advanced/MCPSettings.ts(1 hunks)front_end/panels/ai_chat/ui/settings/advanced/TracingSettings.ts(1 hunks)front_end/panels/ai_chat/ui/settings/advanced/VectorDBSettings.ts(1 hunks)front_end/panels/ai_chat/ui/settings/i18n-strings.ts(1 hunks)front_end/panels/ai_chat/ui/settings/providers/GroqSettings.ts(1 hunks)front_end/panels/ai_chat/ui/settings/providers/LiteLLMSettings.ts(1 hunks)front_end/panels/ai_chat/ui/settings/providers/OpenAISettings.ts(1 hunks)front_end/panels/ai_chat/ui/settings/providers/OpenRouterSettings.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (9)
front_end/panels/ai_chat/ui/settings/advanced/EvaluationSettings.ts (3)
front_end/panels/ai_chat/core/Logger.ts (1)
createLogger(316-318)front_end/panels/ai_chat/ui/settings/i18n-strings.ts (2)
i18nString(470-470)UIStrings(10-460)front_end/panels/ai_chat/common/EvaluationConfig.ts (7)
getEvaluationConfig(213-215)isEvaluationEnabled(221-223)isEvaluationConnected(237-239)setEvaluationConfig(217-219)connectToEvaluationService(225-227)getEvaluationClientId(233-235)disconnectFromEvaluationService(229-231)
front_end/panels/ai_chat/ui/settings/advanced/VectorDBSettings.ts (3)
front_end/panels/ai_chat/ui/settings/i18n-strings.ts (2)
i18nString(470-470)UIStrings(10-460)front_end/panels/ai_chat/ui/settings/constants.ts (6)
VECTOR_DB_ENABLED_KEY(33-33)MILVUS_ENDPOINT_KEY(34-34)MILVUS_USERNAME_KEY(35-35)MILVUS_PASSWORD_KEY(36-36)MILVUS_OPENAI_KEY(38-38)MILVUS_COLLECTION_KEY(37-37)front_end/panels/ai_chat/tools/VectorDBClient.ts (1)
VectorDBClient(74-469)
front_end/panels/ai_chat/ui/settings/providers/OpenRouterSettings.ts (8)
front_end/panels/ai_chat/core/Logger.ts (1)
createLogger(316-318)front_end/panels/ai_chat/ui/settings/types.ts (5)
UpdateModelOptionsFunction(69-72)GetModelOptionsFunction(77-79)AddCustomModelOptionFunction(84-87)RemoveCustomModelOptionFunction(92-94)ModelOption(8-12)front_end/panels/ai_chat/ui/AIChatPanel.ts (3)
getModelOptions(470-501)updateModelOptions(509-628)localStorage(836-859)front_end/panels/ai_chat/ui/settings/utils/storage.ts (2)
getStorageItem(8-10)setStorageItem(15-21)front_end/panels/ai_chat/ui/settings/constants.ts (4)
OPENROUTER_API_KEY_STORAGE_KEY(23-23)OPENROUTER_MODELS_CACHE_DURATION_MS(28-28)MINI_MODEL_STORAGE_KEY(8-8)NANO_MODEL_STORAGE_KEY(9-9)front_end/panels/ai_chat/LLM/LLMClient.ts (1)
LLMClient(49-385)front_end/panels/ai_chat/ui/settings/utils/validation.ts (1)
getValidModelForProvider(11-34)front_end/panels/ai_chat/ui/settings/components/ModelSelectorFactory.ts (1)
createModelSelector(21-72)
front_end/panels/ai_chat/ui/settings/providers/OpenAISettings.ts (7)
front_end/panels/ai_chat/ui/AIChatPanel.ts (3)
getModelOptions(470-501)addCustomModelOption(636-677)removeCustomModelOption(693-717)front_end/panels/ai_chat/ui/settings/types.ts (3)
GetModelOptionsFunction(77-79)AddCustomModelOptionFunction(84-87)RemoveCustomModelOptionFunction(92-94)front_end/panels/ai_chat/ui/settings/i18n-strings.ts (2)
i18nString(470-470)UIStrings(10-460)front_end/panels/ai_chat/ui/settings/utils/storage.ts (2)
getStorageItem(8-10)setStorageItem(15-21)front_end/panels/ai_chat/ui/settings/constants.ts (3)
OPENAI_API_KEY_STORAGE_KEY(19-19)MINI_MODEL_STORAGE_KEY(8-8)NANO_MODEL_STORAGE_KEY(9-9)front_end/panels/ai_chat/ui/settings/utils/validation.ts (1)
getValidModelForProvider(11-34)front_end/panels/ai_chat/ui/settings/components/ModelSelectorFactory.ts (1)
createModelSelector(21-72)
front_end/panels/ai_chat/ui/settings/advanced/MCPSettings.ts (2)
front_end/panels/ai_chat/core/Logger.ts (1)
createLogger(316-318)front_end/panels/ai_chat/ui/settings/i18n-strings.ts (2)
i18nString(470-470)UIStrings(10-460)
front_end/panels/ai_chat/ui/settings/providers/GroqSettings.ts (8)
front_end/panels/ai_chat/core/Logger.ts (1)
createLogger(316-318)front_end/panels/ai_chat/ui/settings/types.ts (5)
UpdateModelOptionsFunction(69-72)GetModelOptionsFunction(77-79)AddCustomModelOptionFunction(84-87)RemoveCustomModelOptionFunction(92-94)ModelOption(8-12)front_end/panels/ai_chat/ui/AIChatPanel.ts (2)
getModelOptions(470-501)updateModelOptions(509-628)front_end/panels/ai_chat/ui/settings/utils/storage.ts (2)
getStorageItem(8-10)setStorageItem(15-21)front_end/panels/ai_chat/ui/settings/constants.ts (3)
GROQ_API_KEY_STORAGE_KEY(22-22)MINI_MODEL_STORAGE_KEY(8-8)NANO_MODEL_STORAGE_KEY(9-9)front_end/panels/ai_chat/LLM/LLMClient.ts (1)
LLMClient(49-385)front_end/panels/ai_chat/ui/settings/components/ModelSelectorFactory.ts (2)
refreshModelSelectOptions(82-121)createModelSelector(21-72)front_end/panels/ai_chat/ui/settings/utils/validation.ts (1)
getValidModelForProvider(11-34)
front_end/panels/ai_chat/ui/settings/advanced/TracingSettings.ts (2)
front_end/panels/ai_chat/ui/settings/i18n-strings.ts (2)
i18nString(470-470)UIStrings(10-460)front_end/panels/ai_chat/tracing/TracingConfig.ts (3)
getTracingConfig(127-129)isTracingEnabled(141-143)setTracingConfig(134-136)
front_end/panels/ai_chat/ui/settings/providers/LiteLLMSettings.ts (9)
front_end/panels/ai_chat/core/Logger.ts (1)
createLogger(316-318)front_end/panels/ai_chat/ui/settings/types.ts (5)
UpdateModelOptionsFunction(69-72)FetchLiteLLMModelsFunction(61-64)GetModelOptionsFunction(77-79)AddCustomModelOptionFunction(84-87)RemoveCustomModelOptionFunction(92-94)front_end/panels/ai_chat/ui/AIChatPanel.ts (4)
getModelOptions(470-501)updateModelOptions(509-628)litellmModels(1243-1246)localStorage(836-859)front_end/panels/ai_chat/LLM/LLMClient.ts (2)
fetchLiteLLMModels(298-302)LLMClient(49-385)front_end/panels/ai_chat/ui/settings/i18n-strings.ts (2)
i18nString(470-470)UIStrings(10-460)front_end/panels/ai_chat/ui/settings/utils/storage.ts (2)
getStorageItem(8-10)setStorageItem(15-21)front_end/panels/ai_chat/ui/settings/constants.ts (4)
LITELLM_ENDPOINT_KEY(20-20)LITELLM_API_KEY_STORAGE_KEY(21-21)MINI_MODEL_STORAGE_KEY(8-8)NANO_MODEL_STORAGE_KEY(9-9)front_end/panels/ai_chat/ui/settings/components/ModelSelectorFactory.ts (2)
refreshModelSelectOptions(82-121)createModelSelector(21-72)front_end/panels/ai_chat/ui/settings/utils/validation.ts (1)
getValidModelForProvider(11-34)
front_end/panels/ai_chat/ui/settings/advanced/BrowsingHistorySettings.ts (2)
front_end/panels/ai_chat/ui/settings/i18n-strings.ts (2)
i18nString(470-470)UIStrings(10-460)front_end/panels/ai_chat/tools/VisitHistoryManager.ts (1)
VisitHistoryManager(18-434)
🪛 ast-grep (0.39.9)
front_end/panels/ai_chat/ui/settings/advanced/EvaluationSettings.ts
[warning] 36-36: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: this.container.innerHTML = ''
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
front_end/panels/ai_chat/ui/settings/advanced/VectorDBSettings.ts
[warning] 34-34: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: this.container.innerHTML = ''
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
front_end/panels/ai_chat/ui/settings/providers/OpenRouterSettings.ts
[warning] 69-69: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: this.container.innerHTML = ''
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
front_end/panels/ai_chat/ui/settings/providers/OpenAISettings.ts
[warning] 33-33: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: this.container.innerHTML = ''
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
front_end/panels/ai_chat/ui/settings/advanced/MCPSettings.ts
[warning] 56-56: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: this.container.innerHTML = ''
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 508-508: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: this.mcpStatusText.innerHTML = <span style="color: var(--color-text-disabled); font-weight: 500;">⚫ Disabled</span>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 521-521: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: this.mcpStatusText.innerHTML = <span style="color: var(--color-warning); font-weight: 500;">🟡 Connected with issues</span> <span style="color: var(--color-text-secondary); font-size: 12px;">(${toolCount} tools)</span>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 524-524: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: this.mcpStatusText.innerHTML = <span style="color: var(--color-accent-green); font-weight: 500;">🟢 Connected</span> <span style="color: var(--color-text-secondary); font-size: 12px;">(${toolCount} tools)</span>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 558-558: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: this.mcpStatusText.innerHTML = <span style="color: var(--color-error); font-weight: 500;">🔴 Authentication required</span>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 561-561: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: this.mcpStatusText.innerHTML = <span style="color: var(--color-text-disabled); font-weight: 500;">⚪ Not connected</span>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 508-508: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: this.mcpStatusText.innerHTML = <span style="color: var(--color-text-disabled); font-weight: 500;">⚫ Disabled</span>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 521-521: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: this.mcpStatusText.innerHTML = <span style="color: var(--color-warning); font-weight: 500;">🟡 Connected with issues</span> <span style="color: var(--color-text-secondary); font-size: 12px;">(${toolCount} tools)</span>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 524-524: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: this.mcpStatusText.innerHTML = <span style="color: var(--color-accent-green); font-weight: 500;">🟢 Connected</span> <span style="color: var(--color-text-secondary); font-size: 12px;">(${toolCount} tools)</span>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 558-558: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: this.mcpStatusText.innerHTML = <span style="color: var(--color-error); font-weight: 500;">🔴 Authentication required</span>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 561-561: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: this.mcpStatusText.innerHTML = <span style="color: var(--color-text-disabled); font-weight: 500;">⚪ Not connected</span>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
front_end/panels/ai_chat/ui/settings/providers/GroqSettings.ts
[warning] 40-40: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: this.container.innerHTML = ''
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
front_end/panels/ai_chat/ui/settings/advanced/TracingSettings.ts
[warning] 25-25: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: this.container.innerHTML = ''
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
front_end/panels/ai_chat/ui/settings/providers/LiteLLMSettings.ts
[warning] 48-48: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: this.container.innerHTML = ''
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 305-305: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: this.customModelsList.innerHTML = ''
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 339-344: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: checkIcon.innerHTML = <svg width="16" height="16" viewBox="0 0 16 16" fill="none" xmlns="http://www.w3.org/2000/svg"> <path d="M12 5L6.5 10.5L4 8" stroke="currentColor" stroke-width="1.5" stroke-linecap="round" stroke-linejoin="round"/> <circle cx="8" cy="8" r="6.25" stroke="currentColor" stroke-width="1.5"/> </svg>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 358-363: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: trashIcon.innerHTML = <svg width="16" height="16" viewBox="0 0 16 16" fill="none" xmlns="http://www.w3.org/2000/svg"> <path d="M6 2.5V2C6 1.44772 6.44772 1 7 1H9C9.55228 1 10 1.44772 10 2V2.5M2 2.5H14M12.5 2.5V13C12.5 13.5523 12.0523 14 11.5 14H4.5C3.94772 14 3.5 13.5523 3.5 13V2.5M5.5 5.5V11M8 5.5V11M10.5 5.5V11" stroke="currentColor" stroke-width="1.2" stroke-linecap="round" stroke-linejoin="round"/> </svg>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 339-344: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: checkIcon.innerHTML = <svg width="16" height="16" viewBox="0 0 16 16" fill="none" xmlns="http://www.w3.org/2000/svg"> <path d="M12 5L6.5 10.5L4 8" stroke="currentColor" stroke-width="1.5" stroke-linecap="round" stroke-linejoin="round"/> <circle cx="8" cy="8" r="6.25" stroke="currentColor" stroke-width="1.5"/> </svg>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 358-363: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: trashIcon.innerHTML = <svg width="16" height="16" viewBox="0 0 16 16" fill="none" xmlns="http://www.w3.org/2000/svg"> <path d="M6 2.5V2C6 1.44772 6.44772 1 7 1H9C9.55228 1 10 1.44772 10 2V2.5M2 2.5H14M12.5 2.5V13C12.5 13.5523 12.0523 14 11.5 14H4.5C3.94772 14 3.5 13.5523 3.5 13V2.5M5.5 5.5V11M8 5.5V11M10.5 5.5V11" stroke="currentColor" stroke-width="1.2" stroke-linecap="round" stroke-linejoin="round"/> </svg>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
front_end/panels/ai_chat/ui/settings/advanced/BrowsingHistorySettings.ts
[warning] 24-24: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: this.container.innerHTML = ''
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
🪛 ESLint
front_end/panels/ai_chat/ui/settings/advanced/EvaluationSettings.ts
[error] 5-5: ../i18n-strings.js import should occur after import of ../../../core/Logger.js
(import/order)
[error] 41-41: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
[error] 50-50: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
[error] 67-67: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
[error] 73-73: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
[error] 95-95: Missing return type on function.
(@typescript-eslint/explicit-function-return-type)
[error] 118-118: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
[error] 187-187: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
[error] 200-200: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
[error] 201-201: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
[error] 230-230: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
[error] 245-245: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
[error] 246-246: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
front_end/panels/ai_chat/ui/settings/i18n-strings.ts
[error] 10-460: Exporting the UIStrings object is only allowed in ModuleUIStrings.(js|ts) or trace/model/insights
(rulesdir/l10n-no-uistrings-export)
[error] 10-460: Add as const to UIStrings constant object.
(rulesdir/enforce-ui-strings-as-const)
front_end/panels/ai_chat/ui/settings/advanced/VectorDBSettings.ts
[error] 6-13: ../constants.js import should occur before import of ../i18n-strings.js
(import/order)
[error] 39-39: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
[error] 45-45: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
[error] 62-62: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
[error] 68-68: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
[error] 201-201: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
[error] 202-202: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
[error] 206-206: Missing return type on function.
(@typescript-eslint/explicit-function-return-type)
[error] 270-270: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
front_end/panels/ai_chat/ui/settings/providers/OpenRouterSettings.ts
[error] 5-5: There should be at least one empty line between import groups
(import/order)
[error] 6-6: ../components/ModelSelectorFactory.js import should occur before import of ./BaseProviderSettings.js
(import/order)
[error] 7-7: ../i18n-strings.js import should occur before import of ./BaseProviderSettings.js
(import/order)
[error] 8-8: ../utils/validation.js import should occur before import of ./BaseProviderSettings.js
(import/order)
[error] 9-9: ../utils/storage.js import should occur before import of ./BaseProviderSettings.js
(import/order)
[error] 10-10: ../constants.js import should occur before import of ./BaseProviderSettings.js
(import/order)
[error] 11-11: ../types.js type import should occur before import of ./BaseProviderSettings.js
(import/order)
[error] 12-12: ../../../LLM/LLMClient.js import should occur before import of ./BaseProviderSettings.js
(import/order)
[error] 13-13: ../../../core/Logger.js import should occur before import of ./BaseProviderSettings.js
(import/order)
[error] 43-43: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 60-60: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 73-73: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
[error] 118-118: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
[error] 166-166: Missing return type on function.
(@typescript-eslint/explicit-function-return-type)
[error] 193-193: Expected { after 'if' condition.
(curly)
[error] 245-245: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 283-283: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 300-300: Expected { after 'if' condition.
(curly)
[error] 336-336: Expected { after 'if' condition.
(curly)
[error] 456-456: Expected { after 'if' condition.
(curly)
[error] 482-482: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
front_end/panels/ai_chat/ui/settings/providers/OpenAISettings.ts
[error] 5-5: There should be at least one empty line between import groups
(import/order)
[error] 5-5: ./BaseProviderSettings.js import should occur after type import of ../types.js
(import/order)
[error] 7-7: ../i18n-strings.js import should occur after import of ../constants.js
(import/order)
[error] 8-8: ../utils/validation.js import should occur after type import of ../types.js
(import/order)
[error] 9-9: ../utils/storage.js import should occur after type import of ../types.js
(import/order)
[error] 37-37: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
[error] 69-69: Expected { after 'if' condition.
(curly)
[error] 87-87: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
front_end/panels/ai_chat/ui/settings/advanced/MCPSettings.ts
[error] 6-11: ../../../mcp/MCPConfig.js import should occur before import of ../i18n-strings.js
(import/order)
[error] 12-12: ../../../mcp/MCPRegistry.js import should occur before import of ../i18n-strings.js
(import/order)
[error] 13-13: ../../mcp/MCPConnectionsDialog.js import should occur before import of ../i18n-strings.js
(import/order)
[error] 14-14: ../../../core/Logger.js import should occur before import of ../i18n-strings.js
(import/order)
[error] 62-62: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
[error] 70-70: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
[error] 91-91: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
[error] 108-108: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
[error] 190-190: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
[error] 195-195: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
[error] 200-200: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
[error] 249-249: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
[error] 333-333: Missing return type on function.
(@typescript-eslint/explicit-function-return-type)
[error] 348-348: Expected { after 'if' condition.
(curly)
[error] 353-353: Expected { after 'if' condition.
(curly)
[error] 377-377: Missing return type on function.
(@typescript-eslint/explicit-function-return-type)
[error] 378-378: Expected { after 'if' condition.
(curly)
[error] 383-383: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
[error] 438-448: Unexpected if as the only statement in an else block.
(no-lonely-if)
[error] 491-491: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
[error] 494-494: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
[error] 503-503: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
[error] 597-597: Expected { after 'if' condition.
(curly)
front_end/panels/ai_chat/ui/settings/providers/GroqSettings.ts
[error] 5-5: There should be at least one empty line between import groups
(import/order)
[error] 6-6: ../components/ModelSelectorFactory.js import should occur before import of ./BaseProviderSettings.js
(import/order)
[error] 7-7: ../i18n-strings.js import should occur before import of ./BaseProviderSettings.js
(import/order)
[error] 8-8: ../utils/validation.js import should occur before import of ./BaseProviderSettings.js
(import/order)
[error] 9-9: ../utils/storage.js import should occur before import of ./BaseProviderSettings.js
(import/order)
[error] 10-10: ../constants.js import should occur before import of ./BaseProviderSettings.js
(import/order)
[error] 11-11: ../types.js type import should occur before import of ./BaseProviderSettings.js
(import/order)
[error] 12-12: ../../../LLM/LLMClient.js import should occur before import of ./BaseProviderSettings.js
(import/order)
[error] 13-13: ../../../core/Logger.js import should occur before import of ./BaseProviderSettings.js
(import/order)
[error] 44-44: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
[error] 93-93: Expected { after 'if' condition.
(curly)
[error] 127-127: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 130-130: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 162-162: Expected { after 'if' condition.
(curly)
[error] 185-185: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
front_end/panels/ai_chat/ui/settings/advanced/TracingSettings.ts
[error] 6-6: ../../../tracing/TracingConfig.js import should occur before import of ../i18n-strings.js
(import/order)
[error] 30-30: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
[error] 39-39: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
[error] 56-56: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
[error] 62-62: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
[error] 135-135: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
[error] 147-147: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
[error] 148-148: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
[error] 149-149: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
front_end/panels/ai_chat/ui/settings/providers/LiteLLMSettings.ts
[error] 5-5: There should be at least one empty line between import groups
(import/order)
[error] 6-6: ../components/ModelSelectorFactory.js import should occur before import of ./BaseProviderSettings.js
(import/order)
[error] 7-7: ../i18n-strings.js import should occur before import of ./BaseProviderSettings.js
(import/order)
[error] 8-8: ../utils/validation.js import should occur before import of ./BaseProviderSettings.js
(import/order)
[error] 9-9: ../utils/storage.js import should occur before import of ./BaseProviderSettings.js
(import/order)
[error] 10-10: ../constants.js import should occur before import of ./BaseProviderSettings.js
(import/order)
[error] 11-11: ../types.js type import should occur before import of ./BaseProviderSettings.js
(import/order)
[error] 11-11: 'ModelOption' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 12-12: ../../../LLM/LLMClient.js import should occur before import of ./BaseProviderSettings.js
(import/order)
[error] 13-13: ../../../core/Logger.js import should occur before import of ./BaseProviderSettings.js
(import/order)
[error] 30-30: Type boolean trivially inferred from a boolean literal, remove type annotation.
(@typescript-eslint/no-inferrable-types)
[error] 52-52: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
[error] 95-95: Missing return type on function.
(@typescript-eslint/explicit-function-return-type)
[error] 121-121: Expected { after 'if' condition.
(curly)
[error] 147-147: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 150-150: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 193-193: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
[error] 246-246: Expected { after 'if' condition.
(curly)
[error] 303-303: Expected { after 'if' condition.
(curly)
[error] 314-314: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
[error] 316-316: Forbidden non-null assertion.
(@typescript-eslint/no-non-null-assertion)
[error] 419-419: Expected { after 'if' condition.
(curly)
[error] 450-456: Unnecessary 'else' after 'return'.
(no-else-return)
[error] 467-467: Expected { after 'if' condition.
(curly)
[error] 488-488: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
[error] 498-498: Missing return type on function.
(@typescript-eslint/explicit-function-return-type)
front_end/panels/ai_chat/ui/settings/advanced/BrowsingHistorySettings.ts
[error] 6-6: ../../../../../core/common/common.js import should occur before import of ../i18n-strings.js
(import/order)
[error] 29-29: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
[error] 35-35: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
[error] 41-41: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
[error] 48-48: Prefer template literals over imperative DOM API calls
(rulesdir/no-imperative-dom-api)
| import { i18nString, UIStrings } from '../i18n-strings.js'; | ||
| import { getTracingConfig, setTracingConfig, isTracingEnabled } from '../../../tracing/TracingConfig.js'; |
There was a problem hiding this comment.
Restore canonical import order.
../../../tracing/TracingConfig.js is a higher-level module and must precede the local UI import to satisfy import/order.
-import { i18nString, UIStrings } from '../i18n-strings.js';
-import { getTracingConfig, setTracingConfig, isTracingEnabled } from '../../../tracing/TracingConfig.js';
+import { getTracingConfig, setTracingConfig, isTracingEnabled } from '../../../tracing/TracingConfig.js';
+import { i18nString, UIStrings } from '../i18n-strings.js';As per coding guidelines.
📝 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.
| import { i18nString, UIStrings } from '../i18n-strings.js'; | |
| import { getTracingConfig, setTracingConfig, isTracingEnabled } from '../../../tracing/TracingConfig.js'; | |
| import { getTracingConfig, setTracingConfig, isTracingEnabled } from '../../../tracing/TracingConfig.js'; | |
| import { i18nString, UIStrings } from '../i18n-strings.js'; |
🧰 Tools
🪛 ESLint
[error] 6-6: ../../../tracing/TracingConfig.js import should occur before import of ../i18n-strings.js
(import/order)
🤖 Prompt for AI Agents
In front_end/panels/ai_chat/ui/settings/advanced/TracingSettings.ts around lines
5 to 6, the import order is wrong — the higher-level tracing module should come
before the local UI i18n import to satisfy import/order; fix by swapping the two
import lines so that getTracingConfig, setTracingConfig, isTracingEnabled are
imported from '../../../tracing/TracingConfig.js' first, and then import
i18nString, UIStrings from '../i18n-strings.js', preserving original named
imports and formatting.
| refreshModelSelectOptions(this.miniModelSelector as any, allLiteLLMModels, miniModel, i18nString(UIStrings.defaultMiniOption)); | ||
| } | ||
| if (this.nanoModelSelector) { | ||
| refreshModelSelectOptions(this.nanoModelSelector as any, allLiteLLMModels, nanoModel, i18nString(UIStrings.defaultNanoOption)); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace as any with proper type assertion.
The as any casts bypass TypeScript's type safety. Use as unknown as ModelSelectorElement instead.
Apply this diff:
if (this.miniModelSelector) {
- refreshModelSelectOptions(this.miniModelSelector as any, allLiteLLMModels, miniModel, i18nString(UIStrings.defaultMiniOption));
+ refreshModelSelectOptions(this.miniModelSelector as unknown as ModelSelectorElement, allLiteLLMModels, miniModel, i18nString(UIStrings.defaultMiniOption));
}
if (this.nanoModelSelector) {
- refreshModelSelectOptions(this.nanoModelSelector as any, allLiteLLMModels, nanoModel, i18nString(UIStrings.defaultNanoOption));
+ refreshModelSelectOptions(this.nanoModelSelector as unknown as ModelSelectorElement, allLiteLLMModels, nanoModel, i18nString(UIStrings.defaultNanoOption));
}📝 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.
| refreshModelSelectOptions(this.miniModelSelector as any, allLiteLLMModels, miniModel, i18nString(UIStrings.defaultMiniOption)); | |
| } | |
| if (this.nanoModelSelector) { | |
| refreshModelSelectOptions(this.nanoModelSelector as any, allLiteLLMModels, nanoModel, i18nString(UIStrings.defaultNanoOption)); | |
| refreshModelSelectOptions(this.miniModelSelector as unknown as ModelSelectorElement, allLiteLLMModels, miniModel, i18nString(UIStrings.defaultMiniOption)); | |
| } | |
| if (this.nanoModelSelector) { | |
| refreshModelSelectOptions(this.nanoModelSelector as unknown as ModelSelectorElement, allLiteLLMModels, nanoModel, i18nString(UIStrings.defaultNanoOption)); |
🧰 Tools
🪛 ESLint
[error] 147-147: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 150-150: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🤖 Prompt for AI Agents
In front_end/panels/ai_chat/ui/settings/providers/LiteLLMSettings.ts around
lines 147 to 150, the two calls use "as any" which circumvents TypeScript
safety; replace those casts with "as unknown as ModelSelectorElement" for both
this.miniModelSelector and this.nanoModelSelector so the calls become type-safe
while preserving the runtime value; ensure ModelSelectorElement is imported or
available in scope before changing the assertions.
| import { BaseProviderSettings } from './BaseProviderSettings.js'; | ||
| import { createModelSelector } from '../components/ModelSelectorFactory.js'; | ||
| import { i18nString, UIStrings } from '../i18n-strings.js'; | ||
| import { getValidModelForProvider } from '../utils/validation.js'; | ||
| import { getStorageItem, setStorageItem } from '../utils/storage.js'; | ||
| import { OPENROUTER_API_KEY_STORAGE_KEY, MINI_MODEL_STORAGE_KEY, NANO_MODEL_STORAGE_KEY, OPENROUTER_MODELS_CACHE_DURATION_MS } from '../constants.js'; | ||
| import type { UpdateModelOptionsFunction, GetModelOptionsFunction, AddCustomModelOptionFunction, RemoveCustomModelOptionFunction, ModelOption } from '../types.js'; | ||
| import { LLMClient } from '../../../LLM/LLMClient.js'; | ||
| import { createLogger } from '../../../core/Logger.js'; |
There was a problem hiding this comment.
Group imports to satisfy lint.
Reorder the imports so non-relative/core modules come before sibling ones; the current sequence still violates import/order.
-import { BaseProviderSettings } from './BaseProviderSettings.js';
-import { createModelSelector } from '../components/ModelSelectorFactory.js';
-import { i18nString, UIStrings } from '../i18n-strings.js';
-import { getValidModelForProvider } from '../utils/validation.js';
-import { getStorageItem, setStorageItem } from '../utils/storage.js';
-import { OPENROUTER_API_KEY_STORAGE_KEY, MINI_MODEL_STORAGE_KEY, NANO_MODEL_STORAGE_KEY, OPENROUTER_MODELS_CACHE_DURATION_MS } from '../constants.js';
-import type { UpdateModelOptionsFunction, GetModelOptionsFunction, AddCustomModelOptionFunction, RemoveCustomModelOptionFunction, ModelOption } from '../types.js';
-import { LLMClient } from '../../../LLM/LLMClient.js';
-import { createLogger } from '../../../core/Logger.js';
+import { LLMClient } from '../../../LLM/LLMClient.js';
+import { createLogger } from '../../../core/Logger.js';
+import { createModelSelector } from '../components/ModelSelectorFactory.js';
+import { i18nString, UIStrings } from '../i18n-strings.js';
+import { getValidModelForProvider } from '../utils/validation.js';
+import { getStorageItem, setStorageItem } from '../utils/storage.js';
+import { OPENROUTER_API_KEY_STORAGE_KEY, MINI_MODEL_STORAGE_KEY, NANO_MODEL_STORAGE_KEY, OPENROUTER_MODELS_CACHE_DURATION_MS } from '../constants.js';
+import type { UpdateModelOptionsFunction, GetModelOptionsFunction, AddCustomModelOptionFunction, RemoveCustomModelOptionFunction, ModelOption } from '../types.js';
+import { BaseProviderSettings } from './BaseProviderSettings.js';Adjust grouping per project import conventions. As per coding guidelines.
📝 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.
| import { BaseProviderSettings } from './BaseProviderSettings.js'; | |
| import { createModelSelector } from '../components/ModelSelectorFactory.js'; | |
| import { i18nString, UIStrings } from '../i18n-strings.js'; | |
| import { getValidModelForProvider } from '../utils/validation.js'; | |
| import { getStorageItem, setStorageItem } from '../utils/storage.js'; | |
| import { OPENROUTER_API_KEY_STORAGE_KEY, MINI_MODEL_STORAGE_KEY, NANO_MODEL_STORAGE_KEY, OPENROUTER_MODELS_CACHE_DURATION_MS } from '../constants.js'; | |
| import type { UpdateModelOptionsFunction, GetModelOptionsFunction, AddCustomModelOptionFunction, RemoveCustomModelOptionFunction, ModelOption } from '../types.js'; | |
| import { LLMClient } from '../../../LLM/LLMClient.js'; | |
| import { createLogger } from '../../../core/Logger.js'; | |
| import { LLMClient } from '../../../LLM/LLMClient.js'; | |
| import { createLogger } from '../../../core/Logger.js'; | |
| import { createModelSelector } from '../components/ModelSelectorFactory.js'; | |
| import { i18nString, UIStrings } from '../i18n-strings.js'; | |
| import { getValidModelForProvider } from '../utils/validation.js'; | |
| import { getStorageItem, setStorageItem } from '../utils/storage.js'; | |
| import { OPENROUTER_API_KEY_STORAGE_KEY, MINI_MODEL_STORAGE_KEY, NANO_MODEL_STORAGE_KEY, OPENROUTER_MODELS_CACHE_DURATION_MS } from '../constants.js'; | |
| import type { UpdateModelOptionsFunction, GetModelOptionsFunction, AddCustomModelOptionFunction, RemoveCustomModelOptionFunction, ModelOption } from '../types.js'; | |
| import { BaseProviderSettings } from './BaseProviderSettings.js'; |
🧰 Tools
🪛 ESLint
[error] 5-5: There should be at least one empty line between import groups
(import/order)
[error] 6-6: ../components/ModelSelectorFactory.js import should occur before import of ./BaseProviderSettings.js
(import/order)
[error] 7-7: ../i18n-strings.js import should occur before import of ./BaseProviderSettings.js
(import/order)
[error] 8-8: ../utils/validation.js import should occur before import of ./BaseProviderSettings.js
(import/order)
[error] 9-9: ../utils/storage.js import should occur before import of ./BaseProviderSettings.js
(import/order)
[error] 10-10: ../constants.js import should occur before import of ./BaseProviderSettings.js
(import/order)
[error] 11-11: ../types.js type import should occur before import of ./BaseProviderSettings.js
(import/order)
[error] 12-12: ../../../LLM/LLMClient.js import should occur before import of ./BaseProviderSettings.js
(import/order)
[error] 13-13: ../../../core/Logger.js import should occur before import of ./BaseProviderSettings.js
(import/order)
🤖 Prompt for AI Agents
In front_end/panels/ai_chat/ui/settings/providers/OpenRouterSettings.ts around
lines 5 to 13, the imports are not grouped per project import/order rules;
reorder them so third-party and absolute/core imports come first (e.g.,
createLogger, LLMClient), then grouped internal modules (constants, types,
utils, i18n, components), and finally relative sibling imports; also keep
type-only imports grouped together and maintain alphabetic order within groups
where applicable to satisfy the lint rule.
| if (!this.oauthButton) return; | ||
|
|
||
| const OAuth = await this.getOpenRouterOAuth(); | ||
| this.oauthButton.disabled = true; | ||
|
|
||
| try { | ||
| if (await OAuth.isOAuthAuthenticated()) { | ||
| // Disconnect | ||
| if (confirm('Are you sure you want to disconnect your OpenRouter account?')) { | ||
| await OAuth.revokeToken(); | ||
| updateOAuthButton(); | ||
| } | ||
| } else { | ||
| // Connect - provide clear feedback for tab-based flow | ||
| this.oauthButton.textContent = 'Redirecting to OpenRouter...'; | ||
| if (this.oauthStatus) { | ||
| this.oauthStatus.textContent = 'You will be redirected to OpenRouter to authorize access. The page will return here automatically after authorization.'; | ||
| this.oauthStatus.style.color = 'var(--color-text-secondary)'; | ||
| this.oauthStatus.style.display = 'block'; | ||
| } | ||
|
|
||
| await OAuth.startAuthFlow(); | ||
| updateOAuthButton(); | ||
| } | ||
| } catch (error) { | ||
| logger.error('OAuth flow error:', error); | ||
| if (this.oauthStatus) { | ||
| this.oauthStatus.textContent = `Error: ${error instanceof Error ? error.message : 'Unknown error'}`; | ||
| this.oauthStatus.style.color = 'var(--color-accent-red)'; | ||
| this.oauthStatus.style.display = 'block'; | ||
| } | ||
| } finally { | ||
| this.oauthButton.disabled = false; | ||
| if (!await OAuth.isOAuthAuthenticated()) { | ||
| this.oauthButton.textContent = 'Connect with OpenRouter'; | ||
| if (this.oauthStatus) { | ||
| this.oauthStatus.style.display = 'none'; | ||
| } | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| // Handle OAuth events | ||
| this.handleOAuthSuccess = () => { | ||
| updateOAuthButton(); | ||
| if (this.oauthStatus) { | ||
| this.oauthStatus.textContent = '✓ Successfully connected to OpenRouter'; | ||
| this.oauthStatus.style.color = 'var(--color-accent-green)'; | ||
| this.oauthStatus.style.display = 'block'; | ||
| } | ||
|
|
||
| // Trigger chat panel refresh to recognize new credentials | ||
| const chatPanel = document.querySelector('ai-chat-panel') as any; | ||
| if (chatPanel && typeof chatPanel.refreshCredentials === 'function') { | ||
| chatPanel.refreshCredentials(); | ||
| } | ||
|
|
||
| // Auto-save settings and close dialog after successful OAuth | ||
| this.onSettingsSaved(); | ||
| setTimeout(() => { | ||
| this.onDialogHide(); | ||
| }, 2000); | ||
| }; | ||
|
|
||
| this.handleOAuthError = (event: Event) => { | ||
| const customEvent = event as CustomEvent; | ||
| if (this.oauthStatus) { | ||
| this.oauthStatus.textContent = `Error: ${customEvent.detail.error}`; | ||
| this.oauthStatus.style.color = 'var(--color-accent-red)'; | ||
| this.oauthStatus.style.display = 'block'; | ||
| } | ||
| }; | ||
|
|
||
| this.handleOAuthLogout = () => { | ||
| // Clear the API key input field | ||
| if (this.apiKeyInput) { | ||
| this.apiKeyInput.value = ''; | ||
| } | ||
|
|
||
| // Update OAuth button state | ||
| updateOAuthButton(); | ||
|
|
||
| // Show logout confirmation | ||
| if (this.oauthStatus) { | ||
| this.oauthStatus.textContent = '✓ Disconnected from OpenRouter'; | ||
| this.oauthStatus.style.color = 'var(--color-text-secondary)'; | ||
| this.oauthStatus.style.display = 'block'; | ||
| } | ||
|
|
||
| // Refresh chat panel to recognize credential removal | ||
| const chatPanel = document.querySelector('ai-chat-panel') as any; | ||
| if (chatPanel && typeof chatPanel.refreshCredentials === 'function') { | ||
| chatPanel.refreshCredentials(); | ||
| } | ||
|
|
||
| // Auto-close dialog after showing disconnect message | ||
| setTimeout(() => { | ||
| this.onDialogHide(); | ||
| }, 2000); | ||
| }; | ||
|
|
||
| window.addEventListener('openrouter-oauth-success', this.handleOAuthSuccess); | ||
| window.addEventListener('openrouter-oauth-error', this.handleOAuthError); | ||
| window.addEventListener('openrouter-oauth-logout', this.handleOAuthLogout); | ||
|
|
||
| // Update API key input behavior for OAuth compatibility | ||
| this.apiKeyInput.addEventListener('input', async () => { | ||
| if (!this.apiKeyInput) return; | ||
|
|
||
| if (this.apiKeyInput.value.trim()) { | ||
| // Switch to manual API key method | ||
| localStorage.setItem('openrouter_auth_method', 'api_key'); | ||
| const OAuth = await this.getOpenRouterOAuth(); | ||
| if (await OAuth.isOAuthAuthenticated()) { | ||
| OAuth.switchToManualApiKey(); | ||
| } | ||
| } | ||
|
|
||
| // Update fetch button state | ||
| if (this.fetchModelsButton) { | ||
| this.fetchModelsButton.disabled = !this.apiKeyInput.value.trim(); | ||
| } | ||
| }); | ||
|
|
||
| // Fetch OpenRouter models button | ||
| const openrouterFetchButtonContainer = document.createElement('div'); | ||
| openrouterFetchButtonContainer.className = 'fetch-button-container'; | ||
| openrouterSettingsSection.appendChild(openrouterFetchButtonContainer); | ||
|
|
||
| this.fetchModelsButton = document.createElement('button'); | ||
| this.fetchModelsButton.className = 'settings-button'; | ||
| this.fetchModelsButton.setAttribute('type', 'button'); | ||
| this.fetchModelsButton.textContent = i18nString(UIStrings.fetchOpenRouterModelsButton); | ||
| this.fetchModelsButton.disabled = !this.apiKeyInput.value.trim(); | ||
| openrouterFetchButtonContainer.appendChild(this.fetchModelsButton); | ||
|
|
||
| this.fetchModelsStatus = document.createElement('div'); | ||
| this.fetchModelsStatus.className = 'settings-status'; | ||
| this.fetchModelsStatus.style.display = 'none'; | ||
| openrouterFetchButtonContainer.appendChild(this.fetchModelsStatus); | ||
|
|
||
| // Add click handler for fetch OpenRouter models button | ||
| this.fetchModelsButton.addEventListener('click', async () => { | ||
| if (!this.fetchModelsButton || !this.fetchModelsStatus || !this.apiKeyInput) return; | ||
|
|
||
| this.fetchModelsButton.disabled = true; | ||
| this.fetchModelsStatus.textContent = i18nString(UIStrings.fetchingModels); | ||
| this.fetchModelsStatus.style.display = 'block'; | ||
| this.fetchModelsStatus.style.backgroundColor = 'var(--color-accent-blue-background)'; | ||
| this.fetchModelsStatus.style.color = 'var(--color-accent-blue)'; | ||
|
|
||
| try { | ||
| const openrouterApiKey = this.apiKeyInput.value.trim(); | ||
|
|
||
| // Fetch OpenRouter models using LLMClient static method | ||
| const openrouterModels = await LLMClient.fetchOpenRouterModels(openrouterApiKey); | ||
|
|
||
| // Convert OpenRouter models to ModelOption format | ||
| const modelOptions: ModelOption[] = openrouterModels.map(model => ({ | ||
| value: model.id, | ||
| label: model.name || model.id, | ||
| type: 'openrouter' as const | ||
| })); | ||
|
|
||
| // Update model options with fetched OpenRouter models | ||
| this.updateModelOptions(modelOptions, false); | ||
|
|
||
| // Update timestamp for cache management | ||
| localStorage.setItem('openrouter_models_cache_timestamp', Date.now().toString()); | ||
|
|
||
| const actualModelCount = openrouterModels.length; | ||
|
|
||
| // Update the model selectors with the new models | ||
| await this.updateModelSelectors(); | ||
|
|
||
| // Update status to show success | ||
| this.fetchModelsStatus.textContent = i18nString(UIStrings.fetchedModels, {PH1: actualModelCount}); | ||
| this.fetchModelsStatus.style.backgroundColor = 'var(--color-accent-green-background)'; | ||
| this.fetchModelsStatus.style.color = 'var(--color-accent-green)'; | ||
|
|
||
| logger.debug(`Successfully fetched ${actualModelCount} OpenRouter models`); | ||
| } catch (error) { | ||
| logger.error('Error fetching OpenRouter models:', error); | ||
| this.fetchModelsStatus.textContent = `Error: ${error instanceof Error ? error.message : 'Unknown error'}`; | ||
| this.fetchModelsStatus.style.backgroundColor = 'var(--color-accent-red-background)'; | ||
| this.fetchModelsStatus.style.color = 'var(--color-accent-red)'; | ||
| } finally { | ||
| this.fetchModelsButton.disabled = false; | ||
|
|
||
| // Hide status message after 3 seconds | ||
| setTimeout(() => { | ||
| if (this.fetchModelsStatus) { | ||
| this.fetchModelsStatus.style.display = 'none'; | ||
| } | ||
| }, 3000); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Wrap single-line conditionals in braces.
Statements like if (!this.oauthButton) return; (and similar checks later) violate the curly lint rule and keep the branch ambiguous. Add braces to each of these guard clauses.
- this.oauthButton.addEventListener('click', async () => {
- if (!this.oauthButton) return;
+ this.oauthButton.addEventListener('click', async () => {
+ if (!this.oauthButton) {
+ return;
+ }
@@
- this.apiKeyInput.addEventListener('input', async () => {
- if (!this.apiKeyInput) return;
+ this.apiKeyInput.addEventListener('input', async () => {
+ if (!this.apiKeyInput) {
+ return;
+ }
@@
- this.fetchModelsButton.addEventListener('click', async () => {
- if (!this.fetchModelsButton || !this.fetchModelsStatus || !this.apiKeyInput) return;
+ this.fetchModelsButton.addEventListener('click', async () => {
+ if (!this.fetchModelsButton || !this.fetchModelsStatus || !this.apiKeyInput) {
+ return;
+ }Repeat the pattern for the other guard clauses in this block. As per coding guidelines.
🧰 Tools
🪛 ESLint
[error] 193-193: Expected { after 'if' condition.
(curly)
[error] 245-245: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 283-283: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 300-300: Expected { after 'if' condition.
(curly)
[error] 336-336: Expected { after 'if' condition.
(curly)
| // Trigger chat panel refresh to recognize new credentials | ||
| const chatPanel = document.querySelector('ai-chat-panel') as any; | ||
| if (chatPanel && typeof chatPanel.refreshCredentials === 'function') { | ||
| chatPanel.refreshCredentials(); | ||
| } | ||
|
|
||
| // Auto-save settings and close dialog after successful OAuth | ||
| this.onSettingsSaved(); | ||
| setTimeout(() => { | ||
| this.onDialogHide(); | ||
| }, 2000); | ||
| }; | ||
|
|
||
| this.handleOAuthError = (event: Event) => { | ||
| const customEvent = event as CustomEvent; | ||
| if (this.oauthStatus) { | ||
| this.oauthStatus.textContent = `Error: ${customEvent.detail.error}`; | ||
| this.oauthStatus.style.color = 'var(--color-accent-red)'; | ||
| this.oauthStatus.style.display = 'block'; | ||
| } | ||
| }; | ||
|
|
||
| this.handleOAuthLogout = () => { | ||
| // Clear the API key input field | ||
| if (this.apiKeyInput) { | ||
| this.apiKeyInput.value = ''; | ||
| } | ||
|
|
||
| // Update OAuth button state | ||
| updateOAuthButton(); | ||
|
|
||
| // Show logout confirmation | ||
| if (this.oauthStatus) { | ||
| this.oauthStatus.textContent = '✓ Disconnected from OpenRouter'; | ||
| this.oauthStatus.style.color = 'var(--color-text-secondary)'; | ||
| this.oauthStatus.style.display = 'block'; | ||
| } | ||
|
|
||
| // Refresh chat panel to recognize credential removal | ||
| const chatPanel = document.querySelector('ai-chat-panel') as any; | ||
| if (chatPanel && typeof chatPanel.refreshCredentials === 'function') { | ||
| chatPanel.refreshCredentials(); | ||
| } | ||
|
|
||
| // Auto-close dialog after showing disconnect message | ||
| setTimeout(() => { | ||
| this.onDialogHide(); | ||
| }, 2000); |
There was a problem hiding this comment.
Avoid any when accessing ai-chat-panel.
Casting document.querySelector('ai-chat-panel') to any still violates lint. Narrow to the specific shape you need.
- const chatPanel = document.querySelector('ai-chat-panel') as any;
- if (chatPanel && typeof chatPanel.refreshCredentials === 'function') {
- chatPanel.refreshCredentials();
- }
+ const chatPanel = document.querySelector('ai-chat-panel') as (HTMLElement & { refreshCredentials?: () => void }) | null;
+ chatPanel?.refreshCredentials?.();Apply the same fix in the logout handler. As per coding guidelines.
📝 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.
| // Trigger chat panel refresh to recognize new credentials | |
| const chatPanel = document.querySelector('ai-chat-panel') as any; | |
| if (chatPanel && typeof chatPanel.refreshCredentials === 'function') { | |
| chatPanel.refreshCredentials(); | |
| } | |
| // Auto-save settings and close dialog after successful OAuth | |
| this.onSettingsSaved(); | |
| setTimeout(() => { | |
| this.onDialogHide(); | |
| }, 2000); | |
| }; | |
| this.handleOAuthError = (event: Event) => { | |
| const customEvent = event as CustomEvent; | |
| if (this.oauthStatus) { | |
| this.oauthStatus.textContent = `Error: ${customEvent.detail.error}`; | |
| this.oauthStatus.style.color = 'var(--color-accent-red)'; | |
| this.oauthStatus.style.display = 'block'; | |
| } | |
| }; | |
| this.handleOAuthLogout = () => { | |
| // Clear the API key input field | |
| if (this.apiKeyInput) { | |
| this.apiKeyInput.value = ''; | |
| } | |
| // Update OAuth button state | |
| updateOAuthButton(); | |
| // Show logout confirmation | |
| if (this.oauthStatus) { | |
| this.oauthStatus.textContent = '✓ Disconnected from OpenRouter'; | |
| this.oauthStatus.style.color = 'var(--color-text-secondary)'; | |
| this.oauthStatus.style.display = 'block'; | |
| } | |
| // Refresh chat panel to recognize credential removal | |
| const chatPanel = document.querySelector('ai-chat-panel') as any; | |
| if (chatPanel && typeof chatPanel.refreshCredentials === 'function') { | |
| chatPanel.refreshCredentials(); | |
| } | |
| // Auto-close dialog after showing disconnect message | |
| setTimeout(() => { | |
| this.onDialogHide(); | |
| }, 2000); | |
| // Trigger chat panel refresh to recognize new credentials | |
| const chatPanel = document.querySelector('ai-chat-panel') as (HTMLElement & { refreshCredentials?: () => void }) | null; | |
| chatPanel?.refreshCredentials?.(); | |
| // Auto-save settings and close dialog after successful OAuth | |
| this.onSettingsSaved(); | |
| setTimeout(() => { | |
| this.onDialogHide(); | |
| }, 2000); | |
| }; | |
| this.handleOAuthError = (event: Event) => { | |
| const customEvent = event as CustomEvent; | |
| if (this.oauthStatus) { | |
| this.oauthStatus.textContent = `Error: ${customEvent.detail.error}`; | |
| this.oauthStatus.style.color = 'var(--color-accent-red)'; | |
| this.oauthStatus.style.display = 'block'; | |
| } | |
| }; | |
| this.handleOAuthLogout = () => { | |
| // Clear the API key input field | |
| if (this.apiKeyInput) { | |
| this.apiKeyInput.value = ''; | |
| } | |
| // Update OAuth button state | |
| updateOAuthButton(); | |
| // Show logout confirmation | |
| if (this.oauthStatus) { | |
| this.oauthStatus.textContent = '✓ Disconnected from OpenRouter'; | |
| this.oauthStatus.style.color = 'var(--color-text-secondary)'; | |
| this.oauthStatus.style.display = 'block'; | |
| } | |
| // Refresh chat panel to recognize credential removal | |
| const chatPanel = document.querySelector('ai-chat-panel') as (HTMLElement & { refreshCredentials?: () => void }) | null; | |
| chatPanel?.refreshCredentials?.(); | |
| // Auto-close dialog after showing disconnect message | |
| setTimeout(() => { | |
| this.onDialogHide(); | |
| }, 2000); |
🧰 Tools
🪛 ESLint
[error] 245-245: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 283-283: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🤖 Prompt for AI Agents
front_end/panels/ai_chat/ui/settings/providers/OpenRouterSettings.ts lines
244-291: the code casts document.querySelector('ai-chat-panel') to any which
violates lint; instead declare a narrow interface (e.g. { refreshCredentials?:
() => void }) and use a safe typed cast or type guard so you treat the element
as that interface only when it exists, then call refreshCredentials(); apply the
same change in the logout handler so both querySelector calls avoid any and use
the specific shape needed.
Summary of what we fixed:
- BaseProviderSettings: Logger path (already correct)
- BrowsingHistorySettings: Changed ../../../../ to ../../../../../ for Common import
Summary by CodeRabbit