apps/frontend/src/renderer/components/AgentTools.tsx (1)

1426-1458: ⚠️ Potential issue | 🟠 Major

Always render the global MCP card container so retry actions remain available.

Line 1426 hides the whole section when allGlobalServers.length === 0, which also hides refresh and health-check controls after empty/failed loads.

Proposed fix direction

-          {allGlobalServers.length > 0 && (
+          {(isLoadingGlobalMcps || globalMcps !== null) && (
             <div className="rounded-lg border border-border bg-card p-4">
               <div className="flex items-center justify-between mb-4">
                 ...
               </div>
-              <div className="space-y-2">
-                {allGlobalServers.map((server) => {
+              {isLoadingGlobalMcps ? (
+                <p className="text-xs text-muted-foreground">{t('settings:mcp.globalMcps.loading')}</p>
+              ) : allGlobalServers.length === 0 ? (
+                <p className="text-xs text-muted-foreground">{t('settings:mcp.globalMcps.empty')}</p>
+              ) : (
+                <div className="space-y-2">
+                  {allGlobalServers.map((server) => {
                     ...
-                })}
-              </div>
+                  })}
+                </div>
+              )}
             </div>
           )}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/frontend/src/renderer/components/AgentTools.tsx` around lines 1426 -
1458, The MCP card is currently hidden when allGlobalServers.length === 0,
removing the retry/refresh controls; change rendering so the outer container
(the div with class "rounded-lg border...") always renders, while the list of
servers inside is conditional — keep the header and the Buttons (onClick
handlers checkGlobalMcpHealth and loadGlobalMcps, and states
isCheckingGlobalHealth/isLoadingGlobalMcps) visible at all times and only hide
or show the server list/content based on allGlobalServers.length; update the JSX
to move the conditional from the wrapper to the inner server-list section so
retry actions remain available when the list is empty.

apps/frontend/src/shared/types/ipc.ts (1)

458-459: ⚠️ Potential issue | 🟡 Minor

Include projectId in onIndexProgress payload for event scoping.

The onIndexProgress callback payload lacks a projectId field. When multiple projects are being indexed (or rapidly switched), progress events cannot be reliably associated with the correct project, potentially causing UI updates to be misrouted.

🔧 Suggested fix

-  onIndexProgress: (callback: (data: { message: string; current?: number; total?: number }) => void) => () => void;
+  onIndexProgress: (callback: (data: { projectId: string; message: string; current?: number; total?: number }) => void) => () => void;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/frontend/src/shared/types/ipc.ts` around lines 458 - 459, The
onIndexProgress callback payload is missing projectId which prevents scoping
progress events; update the IPC type signature for onIndexProgress to include
projectId: string in the payload (so its callback becomes (data: { projectId:
string; message: string; current?: number; total?: number }) => void) and then
adjust any emitters/listeners that use onIndexProgress and refreshProjectIndex
to pass and check projectId so progress events are correctly routed to the
matching project.

apps/frontend/src/renderer/components/AgentTools.tsx (3)

1425-1427: ⚠️ Potential issue | 🟡 Minor

Don't hide the entire section when data is empty or on first load failure.

The condition allGlobalServers.length > 0 hides the card completely when there are no servers or when the initial fetch fails. This also hides the refresh button, making recovery harder.

Suggested direction

-          {allGlobalServers.length > 0 && (
+          {(isLoadingGlobalMcps || globalMcps !== null) && (
             <div className="rounded-lg border border-border bg-card p-4">

Then render:

• A loading state when isLoadingGlobalMcps is true
• An empty state with messaging when allGlobalServers.length === 0
• The populated list otherwise

This keeps the refresh control accessible for retry.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/frontend/src/renderer/components/AgentTools.tsx` around lines 1425 -
1427, The current JSX hides the entire Claude Code Global MCPs card when
allGlobalServers.length === 0, which also hides the refresh control; change the
render logic around the card (the block currently gated by
allGlobalServers.length > 0) to always render the card and inside it: show a
loading state when isLoadingGlobalMcps is true, show an explicit empty
state/message when allGlobalServers.length === 0 (with the refresh button
visible), and otherwise render the populated list using allGlobalServers; ensure
the refresh control remains outside the length check so it is always accessible
for retries.

---

50-52: 🛠️ Refactor suggestion | 🟠 Major

Use renderer path aliases instead of relative imports.

Lines 50-51 use relative imports where @/* aliases are required per coding guidelines.

Proposed fix

-import { useSettingsStore, saveSettings } from '../stores/settings-store';
-import { cn } from '../lib/utils';
+import { useSettingsStore, saveSettings } from '@/stores/settings-store';
+import { cn } from '@/lib/utils';

As per coding guidelines: "Use path aliases from tsconfig.json in frontend code: @/* for renderer."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/frontend/src/renderer/components/AgentTools.tsx` around lines 50 - 52,
The imports in AgentTools.tsx use relative paths instead of the renderer path
alias; update the import statements that reference useSettingsStore,
saveSettings, cn, and useProjectStore to use the `@/...` alias (e.g., import
from '@/stores/settings-store' and '@/lib/utils' and '@/stores/project-store')
so they follow the project's tsconfig renderer alias convention; ensure all four
imported symbols (useSettingsStore, saveSettings, cn, useProjectStore) are
imported from their aliased module paths.

---

749-753: ⚠️ Potential issue | 🟡 Minor

Translate the fallback health-check message.

Line 752 uses a hardcoded English string 'Health check failed' that surfaces in status tooltips. This violates the i18n requirement.

Proposed fix

         } catch {
           results[server.serverId] = {
             serverId: server.serverId,
             status: 'unknown',
-            message: 'Health check failed',
+            message: t('settings:mcp.globalMcps.healthCheckFailed'),
             checkedAt: new Date().toISOString(),
           };
         }

And update the dependency array on line 761:

-  }, [allGlobalServers]);
+  }, [allGlobalServers, t]);

As per coding guidelines: "All frontend user-facing text must use react-i18next translation keys. Never hardcode strings in JSX/TSX."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/frontend/src/renderer/components/AgentTools.tsx` around lines 749 - 753,
Replace the hardcoded message 'Health check failed' in the results assignment
(the object created at results[server.serverId]) with a translation call, e.g.
use t('agent.healthCheckFailed') from react-i18next so the tooltip uses i18n;
then ensure the hook that builds/returns these results (the enclosing
useCallback/useEffect that references results) includes the translation function
t in its dependency array so translations update correctly when locale changes.

apps/frontend/src/renderer/components/CustomerReposModal.tsx (1)

136-145: ⚠️ Potential issue | 🟡 Minor

Use i18n translation key for the aria-label attribute.

The aria-label="Clear search" on line 140 is a hardcoded English string. Screen reader labels are user-facing text and must use translation keys per coding guidelines.

♿ Suggested fix

           {search && (
             <button
               type="button"
               onClick={() => setSearch('')}
-              aria-label="Clear search"
+              aria-label={t('customerRepos.clearSearch')}
               className="absolute right-3 top-1/2 -translate-y-1/2 text-muted-foreground hover:text-foreground"
             >
               <X className="h-4 w-4" />
             </button>
           )}

Ensure customerRepos.clearSearch is added to both en/*.json and fr/*.json translation files. As per coding guidelines, "All frontend user-facing text must use react-i18next translation keys."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/frontend/src/renderer/components/CustomerReposModal.tsx` around lines
136 - 145, Replace the hardcoded aria-label in CustomerReposModal's clear button
with a react-i18next translation key: use the useTranslation hook and call
t('customerRepos.clearSearch') for the aria-label on the button (the component
and element to change are in CustomerReposModal.tsx where the button with
onClick={() => setSearch('')} and <X /> lives); also add the key
"customerRepos.clearSearch" with appropriate English and French values to both
en/*.json and fr/*.json translation files so the label is localized for screen
readers.

apps/frontend/src/renderer/components/GitHubIssues.tsx (1)

257-257: ⚠️ Potential issue | 🔴 Critical

Map lookup uses wrong key type/format — will never match.

The issueToTaskMap uses string keys like "owner/repo#123" or "#123", but the lookup passes selectedIssue.number (a plain number). This causes both a TypeScript type mismatch and a runtime bug where linked tasks are never found.

🐛 Proposed fix to use repo-scoped key for lookup

             linkedTaskId={issueToTaskMap.get(selectedIssue.number)}
+            linkedTaskId={issueToTaskMap.get(
+              selectedIssue.repoFullName
+                ? `${selectedIssue.repoFullName}#${selectedIssue.number}`
+                : `#${selectedIssue.number}`
+            )}

Alternatively, extract the key computation into a helper to keep it DRY with lines 145-147:

const getIssueTaskKey = (repo: string | undefined, issueNumber: number): string =>
  repo ? `${repo}#${issueNumber}` : `#${issueNumber}`;

Then use it in both the map population and lookup.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/frontend/src/renderer/components/GitHubIssues.tsx` at line 257, The
lookup into issueToTaskMap is using selectedIssue.number (a number) but the map
keys are strings like "owner/repo#123" or "#123", so change the lookup to use
the same string key format used when populating the map; add or reuse a helper
(e.g., getIssueTaskKey(repo, issueNumber)) that returns repo ?
`${repo}#${issueNumber}` : `#${issueNumber}` and call it where linkedTaskId is
computed (replace issueToTaskMap.get(selectedIssue.number) with
issueToTaskMap.get(getIssueTaskKey(selectedRepo, selectedIssue.number))),
ensuring the helper is also used where the map is populated to keep keys
consistent.

apps/frontend/src/main/claude-code-settings/reader.ts (1)

103-104: ⚠️ Potential issue | 🟠 Major

Block prototype-pollution keys in sanitized maps.

The sanitizer functions write untrusted JSON keys into plain {} objects without guarding against prototype-pollution keys (__proto__, constructor, prototype). This affects:

• sanitizeMcpServers() at Line 103 (mcpServers map) and Line 140 (headers map)
• sanitizeEnabledPlugins() at Line 200

Use Object.create(null) for all sanitized maps and validate keys before assignment.

🔒 Proposed hardening

+const FORBIDDEN_KEYS = new Set(['__proto__', 'constructor', 'prototype']);
+
+function isSafeKey(key: string): boolean {
+  return !FORBIDDEN_KEYS.has(key);
+}
+
 function sanitizeMcpServers(mcpServers: unknown): Record<string, ClaudeCodeMcpServerConfig> | undefined {
   if (!isPlainObject(mcpServers)) {
     return undefined;
   }

-  const sanitized: Record<string, ClaudeCodeMcpServerConfig> = {};
+  const sanitized: Record<string, ClaudeCodeMcpServerConfig> = Object.create(null);
   let hasValidEntries = false;

   for (const [key, value] of Object.entries(mcpServers)) {
+    if (!isSafeKey(key)) {
+      debugLog(`${LOG_PREFIX} Skipping unsafe mcpServers key:`, key);
+      continue;
+    }
     if (!isPlainObject(value)) {

Apply the same pattern to headers (Line 140) and sanitizeEnabledPlugins (Line 200).

Also applies to: 140-140, 200-200

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/frontend/src/main/claude-code-settings/reader.ts` around lines 103 -
104, The sanitized maps in sanitizeMcpServers() and sanitizeEnabledPlugins()
currently use plain {} which allows prototype-pollution via keys like
"__proto__", "constructor", or "prototype"; change the maps (the local variables
currently named sanitized/mcpServers, headers, and enabledPlugins) to be created
with Object.create(null) and, before assigning any untrusted key into these maps
inside sanitizeMcpServers() and sanitizeEnabledPlugins(), validate the key
against a deny-list (at minimum "__proto__", "constructor", "prototype") or a
safe-key regex and skip/ignore any disallowed keys so they are never written
into the prototype chain.

apps/frontend/src/renderer/components/AddCustomerModal.tsx (1)

44-48: ⚠️ Potential issue | 🟠 Major

Handle project registration failures explicitly.

When window.electronAPI.addProject() fails, registerAndInitCustomer() returns silently without surfacing any error to the user. Both handleOpenExisting and handleCreateFolder call this function, but neither will display an error if registration fails since no exception is thrown.

🔧 Suggested fix

   const registerAndInitCustomer = async (path: string) => {
     // Pass type: 'customer' through IPC so it's persisted to disk (projects.json)
     const result = await window.electronAPI.addProject(path, 'customer');
-    if (!result.success || !result.data) return;
+    if (!result.success || !result.data) {
+      throw new Error(result.error || t('addCustomer.failedToCreate'));
+    }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/frontend/src/renderer/components/AddCustomerModal.tsx` around lines 44 -
48, registerAndInitCustomer currently swallows failures from
window.electronAPI.addProject, causing handleOpenExisting and handleCreateFolder
to return silently; modify registerAndInitCustomer to explicitly handle
unsuccessful results by either throwing an Error or returning a rejected Promise
with a descriptive message, and ensure callers (handleOpenExisting and
handleCreateFolder) catch that rejection and surface it to the user (e.g., via
the existing UI error state / notification mechanism or by setting an error
message in component state). Locate registerAndInitCustomer,
window.electronAPI.addProject, handleOpenExisting, and handleCreateFolder and
add explicit error propagation and user-facing reporting rather than a silent
return.

apps/frontend/src/renderer/components/github-issues/hooks/useMultiRepoGitHubIssues.ts (1)

70-76: ⚠️ Potential issue | 🟡 Minor

Missing cancellation check in the else branch.

The else branch at line 70 sets state without first checking if (cancelled) return, which is inconsistent with the pattern used elsewhere in this effect (lines 62, 78). If the component unmounts or customerId changes during the async call, this branch could still update state.

🔧 Suggested fix

         } else {
+          if (cancelled) return;
           setState(prev => ({
             ...prev,
             syncStatus: { connected: false, repos: [], error: result.error },
             error: result.error || t('issues.multiRepo.failedToCheckConnection'),
           }));
         }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@apps/frontend/src/renderer/components/github-issues/hooks/useMultiRepoGitHubIssues.ts`
around lines 70 - 76, The else branch that calls setState(...) should guard
against updates after unmount by checking the same cancellation flag used
elsewhere; before calling setState in the else branch (the block handling a
failed connection result inside the async effect in useMultiRepoGitHubIssues),
add "if (cancelled) return" (or the existing cancellation variable check) to
avoid updating state after the component is unmounted or customerId changes.

apps/frontend/src/renderer/components/github-prs/GitHubPRs.tsx (1)

491-517: ⚠️ Potential issue | 🟡 Minor

Strengthen the guard to verify PR belongs to the resolved child project.

The current check multiRepoSelectedNumber === selectedPR.number only validates the PR number, but different repos can have PRs with the same number. During transitions when resolvedChildProjectId updates but singleRepo.selectedPR hasn't reloaded yet, this could briefly pair a stale PR from the wrong repo with the new project context.

🛡️ Proposed additional guard

         fullPRDetail={
-          resolvedChildProjectId && selectedPR && multiRepoSelectedNumber === selectedPR.number ? (
+          resolvedChildProjectId && selectedPR && 
+          multiRepoSelectedNumber === selectedPR.number &&
+          multiRepo.selectedPR?.repoFullName === selectedPR.repoFullName ? (
             <PRDetail
               pr={selectedPR}
               projectId={resolvedChildProjectId}

This adds a check that the selected PR's repo matches the multi-repo selection's repo, ensuring the detail view is only rendered when both contexts are in sync.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/frontend/src/renderer/components/github-prs/GitHubPRs.tsx` around lines
491 - 517, The conditional rendering for fullPRDetail in the PR list only checks
PR number (multiRepoSelectedNumber === selectedPR.number) which can match across
different repos; update the guard inside the fullPRDetail conditional (around
resolvedChildProjectId, selectedPR and multiRepoSelectedNumber) to also verify
the selected PR belongs to the same repo as the multi-repo selection — e.g.,
compare a unique repo identifier on selectedPR (repositoryId, repoFullName, or
owner/name) against the multi-repo selected repo id/name (or
resolvedChildProjectId if that maps to the repo) so the detail view is only
shown when both PR number and repo match. Ensure you use the existing symbols
selectedPR, resolvedChildProjectId and multiRepoSelectedNumber to locate and
change the check.

apps/frontend/src/main/ipc-handlers/memory-handlers.ts (1)

74-112: ⚠️ Potential issue | 🟠 Major

Fail fast for unknown models instead of returning guessed dimensions.

The "fallback" path still lets unknown models succeed with a guessed vector size. That can surface later as embedding-dimension mismatches, and this concern was already raised in a prior review.

🛠️ Proposed fix

-function lookupEmbeddingDim(modelName: string): { dim: number; source: 'known' | 'fallback' } | null {
+function lookupEmbeddingDim(modelName: string): { dim: number; source: 'known' } | null {
   const nameLower = modelName.toLowerCase();
   ...
-  // Heuristic fallback based on name patterns.
-  // WARNING: These are guesses and may be incorrect for unknown models.
-  // The 'fallback' source flag allows callers to surface this uncertainty.
-  if (nameLower.includes('large')) {
-    console.warn(`[OllamaEmbedding] Using heuristic dimension guess (1024) for unknown model: ${modelName}`);
-    return { dim: 1024, source: 'fallback' };
-  }
-  if (nameLower.includes('base')) {
-    console.warn(`[OllamaEmbedding] Using heuristic dimension guess (768) for unknown model: ${modelName}`);
-    return { dim: 768, source: 'fallback' };
-  }
-  if (nameLower.includes('small') || nameLower.includes('mini')) {
-    console.warn(`[OllamaEmbedding] Using heuristic dimension guess (384) for unknown model: ${modelName}`);
-    return { dim: 384, source: 'fallback' };
-  }
-
   return null;
 }

Also narrow the IPC payload type at Line 959 to remove 'fallback'.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/frontend/src/main/ipc-handlers/memory-handlers.ts` around lines 74 -
112, Update lookupEmbeddingDim to fail fast for unknown models: remove the
heuristic "fallback" branches and their console warnings so the function only
returns known dimensions (source: 'known') or null; callers should handle null
as an unknown/unsupported model. Also update the IPC payload type that currently
allows a 'fallback' source (the IPC message type that carries lookupEmbeddingDim
results) to exclude 'fallback' so its source can only be 'known' (or the payload
allows null/unsupported), and adjust any call sites that assumed fallback to
handle null/unsupported appropriately.

apps/backend/agents/custom_agents.py (1)

155-171: ⚠️ Potential issue | 🟡 Minor

Root-level agents are excluded from load_all_agents() but included in load_custom_agent().

load_custom_agent() searches both category subdirectories (lines 139-144) AND the root agents directory (lines 147-149), but load_all_agents() only iterates subdirectories. This means an agent at ~/.claude/agents/my-agent.md can be loaded by ID but won't appear in the full agent list or catalog.

🔧 Suggested fix

 def load_all_agents() -> list[CustomAgentConfig]:
     """Load all custom agents from all categories in ~/.claude/agents/."""
     agents_dir = get_agents_dir()
     if not agents_dir.exists():
         return []
 
     agents = []
+    # Load root-level agents first
+    for agent_file in sorted(agents_dir.glob("*.md")):
+        if agent_file.name == "README.md":
+            continue
+        agent = parse_agent_file(agent_file)
+        if agent:
+            agents.append(agent)
+
+    # Then load from category subdirectories
     for category_dir in sorted(agents_dir.iterdir()):

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/agents/custom_agents.py` around lines 155 - 171, load_all_agents
currently only iterates category subdirectories and therefore omits root-level
agent files that load_custom_agent can find; update load_all_agents (which uses
get_agents_dir and parse_agent_file) to also scan agents_dir.glob("*.md")
(skipping README.md) and parse/append those files before or after iterating
category_dir files so root-level agents are included in the returned list.

apps/backend/analysis/analyzers/route_detector.py (2)

650-656: ⚠️ Potential issue | 🟠 Major

Minimal API auth detection can miss .RequireAuthorization() when lambdas contain internal semicolons.

Line 651 uses content.find(";", match.end()) to find the statement end, but block lambdas like () => { var a = 1; return a; } contain internal semicolons. This causes the search to stop prematurely, missing chained .RequireAuthorization() calls.

🛠️ Suggested fix

Implement a bracket-aware statement end finder:

def _find_statement_end(self, content: str, start: int, lookahead: int = 1200) -> int:
    """Find statement end, accounting for nested braces/parens in lambdas."""
    end_limit = min(len(content), start + lookahead)
    paren = brace = bracket = 0
    in_str: str | None = None
    i = start
    while i < end_limit:
        ch = content[i]
        prev = content[i - 1] if i > 0 else ""
        if in_str:
            if ch == in_str and prev != "\\":
                in_str = None
        else:
            if ch in {"'", '"'}:
                in_str = ch
            elif ch == "(":
                paren += 1
            elif ch == ")":
                paren = max(0, paren - 1)
            elif ch == "{":
                brace += 1
            elif ch == "}":
                brace = max(0, brace - 1)
            elif ch == ";":
                if paren == 0 and brace == 0 and bracket == 0:
                    return i + 1
        i += 1
    return end_limit

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/analysis/analyzers/route_detector.py` around lines 650 - 656,
The current detection in route_detector.py stops at the first semicolon after
match (using content.find) which breaks when lambdas contain internal
semicolons; replace that logic by implementing a bracket- and string-aware
helper (e.g., _find_statement_end) that scans from match.end(), tracks
paren/brace/bracket nesting and ignores semicolons inside strings or nested
blocks, returns the correct statement end (or a lookahead limit) and then use it
to build route_stmt and compute requires_auth (instead of the simple
content.find result) so chained .RequireAuthorization(...) is reliably detected.

---

484-589: ⚠️ Potential issue | 🟠 Major

Controller context is only extracted from the first class, causing route misattribution in multi-controller files.

Line 484 uses re.search() which captures only the first class declaration, but line 521 iterates over all [Http*] attributes in the entire file. This means routes in the second and subsequent controllers will incorrectly use the first controller's route prefix and authorization rules.

🛠️ Suggested approach

Find all class declarations using re.finditer(), compute each class's span, then associate each [Http*] match with the nearest preceding class by comparing positions:

# Find all controller classes with their spans
class_pattern = re.compile(r"class\s+(\w+)")
classes = [(m.start(), m.group(1)) for m in class_pattern.finditer(content)]

# For each Http* match, find which class it belongs to
for match in http_method_pattern.finditer(content):
    # Find the class this method belongs to (largest start pos < match.start())
    owning_class = None
    for class_start, class_name in classes:
        if class_start < match.start():
            owning_class = (class_start, class_name)
    # Use owning_class context for route prefix and auth

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/analysis/analyzers/route_detector.py` around lines 484 - 589,
The code currently uses a single class_match (class_match) and shared
class_route_prefix/class_authorize while iterating all Http attributes
(http_method_pattern), causing later controller methods to inherit the first
class's context; fix by finding all classes with re.finditer (e.g.,
class_pattern = re.compile(r"class\s+(\w+)") and capture their start/end spans
and names), then for each http_method_pattern.match determine the owning class
by selecting the nearest preceding class start, compute that class's
controller_name, class_route_prefix and class_authorize from the owning class's
pre-class section (instead of the top-level variables), and then proceed to
compute full_path, method_authorize and requires_auth for that specific class
before appending to routes (keep usage of _normalize_aspnet_path and existing
method_name search logic).

apps/frontend/src/renderer/components/settings/AgentProfileSettings.tsx (1)

386-438: 🧹 Nitpick | 🔵 Trivial

Constrain catalog panel height to avoid viewport overflow.

The expanded agents catalog can become very tall with many categories/agents. Add fixed max-height with internal scrolling to match settings dropdown behavior in this area.

♻️ Proposed fix

             {showAgentsCatalog && (
-              <div className="border-t border-border p-4 space-y-1">
+              <div className="border-t border-border p-4 space-y-1 max-h-60 overflow-y-auto">
                 <p className="text-xs text-muted-foreground mb-3">

Based on learnings, "Dropdown components in apps/frontend/src/renderer/components/settings use a fixed max-height (e.g., max-h-60) with internal overflow-y-auto scrolling... If you add new dropdowns in this area, follow the same fixed-height + internal-scroll approach for consistency."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/frontend/src/renderer/components/settings/AgentProfileSettings.tsx`
around lines 386 - 438, The agents catalog panel can grow beyond the viewport;
wrap the category list (the JSX block rendered when showAgentsCatalog is true —
currently the div with className "border-t border-border p-4 space-y-1") or the
inner container that maps agentsInfo.categories with a fixed max-height and
internal scrolling (e.g., add a utility class like max-h-60 and overflow-y-auto)
so the expandedAgentCategories toggle still works but the panel scrolls
internally; update the element around agentsInfo.categories mapping in
AgentProfileSettings (the section using expandedAgentCategories and
setExpandedAgentCategories) to include the max-height + overflow-y-auto classes
for consistent dropdown behavior.

apps/backend/analysis/analyzers/database_detector.py (1)

477-488: ⚠️ Potential issue | 🟠 Major

Still exclude entity-reference navigations before the PascalCase fallback.

This is the same gap raised earlier and it still looks unresolved: Customer Customer { get; set; } passes the uppercase-type filter and is emitted as a scalar field. That overstates columns for EF Core models with non-virtual reference navigations.

Proposed fix

                     is_nullable = raw_type.endswith("?")
                     clean_type = raw_type.rstrip("?")

                     if not clean_type:
                         continue

+                    if clean_type in known_entities and not prop_name.endswith(
+                        ("Id", "ID")
+                    ):
+                        continue
+
                     # Only include properties with recognized types or common patterns
                     if clean_type not in csharp_types and (
                         not clean_type or not clean_type[0].isupper()
                     ):
                         continue

Read-only verification

#!/bin/bash
python - <<'PY'
from pathlib import Path
import re

excluded = {"bin", "obj", "node_modules", ".git", "TestResults"}
cs_files = [
    p for p in Path(".").rglob("*.cs")
    if not any(part in excluded for part in p.parts)
]

dbset_pattern = re.compile(r"DbSet<(\w+)>\s+(\w+)")
config_pattern = re.compile(r"IEntityTypeConfiguration<(\w+)>")
prop_pattern = re.compile(
    r"public\s+(virtual\s+)?"
    r"([\w<>,?\[\]\s]+?)\s+"
    r"(\w+)\s*\{\s*get;\s*set;\s*\}"
)

known_entities = set()
for path in cs_files:
    try:
        text = path.read_text(encoding="utf-8")
    except (OSError, UnicodeDecodeError):
        continue
    known_entities.update(m.group(1) for m in dbset_pattern.finditer(text))
    known_entities.update(m.group(1) for m in config_pattern.finditer(text))

for path in cs_files:
    try:
        text = path.read_text(encoding="utf-8")
    except (OSError, UnicodeDecodeError):
        continue
    for match in prop_pattern.finditer(text):
        is_virtual = match.group(1) is not None
        clean_type = match.group(2).strip().rstrip("?")
        prop_name = match.group(3)
        if not is_virtual and clean_type in known_entities and not prop_name.endswith(("Id", "ID")):
            line = text.count("\n", 0, match.start()) + 1
            print(f"{path}:{line}: public {clean_type} {prop_name} {{ get; set; }}")
PY

Expected result: any output is currently being misclassified as a scalar field.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/analysis/analyzers/database_detector.py` around lines 477 - 488,
The parser currently treats uppercase type names like "Customer Customer { get;
set; }" as scalar because the PascalCase fallback runs before excluding EF
reference navigations; modify the logic around raw_type/is_nullable/clean_type
(and before the PascalCase fallback using csharp_types) to skip non-virtual
properties whose clean_type matches a discovered EF entity type (use the
module's known entity set—e.g., known_entities or entity_type_names populated
from DbSet<> and IEntityTypeConfiguration detections) and whose property name
does not end with "Id" or "ID"; in short, add a guard that if not is_virtual and
clean_type in known_entities and not prop_name.endswith(("Id","ID")) then
continue so reference navigations are not emitted as scalar fields.

apps/frontend/src/main/ipc-handlers/github/customer-github-handlers.ts (1)

193-195: ⚠️ Potential issue | 🟡 Minor

Fractional IPC inputs still pass validation.

Line 193 and Line 287 only check for finite positive numbers, so page = 1.5 and issueNumber = 7.2 still produce malformed GitHub endpoints. Tighten both paths to Number.isInteger(...) && > 0 before building the request.

Also applies to: 287-289

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/frontend/src/main/ipc-handlers/github/customer-github-handlers.ts`
around lines 193 - 195, The current validation allows fractional values like
page = 1.5 and issueNumber = 7.2; update both checks so they only accept
positive integers by replacing the loose finite checks with
Number.isInteger(page) && page > 0 for the page validation and
Number.isInteger(issueNumber) && issueNumber > 0 for the issueNumber validation
(locate the checks that currently read typeof ... Number.isFinite(...) and
tighten them in the handler that constructs the GitHub endpoints).

apps/backend/analysis/analyzers/context/env_detector.py (1)

44-45: ⚠️ Potential issue | 🟠 Major

Placeholder entries still block launchSettings.json values.

Line 44/45 seeds .env.example and docker-compose placeholders before Line 51 reads launchSettings.json, and _parse_launch_settings() refuses to overwrite existing keys. If both declare something like ASPNETCORE_ENVIRONMENT, the detector keeps value=None instead of the concrete local value. Parse those placeholder sources later, or let launchSettings replace entries whose current value is None.

Also applies to: 50-51

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/analysis/analyzers/context/env_detector.py` around lines 44 -
45, The detector seeds placeholders via _parse_env_example(env_vars,
required_vars) and _parse_docker_compose(env_vars) before reading launchSettings
with _parse_launch_settings(), causing launchSettings values (e.g.,
ASPNETCORE_ENVIRONMENT) to be blocked when placeholders set value=None; either
move the call to _parse_launch_settings() to run before
_parse_env_example/_parse_docker_compose or change _parse_launch_settings() to
explicitly overwrite existing keys whose current value is None in the env_vars
dict; update references to env_vars and required_vars accordingly so
launchSettings wins for concrete local values.

apps/backend/analysis/analyzers/framework_analyzer.py (1)

733-743: ⚠️ Potential issue | 🔴 Critical

Resolve solution project paths before relative_to().

Path.relative_to() is lexical. At Line 742, self.path / "../Shared/Shared.csproj" can still yield ../Shared instead of being rejected, so projects outside the analyzed root get stored in dotnet_solution and can be reused by other analyzers. Resolve csproj_path first and compare against self.path.resolve().

Suggested change

-            csproj_path = self.path / project_rel_path
+            root = self.path.resolve()
+            csproj_path = (self.path / project_rel_path).resolve()
             if not csproj_path.exists():
                 continue
 
             # Get the project directory path relative to solution root.
             # Skip projects that resolve outside the solution directory
             # (e.g., via ".." segments in the .sln reference).
             try:
-                project_dir = str(csproj_path.parent.relative_to(self.path))
+                project_dir = str(csproj_path.relative_to(root).parent)
             except ValueError:
                 continue

Run this to confirm the containment check. Expected result: relative_to(root) prints a path with .., while the resolved path is outside the root.

#!/bin/bash
set -euo pipefail

sed -n '733,743p' apps/backend/analysis/analyzers/framework_analyzer.py

python - <<'PY'
from pathlib import Path
import tempfile

with tempfile.TemporaryDirectory() as d:
    root = Path(d)
    outside = root.parent / "outside-cr"
    outside.mkdir(exist_ok=True)
    csproj = root / ".." / "outside-cr" / "Shared.csproj"
    csproj.touch()

    print("exists:", csproj.exists())
    print("relative_to(root):", csproj.parent.relative_to(root))
    print("resolved inside root:", csproj.resolve().is_relative_to(root.resolve()))
PY

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/analysis/analyzers/framework_analyzer.py` around lines 733 -
743, The code uses csproj_path.parent.relative_to(self.path) which is lexical
and can incorrectly accept paths with ".."; change to resolve the csproj path
first and perform the containment check against the resolved solution root:
compute csproj_resolved = (self.path / project_rel_path).resolve() (or resolve
csproj_path then csproj_path.parent.resolve()), compute root_resolved =
self.path.resolve(), and then use
csproj_resolved.parent.relative_to(root_resolved) (or root_resolved
in/is_relative_to) inside the try/except to skip projects that resolve outside
the solution; update the references to csproj_path, project_dir and the existing
except ValueError branch accordingly so external projects are not added to
dotnet_solution.

apps/backend/integrations/graphiti/config.py (1)

243-246: ⚠️ Potential issue | 🟡 Minor

Warn when GRAPHITI_EPISODE_TTL_DAYS is non-numeric.

A typo like GRAPHITI_EPISODE_TTL_DAYS=seven currently disables TTL cleanup silently by falling back to 0. Please log the raw invalid value in the except path before defaulting.

Suggested change

-        try:
-            episode_ttl_days = int(os.environ.get("GRAPHITI_EPISODE_TTL_DAYS", "0"))
+        raw_episode_ttl_days = os.environ.get("GRAPHITI_EPISODE_TTL_DAYS", "0")
+        try:
+            episode_ttl_days = int(raw_episode_ttl_days)
         except ValueError:
+            logger.warning(
+                "Invalid GRAPHITI_EPISODE_TTL_DAYS=%r; defaulting episode_ttl_days to 0",
+                raw_episode_ttl_days,
+            )
             episode_ttl_days = 0

As per coding guidelines, "Check for proper error handling and security considerations."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/integrations/graphiti/config.py` around lines 243 - 246, The
try/except around parsing GRAPHITI_EPISODE_TTL_DAYS quietly falls back to 0 on
ValueError; update the except block for the episode_ttl_days parsing to log the
raw invalid environment value (os.environ.get("GRAPHITI_EPISODE_TTL_DAYS")) and
the exception before setting episode_ttl_days = 0 so the bad input is visible in
logs; reference the episode_ttl_days variable and GRAPHITI_EPISODE_TTL_DAYS env
key and use the module's logger (or logging.warning/error) to record the invalid
value and the ValueError.

apps/backend/integrations/graphiti/queries_pkg/schema.py (1)

29-32: ⚠️ Potential issue | 🟠 Major

Clamp GRAPHITI_MAX_RESULTS to a positive integer.

0 and negative values still make it through this parser. Downstream min(num_results, MAX_CONTEXT_RESULTS) can then pass 0/negative num_results into search and effectively disable context retrieval.

Suggested change

 try:
-    MAX_CONTEXT_RESULTS = int(os.getenv("GRAPHITI_MAX_RESULTS", "10"))
+    MAX_CONTEXT_RESULTS = max(1, int(os.getenv("GRAPHITI_MAX_RESULTS", "10")))
 except ValueError:
     MAX_CONTEXT_RESULTS = 10

As per coding guidelines, "Check for proper error handling and security considerations."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/integrations/graphiti/queries_pkg/schema.py` around lines 29 -
32, The env parsing for GRAPHITI_MAX_RESULTS must enforce a positive integer:
when reading os.getenv("GRAPHITI_MAX_RESULTS", "10") and converting to int for
MAX_CONTEXT_RESULTS, catch ValueError as before but also clamp the parsed value
to at least 1 (e.g., MAX_CONTEXT_RESULTS = max(1, parsed_value)) so 0 or
negative values cannot propagate; keep the existing default of 10 when parsing
fails or env is absent and reference the MAX_CONTEXT_RESULTS symbol in your
change.

apps/backend/integrations/graphiti/tests/test_integration_graphiti.py (1)

31-33: ⚠️ Potential issue | 🟠 Major

integration + autouse mocks leaves both gaps: no live coverage and skipped CI.

Every test in this module replaces graphiti_core, so the suite is unit-level. With pytest.mark.integration, the default CI selection can skip it entirely while the real LadybugDB/Graphiti path still goes untested.

Also applies to: 40-95

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/integrations/graphiti/tests/test_integration_graphiti.py` around
lines 31 - 33, The tests are marked as integration via the module-level
pytestmark but they autouse-mock graphiti_core, making them unit tests; remove
or replace the module-level pytestmark = [pytest.mark.integration] with a more
accurate marker (e.g., pytest.mark.unit or no marker) so CI doesn't skip live
coverage, and/or adjust the autouse fixture that mocks graphiti_core (the
autouse mock definition referring to graphiti_core) so that tests either
actually run integration against LadybugDB/Graphiti or are correctly labeled as
unit tests; update the marker declaration and/or the autouse mock fixture to
keep test intent and CI selection consistent.

apps/backend/integrations/graphiti/tests/test_integration_ollama.py (1)

31-36: ⚠️ Potential issue | 🟠 Major

Don’t keep fully mocked tests under pytest.mark.integration.

With the current autouse mocks, this module never touches live Ollama/Graphiti behavior. Keeping the marker means the default -m "not integration" CI run skips unit-level coverage entirely while still not providing real integration coverage.

Also applies to: 43-93

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/integrations/graphiti/tests/test_integration_ollama.py` around
lines 31 - 36, The module-level pytest marker pytestmark = [
pytest.mark.integration ] incorrectly labels fully mocked unit tests as
integration tests; remove or change that marker so CI doesn't skip these tests
(e.g., delete the pytestmark definition or replace it with pytest.mark.unit),
and ensure any truly integration tests that require a live Ollama/Graphiti
server are moved to a separate file or given the correct pytest.mark.integration
marker (also update the same marker usage in the other block spanning the tests
referenced at 43-93).

apps/backend/integrations/graphiti/queries_pkg/search.py (1)

64-79: ⚠️ Potential issue | 🟠 Major

Verify the GraphitiConfig import here and only mark validation complete after success.

This block imports graphiti_config, while the config class is exposed as integrations.graphiti.config elsewhere in this PR. If that alias does not exist, Line 66 flips _dimension_validated before the ImportError, the exception is swallowed, and every later search skips the validator permanently.

Run this read-only check to confirm the module path:

#!/bin/bash
set -euo pipefail
fd -HI 'graphiti_config\.py$'
rg -n --type=py 'class GraphitiConfig|from graphiti_config import GraphitiConfig|from integrations\.graphiti\.config import GraphitiConfig|from \.\.?config import GraphitiConfig'

Expected result: either a real graphiti_config.py alias exists, or GraphitiConfig is only defined/imported under apps/backend/integrations/graphiti/config.py / integrations.graphiti.config, in which case this import should be updated and _dimension_validated should only be set after the lookup succeeds.

As per coding guidelines, "Check for proper error handling and security considerations."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/integrations/graphiti/queries_pkg/search.py` around lines 64 -
79, The code prematurely sets GraphitiSearch._dimension_validated = True before
the import/validation can succeed and uses the wrong module path; move the
assignment so it only runs after a successful validation, and import
GraphitiConfig from the canonical integrations.graphiti.config module (i.e.
replace from graphiti_config import GraphitiConfig with from
integrations.graphiti.config import GraphitiConfig or the correct package
alias), wrap the import and config.get_embedding_dimension() call in the
try/except so only on success you set GraphitiSearch._dimension_validated =
True, and keep the exception handling but ensure ImportError/AttributeError are
logged at debug and do not flip the validated flag.