feat: port memory system + custom agents to desktop architecture#1955
Open
vitorafgomes wants to merge 3 commits intoAndyMik90:developfrom
Open
feat: port memory system + custom agents to desktop architecture#1955vitorafgomes wants to merge 3 commits intoAndyMik90:developfrom
vitorafgomes wants to merge 3 commits intoAndyMik90:developfrom
Conversation
…itecture Port viable frontend changes from PR AndyMik90#1920 to the current apps/desktop/ structure after upstream restructured apps/frontend/ → apps/desktop/ and migrated backend from Python to TypeScript (Vercel AI SDK v6). New features ported: - Custom Agents: IPC handler to read ~/.claude/agents/ definitions, specialist agents catalog in AgentProfileSettings - Global MCP: IPC handler for Claude Code MCP config, health checks, pipeline phase assignment UI in AgentTools - Customer Projects: AddCustomerModal, CustomerReposModal, multi-repo GitHub hooks (issues + PRs), RepoFilterDropdown, Sidebar integration - Memory: Ollama embedding dimension lookup with 30+ model mappings, index progress tracking in context store - i18n: New context namespace, 100+ translation keys (en + fr) for settings, dialogs, navigation, tasks Also fixes code scanning issues from original PR: - Export getUserConfigDir from claude-code-settings/reader - Export transformIssue from github/issue-handlers - Add type annotations to eliminate implicit any errors Supersedes AndyMik90#1920 (closed due to unmergeable conflicts) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
|
Contributor
📝 WalkthroughWalkthroughAdds customer multi-repo GitHub support, Claude agents discovery, global MCP aggregation and health checks, Ollama embedding-dim lookup, new IPC endpoints/types, new UI components and hooks for customer flows and MCPs, i18n additions, and store updates for index progress. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Renderer as Renderer (UI)
participant IPC as Renderer→IPC
participant Main as Main (Electron)
participant FS as Filesystem
User->>Renderer: Open AddCustomerModal / choose folder
Renderer->>IPC: addProject(path, 'customer') / createFolder
IPC->>Main: register project
Main->>FS: mkdir projectPath
FS-->>Main: success
Main-->>IPC: project registered
IPC-->>Renderer: project added
Renderer->>IPC: initializeCustomerProject(projectId)
IPC->>Main: create .auto-claude
Main->>FS: mkdir .auto-claude
FS-->>Main: success
Main-->>IPC: initialized
IPC-->>Renderer: initialized
Renderer->>User: open/select new project
sequenceDiagram
participant Renderer as Renderer (UI)
participant IPC as Renderer→IPC
participant PluginCache as Plugin Cache
participant FS as Filesystem
participant Resolver as Aggregator
Renderer->>IPC: getGlobalMcps()
IPC->>PluginCache: resolvePluginServers()
PluginCache->>FS: list plugin cache dirs
FS-->>PluginCache: dir metadata
PluginCache->>FS: read .mcp.json for latest hash
FS-->>PluginCache: plugin server entries
IPC->>FS: read settings.mcpServers (inline)
FS-->>IPC: inline entries
IPC->>FS: read ~/.claude.json
FS-->>IPC: claude.json entries
IPC->>Resolver: aggregate plugin + inline + claude.json
Resolver-->>Renderer: GlobalMcpInfo
sequenceDiagram
participant Renderer as Renderer (UI)
participant IPC as Renderer→IPC
participant FS as Filesystem
participant GH as GitHub API
Renderer->>IPC: checkMultiRepoConnection(customerId)
IPC->>FS: read customer .env / child repo configs / git remotes
FS-->>IPC: repo list
Renderer->>IPC: getMultiRepoIssues(customerId, state, page)
IPC->>GH: parallel fetch issues per repo
GH-->>IPC: issues per repo
IPC-->>Renderer: aggregated issues + repos
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Contributor
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly updates the desktop application by porting critical frontend features from a previous, unmergeable branch to the new Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
5 tasks
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request is a significant feature port, bringing custom agents, a memory system, and multi-repo customer project support to the new desktop architecture. The changes are extensive, adding new IPC handlers, frontend components, and updating type definitions.
My review focuses on the new backend logic in the main process. I've identified several instances where synchronous file system operations are used within asynchronous IPC handlers. While this might work for small files/directories, it can block the main process and lead to UI unresponsiveness. I recommend switching to asynchronous file system methods to improve performance and robustness.
Additionally, I've pointed out a case where an error is caught but not logged, which can hinder debugging. I've also included a suggestion to refactor a piece of validation logic for better conciseness and maintainability.
Overall, the PR is well-structured, and the new features are a great addition. Addressing these points will help ensure the application remains responsive and maintainable.
Comment on lines
+153
to
+163
| const plugins: Record<string, boolean> = {}; | ||
| let hasValidPlugins = false; | ||
| for (const [key, value] of Object.entries(obj.enabledPlugins as Record<string, unknown>)) { | ||
| if (typeof value === 'boolean') { | ||
| plugins[key] = value; | ||
| hasValidPlugins = true; | ||
| } | ||
| } | ||
| if (hasValidPlugins) { | ||
| sanitized.enabledPlugins = plugins; | ||
| hasValidFields = true; |
Contributor
There was a problem hiding this comment.
This block of code for sanitizing enabledPlugins can be made more concise and declarative by using Object.fromEntries and Array.prototype.filter.
const plugins = Object.fromEntries(
Object.entries(obj.enabledPlugins as Record<string, unknown>).filter(
([, value]) => typeof value === 'boolean'
)
);
if (Object.keys(plugins).length > 0) {
sanitized.enabledPlugins = plugins as Record<string, boolean>;
hasValidFields = true;
}| const categories: ClaudeAgentCategory[] = []; | ||
| let totalAgents = 0; | ||
|
|
||
| const entries = readdirSync(agentsDir, { withFileTypes: true }); |
Contributor
There was a problem hiding this comment.
Using synchronous file system methods like readdirSync in an async handler can block the main process, impacting UI responsiveness. Consider using the asynchronous version fs.promises.readdir along with Promise.all to process directories concurrently.
| const envPath = path.join(customer.path, customer.autoBuildPath, '.env'); | ||
| if (existsSync(envPath)) { | ||
| try { | ||
| const content = readFileSync(envPath, 'utf-8'); |
Contributor
There was a problem hiding this comment.
Using readFileSync in an async function blocks the main process, which can lead to UI unresponsiveness. Since this function is already async, it's better to use the asynchronous version fs.promises.readFile. This will require updating the fs import at the top of the file to use fs/promises.
Comment on lines
+128
to
+130
| } catch { | ||
| debugLog('[Customer GitHub] Could not detect git remote for child:', child.path); | ||
| } |
Contributor
There was a problem hiding this comment.
The error object is not being logged in this catch block, which can make debugging difficult. It's a good practice to always log the error that was caught.
Suggested change
| } catch { | |
| debugLog('[Customer GitHub] Could not detect git remote for child:', child.path); | |
| } | |
| } catch (error) { | |
| debugLog('[Customer GitHub] Could not detect git remote for child:', child.path, error); | |
| } |
| const dotAutoClaude = path.join(project.path, '.auto-claude'); | ||
|
|
||
| if (!existsSync(dotAutoClaude)) { | ||
| mkdirSync(dotAutoClaude, { recursive: true }); |
Contributor
There was a problem hiding this comment.
Using mkdirSync in an async handler can block the main process. It's better to use the asynchronous version from fs/promises. This would require updating the fs import at the top of the file.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 16
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/main/ipc-handlers/github/customer-github-handlers.ts`:
- Around line 368-388: Extract the inline mapping that builds `transformed` into
a new typed helper function `transformPullRequest(pr: GitHubAPIPullRequest,
repoFullName: string)` and add a `GitHubAPIPullRequest` interface modeling the
fields used (number, title, body, state, user.login, head.ref, base.ref,
additions, deletions, changed_files, assignees, created_at, updated_at,
html_url); replace the inline map with `(data as any[]).map(pr =>
transformPullRequest(pr, repoFullName))` and import/align usage like the
existing `transformIssue` helper so the PR transformation is reusable and
type-safe.
- Around line 193-195: The code currently reassigns the parameter page; instead
create a new local constant (e.g., validatedPage) and perform the checks on page
to set validatedPage = 1 when invalid, then use validatedPage instead of page
when building the GitHub API endpoint (replace the usage in the endpoint
construction where page is interpolated); update any other references in the
function that rely on the sanitized page value to use validatedPage so the
original parameter is not mutated.
In `@apps/desktop/src/renderer/components/AddCustomerModal.tsx`:
- Around line 139-142: The platform separator detection using
window.navigator.platform is deprecated; update the logic in AddCustomerModal
(symbols: sep and folderPreview) to prefer a separator provided by the preload
API (e.g., window.electronAPI.pathSep) and fall back to inferring the separator
from the provided location string (check if location includes '\\' then use '\\'
else '/'), so set sep from window.electronAPI.pathSep ??
(location?.includes('\\') ? '\\' : '/') before building folderPreview; ensure
you preserve the existing behavior when customerName.trim() and location are
present.
In `@apps/desktop/src/renderer/components/AgentTools.tsx`:
- Around line 764-769: The useEffect that auto-checks health can re-run if the
identity of checkGlobalMcpHealth changes; to guard against unnecessary loops,
update the effect to explicitly depend on allGlobalServers (and optionally keep
checkGlobalMcpHealth) and add an internal guard that returns early unless
allGlobalServers.length > 0 before calling checkGlobalMcpHealth; reference the
existing useEffect, the allGlobalServers value, and the checkGlobalMcpHealth
callback when making this change.
- Line 1477: The inline fallback string in the t(...) call inside AgentTools.tsx
uses 'No global MCP servers found.' — remove that inline English fallback and
ensure the translation key mcp.globalMcps.noServers is defined in the
translation files; add the corresponding entry to en/settings.json and
fr/settings.json with appropriate localized strings, then call
t('settings:mcp.globalMcps.noServers') (no inline fallback) in the component to
rely solely on react-i18next.
In `@apps/desktop/src/renderer/components/CustomerReposModal.tsx`:
- Line 140: Replace the hardcoded aria-label in CustomerReposModal.tsx with a
react-i18next translation key: import and use the useTranslation hook (t) in the
component and change aria-label="Clear search" to
aria-label={t('dialogs:clearSearch')} (or another clear key name), then add the
same key and localized strings to en/dialogs.json and fr/dialogs.json (e.g.
"clearSearch": "Clear search" and French translation) so the label is localized
across locales.
In
`@apps/desktop/src/renderer/components/github-issues/components/RepoFilterDropdown.tsx`:
- Around line 3-9: Replace the relative import for the UI select components with
the project path alias: change the import of Select, SelectContent, SelectItem,
SelectTrigger, SelectValue (currently from '../../ui/select') to use the
configured alias (e.g. '@components/ui/select' or '@/components/ui/select' per
renderer tsconfig). Update the import statement in RepoFilterDropdown.tsx to the
aliased path and ensure your tsconfig/webpack supports the chosen alias so the
module resolves.
In
`@apps/desktop/src/renderer/components/github-issues/hooks/useMultiRepoGitHubIssues.ts`:
- Around line 195-215: The callbacks getFilteredIssues and getOpenIssuesCount
are recreating on any state change because they depend on the whole state
object; to fix, move the destructuring const { issues, selectedRepo } = state
into the surrounding scope (above the useCallback definitions) and change the
useCallback dependencies to the specific pieces: [issues, selectedRepo]; update
both getFilteredIssues and getOpenIssuesCount to use those outer-scope variables
so their dependency arrays reference only issues and selectedRepo rather than
state.
- Around line 90-132: Destructure the specific pieces of state used by the
effect (e.g. const { filterState } = state and const syncConnected =
state.syncStatus?.connected) and then update the useEffect to depend on
customerId, filterState, syncConnected, and t instead of referencing
state.filterState and state.syncStatus?.connected directly; keep the existing
loadIssues, setState calls, and cancelled flag logic but ensure the effect
closure only captures those destructured primitives to avoid stale closures and
unnecessary reruns.
In
`@apps/desktop/src/renderer/components/github-prs/hooks/useMultiRepoGitHubPRs.ts`:
- Around line 215-216: The isMultiRepo flag is hardcoded to true; change it to
derive from the actual repository list/count in the hook (useMultiRepoGitHubPRs)
— e.g., compute isMultiRepo = (repos?.length ?? repoCount ?? 0) > 1 using the
existing repo array or count variable used earlier in the hook, and return that
value instead of the literal true so the flag reflects the real multi-repo
state.
In `@apps/desktop/src/renderer/components/GitHubSetupModal.tsx`:
- Around line 130-137: The code uses an unnecessary type assertion "(project as
Project & { type?: string }).type" when checking project.type; remove the cast
and use the existing Project type directly (i.e., reference project.type) in the
condition where hasGitHubAuth is checked, keeping the surrounding logic that
calls onComplete with ghTokenResult.data!.token, githubRepo, mainBranch and
githubAuthMethod unchanged; update the condition that references hasGitHubAuth
and project.type to simply use project.type (and ensure imports/definitions
still reference the Project interface).
- Around line 252-261: The code in the GitHubSetupModal component uses a
redundant type assertion when checking project type; simplify the conditional by
accessing project.type directly (remove the unnecessary "(project as Project & {
type?: string })" cast) in the block that calls onComplete with
githubToken/githubRepo/mainBranch/githubAuthMethod for customers, keeping the
existing call to onComplete and the early return unchanged.
In
`@apps/desktop/src/renderer/components/settings/integrations/GitHubIntegration.tsx`:
- Around line 432-566: Replace the hardcoded English UI strings in the
customer-specific clone section of GitHubIntegration (the JSX that renders
Cloned Repositories, Clone Repositories, button labels, search placeholder, and
per-repo status labels) with the corresponding react-i18next keys from the
settings:github namespace (e.g., use t('settings:github.clonedRepositories'),
t('settings:github.cloneRepositories'), t('settings:github.refresh') /
t('settings:github.loadRepos'), t('settings:github.searchRepos'),
t('settings:github.cloned'), t('settings:github.cloning'),
t('settings:github.clone')). Ensure the component uses the useTranslation hook
(t) already present in the file and update strings in the JSX blocks that
reference customerChildProjects, loadCustomerRepos, customerRepoSearch,
filteredCustomerRepos, cloneStatuses, cloneErrors, and handleCloneRepo so all
user-facing text uses t(...).
In `@apps/desktop/src/renderer/components/Sidebar.tsx`:
- Around line 140-146: Remove the redundant broad type assertions around
selectedProject and project elements; directly use the existing Project.type
optional union. In the memo that computes parentCustomer (references:
selectedProject, projects, normalizePath, parentCustomer), replace occurrences
like (selectedProject as Project & { type?: string }).type with
selectedProject.type and similarly remove casts on p in the projects.find
callback so you compare p.type === 'customer' and rely on TypeScript's Project
definition; ensure null/undefined checks remain for optional type where needed.
In `@apps/desktop/src/renderer/lib/browser-mock.ts`:
- Around line 144-145: The mock function checkGlobalMcpHealth currently types
its parameter as any; update its signature to use the CustomMcpServer type so it
matches the ElectronAPI interface and allows using server.id safely—locate
checkGlobalMcpHealth in browser-mock.ts and replace the server parameter type
with CustomMcpServer (importing or referencing CustomMcpServer from the shared
types if necessary) to align the mock with the interface.
In `@apps/desktop/src/shared/types/task.ts`:
- Line 239: Replace the inline type import used for phaseCustomAgents with a
named import from ./settings to keep import style consistent: add
PhaseCustomAgentsConfig to the existing import list from './settings' (the same
import that currently brings in other types) and then change the property
declaration phaseCustomAgents?: import('./settings').PhaseCustomAgentsConfig; to
phaseCustomAgents?: PhaseCustomAgentsConfig; so the file consistently uses the
named type import.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e9c4d2a9-0a41-489e-b93f-f1867ee216e9
📒 Files selected for processing (44)
apps/desktop/src/main/claude-code-settings/reader.tsapps/desktop/src/main/claude-code-settings/types.tsapps/desktop/src/main/ipc-handlers/claude-agents-handlers.tsapps/desktop/src/main/ipc-handlers/claude-mcp-handlers.tsapps/desktop/src/main/ipc-handlers/github/customer-github-handlers.tsapps/desktop/src/main/ipc-handlers/github/issue-handlers.tsapps/desktop/src/main/ipc-handlers/index.tsapps/desktop/src/main/ipc-handlers/mcp-handlers.tsapps/desktop/src/main/ipc-handlers/memory-handlers.tsapps/desktop/src/main/ipc-handlers/project-handlers.tsapps/desktop/src/preload/api/modules/github-api.tsapps/desktop/src/renderer/components/AddCustomerModal.tsxapps/desktop/src/renderer/components/AgentTools.tsxapps/desktop/src/renderer/components/CustomerReposModal.tsxapps/desktop/src/renderer/components/GitHubSetupModal.tsxapps/desktop/src/renderer/components/Sidebar.tsxapps/desktop/src/renderer/components/github-issues/components/RepoFilterDropdown.tsxapps/desktop/src/renderer/components/github-issues/hooks/useMultiRepoGitHubIssues.tsapps/desktop/src/renderer/components/github-prs/GitHubPRs.tsxapps/desktop/src/renderer/components/github-prs/hooks/useMultiRepoGitHubPRs.tsapps/desktop/src/renderer/components/ideation/Ideation.tsxapps/desktop/src/renderer/components/settings/AgentProfileSettings.tsxapps/desktop/src/renderer/components/settings/integrations/GitHubIntegration.tsxapps/desktop/src/renderer/lib/browser-mock.tsapps/desktop/src/renderer/stores/context-store.tsapps/desktop/src/shared/constants/ipc.tsapps/desktop/src/shared/i18n/index.tsapps/desktop/src/shared/i18n/locales/en/common.jsonapps/desktop/src/shared/i18n/locales/en/context.jsonapps/desktop/src/shared/i18n/locales/en/dialogs.jsonapps/desktop/src/shared/i18n/locales/en/navigation.jsonapps/desktop/src/shared/i18n/locales/en/settings.jsonapps/desktop/src/shared/i18n/locales/en/tasks.jsonapps/desktop/src/shared/i18n/locales/fr/common.jsonapps/desktop/src/shared/i18n/locales/fr/context.jsonapps/desktop/src/shared/i18n/locales/fr/dialogs.jsonapps/desktop/src/shared/i18n/locales/fr/navigation.jsonapps/desktop/src/shared/i18n/locales/fr/settings.jsonapps/desktop/src/shared/i18n/locales/fr/tasks.jsonapps/desktop/src/shared/types/integrations.tsapps/desktop/src/shared/types/ipc.tsapps/desktop/src/shared/types/project.tsapps/desktop/src/shared/types/settings.tsapps/desktop/src/shared/types/task.ts
Comment on lines
+193
to
+195
| if (typeof page !== 'number' || !Number.isFinite(page) || page < 1) { | ||
| page = 1; | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Avoid reassigning function parameters.
Reassigning page can lead to confusion and may be flagged by strict linters. Use a local variable instead.
♻️ Proposed fix
- if (typeof page !== 'number' || !Number.isFinite(page) || page < 1) {
- page = 1;
- }
+ const validatedPage = (typeof page !== 'number' || !Number.isFinite(page) || page < 1) ? 1 : page;Then use validatedPage in the endpoint construction on line 217.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/main/ipc-handlers/github/customer-github-handlers.ts` around
lines 193 - 195, The code currently reassigns the parameter page; instead create
a new local constant (e.g., validatedPage) and perform the checks on page to set
validatedPage = 1 when invalid, then use validatedPage instead of page when
building the GitHub API endpoint (replace the usage in the endpoint construction
where page is interpolated); update any other references in the function that
rely on the sanitized page value to use validatedPage so the original parameter
is not mutated.
Comment on lines
+368
to
+388
| // TODO: Add a typed interface (e.g. GitHubAPIPullRequest) for the GitHub PR API response shape | ||
| // biome-ignore lint/suspicious/noExplicitAny: GitHub REST API response shape | ||
| const transformed = (data as any[]).map((pr) => ({ | ||
| number: pr.number, | ||
| title: pr.title, | ||
| body: pr.body || '', | ||
| state: pr.state.toLowerCase(), | ||
| author: { login: pr.user.login }, | ||
| headRefName: pr.head.ref, | ||
| baseRefName: pr.base.ref, | ||
| additions: pr.additions ?? 0, | ||
| deletions: pr.deletions ?? 0, | ||
| changedFiles: pr.changed_files ?? 0, | ||
| // biome-ignore lint/suspicious/noExplicitAny: GitHub REST API assignee shape | ||
| assignees: (pr.assignees || []).map((a: any) => ({ login: a.login })), | ||
| createdAt: pr.created_at, | ||
| updatedAt: pr.updated_at, | ||
| htmlUrl: pr.html_url, | ||
| repoFullName, | ||
| })); | ||
| allPRs.push(...transformed); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider extracting PR transformation to a typed helper.
Similar to how transformIssue is imported and used for issues, consider creating a transformPullRequest helper with a proper GitHubAPIPullRequest interface to improve type safety and reusability.
Do you want me to help create a typed interface and transformation function for GitHub PRs?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/main/ipc-handlers/github/customer-github-handlers.ts` around
lines 368 - 388, Extract the inline mapping that builds `transformed` into a new
typed helper function `transformPullRequest(pr: GitHubAPIPullRequest,
repoFullName: string)` and add a `GitHubAPIPullRequest` interface modeling the
fields used (number, title, body, state, user.login, head.ref, base.ref,
additions, deletions, changed_files, assignees, created_at, updated_at,
html_url); replace the inline map with `(data as any[]).map(pr =>
transformPullRequest(pr, repoFullName))` and import/align usage like the
existing `transformIssue` helper so the PR transformation is reusable and
type-safe.
Comment on lines
+139
to
+142
| const sep = window.navigator.platform.startsWith('Win') ? '\\' : '/'; | ||
| const folderPreview = customerName.trim() && location | ||
| ? `${location}${sep}${customerName.trim()}` | ||
| : null; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Platform detection could be more robust.
The current approach works but navigator.platform is deprecated. Consider using a more reliable method.
✨ Alternative approach
- const sep = window.navigator.platform.startsWith('Win') ? '\\' : '/';
+ // Use path.sep if available via preload, or detect from existing path
+ const sep = location.includes('\\') ? '\\' : '/';Or if the preload API exposes path.sep:
const sep = window.electronAPI.pathSep ?? (location.includes('\\') ? '\\' : '/');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/components/AddCustomerModal.tsx` around lines 139 -
142, The platform separator detection using window.navigator.platform is
deprecated; update the logic in AddCustomerModal (symbols: sep and
folderPreview) to prefer a separator provided by the preload API (e.g.,
window.electronAPI.pathSep) and fall back to inferring the separator from the
provided location string (check if location includes '\\' then use '\\' else
'/'), so set sep from window.electronAPI.pathSep ?? (location?.includes('\\') ?
'\\' : '/') before building folderPreview; ensure you preserve the existing
behavior when customerName.trim() and location are present.
Comment on lines
+764
to
+769
| // Auto-check health when global MCPs are loaded | ||
| useEffect(() => { | ||
| if (allGlobalServers.length > 0) { | ||
| checkGlobalMcpHealth(); | ||
| } | ||
| }, [checkGlobalMcpHealth]); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Potential infinite re-check loop if health check modifies state improperly.
The effect depends on checkGlobalMcpHealth, which itself depends on allGlobalServers. While the current implementation is correct because allGlobalServers is memoized and won't change unless globalMcps changes, consider adding a guard to prevent unnecessary re-checks.
💡 Optional: Add explicit guard
// Auto-check health when global MCPs are loaded
useEffect(() => {
- if (allGlobalServers.length > 0) {
+ if (allGlobalServers.length > 0 && !isCheckingGlobalHealth) {
checkGlobalMcpHealth();
}
- }, [checkGlobalMcpHealth]);
+ }, [allGlobalServers]); // Depend on the data, not the callbackThis makes the dependency more explicit and prevents potential issues if the callback identity changes unexpectedly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/components/AgentTools.tsx` around lines 764 - 769,
The useEffect that auto-checks health can re-run if the identity of
checkGlobalMcpHealth changes; to guard against unnecessary loops, update the
effect to explicitly depend on allGlobalServers (and optionally keep
checkGlobalMcpHealth) and add an internal guard that returns early unless
allGlobalServers.length > 0 before calling checkGlobalMcpHealth; reference the
existing useEffect, the allGlobalServers value, and the checkGlobalMcpHealth
callback when making this change.
| <div className="space-y-2"> | ||
| {allGlobalServers.length === 0 && ( | ||
| <p className="text-sm text-muted-foreground text-center py-3"> | ||
| {t('settings:mcp.globalMcps.noServers', 'No global MCP servers found.')} |
Contributor
There was a problem hiding this comment.
Inline fallback string should be moved to translation files.
Using an inline fallback string 'No global MCP servers found.' is inconsistent with the rest of the codebase that relies solely on translation keys. This fallback will show in English for all locales if the key is missing.
🌐 Proposed fix
- {t('settings:mcp.globalMcps.noServers', 'No global MCP servers found.')}
+ {t('settings:mcp.globalMcps.noServers')}Ensure the key mcp.globalMcps.noServers exists in both en/settings.json and fr/settings.json.
As per coding guidelines: "All frontend user-facing text must use react-i18next translation keys. Add keys to both en/*.json and fr/*.json translation files."
📝 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.
Suggested change
| {t('settings:mcp.globalMcps.noServers', 'No global MCP servers found.')} | |
| {t('settings:mcp.globalMcps.noServers')} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/components/AgentTools.tsx` at line 1477, The inline
fallback string in the t(...) call inside AgentTools.tsx uses 'No global MCP
servers found.' — remove that inline English fallback and ensure the translation
key mcp.globalMcps.noServers is defined in the translation files; add the
corresponding entry to en/settings.json and fr/settings.json with appropriate
localized strings, then call t('settings:mcp.globalMcps.noServers') (no inline
fallback) in the component to rely solely on react-i18next.
Comment on lines
+252
to
+261
| // For customers, we only need the GitHub token -- skip repo/branch/claude steps | ||
| if ((project as Project & { type?: string }).type === 'customer') { | ||
| onComplete({ | ||
| githubToken: token, | ||
| githubRepo: '', | ||
| mainBranch: '', | ||
| githubAuthMethod: 'oauth' | ||
| }); | ||
| return; | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Same redundant type assertion.
Apply the same simplification here.
✨ Proposed simplification
// For customers, we only need the GitHub token -- skip repo/branch/claude steps
- if ((project as Project & { type?: string }).type === 'customer') {
+ if (project.type === 'customer') {
onComplete({
githubToken: token,
githubRepo: '',
mainBranch: '',
githubAuthMethod: 'oauth'
});
return;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/components/GitHubSetupModal.tsx` around lines 252 -
261, The code in the GitHubSetupModal component uses a redundant type assertion
when checking project type; simplify the conditional by accessing project.type
directly (remove the unnecessary "(project as Project & { type?: string })"
cast) in the block that calls onComplete with
githubToken/githubRepo/mainBranch/githubAuthMethod for customers, keeping the
existing call to onComplete and the early return unchanged.
Comment on lines
+432
to
+566
| {/* Customer-specific: Clone Repositories */} | ||
| {projectType === 'customer' && envConfig.githubToken && ( | ||
| <> | ||
| <Separator /> | ||
|
|
||
| {/* Default Branch Selector */} | ||
| {projectPath && ( | ||
| <div className="space-y-2"> | ||
| <div className="flex items-center justify-between"> | ||
| <div className="space-y-0.5"> | ||
| <div className="flex items-center gap-2"> | ||
| <GitBranch className="h-4 w-4 text-info" /> | ||
| <Label className="text-sm font-medium text-foreground"> | ||
| {t('settings:projectSections.github.defaultBranch.label')} | ||
| </Label> | ||
| {/* Already cloned repos */} | ||
| {customerChildProjects.length > 0 && ( | ||
| <div className="space-y-2"> | ||
| <Label className="text-sm font-medium text-foreground flex items-center gap-2"> | ||
| <FolderGit2 className="h-4 w-4" /> | ||
| Cloned Repositories | ||
| </Label> | ||
| <div className="space-y-1.5"> | ||
| {customerChildProjects.map((child) => ( | ||
| <div | ||
| key={child.id} | ||
| className="flex items-center gap-2 rounded-md border border-border bg-muted/50 px-3 py-2 text-sm" | ||
| > | ||
| <CheckCircle2 className="h-4 w-4 text-green-500 shrink-0" /> | ||
| <span className="truncate font-medium">{child.name}</span> | ||
| <span className="text-xs text-muted-foreground truncate ml-auto">{child.path}</span> | ||
| </div> | ||
| ))} | ||
| </div> | ||
| <p className="text-xs text-muted-foreground pl-6"> | ||
| {t('settings:projectSections.github.defaultBranch.description')} | ||
| </p> | ||
| </div> | ||
| <Button | ||
| variant="ghost" | ||
| size="sm" | ||
| onClick={fetchBranches} | ||
| disabled={isLoadingBranches} | ||
| className="h-7 px-2" | ||
| > | ||
| <RefreshCw className={`h-3 w-3 ${isLoadingBranches ? 'animate-spin' : ''}`} /> | ||
| </Button> | ||
| </div> | ||
| )} | ||
|
|
||
| {branchesError && ( | ||
| <div className="flex items-center gap-2 text-xs text-destructive pl-6"> | ||
| <AlertCircle className="h-3 w-3" /> | ||
| {branchesError} | ||
| {/* Clone new repos */} | ||
| <div className="space-y-3"> | ||
| <div className="flex items-center justify-between"> | ||
| <Label className="text-sm font-medium text-foreground flex items-center gap-2"> | ||
| <Download className="h-4 w-4" /> | ||
| Clone Repositories | ||
| </Label> | ||
| <Button | ||
| variant="outline" | ||
| size="sm" | ||
| onClick={loadCustomerRepos} | ||
| disabled={isLoadingCustomerRepos} | ||
| > | ||
| {isLoadingCustomerRepos ? ( | ||
| <Loader2 className="mr-1.5 h-3.5 w-3.5 animate-spin" /> | ||
| ) : ( | ||
| <RefreshCw className="mr-1.5 h-3.5 w-3.5" /> | ||
| )} | ||
| {customerRepos.length > 0 ? 'Refresh' : 'Load Repos'} | ||
| </Button> | ||
| </div> | ||
| )} | ||
|
|
||
| <div className="pl-6"> | ||
| <Combobox | ||
| options={branchOptions} | ||
| value={selectedBranch} | ||
| onValueChange={handleBranchChange} | ||
| placeholder={t('settings:projectSections.github.defaultBranch.autoDetect')} | ||
| searchPlaceholder={t('settings:projectSections.github.defaultBranch.searchPlaceholder')} | ||
| emptyMessage={t('settings:projectSections.github.defaultBranch.noBranchesFound')} | ||
| disabled={isLoadingBranches} | ||
| className="w-full" | ||
| /> | ||
| {customerReposError && ( | ||
| <div className="flex items-center gap-2 text-xs text-destructive"> | ||
| <AlertCircle className="h-3 w-3" /> | ||
| {customerReposError} | ||
| </div> | ||
| )} | ||
|
|
||
| {customerRepos.length > 0 && ( | ||
| <> | ||
| {/* Search */} | ||
| <Input | ||
| type="text" | ||
| value={customerRepoSearch} | ||
| onChange={(e) => setCustomerRepoSearch(e.target.value)} | ||
| placeholder="Search repositories..." | ||
| className="h-8 text-xs" | ||
| /> | ||
|
|
||
| {/* Repo list */} | ||
| <div className="space-y-1.5 max-h-[300px] overflow-y-auto"> | ||
| {filteredCustomerRepos.map((repo) => { | ||
| const status = cloneStatuses[repo.fullName] || 'idle'; | ||
| const alreadyCloned = customerChildProjects.some( | ||
| p => p.name === repo.fullName.split('/').pop() | ||
| ); | ||
| const cloneError = cloneErrors[repo.fullName]; | ||
|
|
||
| return ( | ||
| <div | ||
| key={repo.fullName} | ||
| className={cn( | ||
| 'flex items-center gap-2 rounded-md border px-3 py-2 text-sm', | ||
| status === 'done' || alreadyCloned | ||
| ? 'border-green-500/30 bg-green-500/5' | ||
| : 'border-border' | ||
| )} | ||
| > | ||
| <Github className="h-4 w-4 shrink-0 text-muted-foreground" /> | ||
| <div className="flex-1 min-w-0"> | ||
| <div className="flex items-center gap-1.5"> | ||
| <span className="text-xs font-medium truncate">{repo.fullName}</span> | ||
| {repo.isPrivate ? ( | ||
| <Lock className="h-3 w-3 shrink-0 text-muted-foreground" /> | ||
| ) : ( | ||
| <Globe className="h-3 w-3 shrink-0 text-muted-foreground" /> | ||
| )} | ||
| </div> | ||
| {repo.description && ( | ||
| <p className="text-xs text-muted-foreground truncate">{repo.description}</p> | ||
| )} | ||
| {cloneError && ( | ||
| <p className="text-xs text-destructive">{cloneError}</p> | ||
| )} | ||
| </div> | ||
|
|
||
| <div className="shrink-0"> | ||
| {alreadyCloned || status === 'done' ? ( | ||
| <span className="flex items-center gap-1 text-xs text-green-600"> | ||
| <CheckCircle2 className="h-3.5 w-3.5" /> | ||
| Cloned | ||
| </span> | ||
| ) : status === 'cloning' ? ( | ||
| <Button variant="outline" size="sm" disabled className="h-7 text-xs"> | ||
| <Loader2 className="mr-1 h-3 w-3 animate-spin" /> | ||
| Cloning... | ||
| </Button> | ||
| ) : ( | ||
| <Button | ||
| variant="outline" | ||
| size="sm" | ||
| onClick={() => handleCloneRepo(repo)} | ||
| className="h-7 text-xs" | ||
| > | ||
| <Download className="mr-1 h-3 w-3" /> | ||
| Clone | ||
| </Button> | ||
| )} | ||
| </div> | ||
| </div> | ||
| ); | ||
| })} | ||
| </div> | ||
| </> | ||
| )} | ||
| </div> | ||
|
|
||
| {selectedBranch && ( | ||
| <p className="text-xs text-muted-foreground pl-6"> | ||
| {t('settings:projectSections.github.defaultBranch.selectedBranchHelp', { branch: selectedBranch })} | ||
| </p> | ||
| )} | ||
| </div> | ||
| </> |
Contributor
There was a problem hiding this comment.
Hardcoded strings violate i18n guidelines.
The customer-specific clone section contains multiple hardcoded English strings that break localization. These should use translation keys from the settings:github namespace that were added in this PR.
Affected strings:
- Line 442:
"Cloned Repositories"→t('settings:github.clonedRepositories') - Line 464:
"Clone Repositories"→t('settings:github.cloneRepositories') - Line 477:
'Refresh'/'Load Repos'→t('settings:github.refresh')/t('settings:github.loadRepos') - Line 495:
"Search repositories..."→t('settings:github.searchRepos') - Line 540:
"Cloned"→t('settings:github.cloned') - Line 545:
"Cloning..."→t('settings:github.cloning') - Line 555:
"Clone"→t('settings:github.clone')
As per coding guidelines: "All frontend user-facing text must use react-i18next translation keys."
🌐 Proposed fix for i18n compliance
<Label className="text-sm font-medium text-foreground flex items-center gap-2">
<FolderGit2 className="h-4 w-4" />
- Cloned Repositories
+ {t('settings:github.clonedRepositories')}
</Label> <Label className="text-sm font-medium text-foreground flex items-center gap-2">
<Download className="h-4 w-4" />
- Clone Repositories
+ {t('settings:github.cloneRepositories')}
</Label>- {customerRepos.length > 0 ? 'Refresh' : 'Load Repos'}
+ {customerRepos.length > 0 ? t('settings:github.refresh') : t('settings:github.loadRepos')} <Input
type="text"
value={customerRepoSearch}
onChange={(e) => setCustomerRepoSearch(e.target.value)}
- placeholder="Search repositories..."
+ placeholder={t('settings:github.searchRepos')}
className="h-8 text-xs"
/> {alreadyCloned || status === 'done' ? (
<span className="flex items-center gap-1 text-xs text-green-600">
<CheckCircle2 className="h-3.5 w-3.5" />
- Cloned
+ {t('settings:github.cloned')}
</span>
) : status === 'cloning' ? (
<Button variant="outline" size="sm" disabled className="h-7 text-xs">
<Loader2 className="mr-1 h-3 w-3 animate-spin" />
- Cloning...
+ {t('settings:github.cloning')}
</Button>
) : (
<Button
variant="outline"
size="sm"
onClick={() => handleCloneRepo(repo)}
className="h-7 text-xs"
>
<Download className="mr-1 h-3 w-3" />
- Clone
+ {t('settings:github.clone')}
</Button>📝 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.
Suggested change
| {/* Customer-specific: Clone Repositories */} | |
| {projectType === 'customer' && envConfig.githubToken && ( | |
| <> | |
| <Separator /> | |
| {/* Default Branch Selector */} | |
| {projectPath && ( | |
| <div className="space-y-2"> | |
| <div className="flex items-center justify-between"> | |
| <div className="space-y-0.5"> | |
| <div className="flex items-center gap-2"> | |
| <GitBranch className="h-4 w-4 text-info" /> | |
| <Label className="text-sm font-medium text-foreground"> | |
| {t('settings:projectSections.github.defaultBranch.label')} | |
| </Label> | |
| {/* Already cloned repos */} | |
| {customerChildProjects.length > 0 && ( | |
| <div className="space-y-2"> | |
| <Label className="text-sm font-medium text-foreground flex items-center gap-2"> | |
| <FolderGit2 className="h-4 w-4" /> | |
| Cloned Repositories | |
| </Label> | |
| <div className="space-y-1.5"> | |
| {customerChildProjects.map((child) => ( | |
| <div | |
| key={child.id} | |
| className="flex items-center gap-2 rounded-md border border-border bg-muted/50 px-3 py-2 text-sm" | |
| > | |
| <CheckCircle2 className="h-4 w-4 text-green-500 shrink-0" /> | |
| <span className="truncate font-medium">{child.name}</span> | |
| <span className="text-xs text-muted-foreground truncate ml-auto">{child.path}</span> | |
| </div> | |
| ))} | |
| </div> | |
| <p className="text-xs text-muted-foreground pl-6"> | |
| {t('settings:projectSections.github.defaultBranch.description')} | |
| </p> | |
| </div> | |
| <Button | |
| variant="ghost" | |
| size="sm" | |
| onClick={fetchBranches} | |
| disabled={isLoadingBranches} | |
| className="h-7 px-2" | |
| > | |
| <RefreshCw className={`h-3 w-3 ${isLoadingBranches ? 'animate-spin' : ''}`} /> | |
| </Button> | |
| </div> | |
| )} | |
| {branchesError && ( | |
| <div className="flex items-center gap-2 text-xs text-destructive pl-6"> | |
| <AlertCircle className="h-3 w-3" /> | |
| {branchesError} | |
| {/* Clone new repos */} | |
| <div className="space-y-3"> | |
| <div className="flex items-center justify-between"> | |
| <Label className="text-sm font-medium text-foreground flex items-center gap-2"> | |
| <Download className="h-4 w-4" /> | |
| Clone Repositories | |
| </Label> | |
| <Button | |
| variant="outline" | |
| size="sm" | |
| onClick={loadCustomerRepos} | |
| disabled={isLoadingCustomerRepos} | |
| > | |
| {isLoadingCustomerRepos ? ( | |
| <Loader2 className="mr-1.5 h-3.5 w-3.5 animate-spin" /> | |
| ) : ( | |
| <RefreshCw className="mr-1.5 h-3.5 w-3.5" /> | |
| )} | |
| {customerRepos.length > 0 ? 'Refresh' : 'Load Repos'} | |
| </Button> | |
| </div> | |
| )} | |
| <div className="pl-6"> | |
| <Combobox | |
| options={branchOptions} | |
| value={selectedBranch} | |
| onValueChange={handleBranchChange} | |
| placeholder={t('settings:projectSections.github.defaultBranch.autoDetect')} | |
| searchPlaceholder={t('settings:projectSections.github.defaultBranch.searchPlaceholder')} | |
| emptyMessage={t('settings:projectSections.github.defaultBranch.noBranchesFound')} | |
| disabled={isLoadingBranches} | |
| className="w-full" | |
| /> | |
| {customerReposError && ( | |
| <div className="flex items-center gap-2 text-xs text-destructive"> | |
| <AlertCircle className="h-3 w-3" /> | |
| {customerReposError} | |
| </div> | |
| )} | |
| {customerRepos.length > 0 && ( | |
| <> | |
| {/* Search */} | |
| <Input | |
| type="text" | |
| value={customerRepoSearch} | |
| onChange={(e) => setCustomerRepoSearch(e.target.value)} | |
| placeholder="Search repositories..." | |
| className="h-8 text-xs" | |
| /> | |
| {/* Repo list */} | |
| <div className="space-y-1.5 max-h-[300px] overflow-y-auto"> | |
| {filteredCustomerRepos.map((repo) => { | |
| const status = cloneStatuses[repo.fullName] || 'idle'; | |
| const alreadyCloned = customerChildProjects.some( | |
| p => p.name === repo.fullName.split('/').pop() | |
| ); | |
| const cloneError = cloneErrors[repo.fullName]; | |
| return ( | |
| <div | |
| key={repo.fullName} | |
| className={cn( | |
| 'flex items-center gap-2 rounded-md border px-3 py-2 text-sm', | |
| status === 'done' || alreadyCloned | |
| ? 'border-green-500/30 bg-green-500/5' | |
| : 'border-border' | |
| )} | |
| > | |
| <Github className="h-4 w-4 shrink-0 text-muted-foreground" /> | |
| <div className="flex-1 min-w-0"> | |
| <div className="flex items-center gap-1.5"> | |
| <span className="text-xs font-medium truncate">{repo.fullName}</span> | |
| {repo.isPrivate ? ( | |
| <Lock className="h-3 w-3 shrink-0 text-muted-foreground" /> | |
| ) : ( | |
| <Globe className="h-3 w-3 shrink-0 text-muted-foreground" /> | |
| )} | |
| </div> | |
| {repo.description && ( | |
| <p className="text-xs text-muted-foreground truncate">{repo.description}</p> | |
| )} | |
| {cloneError && ( | |
| <p className="text-xs text-destructive">{cloneError}</p> | |
| )} | |
| </div> | |
| <div className="shrink-0"> | |
| {alreadyCloned || status === 'done' ? ( | |
| <span className="flex items-center gap-1 text-xs text-green-600"> | |
| <CheckCircle2 className="h-3.5 w-3.5" /> | |
| Cloned | |
| </span> | |
| ) : status === 'cloning' ? ( | |
| <Button variant="outline" size="sm" disabled className="h-7 text-xs"> | |
| <Loader2 className="mr-1 h-3 w-3 animate-spin" /> | |
| Cloning... | |
| </Button> | |
| ) : ( | |
| <Button | |
| variant="outline" | |
| size="sm" | |
| onClick={() => handleCloneRepo(repo)} | |
| className="h-7 text-xs" | |
| > | |
| <Download className="mr-1 h-3 w-3" /> | |
| Clone | |
| </Button> | |
| )} | |
| </div> | |
| </div> | |
| ); | |
| })} | |
| </div> | |
| </> | |
| )} | |
| </div> | |
| {selectedBranch && ( | |
| <p className="text-xs text-muted-foreground pl-6"> | |
| {t('settings:projectSections.github.defaultBranch.selectedBranchHelp', { branch: selectedBranch })} | |
| </p> | |
| )} | |
| </div> | |
| </> | |
| {/* Customer-specific: Clone Repositories */} | |
| {projectType === 'customer' && envConfig.githubToken && ( | |
| <> | |
| <Separator /> | |
| {/* Already cloned repos */} | |
| {customerChildProjects.length > 0 && ( | |
| <div className="space-y-2"> | |
| <Label className="text-sm font-medium text-foreground flex items-center gap-2"> | |
| <FolderGit2 className="h-4 w-4" /> | |
| {t('settings:github.clonedRepositories')} | |
| </Label> | |
| <div className="space-y-1.5"> | |
| {customerChildProjects.map((child) => ( | |
| <div | |
| key={child.id} | |
| className="flex items-center gap-2 rounded-md border border-border bg-muted/50 px-3 py-2 text-sm" | |
| > | |
| <CheckCircle2 className="h-4 w-4 text-green-500 shrink-0" /> | |
| <span className="truncate font-medium">{child.name}</span> | |
| <span className="text-xs text-muted-foreground truncate ml-auto">{child.path}</span> | |
| </div> | |
| ))} | |
| </div> | |
| </div> | |
| )} | |
| {/* Clone new repos */} | |
| <div className="space-y-3"> | |
| <div className="flex items-center justify-between"> | |
| <Label className="text-sm font-medium text-foreground flex items-center gap-2"> | |
| <Download className="h-4 w-4" /> | |
| {t('settings:github.cloneRepositories')} | |
| </Label> | |
| <Button | |
| variant="outline" | |
| size="sm" | |
| onClick={loadCustomerRepos} | |
| disabled={isLoadingCustomerRepos} | |
| > | |
| {isLoadingCustomerRepos ? ( | |
| <Loader2 className="mr-1.5 h-3.5 w-3.5 animate-spin" /> | |
| ) : ( | |
| <RefreshCw className="mr-1.5 h-3.5 w-3.5" /> | |
| )} | |
| {customerRepos.length > 0 ? t('settings:github.refresh') : t('settings:github.loadRepos')} | |
| </Button> | |
| </div> | |
| {customerReposError && ( | |
| <div className="flex items-center gap-2 text-xs text-destructive"> | |
| <AlertCircle className="h-3 w-3" /> | |
| {customerReposError} | |
| </div> | |
| )} | |
| {customerRepos.length > 0 && ( | |
| <> | |
| {/* Search */} | |
| <Input | |
| type="text" | |
| value={customerRepoSearch} | |
| onChange={(e) => setCustomerRepoSearch(e.target.value)} | |
| placeholder={t('settings:github.searchRepos')} | |
| className="h-8 text-xs" | |
| /> | |
| {/* Repo list */} | |
| <div className="space-y-1.5 max-h-[300px] overflow-y-auto"> | |
| {filteredCustomerRepos.map((repo) => { | |
| const status = cloneStatuses[repo.fullName] || 'idle'; | |
| const alreadyCloned = customerChildProjects.some( | |
| p => p.name === repo.fullName.split('/').pop() | |
| ); | |
| const cloneError = cloneErrors[repo.fullName]; | |
| return ( | |
| <div | |
| key={repo.fullName} | |
| className={cn( | |
| 'flex items-center gap-2 rounded-md border px-3 py-2 text-sm', | |
| status === 'done' || alreadyCloned | |
| ? 'border-green-500/30 bg-green-500/5' | |
| : 'border-border' | |
| )} | |
| > | |
| <Github className="h-4 w-4 shrink-0 text-muted-foreground" /> | |
| <div className="flex-1 min-w-0"> | |
| <div className="flex items-center gap-1.5"> | |
| <span className="text-xs font-medium truncate">{repo.fullName}</span> | |
| {repo.isPrivate ? ( | |
| <Lock className="h-3 w-3 shrink-0 text-muted-foreground" /> | |
| ) : ( | |
| <Globe className="h-3 w-3 shrink-0 text-muted-foreground" /> | |
| )} | |
| </div> | |
| {repo.description && ( | |
| <p className="text-xs text-muted-foreground truncate">{repo.description}</p> | |
| )} | |
| {cloneError && ( | |
| <p className="text-xs text-destructive">{cloneError}</p> | |
| )} | |
| </div> | |
| <div className="shrink-0"> | |
| {alreadyCloned || status === 'done' ? ( | |
| <span className="flex items-center gap-1 text-xs text-green-600"> | |
| <CheckCircle2 className="h-3.5 w-3.5" /> | |
| {t('settings:github.cloned')} | |
| </span> | |
| ) : status === 'cloning' ? ( | |
| <Button variant="outline" size="sm" disabled className="h-7 text-xs"> | |
| <Loader2 className="mr-1 h-3 w-3 animate-spin" /> | |
| {t('settings:github.cloning')} | |
| </Button> | |
| ) : ( | |
| <Button | |
| variant="outline" | |
| size="sm" | |
| onClick={() => handleCloneRepo(repo)} | |
| className="h-7 text-xs" | |
| > | |
| <Download className="mr-1 h-3 w-3" /> | |
| {t('settings:github.clone')} | |
| </Button> | |
| )} | |
| </div> | |
| </div> | |
| ); | |
| })} | |
| </div> | |
| </> | |
| )} | |
| </div> | |
| </> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/components/settings/integrations/GitHubIntegration.tsx`
around lines 432 - 566, Replace the hardcoded English UI strings in the
customer-specific clone section of GitHubIntegration (the JSX that renders
Cloned Repositories, Clone Repositories, button labels, search placeholder, and
per-repo status labels) with the corresponding react-i18next keys from the
settings:github namespace (e.g., use t('settings:github.clonedRepositories'),
t('settings:github.cloneRepositories'), t('settings:github.refresh') /
t('settings:github.loadRepos'), t('settings:github.searchRepos'),
t('settings:github.cloned'), t('settings:github.cloning'),
t('settings:github.clone')). Ensure the component uses the useTranslation hook
(t) already present in the file and update strings in the JSX blocks that
reference customerChildProjects, loadCustomerRepos, customerRepoSearch,
filteredCustomerRepos, cloneStatuses, cloneErrors, and handleCloneRepo so all
user-facing text uses t(...).
Comment on lines
+140
to
+146
| if ((selectedProject as Project & { type?: string }).type === 'customer') return selectedProject; | ||
| const normalizedSelected = normalizePath(selectedProject.path); | ||
| const parentCustomer = projects.find( | ||
| p => (p as Project & { type?: string }).type === 'customer' && normalizedSelected.startsWith(normalizePath(p.path) + '/') | ||
| ); | ||
| return parentCustomer ?? null; | ||
| }, [selectedProject, projects]); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if Project type already has a 'type' field
ast-grep --pattern 'interface Project {
$$$
}'
# Also check type definition files
rg -n "type Project\s*=" --type ts
rg -n "interface Project" --type ts -A 10Repository: AndyMik90/Aperant
Length of output: 19698
🏁 Script executed:
# Search for all type assertions related to Project & { type?: string } in Sidebar.tsx
rg -n "Project.*type\?" apps/desktop/src/renderer/components/Sidebar.tsx -A 2 -B 2
# Also check TypeScript configuration
cat apps/desktop/tsconfig.json | head -30Repository: AndyMik90/Aperant
Length of output: 1917
Remove unnecessary type assertions for the type field.
The Project interface already includes type?: 'project' | 'customer' (defined in apps/desktop/src/shared/types/project.ts), making the repeated assertions (selectedProject as Project & { type?: string }) on lines 140, 143, 258, and 451 redundant. Remove these assertions and access the field directly: selectedProject.type === 'customer'. Additionally, the assertions widen the type to string, losing type safety—use the existing type definition instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/components/Sidebar.tsx` around lines 140 - 146,
Remove the redundant broad type assertions around selectedProject and project
elements; directly use the existing Project.type optional union. In the memo
that computes parentCustomer (references: selectedProject, projects,
normalizePath, parentCustomer), replace occurrences like (selectedProject as
Project & { type?: string }).type with selectedProject.type and similarly remove
casts on p in the projects.find callback so you compare p.type === 'customer'
and rely on TypeScript's Project definition; ensure null/undefined checks remain
for optional type where needed.
Comment on lines
+144
to
+145
| checkGlobalMcpHealth: async (server: any) => ({ | ||
| success: true, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify remaining explicit `any` in this mock file
rg -nP '\b:\s*any\b' apps/desktop/src/renderer/lib/browser-mock.tsRepository: AndyMik90/Aperant
Length of output: 114
🏁 Script executed:
# Find ElectronAPI interface definition
fd -t f -e ts -e tsx | xargs rg -l "interface ElectronAPI|type ElectronAPI" | head -5Repository: AndyMik90/Aperant
Length of output: 135
🏁 Script executed:
# Check tsconfig.json for strict mode
cat apps/desktop/tsconfig.json 2>/dev/null || cat tsconfig.json 2>/dev/null || find . -name "tsconfig.json" -type f | head -3Repository: AndyMik90/Aperant
Length of output: 876
🏁 Script executed:
# View context around line 144 in browser-mock.ts
sed -n '140,160p' apps/desktop/src/renderer/lib/browser-mock.tsRepository: AndyMik90/Aperant
Length of output: 651
🏁 Script executed:
# Check the ElectronAPI interface definition for checkGlobalMcpHealth
rg -A 3 "checkGlobalMcpHealth" apps/desktop/src/preload/api/index.ts apps/desktop/src/shared/types/ipc.tsRepository: AndyMik90/Aperant
Length of output: 441
🏁 Script executed:
# Also view the full ElectronAPI type definition to understand server parameter type
rg -B 5 "checkGlobalMcpHealth" apps/desktop/src/shared/types/ipc.ts | head -20Repository: AndyMik90/Aperant
Length of output: 314
🏁 Script executed:
# Check CustomMcpServer definition
rg -B 2 -A 5 "type CustomMcpServer|interface CustomMcpServer" apps/desktop/src/shared/types/Repository: AndyMik90/Aperant
Length of output: 662
Use the correct type for server parameter in mock.
checkGlobalMcpHealth should use CustomMcpServer type to match the ElectronAPI interface signature:
- checkGlobalMcpHealth: async (server: any) => ({
+ checkGlobalMcpHealth: async (server: CustomMcpServer) => ({The interface in apps/desktop/src/shared/types/ipc.ts defines (server: CustomMcpServer), and the mock function accesses server.id, confirming this is the correct type.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/lib/browser-mock.ts` around lines 144 - 145, The
mock function checkGlobalMcpHealth currently types its parameter as any; update
its signature to use the CustomMcpServer type so it matches the ElectronAPI
interface and allows using server.id safely—locate checkGlobalMcpHealth in
browser-mock.ts and replace the server parameter type with CustomMcpServer
(importing or referencing CustomMcpServer from the shared types if necessary) to
align the mock with the interface.
| isAutoProfile?: boolean; // True when using Auto (Optimized) profile | ||
| phaseModels?: PhaseModelConfig; // Per-phase model configuration | ||
| phaseThinking?: PhaseThinkingConfig; // Per-phase thinking configuration | ||
| phaseCustomAgents?: import('./settings').PhaseCustomAgentsConfig; // Per-phase custom agent IDs |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using consistent import style.
The inline import('./settings').PhaseCustomAgentsConfig type works, but Line 5 already imports other types from ./settings. Adding PhaseCustomAgentsConfig to that import statement would be more consistent.
♻️ Suggested refactor
Update the import at Line 5:
-import type { ThinkingLevel, PhaseModelConfig, PhaseThinkingConfig } from './settings';
+import type { ThinkingLevel, PhaseModelConfig, PhaseThinkingConfig, PhaseCustomAgentsConfig } from './settings';Then simplify Line 239:
- phaseCustomAgents?: import('./settings').PhaseCustomAgentsConfig; // Per-phase custom agent IDs
+ phaseCustomAgents?: PhaseCustomAgentsConfig; // Per-phase custom agent IDs🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/shared/types/task.ts` at line 239, Replace the inline type
import used for phaseCustomAgents with a named import from ./settings to keep
import style consistent: add PhaseCustomAgentsConfig to the existing import list
from './settings' (the same import that currently brings in other types) and
then change the property declaration phaseCustomAgents?:
import('./settings').PhaseCustomAgentsConfig; to phaseCustomAgents?:
PhaseCustomAgentsConfig; so the file consistently uses the named type import.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| setIsLoading(true); | ||
| setError(null); | ||
| try { | ||
| const result = await window.electronAPI.listGitHubUserRepos(); |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| title: pr.title, | ||
| body: pr.body || '', | ||
| state: pr.state.toLowerCase(), | ||
| author: { login: pr.user.login }, |
There was a problem hiding this comment.
Bug: The code accesses pr.user.login without a null check, causing a TypeError and failing the entire PR fetch if any PR is from a deleted user.
Severity: HIGH
Suggested Fix
Use optional chaining and a nullish coalescing operator to safely access the author's login, similar to other parts of the codebase. Change pr.user.login to pr.user?.login ?? 'unknown'. This will prevent the TypeError and ensure the operation can complete successfully.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: apps/desktop/src/main/ipc-handlers/github/customer-github-handlers.ts#L375
Potential issue: In the `getMultiRepoPRs` handler, the code transforms pull request data
by accessing `pr.user.login` without a null check. The GitHub API can return a `null`
value for the `user` object if the author's account has been deleted. This will throw a
`TypeError`, which is caught but causes the entire multi-repo PR fetch operation to fail
for the customer, even if only one PR has a null author. Other parts of the codebase
already handle this case safely.
Did we get this right? 👍 / 👎 to inform future reviews.
Comment on lines
+83
to
+87
| // Fallback: try getGitHubConfig which also checks gh CLI | ||
| if (!token) { | ||
| const fallbackConfig = getGitHubConfig(customer); | ||
| token = fallbackConfig?.token; | ||
| } |
There was a problem hiding this comment.
Bug: A race condition during new customer project creation causes GitHub operations to fail on first use because autoBuildPath is not yet available when getCustomerGitHubConfig is called.
Severity: HIGH
Suggested Fix
Modify the PROJECT_ADD IPC handler to accept and persist the type: 'customer' parameter when adding a project. This ensures that the project object is correctly configured from the start, or alternatively, make getCustomerGitHubConfig wait for the .auto-claude/ directory to exist before proceeding.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: apps/desktop/src/main/ipc-handlers/github/customer-github-handlers.ts#L83-L87
Potential issue: When a new customer project is created, the `addProject` IPC handler
does not persist the project's `type`. A separate, asynchronous call to
`initializeCustomerProject` is responsible for creating the `.auto-claude/` directory
and setting the `autoBuildPath`. This creates a race condition where GitHub-related IPC
handlers like `getMultiRepoIssues` can be called before `autoBuildPath` is set. The
`getCustomerGitHubConfig` function then returns `null`, causing all multi-repo GitHub
operations to fail until the application is restarted.
Did we get this right? 👍 / 👎 to inform future reviews.
Adds a compact status banner in TaskCard that appears when an agent is actively running. Shows: - Current subtask being analyzed (with phase-colored pulsing dot) - Next pending subtask in the queue - Live elapsed timer (MM:SS, auto-updating) - Activity summary: completed/in-progress counts from last 30 minutes - Phase transition message when available The banner renders only during active execution phases and disappears when idle, complete, or failed. Uses motion/react for smooth entrance animation and i18n for all text (en + fr translations included). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment on lines
+59
to
+65
| const result = await window.electronAPI.getClaudeAgents(); | ||
| if (result.success && result.data) { | ||
| setAgentsInfo(result.data); | ||
| } | ||
| } catch { | ||
| // Silently fail - custom agents are optional | ||
| } |
There was a problem hiding this comment.
Bug: The getClaudeAgents function is called in the renderer but is not exposed on the electronAPI in the preload script, causing the call to fail silently.
Severity: CRITICAL
Suggested Fix
Expose the getClaudeAgents function in the preload API. In a relevant preload API module, create a function that uses invokeIpc to call the CLAUDE_AGENTS_GET IPC channel and add it to the electronAPI interface.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: apps/desktop/src/renderer/components/settings/AgentProfileSettings.tsx#L59-L65
Potential issue: The `AgentProfileSettings.tsx` component attempts to call
`window.electronAPI.getClaudeAgents()` to fetch custom agents. However, this function is
not exposed on the `electronAPI` object in the preload script. While an IPC handler for
`CLAUDE_AGENTS_GET` exists in the main process, the necessary bridge in the preload API
to connect the renderer's call to the main process handler is missing. This results in a
`TypeError` at runtime, which is caught and suppressed, causing the feature to fail
silently. Consequently, the custom agents catalog will always appear empty to the user.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/components/TaskCard.tsx (1)
101-102:⚠️ Potential issue | 🟡 MinorConsider adding
currentSubtaskto memo comparator.The
AgentAnalysisBannerdisplaysexecutionProgress?.currentSubtask, buttaskCardPropsAreEqualonly comparesphaseandphaseProgress. IfcurrentSubtaskchanges independently, the card won't re-render and the banner will show stale data.🛠️ Suggested fix
prevTask.executionProgress?.phase === nextTask.executionProgress?.phase && prevTask.executionProgress?.phaseProgress === nextTask.executionProgress?.phaseProgress && + prevTask.executionProgress?.currentSubtask === nextTask.executionProgress?.currentSubtask && prevTask.subtasks.length === nextTask.subtasks.length &&🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/TaskCard.tsx` around lines 101 - 102, taskCardPropsAreEqual's memo comparator only checks executionProgress.phase and phaseProgress, so AgentAnalysisBanner's executionProgress?.currentSubtask can change without triggering a re-render; update the comparator function taskCardPropsAreEqual to also compare prevTask.executionProgress?.currentSubtask === nextTask.executionProgress?.currentSubtask (handling undefined/null safely) so the card re-renders when currentSubtask changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/renderer/components/AgentAnalysisBanner.tsx`:
- Around line 124-128: The activitySummary computed in the useMemo (const
activitySummary = useMemo(...)) ignores subtasks with status 'failed' even
though SubtaskStatus includes 'failed'; update the calculation to include a
failedCount (e.g., count subtasks where s.status === 'failed') and return it
from activitySummary, then update any UI that consumes activitySummary (the
AgentAnalysisBanner component) to display or appropriately group failedCount (or
merge it with completedCount if that behavior is desired).
- Around line 4-5: Replace the relative imports in AgentAnalysisBanner.tsx with
the configured path aliases: change the import of cn from '../lib/utils' to use
'@lib/utils' and change the type imports (ExecutionPhase, ExecutionProgress,
Subtask) from '../../shared/types' to '@shared/types' so the file uses the
tsconfig path aliases; update any other similar relative imports in this file to
their `@lib/`* or `@shared/`* equivalents to match project guidelines.
- Around line 123-129: The activitySummary currently counts all subtasks but the
UI label (I18N.last30min) implies a 30-minute window; update the useMemo in
activitySummary to filter subtasks by a timestamp (e.g., subtask.statusUpdatedAt
or subtask.updatedAt) within the last 30 minutes before computing
completedCount, inProgressCount, and pendingCount, using Date.now() - 30*60*1000
for the cutoff; if subtasks lack a timestamp field, add/propagate a
statusUpdatedAt (or updatedAt) to each subtask when its status changes and then
use that field in the filter, leaving I18N.last30min as-is.
---
Outside diff comments:
In `@apps/desktop/src/renderer/components/TaskCard.tsx`:
- Around line 101-102: taskCardPropsAreEqual's memo comparator only checks
executionProgress.phase and phaseProgress, so AgentAnalysisBanner's
executionProgress?.currentSubtask can change without triggering a re-render;
update the comparator function taskCardPropsAreEqual to also compare
prevTask.executionProgress?.currentSubtask ===
nextTask.executionProgress?.currentSubtask (handling undefined/null safely) so
the card re-renders when currentSubtask changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5ed5315c-8c63-4160-b4b0-c3c6952e253a
📒 Files selected for processing (4)
apps/desktop/src/renderer/components/AgentAnalysisBanner.tsxapps/desktop/src/renderer/components/TaskCard.tsxapps/desktop/src/shared/i18n/locales/en/tasks.jsonapps/desktop/src/shared/i18n/locales/fr/tasks.json
Comment on lines
+4
to
+5
| import { cn } from '../lib/utils'; | ||
| import type { ExecutionPhase, ExecutionProgress, Subtask } from '../../shared/types'; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use path aliases for imports.
The coding guidelines specify using path aliases defined in tsconfig.json for imports. Consider using @lib/* and @shared/* aliases instead of relative paths.
♻️ Suggested refactor
-import { cn } from '../lib/utils';
-import type { ExecutionPhase, ExecutionProgress, Subtask } from '../../shared/types';
+import { cn } from '@lib/utils';
+import type { ExecutionPhase, ExecutionProgress, Subtask } from '@shared/types';📝 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.
Suggested change
| import { cn } from '../lib/utils'; | |
| import type { ExecutionPhase, ExecutionProgress, Subtask } from '../../shared/types'; | |
| import { cn } from '@lib/utils'; | |
| import type { ExecutionPhase, ExecutionProgress, Subtask } from '@shared/types'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/components/AgentAnalysisBanner.tsx` around lines 4
- 5, Replace the relative imports in AgentAnalysisBanner.tsx with the configured
path aliases: change the import of cn from '../lib/utils' to use '@lib/utils'
and change the type imports (ExecutionPhase, ExecutionProgress, Subtask) from
'../../shared/types' to '@shared/types' so the file uses the tsconfig path
aliases; update any other similar relative imports in this file to their `@lib/`*
or `@shared/`* equivalents to match project guidelines.
Comment on lines
+123
to
+129
| // Activity summary for the last 30 minutes | ||
| const activitySummary = useMemo(() => { | ||
| const completedCount = subtasks.filter((s) => s.status === 'completed').length; | ||
| const inProgressCount = subtasks.filter((s) => s.status === 'in_progress').length; | ||
| const pendingCount = subtasks.filter((s) => s.status === 'pending').length; | ||
| return { completedCount, inProgressCount, pendingCount }; | ||
| }, [subtasks]); |
Contributor
There was a problem hiding this comment.
Activity summary label does not match implementation.
The UI displays "Last 30min" (line 195 via I18N.last30min), but activitySummary counts all subtasks regardless of when they changed status. This is misleading to users who expect time-filtered metrics.
Either:
- Remove the "Last 30min" qualifier and just show totals, or
- Add timestamp tracking to subtasks and filter by the last 30 minutes
🐛 Option 1: Remove time qualifier from label
Update i18n keys to remove the time reference:
// In en/tasks.json and fr/tasks.json
-"last30min": "Last 30min",
+"activity": "Activity",And update the I18N constant:
const I18N = {
// ...
- last30min: 'analysisBanner.last30min',
+ activity: 'analysisBanner.activity',
// ...
} as const;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/components/AgentAnalysisBanner.tsx` around lines
123 - 129, The activitySummary currently counts all subtasks but the UI label
(I18N.last30min) implies a 30-minute window; update the useMemo in
activitySummary to filter subtasks by a timestamp (e.g., subtask.statusUpdatedAt
or subtask.updatedAt) within the last 30 minutes before computing
completedCount, inProgressCount, and pendingCount, using Date.now() - 30*60*1000
for the cutoff; if subtasks lack a timestamp field, add/propagate a
statusUpdatedAt (or updatedAt) to each subtask when its status changes and then
use that field in the filter, leaving I18N.last30min as-is.
Comment on lines
+124
to
+128
| const activitySummary = useMemo(() => { | ||
| const completedCount = subtasks.filter((s) => s.status === 'completed').length; | ||
| const inProgressCount = subtasks.filter((s) => s.status === 'in_progress').length; | ||
| const pendingCount = subtasks.filter((s) => s.status === 'pending').length; | ||
| return { completedCount, inProgressCount, pendingCount }; |
Contributor
There was a problem hiding this comment.
Consider handling 'failed' subtask status.
Per apps/desktop/src/shared/types/task.ts, SubtaskStatus includes 'failed', but it's not counted in activitySummary. Failed subtasks will be invisible in the activity display.
If intentional, this is fine. Otherwise, consider adding a failed count or grouping failed with completed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/components/AgentAnalysisBanner.tsx` around lines
124 - 128, The activitySummary computed in the useMemo (const activitySummary =
useMemo(...)) ignores subtasks with status 'failed' even though SubtaskStatus
includes 'failed'; update the calculation to include a failedCount (e.g., count
subtasks where s.status === 'failed') and return it from activitySummary, then
update any UI that consumes activitySummary (the AgentAnalysisBanner component)
to display or appropriately group failedCount (or merge it with completedCount
if that behavior is desired).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Ports the viable frontend changes from PR #1920 to the current
apps/desktop/structure, after upstream restructuredapps/frontend/→apps/desktop/and migrated the backend from Python to TypeScript (Vercel AI SDK v6).What's included
~/.claude/agents/definitions, specialist agents catalog in AgentProfileSettingscontextnamespace, 100+ translation keys (en + fr) for settings, dialogs, navigation, tasksgetUserConfigDirandtransformIssue, added type annotations eliminating implicitanyerrorsWhat was dropped (incompatible with new architecture)
apps/backend/Python changes (backend migrated to TypeScript)apps/desktop/src/main/ai/)test_episode_types.py,test_integration_graphiti.py,test_integration_ollama.py)Supersedes
apps/frontend/→apps/desktop/rename + backend Python deletion)Test plan
npx tsc --noEmit— 0 new errors)apps/desktop/pathsenandfrlocales🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements