front_end/panels/ai_chat/ui/FileContentViewer.ts (2)

8-8: Fix import order.

Per ESLint, SDK imports should occur before local imports.

-import { createLogger } from '../core/Logger.js';
-import type { FileSummary } from '../tools/FileStorageManager.js';
-import * as Marked from '../../../third_party/marked/marked.js';
-import * as SDK from '../../../core/sdk/sdk.js';
+import * as SDK from '../../../core/sdk/sdk.js';
+
+import { createLogger } from '../core/Logger.js';
+import type { FileSummary } from '../tools/FileStorageManager.js';
+import * as Marked from '../../../third_party/marked/marked.js';

---

315-315: Remove inferrable type annotations.

Per ESLint, the boolean type annotations are trivially inferrable from the default values.

-  private static async buildHTML(file: FileSummary, content: string, editable: boolean = false): Promise<string> {
+  private static async buildHTML(file: FileSummary, content: string, editable = false): Promise<string> {

-  private static buildCSS(editable: boolean = false): string {
+  private static buildCSS(editable = false): string {

-  private static buildJS(fileName: string, content: string, editable: boolean = false): string {
+  private static buildJS(fileName: string, content: string, editable = false): string {

Also applies to: 398-398, 964-964

front_end/panels/ai_chat/tools/FileStorageManager.ts (1)

51-54: Clarify behavior for files created before setSessionId is called

Initializing sessionId to a temp UUID is safer than a global default, but it changes semantics: any files created before setSessionId() runs will stay under the temp session and become invisible once the real conversation ID is set.

If tools can create files before the conversation/session ID is known, you may want either:

• to migrate temp-session files into the real session on setSessionId, or
• to explicitly prevent file operations until a stable sessionId is established.

Otherwise you risk silently orphaning files in IndexedDB.

front_end/panels/ai_chat/ui/settings/utils/styles.ts (1)

505-601: Memory blocks styles look consistent and scoped

The new .memory-blocks-* styles are self-contained, reuse existing design tokens, and include hover/focus affordances on the clickable item. No functional or styling red flags from this change.

front_end/panels/ai_chat/agent_framework/implementation/ConfiguredAgents.ts (1)

11-11: Adjust memory import ordering to satisfy ESLint

The memory utilities import is correct, but ESLint flags its position relative to ../../tools/FetcherTool.js.

Consider moving this line above the tool imports block, e.g.:

-import { FetcherTool } from '../../tools/FetcherTool.js';
+import { SearchMemoryTool, UpdateMemoryTool, ListMemoryBlocksTool, createMemoryAgentConfig } from '../../memory/index.js';
+import { FetcherTool } from '../../tools/FetcherTool.js';

(and keep the rest of the tool imports following FetcherTool).

That should resolve the import/order violation without changing behavior.

front_end/panels/ai_chat/ui/SettingsDialog.ts (2)

29-37: Reorder MemorySettings import to satisfy ESLint rules

ESLint reports:

• import/first: the MemorySettings import appears after a non-import (export { isVectorDBEnabled };).
• import/order: it should come before ./settings/i18n-strings.js.

A minimal fix is:

-// Import settings utilities
-import { i18nString, UIStrings } from './settings/i18n-strings.js';
-import { PROVIDER_SELECTION_KEY, MINI_MODEL_STORAGE_KEY, NANO_MODEL_STORAGE_KEY, ADVANCED_SETTINGS_ENABLED_KEY } from './settings/constants.js';
-import { applySettingsStyles } from './settings/utils/styles.js';
-import { isVectorDBEnabled } from './settings/utils/storage.js';
+// Import settings utilities
+import { MemorySettings } from './settings/advanced/MemorySettings.js';
+import { i18nString, UIStrings } from './settings/i18n-strings.js';
+import { PROVIDER_SELECTION_KEY, MINI_MODEL_STORAGE_KEY, NANO_MODEL_STORAGE_KEY, ADVANCED_SETTINGS_ENABLED_KEY } from './settings/constants.js';
+import { applySettingsStyles } from './settings/utils/styles.js';
+import { isVectorDBEnabled } from './settings/utils/storage.js';
+export { isVectorDBEnabled };
-
-// Re-export for backward compatibility
-export { isVectorDBEnabled };
+// Re-export for backward compatibility

(or equivalent ordering), ensuring all imports come before the re-export.

---

467-508: Memory settings section is integrated consistently with advanced settings

• memorySection is created and classed like other advanced sections, and MemorySettings is instantiated and rendered into it.
• memorySettings is added to advancedFeatures, so its cleanup() will run with the others when the dialog is closed.
• toggleAdvancedSections now includes memorySection, so the advanced toggle appropriately controls its visibility.

Given MemorySettings.save() is a no-op (auto-saves on change), it’s reasonable that the save button only calls vectorDBSettings.save(), tracingSettings.save(), and evaluationSettings.save().

No functional issues from this wiring.

Also applies to: 514-522

front_end/panels/ai_chat/core/PageInfoManager.ts (1)

9-9: Fix import order per ESLint rules.

The import should be grouped with other relative imports and ordered before ../tools/VisitHistoryManager.js.

 import * as SDK from '../../../core/sdk/sdk.js';
 import * as Utils from '../common/utils.js'; // Path relative to core/ assuming utils.ts will be in common/ later, this will be common/utils.js
+import { MemoryBlockManager } from '../memory/index.js';
 import { VisitHistoryManager } from '../tools/VisitHistoryManager.js'; // Path relative to core/ assuming VisitHistoryManager.ts will be in core/
 import { FileStorageManager } from '../tools/FileStorageManager.js';
-import { MemoryBlockManager } from '../memory/index.js';
 import { createLogger } from './Logger.js';

front_end/panels/ai_chat/ui/settings/utils/validation.ts (1)

7-7: Fix import order per ESLint rules.

The import should be ordered before ../../AIChatPanel.js.

+import { findClosestModel } from '../../../LLM/FuzzyModelMatcher.js';
 import { DEFAULT_PROVIDER_MODELS } from '../../AIChatPanel.js';
 import type { ModelOption, ProviderType, ModelTier } from '../types.js';
-import { findClosestModel } from '../../../LLM/FuzzyModelMatcher.js';

front_end/panels/ai_chat/agent_framework/implementation/agents/SearchMemoryAgent.ts (1)

5-9: Fix import ordering and consolidate type imports.

ESLint reports multiple issues: type imports should be consolidated, and import groups need proper ordering.

-import type { AgentToolConfig, ConfigurableAgentArgs } from '../../ConfigurableAgentTool.js';
-import { ChatMessageEntity } from '../../../models/ChatTypes.js';
-import type { ChatMessage } from '../../../models/ChatTypes.js';
-import { MODEL_SENTINELS } from '../../../core/Constants.js';
-import { AGENT_VERSION } from './AgentVersion.js';
+import { ChatMessageEntity, type ChatMessage } from '../../../models/ChatTypes.js';
+import { MODEL_SENTINELS } from '../../../core/Constants.js';
+
+import type { AgentToolConfig, ConfigurableAgentArgs } from '../../ConfigurableAgentTool.js';
+import { AGENT_VERSION } from './AgentVersion.js';

front_end/panels/ai_chat/agent_framework/implementation/agents/MemoryAgent.ts (1)

5-10: Consolidate duplicate imports and fix ordering.

ESLint correctly identifies that ConfigurableAgentTool.js is imported twice and type imports should be consolidated.

-import type { AgentToolConfig } from '../../ConfigurableAgentTool.js';
-import { ChatMessageEntity } from '../../../models/ChatTypes.js';
-import type { ChatMessage } from '../../../models/ChatTypes.js';
-import type { ConfigurableAgentArgs } from '../../ConfigurableAgentTool.js';
-import { MODEL_SENTINELS } from '../../../core/Constants.js';
-import { AGENT_VERSION } from './AgentVersion.js';
+import { ChatMessageEntity, type ChatMessage } from '../../../models/ChatTypes.js';
+import { MODEL_SENTINELS } from '../../../core/Constants.js';
+
+import type { AgentToolConfig, ConfigurableAgentArgs } from '../../ConfigurableAgentTool.js';
+import { AGENT_VERSION } from './AgentVersion.js';

front_end/panels/ai_chat/ui/settings/advanced/MemorySettings.ts (2)

5-10: Fix import ordering per ESLint rules.

Type imports should come after value imports from the same module group.

+import type { MemoryBlock } from '../../../memory/types.js';
+import type { FileSummary } from '../../../tools/FileStorageManager.js';
+
 import { i18nString, UIStrings } from '../i18n-strings.js';
 import { MEMORY_ENABLED_KEY } from '../constants.js';
 import { MemoryBlockManager } from '../../../memory/MemoryBlockManager.js';
 import { FileContentViewer } from '../../FileContentViewer.js';
-import type { MemoryBlock } from '../../../memory/types.js';
-import type { FileSummary } from '../../../tools/FileStorageManager.js';

---

128-133: Prefix unused error variable to satisfy ESLint.

The error is caught but not used. Either log it or prefix with underscore.

-    } catch (error) {
+    } catch (_error) {
       const errorMessage = document.createElement('div');
       errorMessage.className = 'memory-blocks-error';
       errorMessage.textContent = 'Failed to load memory blocks.';
       this.blockListContainer.appendChild(errorMessage);
     }

front_end/panels/ai_chat/core/BaseOrchestratorAgent.ts (1)

38-38: Reorder import to satisfy ESLint.

Per static analysis, this import should be ordered before the tool imports (before line 17).

+import { MemoryModule } from '../memory/index.js';
 // Direct imports from Tools.ts
 import { ToolRegistry } from '../agent_framework/ConfigurableAgentTool.js';
 ...
-import { MemoryModule } from '../memory/index.js';

front_end/panels/ai_chat/LLM/OpenAIProvider.ts (1)

422-456: Good dynamic model fetching with robust filtering.

The implementation correctly filters to chat/reasoning models and excludes non-chat variants. Consider using a typed interface for the API response to improve type safety.

+interface OpenAIModelListResponse {
+  data: Array<{ id: string; object: string; created: number; owned_by: string }>;
+}
+
 async fetchModels(): Promise<Array<{id: string, name: string}>> {
   // ...
-  const data = await response.json();
+  const data: OpenAIModelListResponse = await response.json();
   // ...
   return data.data
-    .filter((model: any) => {
+    .filter((model) => {

front_end/panels/ai_chat/memory/SearchMemoryTool.ts (1)

43-71: Consider reusing MemoryBlockManager instance.

A new MemoryBlockManager is created on each execute call. If the manager has initialization overhead, consider caching it as an instance property. Also, prefer unknown over any for the catch clause.

 export class SearchMemoryTool implements Tool<SearchMemoryArgs, SearchMemoryResult> {
   name = 'search_memory';
   description = '...';
+  private manager = new MemoryBlockManager();

   async execute(args: SearchMemoryArgs, _ctx?: LLMContext): Promise<SearchMemoryResult> {
     logger.info('Executing search memory', { query: args.query });

     try {
-      const manager = new MemoryBlockManager();
-      const searchResults = await manager.searchBlocks(args.query);
+      const searchResults = await this.manager.searchBlocks(args.query);
       // ...
-    } catch (error: any) {
+    } catch (error: unknown) {
+      const message = error instanceof Error ? error.message : 'Failed to search memory.';
       logger.error('Failed to search memory', { error: error?.message });
       return {
         success: false,
         results: [],
         count: 0,
-        error: error?.message || 'Failed to search memory.'
+        error: message
       };
     }
   }
 }

front_end/panels/ai_chat/core/AgentService.ts (5)

32-33: Fix import ordering per ESLint rules.

The static analysis correctly identifies that these imports should appear earlier in the import list (before ../models/ChatTypes.js).

+import { CustomProviderManager } from './CustomProviderManager.js';
+import { ToolRegistry } from '../agent_framework/ConfigurableAgentTool.js';
+import { MemoryModule } from '../memory/index.js';
 import * as Common from '../../../core/common/common.js';
 // ... other imports
-import { ToolRegistry } from '../agent_framework/ConfigurableAgentTool.js';
-import { MemoryModule } from '../memory/index.js';

---

170-175: Consider adding error handling for background memory processing.

The async call to processUnprocessedConversations() is not awaited and has no .catch() handler, which could lead to unhandled promise rejections if the method throws.

     // Process any old conversations that missed memory extraction
     // Delay to avoid blocking startup and ensure tools are registered
     setTimeout(() => {
-      this.processUnprocessedConversations();
+      this.processUnprocessedConversations().catch(err => {
+        logger.error('[Memory] Failed to process unprocessed conversations on startup:', err);
+      });
     }, 5000);

---

928-933: Add error handling for background memory extraction.

Similar to startup processing, the background memory extraction call lacks error handling which could result in unhandled promise rejections.

     // Fire off memory extraction in background (non-blocking)
     if (endingConversationId) {
-      this.#processConversationMemory(endingConversationId);
+      this.#processConversationMemory(endingConversationId).catch(err => {
+        logger.error('[Memory] Background memory extraction failed:', err);
+      });
     }

---

1005-1014: Avoid any type in toolCalls mapping.

The ESLint error correctly identifies the use of any on line 1013. Consider defining a proper type for the tool call structure.

         toolCallsCount: result.toolCalls?.length || 0,
-        toolCalls: result.toolCalls?.map((tc: any) => ({ name: tc.name, args: tc.args })) || [],
+        toolCalls: result.toolCalls?.map((tc: { name: string; args: unknown }) => ({ name: tc.name, args: tc.args })) || [],

---

1037-1040: Add error handling for parallel memory processing.

Each call to #processConversationMemory is fired without error handling. Consider using Promise.allSettled for proper parallel execution with error handling.

-    for (const conv of toProcess) {
-      // Don't await - process in parallel
-      this.#processConversationMemory(conv.id);
-    }
+    // Process in parallel with error handling
+    await Promise.allSettled(
+      toProcess.map(conv => this.#processConversationMemory(conv.id))
+    );

front_end/panels/ai_chat/LLM/FuzzyModelMatcher.ts (2)

86-93: Remove redundant type annotation.

ESLint correctly identifies that the type number is trivially inferred from the default value 0.5.

 export function findClosestModel(
   targetModel: string,
   availableModels: string[],
-  threshold: number = 0.5
+  threshold = 0.5
 ): string | null {

---

139-146: Remove redundant type annotation and consider reducing duplication.

Same type inference issue as findClosestModel. Also, there's code duplication between these two functions - consider having findClosestModel delegate to findClosestModelWithInfo.

 export function findClosestModelWithInfo(
   targetModel: string,
   availableModels: string[],
-  threshold: number = 0.5
+  threshold = 0.5
 ): FuzzyMatchResult {

Optionally, reduce duplication:

export function findClosestModel(
  targetModel: string,
  availableModels: string[],
  threshold = 0.5
): string | null {
  return findClosestModelWithInfo(targetModel, availableModels, threshold).match;
}

front_end/panels/ai_chat/LLM/LLMProviderRegistry.ts (4)

17-18: Fix import ordering per ESLint rules.

The new imports should be grouped and ordered according to the project's import conventions.

---

122-136: LGTM - Custom provider routing is well-implemented.

Good pattern of checking for custom providers first and routing to GenericOpenAIProvider with config from CustomProviderManager. Minor: remove the redundant type annotation on line 124 (apiKey: string = '' → apiKey = '').

   private static createTemporaryProvider(
     providerType: LLMProvider,
-    apiKey: string = '',
+    apiKey = '',
     endpoint?: string
   ): LLMProviderInterface | null {

---

356-370: Replace any types with proper interfaces for model data.

Multiple uses of any (lines 359, 384, 385, 387) compromise type safety. Consider defining an interface for the model response structure.

+interface RawModelInfo {
+  id?: string;
+  name?: string;
+  capabilities?: Record<string, unknown>;
+}

       try {
         if (typeof provider.fetchModels === 'function') {
           const models = await provider.fetchModels();
-          return models.map((m: any) => ({
+          return models.map((m: RawModelInfo) => ({
             id: m.id || m.name,
             name: m.name || m.id,
             provider: providerType,
             ...(m.capabilities ? { capabilities: m.capabilities } : {}),
-          }));
+          })) as ModelInfo[];
         }

---

381-393: Same type safety issue for built-in provider model mapping.

Apply the same RawModelInfo interface fix here to maintain consistency and type safety.

       // Use fetchModels() if available - it throws on API errors (good for validation)
       // Fall back to getModels() which may swallow errors and return defaults
-      if (typeof (provider as any).fetchModels === 'function') {
-        const models = await (provider as any).fetchModels();
+      if ('fetchModels' in provider && typeof provider.fetchModels === 'function') {
+        const models = await provider.fetchModels();
         // Convert to ModelInfo format if needed
-        return models.map((m: any) => ({
+        return models.map((m: RawModelInfo) => ({
           id: m.id || m.name,
           name: m.name || m.id,
           provider: providerType,
           ...(m.capabilities ? { capabilities: m.capabilities } : {}),
         }));
       }

front_end/panels/ai_chat/memory/ListMemoryBlocksTool.ts (2)

11-13: Replace empty interface with type alias or explicit empty object type.

ESLint and Biome correctly flag that an empty interface allows any non-nullish value. Use a more explicit type.

-export interface ListMemoryBlocksArgs {
-  // No arguments needed
-}
+export type ListMemoryBlocksArgs = Record<string, never>;

---

79-80: Avoid any type in error handling.

Replace any with unknown and use type narrowing for safer error handling.

-    } catch (error: any) {
-      logger.error('Failed to list memory blocks', { error: error?.message });
+    } catch (error: unknown) {
+      const message = error instanceof Error ? error.message : String(error);
+      logger.error('Failed to list memory blocks', { error: message });
       return {
         success: false,
         blocks: [],
         summary: {
           totalBlocks: 0,
           totalChars: 0,
-          maxChars: 9500
+          maxChars: 120000
         },
-        error: error?.message || 'Failed to list memory blocks.'
+        error: message || 'Failed to list memory blocks.'
       };

front_end/panels/ai_chat/memory/UpdateMemoryTool.ts (1)

86-93: Avoid any type in error handling.

Replace any with unknown for type-safe error handling, consistent with TypeScript best practices.

-    } catch (error: any) {
-      logger.error('Failed to update memory block', { error: error?.message });
+    } catch (error: unknown) {
+      const message = error instanceof Error ? error.message : String(error);
+      logger.error('Failed to update memory block', { error: message });
       return {
         success: false,
-        message: error?.message || 'Failed to update memory block',
-        error: error?.message || 'Failed to update memory block'
+        message: message || 'Failed to update memory block',
+        error: message || 'Failed to update memory block'
       };
     }

front_end/panels/ai_chat/memory/MemoryModule.ts (1)

72-74: Consider returning a defensive copy of config.

getConfig() returns a direct reference to the internal this.config object. External callers could mutate it, potentially causing unexpected behavior. Since the config is meant to be constant, consider returning a shallow copy.

   getConfig(): MemoryConfig {
-    return this.config;
+    return { ...this.config, blockLimits: { ...this.config.blockLimits } };
   }

front_end/panels/ai_chat/persistence/ConversationManager.ts (1)

245-253: Silent failure when conversation not found.

markMemoryCompleted silently does nothing if the conversation doesn't exist. Consider logging a warning or returning a boolean to indicate success/failure for better observability.

   async markMemoryCompleted(conversationId: string): Promise<void> {
     const conversation = await this.storageManager.loadConversation(conversationId);
     if (conversation) {
       conversation.memoryStatus = 'completed';
       conversation.memoryProcessedAt = Date.now();
       await this.storageManager.saveConversation(conversation);
       logger.info('Marked memory as completed', {conversationId});
+    } else {
+      logger.warn('Cannot mark memory completed - conversation not found', {conversationId});
     }
   }

front_end/panels/ai_chat/memory/MemoryBlockManager.ts (3)

5-8: Import order inconsistency.

Per ESLint, imports should be ordered alphabetically within groups. Consider reordering:

-import { FileStorageManager } from '../tools/FileStorageManager.js';
-import { createLogger } from '../core/Logger.js';
+import { createLogger } from '../core/Logger.js';
+import { FileStorageManager } from '../tools/FileStorageManager.js';

---

104-111: Remove unused error variable.

The caught error is defined but never used. Either log it or use underscore prefix to indicate intentional non-use.

-      } catch (error) {
+      } catch {
         // Ignore if file doesn't exist
         logger.debug('Block not found for deletion', { type, filename });
       }

---

189-201: Fallback type may mask malformed filenames.

When parseFilename encounters an unrecognized filename pattern, it silently falls back to type: 'facts'. This could hide bugs where files with unexpected naming conventions are incorrectly classified.

Consider logging a warning when the fallback is triggered:

     if (filename.startsWith('memory_project_')) {
       const projectName = filename.replace('memory_project_', '').replace('.md', '');
       return { type: 'project', projectName };
     }
+    logger.warn('Unrecognized memory filename, defaulting to facts', { filename });
     return { type: 'facts' }; // fallback

front_end/panels/ai_chat/ui/OnboardingDialog.ts (2)

236-240: Avoid (this as any) pattern for storing element references.

Using (this as any) to store button references bypasses TypeScript's type checking. Consider adding proper typed private fields instead:

+  private backButton: HTMLButtonElement | null = null;
+  private nextButton: HTMLButtonElement | null = null;
+  private skipButton: HTMLButtonElement | null = null;
   
   // In buildDialog:
-    (this as any).backButton = backButton;
-    (this as any).nextButton = nextButton;
-    (this as any).skipButton = skipButton;
+    this.backButton = backButton;
+    this.nextButton = nextButton;
+    this.skipButton = skipButton;

---

780-784: Magic number for auto-close timeout.

The 5-second delay before auto-closing is hardcoded. Consider extracting this to a named constant for clarity and maintainability:

+const AUTO_CLOSE_DELAY_MS = 5000;
+
   // Auto-close after 5 seconds
   setTimeout(() => {
     this.complete();
-  }, 5000);
+  }, AUTO_CLOSE_DELAY_MS);

front_end/panels/ai_chat/ui/AIChatPanel.ts (3)

17-17: Remove unused import CONFIG_MODEL_PLACEHOLDERS.

This import is defined but never used in the file.

 import {
   LLMConfigurationManager,
   type ModelOption,
   DEFAULT_PROVIDER_MODELS,
   DEFAULT_OPENAI_MODELS,
-  MODEL_PLACEHOLDERS as CONFIG_MODEL_PLACEHOLDERS,
 } from '../core/LLMConfigurationManager.js';

---

27-27: Remove unused import createSetupRequiredBanner.

The createSetupRequiredBanner function is imported but never used in this file.

-import { OnboardingDialog, createSetupRequiredBanner } from './OnboardingDialog.js';
+import { OnboardingDialog } from './OnboardingDialog.js';

---

122-134: Remove unused helper functions.

The functions getModelOptions() and getAllModelOptions() are defined but never called anywhere in this file. If they're only used internally through syncModelOptions(), consider removing them:

-// Helper to get MODEL_OPTIONS from LLMConfigurationManager
-function getModelOptions(): ModelOption[] {
-  return LLMConfigurationManager.getInstance().getModelOptionsForCurrentProvider();
-}
-
-// Helper to get all model options across all providers
-function getAllModelOptions(): ModelOption[] {
-  return LLMConfigurationManager.getInstance().getAllModelOptions();
-}

front_end/panels/ai_chat/core/LLMConfigurationManager.ts (2)

134-136: Use consistent generic type syntax.

ESLint suggests specifying generic type arguments as part of the constructor type arguments:

-  private modelOptionsByProvider: Map<string, ModelOption[]> = new Map();
+  private modelOptionsByProvider = new Map<string, ModelOption[]>();

---

408-412: Add curly braces around single-statement if.

For consistency and to prevent future bugs when adding statements:

-  validateModelSelection(model: string, provider?: string): boolean {
-    if (!model) return false;
+  validateModelSelection(model: string, provider?: string): boolean {
+    if (!model) {
+      return false;
+    }
     const options = this.getModelOptions(provider);
     return options.some(opt => opt.value === model);
   }

front_end/panels/ai_chat/persistence/MemoryBlockManager.ts (1)

129-132: Remove unused error variable (same as memory/ version).

-      } catch (error) {
+      } catch {
         // Ignore if file doesn't exist
         logger.debug('Block not found for deletion', { type, filename });
       }

front_end/panels/ai_chat/ui/OnboardingDialog.ts (1)

730-732: Unreachable code after try-catch block.

Lines 730-731 are unreachable because all execution paths in the preceding try-catch either return or throw. This dead code should be removed.

         }
         return;
       }
-
-      nextButton.disabled = false;
-      nextButton.textContent = 'Next';
     }

     if (currentIndex < STEPS.length - 1) {

front_end/panels/ai_chat/memory/MemoryBlockManager.ts (1)

226-247: Escape memory content/metadata before embedding in XML-like context

Both compileMemoryContext and getUserMemoryContext interpolate label, description, and content directly into XML‑like markup. If any of these contain <, >, or &, the resulting context can become malformed and break prompt parsing.

There is already prior feedback on compileMemoryContext; the same concern applies here and to user memory/project names. You can mitigate by introducing an XML escape helper and using it consistently:

   export class MemoryBlockManager {
+    private escapeXml(str: string): string {
+      return str
+        .replace(/&/g, '&')
+        .replace(/</g, '&lt;')
+        .replace(/>/g, '&gt;');
+    }
@@
-      context += `<${block.label}>\n`;
-      context += `<description>${block.description}</description>\n`;
-      context += `<content>\n${block.content}\n</content>\n`;
-      context += `</${block.label}>\n`;
+      const label = this.escapeXml(block.label);
+      context += `<${label}>\n`;
+      context += `<description>${this.escapeXml(block.description)}</description>\n`;
+      context += `<content>\n${this.escapeXml(block.content)}\n</content>\n`;
+      context += `</${label}>\n`;
@@
-      context += `<user>
-<description>${userBlock.description}</description>
-<content>
-${userBlock.content}
-</content>
-</user>\n`;
+      context += `<user>
+<description>${this.escapeXml(userBlock.description)}</description>
+<content>
+${this.escapeXml(userBlock.content)}
+</content>
+</user>\n`;
@@
-${projectBlocks.map(p => `<project name="${p.label.replace('project:', '')}" />`).join('\n')}
+${projectBlocks
+  .map(p => `<project name="${this.escapeXml(p.label.replace('project:', ''))}" />`)
+  .join('\n')}

This keeps the structure stable even with arbitrary user content.

Also applies to: 253-283

front_end/panels/ai_chat/ui/onboardingStyles.ts (2)

93-96: Consider using CSS variables for hardcoded brand colors.

Multiple instances use hardcoded #00a4fe color values. While other styles use CSS variables like --color-primary and --sys-color-*, these hardcoded values won't adapt to theme changes.

Consider defining a CSS variable for the brand color at the :host level or using existing variables:

     .step-indicator.active {
-      background: #00a4fe;
+      background: var(--color-primary, #00a4fe);
       transform: scale(1.3);
-      box-shadow: 0 0 12px rgba(0, 164, 254, 0.5);
+      box-shadow: 0 0 12px color-mix(in srgb, var(--color-primary, #00a4fe) 50%, transparent);
     }

Also applies to: 227-228, 269-269, 279-284

---

278-284: Selected provider card background uses hardcoded light color.

The .provider-card.selected uses background: #def1fb which is a light blue color. This will likely have poor contrast in dark themes.

     .provider-card.selected {
       border-color: #00a4fe;
-      background: #def1fb;
+      background: var(--color-primary-container, #def1fb);
       box-shadow:
         0 0 0 2px #00a4fe,
         0 8px 24px rgba(0, 164, 254, 0.25);
     }

front_end/panels/ai_chat/ui/OnboardingDialog.ts (2)

496-498: Static analysis XSS warning is a false positive here.

The innerHTML assignment uses provider.getKeyUrl which comes from the hardcoded PROVIDERS array defined in this file (lines 27-64). Since the URL values are developer-controlled constants, there's no XSS risk.

However, for defense in depth, consider using DOM APIs:

-    hint.innerHTML = `Don't have an API key? <a href="proxy.php?url=${provider.getKeyUrl}">Get one here</a>`;
+    hint.textContent = "Don't have an API key? ";
+    const link = document.createElement('a');
+    link.href = provider.getKeyUrl;
+    link.textContent = 'Get one here';
+    hint.appendChild(link);

---

750-788: Auto-close timeout is not cancelled if user interacts.

showSuccessAndClose() starts a 5-second timeout to auto-complete. If the dialog could be closed manually before the timeout fires (e.g., via keyboard), the timeout would still execute complete() on a potentially disposed dialog.

Consider storing the timeout ID and clearing it in complete():

+  private autoCloseTimeout: number | null = null;
   
   private showSuccessAndClose(): void {
     // ... existing code ...
     
-    setTimeout(() => {
+    this.autoCloseTimeout = window.setTimeout(() => {
       this.complete();
     }, 5000);
   }
   
   private complete(): void {
+    if (this.autoCloseTimeout) {
+      clearTimeout(this.autoCloseTimeout);
+      this.autoCloseTimeout = null;
+    }
     // ... existing code ...
   }

front_end/panels/ai_chat/agent_framework/AgentRunnerEventBus.ts (1)

28-31: Background-aware gating at event-bus level is clear but tightly couples bus to UI semantics

The isBackground flag and early-return are straightforward and backwards compatible, but they couple AgentRunnerEventBus to the notion of “background” work and suppress events for all listeners (including any non-UI consumers).

If you expect future non-UI listeners (logging, analytics, orchestration) to react to background runs, consider either:

• Moving the background check to callers and always dispatching here, or
• Including isBackground in the event payload and letting listeners decide what to ignore.

Not a blocker, just a design refinement to keep the bus generic.

front_end/panels/ai_chat/agent_framework/AgentRunner.ts (1)

426-440: Background flag wiring and progress-event gating mostly consistent; consider inheritance to handoffs/child agents

The introduction of background?: boolean in overrides and the derived isBackground flag, combined with passing isBackground into all emitProgress calls, gives you consistent suppression of agent-progress events for this AgentRunner instance. That looks correct and should avoid UI noise for background runs.

Two follow-up points you may want to tighten up:

1. Background inheritance into handoffs/child runs

   • run’s overrides includes background?: boolean, but executeHandoff’s overrides parameter type does not, and the LLM-triggered handoff call (lines 993-1007) as well as the max_iterations handoff (lines 1423-1437) never pass a background flag.
   • Similarly, when executing an agent as a tool (ConfigurableAgentTool path around lines 1115–1129), the call context does not receive a background flag here; whether the nested agent run is background is determined elsewhere.
   • Net effect: a background run that hands off or calls another agent-as-tool will not automatically treat the child as background for progress emissions unless some other layer explicitly passes background: true.

   If the intended semantics are “background all the way down” (parent + handoffs + child agents), consider:

   • Extending executeHandoff’s overrides type to { sessionId?: string; parentSessionId?: string; traceId?: string; background?: boolean }, and
   • Threading background through in the LLM handoff and max_iterations handoff call sites, and in the agent-as-tool execution context where appropriate.

2. Event-bus-level suppression affects all listeners
   Because isBackground is forwarded to AgentRunnerEventBus.emitProgress and gating happens there, any listener on 'agent-progress' (not just UI components) will miss background events. If that’s intentional, this is fine; if you foresee non-UI listeners wanting background telemetry, you might instead:

   • Skip calling emitProgress when isBackground is true (check in AgentRunner), or
   • Include isBackground inside data and let listeners decide.

Overall, the current behavior is coherent; these suggestions are about clarifying and future-proofing the background model.

Also applies to: 467-475, 488-512, 593-601, 836-845, 1015-1023, 1100-1112, 1320-1328, 1380-1388, 1451-1459, 1473-1481

front_end/panels/ai_chat/ui/settings/utils/styles.ts (1)

511-607: Memory block styles look good; consider small a11y tweaks

The block list layout, hover, and focus treatments are coherent and match the rest of the settings UI. Two minor follow‑ups to consider:

• Ensure the clickable .memory-block-item is rendered as a semantic control (<button>/<a>) or gets tabindex and appropriate ARIA roles in the markup so the cursor: pointer and :focus style correspond to actual keyboard focus.
• You may want a dedicated :focus-visible style (especially for .memory-block-delete) so keyboard users get a clear focus indicator without relying solely on hover.

front_end/panels/ai_chat/memory/MemoryBlockManager.ts (2)

119-147: Fallback typing on unknown memory_*.md files could hide issues

getAllBlocks+parseFilename treat any memory_*.md filename that isn’t a known pattern as type: 'facts'. If a future block type is added but parseFilename isn’t updated, those files will silently appear as facts.

Consider at least logging when the fallback branch in parseFilename is hit, or returning a distinct 'unknown' type so misconfigurations are easier to spot.

---

181-201: Filename slugging is sane but can collide for similar project names

getFilename lowercases the project name and replaces non‑[a-z0-9] with _. This is fine for most cases, but:

• Distinct project names like my-project and my project both map to memory_project_my_project.md.
• parseFilename only ever recovers the slug, not the original display name, so downstream consumers (labels, XML project names) see the slug.

If you expect many similarly named projects, you might want to incorporate an ID or hash into the filename, or store the human‑readable name inside the block content/metadata.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

front_end/panels/ai_chat/memory/MemoryBlockManager.ts (1)

236-251: XML injection concern already flagged.

The XML injection vulnerability in compileMemoryContext where block.label, block.description, and block.content are interpolated without escaping has been identified in a previous review. Please address the earlier feedback to escape special XML characters or use CDATA sections.

front_end/panels/ai_chat/ui/OnboardingDialog.ts (1)

527-560: Move API key save to after successful validation.

The API key is saved at line 541 before validation at lines 544-547. If validation fails, the invalid API key remains stored in localStorage. This creates a poor user experience and data integrity issue.

Apply this diff to save the key only after successful validation:

     try {
-      // Save API key temporarily for testing
       const provider = this.selectedProvider!;
-      LLMProviderRegistry.saveProviderApiKey(provider.id as any, this.apiKey);

       // Try to fetch models to verify the key works
       const providerInstance = LLMProviderRegistry.getProvider(provider.id as any);
       if (providerInstance && typeof (providerInstance as any).fetchModels === 'function') {
         await (providerInstance as any).fetchModels(this.apiKey);
       }

+      // Save API key only after successful validation
+      LLMProviderRegistry.saveProviderApiKey(provider.id as any, this.apiKey);
+
       statusDiv.className = 'test-status visible success';
       statusDiv.textContent = 'Connection successful! Your API key is valid.';

front_end/panels/ai_chat/Readme.md (1)

35-35: Add a clarifying comment for the redundant cd devtools-frontend step.

The second cd devtools-frontend on line 35 appears necessary (presumably because gclient sync changes the working directory), but this isn't obvious from the documentation. Adding a brief inline comment will improve clarity for users following these steps.

Apply this diff to clarify the purpose of the directory change:

  gclient sync
+ # Ensure we're in the devtools-frontend directory for the build
  cd devtools-frontend
  npm run build

front_end/panels/ai_chat/ui/OnboardingDialog.ts (1)

541-541: Consider improving type safety for provider IDs.

Multiple instances of provider.id as any bypass TypeScript's type checking when calling LLMProviderRegistry methods. While this works, it reduces type safety and could mask issues if provider IDs change.

If feasible, consider one of these approaches:

1. Ensure ProviderInfo.id type matches the type expected by LLMProviderRegistry
2. Add a type guard or validation function to verify provider IDs
3. Update LLMProviderRegistry to accept the ProviderInfo.id type directly

Also applies to: 544-546, 701-704, 711-711, 804-804

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

front_end/panels/ai_chat/ui/OnboardingDialog.ts (1)

527-560: API key saved before validation; invalid keys remain stored.

The testConnection method saves the API key at line 541 before validating it at lines 543-547. If validation fails, the invalid key remains in localStorage. This mirrors a past review comment that was marked as addressed but is still present.

Compare with goNext() (line 711), which correctly saves only after successful validation.

Apply this diff to save the API key only after successful validation:

     try {
-      // Save API key temporarily for testing
       const provider = this.selectedProvider!;
-      LLMProviderRegistry.saveProviderApiKey(provider.id as any, this.apiKey);

       // Try to fetch models to verify the key works
       const providerInstance = LLMProviderRegistry.getProvider(provider.id as any);
       if (providerInstance && typeof (providerInstance as any).fetchModels === 'function') {
         await (providerInstance as any).fetchModels(this.apiKey);
       }

+      // Save API key only after successful validation
+      LLMProviderRegistry.saveProviderApiKey(provider.id as any, this.apiKey);
+
       statusDiv.className = 'test-status visible success';

front_end/ui/visual_logging/KnownContextValues.ts (1)

832-839: Optional: keep connect* context keys alphabetically ordered

Non-functional, but for consistency with surrounding entries you might want to place 'connectors' before the 'connection*' keys:

-  'connect-workspace',
-  'connection',
-  'connection-id',
-  'connection-info',
-  'connectors',
-  'consent-onboarding',
+  'connect-workspace',
+  'connectors',
+  'connection',
+  'connection-id',
+  'connection-info',
+  'consent-onboarding',

front_end/panels/ai_chat/ui/settings/utils/styles.ts (1)

511-665: Memory block styles look solid; consider button focus states

The overall memory-blocks styling (container, items, confirmation UI, status messages) is coherent and matches the existing visual system. One small a11y improvement would be to add explicit :focus-visible styles for .memory-block-delete, .memory-block-confirm-yes, and .memory-block-confirm-no similar to .memory-block-item:focus so keyboard users get a clear focus indicator on the buttons themselves.

front_end/panels/ai_chat/ui/settings/advanced/MemorySettings.ts (1)

103-106: Localize user-facing strings for consistency with the rest of settings UI

Several user-visible strings here are hard-coded in English instead of going through UIStrings/i18nString, e.g.:

• 'Stored Memory'
• 'No memory blocks stored yet. Memory will be extracted from conversations automatically.'
• 'Failed to load memory blocks.'
• Inline delete confirmation text: 'Delete?', 'Yes', 'No'
• Date labels: 'today', 'yesterday', 'days ago'
• Error message in showStatusMessage calls.

For consistency with other settings panels (which already use UIStrings), it would be good to move these into i18n-strings.ts and reference them via i18nString(UIStrings.…). This can be a follow-up, but doing it now avoids future i18n cleanup work.

Also applies to: 117-121, 221-223, 286-320, 335-343, 391-392

front_end/panels/ai_chat/ui/OnboardingDialog.ts (2)

296-296: Static analysis XSS warnings are false positives here.

The static analysis tool flagged innerHTML usage, but these instances are safe:

• Line 296: Clearing content only
• Line 499: Uses hardcoded URLs from the static PROVIDERS array
• Line 844: Static string literal

However, as a general principle, prefer textContent for purely textual content (line 844) to maintain consistent security practices.

For line 844, you could use:

-  text.innerHTML = '⚠️ API key not configured. Set up a provider to start chatting.';
+  text.textContent = '⚠️ API key not configured. Set up a provider to start chatting.';

Also applies to: 499-499, 844-844

---

20-25: Consider stronger typing for provider IDs.

The ProviderInfo.id is currently typed as string, leading to multiple as any assertions when calling LLMProviderRegistry methods (lines 541, 544, 702, 711, 801).

Consider importing and using the LLMProvider type directly:

+import type { LLMProvider } from '../LLM/LLMProviderRegistry.js';
+
 interface ProviderInfo {
-  id: string;
+  id: LLMProvider;
   name: string;
   description: string;
   getKeyUrl: string;
 }

This would eliminate the need for type assertions throughout the file.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro