feat: JIRA integration, Vault support, and consolidated settings UI#1979
feat: JIRA integration, Vault support, and consolidated settings UI#1979thameema wants to merge 5 commits intoAndyMik90:developfrom
Conversation
Migrate JIRA issue tracking and Vault context integration from the previous Python backend to the new TypeScript/Vercel AI SDK architecture. Backend (main process): - Add JIRA IPC handlers (test connection, CRUD, search, transitions) - Add Vault IPC handlers (path validation, file browsing, context loading) - Register JIRA and Vault MCP servers in mcp/registry.ts - Add JIRA_TOOLS and VAULT_TOOLS to agent-configs.ts - Add McpServerId entries for 'jira' and 'vault' Frontend: - Add JIRA/Vault settings to AppSettings and ProjectEnvConfig types - Add IPC channel constants for all JIRA/Vault operations - Create preload API modules (jira-api.ts, vault-api.ts) - Wire APIs into AgentAPI aggregator - Update browser mock with JIRA/Vault stubs - Add i18n keys for JIRA and Vault settings (en + fr) Security: - SSRF protection for JIRA host URLs (block private IPs) - Path traversal prevention for Vault file operations - Vault path validation (block sensitive system directories) - File size limits on vault reads (50KB max)
JIRA Integration (full feature parity with GitHub/GitLab): - JIRA Issues sidebar view with browse, search, filter, assignee - Create tasks from JIRA issues with investigation flow - JIRA description (ADF-to-text) and comments pulled into task specs - JIRA config persistence in project .env (JIRA_HOST, EMAIL, TOKEN, PROJECT_KEY) - Independent issue tracking flags (separate from source control) - JIRA MCP server registration for AI agent access - JIRA search uses POST /rest/api/3/search/jql (v3 API) - Task metadata: jiraIssueKey, jiraUrl for cross-referencing Vault Integration: - Vault IPC handlers (path validation, file browsing, context loading) - Vault settings UI with validate button - Vault MCP server for agent context injection - Security: path traversal prevention, symlink checks, size limits Consolidated Settings UI: - Project settings: General | Source Control | Issue Tracking | Memory & Context - Source Control tab: GitHub | GitLab (repos, branches, PRs/MRs) - Issue Tracking tab: GitHub | GitLab | Linear | JIRA (independent toggles) - Memory & Context: Memory backend + Vault config - Settings save stays on page (no dialog close) - Save button shows success indicator Sidebar: - JIRA Issues nav item when JIRA enabled - GitLab MRs shown independently of GitLab Issues - Source control and issue tracking use separate enable flags i18n: Full English and French translations for all new features.
📝 WalkthroughWalkthroughAdds JIRA and Vault integrations: new IPC channels and handlers, preload APIs, renderer views and stores, MCP server entries, settings UI, type definitions, i18n strings, browser-mode mocks, and utilities for Jira requests and vault file operations; includes investigation flow that creates local spec artifacts. Changes
Sequence DiagramsequenceDiagram
participant Renderer
participant Preload as Preload API
participant Main as Main IPC
participant ProjectStore as Project Store
participant JiraREST as Jira REST API
participant FS as File System
Renderer->>Preload: investigateJiraIssue(projectId, issueKey)
Preload->>Main: send JIRA_INVESTIGATE_ISSUE (projectId, issueKey)
Main->>ProjectStore: load project env / jira config
ProjectStore-->>Main: project + jiraConfig
Main->>JiraREST: GET /rest/api/3/issue/{issueKey}
JiraREST-->>Main: issue data
Main->>JiraREST: GET /rest/api/3/issue/{issueKey}/comment (paginated)
JiraREST-->>Main: comments pages
Main->>FS: create spec files (TASK.md, metadata, etc.)
Main->>Renderer: emit JIRA_INVESTIGATION_PROGRESS (phases)
Main->>Renderer: emit JIRA_INVESTIGATION_COMPLETE { taskId, specId }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 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)
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 |
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 introduces comprehensive JIRA and Vault integrations, significantly expanding the platform's capabilities for issue tracking and external knowledge management. It also refines the user experience by consolidating project settings into a more intuitive, tabbed interface. These changes enhance agent functionality by providing access to JIRA issues and external context, while improving overall system architecture and maintainability. Highlights
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. Footnotes
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| return null; | ||
| } | ||
|
|
||
| const content = await readFile(claudeMdPath, 'utf-8'); |
Check failure
Code scanning / CodeQL
Potential file system race condition High
| continue; | ||
| } | ||
|
|
||
| const content = await readFile(filePath, 'utf-8'); |
Check failure
Code scanning / CodeQL
Potential file system race condition High
| }; | ||
| } | ||
|
|
||
| const content = await readFile(resolvedPath, 'utf-8'); |
Check failure
Code scanning / CodeQL
Potential file system race condition High
| try { | ||
| const entryStat = await stat(fullPath); | ||
| if (entryStat.size <= MAX_FILE_SIZE) { | ||
| const fileContent = await readFile(fullPath, 'utf-8'); |
Check failure
Code scanning / CodeQL
Potential file system race condition High
| jiraUrl, | ||
| selectedComments | ||
| ); | ||
| await writeFile(path.join(specDir, 'TASK.md'), taskContent, 'utf-8'); |
Check warning
Code scanning / CodeQL
Network data written to file Medium
| const finalName = sanitizedName.endsWith('.md') ? sanitizedName : `${sanitizedName}.md`; | ||
|
|
||
| // Build target path within vault/memory/learnings/ | ||
| const learningsDir = path.join(config.path, 'memory', 'learnings'); |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
| @@ -0,0 +1,180 @@ | |||
| import { useState } from 'react'; | |||
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
| @@ -0,0 +1,180 @@ | |||
| import { useState } from 'react'; | |||
| import { useTranslation } from 'react-i18next'; | |||
| import { CheckCircle2, AlertCircle, ExternalLink } from 'lucide-react'; | |||
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
| * GitHub/GitLab issue tracking uses the connection from Source Control. | ||
| */ | ||
| export function IssueTrackingSection(props: IssueTrackingSectionProps) { | ||
| const { t } = useTranslation('settings'); |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
| * Groups GitHub and GitLab source control configuration in a tabbed layout. | ||
| */ | ||
| export function SourceControlSection(props: SourceControlSectionProps) { | ||
| const { t } = useTranslation('settings'); |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces significant new features, including JIRA and Vault integrations, alongside a major refactoring of the settings UI into a consolidated, tabbed interface. The implementation is extensive and well-structured, particularly the security considerations in the Vault integration such as path traversal prevention and symlink checks. My review focuses on improving the robustness of the new JIRA functionality and addressing some type safety and code style points for better maintainability.
| ipcMain.handle( | ||
| VAULT_CHANNELS.VALIDATE_PATH, | ||
| async (_event, vaultPath: string): Promise<IPCResult<{ valid: boolean }>> => { | ||
| try { | ||
| if (typeof vaultPath !== 'string') { | ||
| return { success: false, error: 'Vault path must be a string' }; | ||
| } | ||
|
|
||
| const result = await isValidVaultPath(vaultPath); | ||
| if (!result.valid) { | ||
| return { success: true, data: { valid: false, error: result.error } as { valid: boolean } }; | ||
| } | ||
|
|
||
| return { success: true, data: { valid: true } }; | ||
| } catch (error) { | ||
| const message = error instanceof Error ? error.message : 'Unknown error validating vault path'; | ||
| console.warn('[Vault] Error validating path:', message); | ||
| return { success: false, error: message }; | ||
| } | ||
| } | ||
| ); |
There was a problem hiding this comment.
The IPC handler for VAULT_CHANNELS.VALIDATE_PATH has a return type of Promise<IPCResult<{ valid: boolean }>>, but when validation fails, it returns an object { success: true, data: { valid: false, error: result.error } } and casts it with as { valid: boolean }. This cast hides a type error because the data object contains an error property that is not in the declared type. The frontend (vault-api.ts) actually expects this error property.
The handler's signature should be updated to match the expected return shape to ensure type safety and remove the unsafe cast.
ipcMain.handle(
VAULT_CHANNELS.VALIDATE_PATH,
async (_event, vaultPath: string): Promise<IPCResult<{ valid: boolean; error?: string }>> => {
try {
if (typeof vaultPath !== 'string') {
return { success: false, error: 'Vault path must be a string' };
}
const result = await isValidVaultPath(vaultPath);
if (!result.valid) {
return { success: true, data: { valid: false, error: result.error } };
}
return { success: true, data: { valid: true } };
} catch (error) {
const message = error instanceof Error ? error.message : 'Unknown error validating vault path';
console.warn('[Vault] Error validating path:', message);
return { success: false, error: message };
}
}
);| const result: JiraInvestigationResult = { | ||
| success: true, | ||
| issueKey, | ||
| analysis: { | ||
| summary: `Investigation of JIRA issue ${issueKey}: ${issue.fields.summary}`, | ||
| proposedSolution: issue.fields.description | ||
| ? 'See task details for more information.' | ||
| : 'No description provided. See task details.', | ||
| affectedFiles: [], | ||
| estimatedComplexity: 'standard', | ||
| acceptanceCriteria: [] | ||
| }, | ||
| taskId: task.id | ||
| }; |
There was a problem hiding this comment.
The analysis object in the JiraInvestigationResult is hardcoded with placeholder values. Fields like proposedSolution, affectedFiles, and estimatedComplexity are not being dynamically generated, which could be misleading as it suggests a deeper analysis is occurring than what is implemented. The current implementation primarily fetches JIRA data and creates a task spec.
To make this more representative of the data, consider populating these fields from the JIRA issue itself. For example, the proposedSolution could be initialized with the issue's description.
| const filteredIssues = assignedToMe | ||
| ? allFilteredIssues.filter(i => i.assignee?.toLowerCase().includes(jiraUserName.toLowerCase())) | ||
| : allFilteredIssues; |
There was a problem hiding this comment.
The filter for "My Issues" relies on matching the start of the user's email (jiraUserName) against the assignee's display name. This is not a robust approach because a user's JIRA display name (e.g., "John Doe") often doesn't match their email username (e.g., "j.doe").
A more reliable method would be to fetch the current user's accountId from the /rest/api/3/myself JIRA endpoint during the connection check and use that for filtering, as the assignee object on an issue typically contains this stable identifier.
| function validateUrl(url: string): string | undefined { | ||
| if (!url) return 'Required'; | ||
| try { | ||
| const parsed = new URL(url); | ||
| if (parsed.protocol !== 'https:' && parsed.protocol !== 'http:') { | ||
| return 'Must use https:// or http://'; | ||
| } | ||
| } catch { | ||
| return 'Invalid URL format'; | ||
| } | ||
| return undefined; | ||
| } | ||
|
|
||
| function validateEmail(email: string): string | undefined { | ||
| if (!email) return 'Required'; | ||
| if (!email.includes('@')) return 'Invalid email format'; | ||
| return undefined; | ||
| } | ||
|
|
||
| function validateToken(token: string): string | undefined { | ||
| if (!token) return 'Required'; | ||
| if (token.length < 8) return 'Token seems too short'; | ||
| return undefined; | ||
| } |
There was a problem hiding this comment.
The validation functions (validateUrl, validateEmail, validateToken) are defined inside the JiraIntegration component. Since these are pure functions, they are recreated on every render, which is inefficient. For better performance and code organization, they should be moved outside the component body to the module scope.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| const resolvedFile = path.resolve(resolvedVault, filePath); | ||
|
|
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 33
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/desktop/src/preload/api/agent-api.ts (1)
92-100: 🧹 Nitpick | 🔵 TrivialConsider re-exporting
JiraAPIandVaultAPItypes for consistency.The re-export list includes types for all other integration APIs (Linear, GitHub, GitLab, Shell) but omits the newly added
JiraAPIandVaultAPI. For consumers who may need to reference these types individually, consider adding them to the export list.♻️ Suggested fix
export type { RoadmapAPI, IdeationAPI, InsightsAPI, ChangelogAPI, LinearAPI, GitHubAPI, GitLabAPI, - ShellAPI + ShellAPI, + JiraAPI, + VaultAPI };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/preload/api/agent-api.ts` around lines 92 - 100, The exported type list in agent-api.ts omits the newly added JiraAPI and VaultAPI types; update the export block that currently lists RoadmapAPI, IdeationAPI, InsightsAPI, ChangelogAPI, LinearAPI, GitHubAPI, GitLabAPI, and ShellAPI to also include JiraAPI and VaultAPI so those types are re-exported for consumers who need them.apps/desktop/src/renderer/App.tsx (1)
913-925:⚠️ Potential issue | 🟡 MinorRoute GitHub/GitLab issue settings to the Issue Tracking section.
Lines 914 and 923 route issue views to
'source-control', while JIRA issue view routes to'issue-tracking'(Line 932). This is inconsistent for the same issue-tracking workflow.🎯 Proposed fix
- setSettingsInitialProjectSection('source-control'); + setSettingsInitialProjectSection('issue-tracking'); ... - setSettingsInitialProjectSection('source-control'); + setSettingsInitialProjectSection('issue-tracking');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/App.tsx` around lines 913 - 925, The onOpenSettings handlers for the GitHubIssues and GitLabIssues views currently call setSettingsInitialProjectSection('source-control') which is inconsistent with the JIRA view; update those handlers to call setSettingsInitialProjectSection('issue-tracking') and still call setIsSettingsDialogOpen(true). Locate the onOpenSettings in the GitHubIssues and GitLabIssues component blocks in App.tsx and replace the 'source-control' argument with 'issue-tracking' so all issue views route to the same Issue Tracking settings section.
🤖 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/ai/config/agent-configs.ts`:
- Around line 91-109: mapMcpServerName() currently doesn't normalize the new MCP
integrations, so agentMcpAdd/agentMcpRemove won't match the JIRA_TOOLS and
VAULT_TOOLS servers; update the normalization map inside mapMcpServerName to
include entries for "jira" and "vault" (mapping whatever canonical server keys
your codebase uses for those integrations to the same normalized form) so that
calls involving JIRA_TOOLS and VAULT_TOOLS resolve correctly when agentMcpAdd or
agentMcpRemove is invoked.
In `@apps/desktop/src/main/ai/mcp/registry.ts`:
- Around line 230-239: The MCP Jira branch currently only verifies JIRA_HOST
before returning createJiraServer; update the validation in the 'jira' case to
require JIRA_HOST, JIRA_EMAIL and JIRA_TOKEN (from options.jiraEnv or fallback
options.env) and return null if any are missing. Locate the 'jira' case block
(variables jiraHost, jiraEnv and the call to createJiraServer) and perform a
combined presence check for jiraEnv.JIRA_HOST, jiraEnv.JIRA_EMAIL and
jiraEnv.JIRA_TOKEN before calling createJiraServer so the MCP config is only
returned when all credentials are present.
In `@apps/desktop/src/main/ipc-handlers/env-handlers.ts`:
- Around line 92-98: generateEnvContent currently sets
existingVars['GITHUB_ISSUES_ENABLED'] and ['GITLAB_ISSUES_ENABLED'] from config
but the output template never emits those keys, so the toggles are lost on save;
update generateEnvContent to include those keys in the generated .env output
when present (e.g., ensure the template/serializer writes
existingVars['GITHUB_ISSUES_ENABLED'] and existingVars['GITLAB_ISSUES_ENABLED']
as GITHUB_ISSUES_ENABLED=true/false lines), and mirror the same fix for the
other similar block that handles these flags (the duplicate logic later in the
function).
- Around line 431-436: The code only sets config.jiraEnabled inside the
JIRA_HOST branch so the saved JIRA_ENABLED toggle is lost when JIRA_HOST is
blank; update the ENV_GET handling so vars['JIRA_ENABLED'] is respected
regardless of vars['JIRA_HOST'] by assigning config.jiraEnabled from
vars['JIRA_ENABLED'] (e.g., config.jiraEnabled =
vars['JIRA_ENABLED']?.toLowerCase() !== 'false') outside or before the
vars['JIRA_HOST'] branch while still keeping the existing assignment of
config.jiraHost within the vars['JIRA_HOST'] block.
In `@apps/desktop/src/main/ipc-handlers/jira/spec-utils.ts`:
- Around line 282-283: Replace the runtime dynamic import "const { jiraFetch,
JiraAPIError } = await import('./utils')" with a static import at the top of the
file (import { jiraFetch, JiraAPIError } from './utils') and remove the
await/dynamic import usage; then update any subsequent error checks that
reference the destructured JiraAPIError to use the statically imported
JiraAPIError symbol directly (ensure any code paths that expected the import to
be async still work synchronously after this change).
- Around line 161-166: The sanitizeIssueKey regex is too strict; update the
pattern in sanitizeIssueKey to allow keys that start with a letter or digit and
to perform a case-insensitive match so lowercase project keys are accepted.
Replace the current /^[A-Z][A-Z0-9_]+-\d+$/ check with a regex that permits an
initial letter or digit (e.g., /^[A-Z0-9][A-Z0-9_]+-\d+$/) and add the
case-insensitive flag (/i), keeping the same return behavior (return value if
match else '').
In `@apps/desktop/src/main/ipc-handlers/jira/types.ts`:
- Around line 55-66: The JiraAPIIssueFields interface currently types
description as string but JIRA v3 returns Atlassian Document Format (ADF);
change the JiraAPIIssueFields.description property type to unknown (matching
JiraAPIComment.body) so callers must explicitly handle ADF-to-text conversion;
update the JiraAPIIssueFields definition (and any imports/uses of
JiraAPIIssueFields.description) to reflect the new unknown type.
In `@apps/desktop/src/main/ipc-handlers/jira/utils.ts`:
- Around line 66-80: Add a short inline comment at the call site where
sanitizeJiraHost invokes isPrivateHost (near the existing private-host check)
clarifying that localhost (and the dev exception for http://localhost) is
intentionally handled earlier in sanitizeJiraHost, so the isPrivateHost call is
meant to catch other private hosts/IPs only; reference the sanitizeJiraHost
function and the isPrivateHost function in the comment to guide future
maintainers.
- Around line 238-243: The response.json() call can throw if the body is not
valid JSON; update the handler that checks response.status (the block returning
null for 204) to defensively parse the body: after the 204 check, attempt
response.json() inside a try/catch and on failure read response.text() and throw
or return a new Error containing the response.status and the raw body text so
callers (and the throw at line 248) get an informative message; reference the
existing response.status check and the code path that currently does "return
response.json()" to locate where to add the try/catch and fallback to
response.text().
In `@apps/desktop/src/main/ipc-handlers/vault/utils.ts`:
- Around line 237-241: The current validation uses path.resolve(filePath) which
resolves relative to CWD and is inconsistent with other helpers; change it to
resolve the file path relative to the vault using the existing
resolveVaultFilePath helper (or path.resolve(resolvedVault, filePath)) so that
resolvedFile is produced by resolveVaultFilePath/resolvedVault + entry path,
then check startsWith(resolvedVault + path.sep) against that resolved value
(refer to resolvedFile, filePath, resolvedVault, and resolveVaultFilePath to
locate the code).
- Around line 31-49: SENSITIVE_DIRECTORIES currently contains hardcoded
forward-slash Windows paths that won't match on Windows; replace this constant
with a platform-aware function getSensitiveDirectories() that uses the platform
abstraction utilities from the platform module to return OS-specific sensitive
paths (using backslashes on Windows and POSIX paths on Unix), and update callers
(e.g., the code referenced at the previous line 122 that used
SENSITIVE_DIRECTORIES) to call getSensitiveDirectories() instead of referencing
the old constant; ensure the function returns a readonly string[] and preserves
the existing Unix entries and Windows-only entries adjusted for correct
separators.
In `@apps/desktop/src/main/ipc-handlers/vault/vault-handlers.ts`:
- Around line 357-363: The path validation in the search loop uses path.resolve
on fullPath (resolvedEntry) which doesn't follow symlinks and can let symlinked
entries escape the vault; change this to use a realpath-based resolution (e.g.,
fs.realpathSync or await fs.promises.realpath) and replace the path.resolve
check with the realpath result, then compare that real path against
resolvedVault + path.sep (the same boundary check currently performed) so
symlink targets outside the vault are rejected; update the search function in
vault-handlers.ts to mirror the symlink-safe behavior used by
resolveVaultFilePath/VAULT_READ_FILE.
- Around line 300-301: The success message currently uses console.warn in the
vault save flow—replace or remove it: remove the console.warn(`[Vault] Learning
saved: ${relativePath}`) line or convert it to the project's DEBUG/logging
pattern used by other handlers (use the same debug namespace/function used
elsewhere in this file instead of console.warn), referencing the variables
relativePath, resolvedVault and targetPath to preserve the same contextual
message if keeping the log.
In `@apps/desktop/src/renderer/App.tsx`:
- Line 44: The import for the JiraIssues component uses a relative path; update
it to use the renderer path alias instead (replace the './components/JiraIssues'
import with the alias import for the renderer such as '@/components/JiraIssues')
so the JiraIssues symbol follows the tsconfig alias convention used by the
renderer.
In
`@apps/desktop/src/renderer/components/jira-issues/components/InvestigationDialog.tsx`:
- Around line 49-87: The effect is re-subscribing listeners whenever `phase`
changes; remove `phase` from the dependency array and only gate registration on
`open` so listeners are registered once per open state. Update the early-return
in the `useEffect` to `if (!open) return;` (remove the `phase === 'idle'` check)
and keep the same registrations for
`window.electronAPI.onJiraInvestigationProgress`, `onJiraInvestigationComplete`,
and `onJiraInvestigationError` with the same cleanup (`removeProgress`,
`removeComplete`, `removeError`), so listeners persist across `phase`
transitions while still being torn down when `open` becomes false. Ensure
references to `projectId`, `setProgress`, `setMessage`, `setPhase`, and
`setError` remain correct inside the callbacks.
In `@apps/desktop/src/renderer/components/jira-issues/index.ts`:
- Around line 2-5: Update the re-exports in this barrel to use the project's
path aliases instead of relative paths: replace the relative export source for
the JiraIssues export and the JiraInvestigationDialog export so they import from
the appropriate `@/`... alias (e.g., use "@/renderer/components/JiraIssues" or the
renderer alias that maps to the same module) ensuring the exported symbols
JiraIssues and JiraInvestigationDialog remain unchanged; verify tsconfig path
entries match the chosen aliases and run a quick build/import check to confirm
resolution.
In `@apps/desktop/src/renderer/components/JiraIssues.tsx`:
- Around line 262-269: Replace the hardcoded user-facing strings in
JiraIssues.tsx with i18n keys: import and use the project's translation hook
(e.g., useTranslation / t) at the top of the JiraIssues component, then replace
'Unassigned' with t('issues.unassigned') (used where issue.assignee is falsy)
and replace the Badge text 'Linked' with t('issues.linked') (used when
linkedTaskId is truthy); ensure the translation keys are added to the locale
resource files for all supported languages.
- Around line 179-186: The "My Issues" button label in the JiraIssues component
is hardcoded; replace it with an i18n translation key by using the project's
translation helper (e.g., call useTranslation() and use t('jira.myIssues') or
the project's equivalent) instead of the literal string in the Button children,
and add the new key ("jira.myIssues") to the appropriate locales files; ensure
the component imports useTranslation (or i18n.t) and keep the rest of the Button
props and the assignedToMe / setAssignedToMe toggle logic unchanged.
In `@apps/desktop/src/renderer/components/settings/integrations/index.ts`:
- Line 8: The export currently uses a relative path; update the barrel to use
the repository path alias instead—replace the relative export of JiraIntegration
in the integrations index with an alias-based export (e.g., export {
JiraIntegration } from '@components/settings/integrations/JiraIntegration') so
it follows the tsconfig alias conventions; ensure the symbol name
JiraIntegration remains unchanged.
In
`@apps/desktop/src/renderer/components/settings/integrations/IssueTrackingSection.tsx`:
- Line 3: The import list in the IssueTrackingSection component includes an
unused symbol ExternalLink; remove ExternalLink from the import statement (leave
CheckCircle2 and AlertCircle) so the component no longer imports an unused icon
and ESLint/build warnings disappear.
- Around line 100-105: The two hardcoded English strings in IssueTrackingSection
(the Label text "Auto-sync on load" and the paragraph "Automatically sync issues
when the project opens") should be replaced with i18n keys; update the component
to use the project's i18n helper (e.g., useTranslation/t or equivalent) and
replace those literals with calls like t('settings.integrations.autoSyncLabel')
and t('settings.integrations.autoSyncDescription') (or your chosen key names),
then add the corresponding keys to the locale files so translations are
available.
- Around line 56-80: Replace the hardcoded English strings in the
IssueTrackingSection component with react-i18next calls: use t('...') for the
connection messages near the top ("Connected via Source Control settings" and
"Configure {label} in Source Control first"), for the section title/label
("Enable {label} Issues") and description ("Sync and import issues from
{label}"), and for the auto-sync toggle labels around lines 100-105; keep using
interpolation for label (e.g., t('integrations.enableIssues', { label })) and
provide sensible translation keys such as integrations.connection.connected,
integrations.connection.configure, integrations.enableIssues,
integrations.syncDescription, and integrations.autoSync to locate and replace
the strings in IssueTrackingSection.
In
`@apps/desktop/src/renderer/components/settings/integrations/JiraIntegration.tsx`:
- Around line 25-47: Replace hardcoded English UI strings in the JiraIntegration
component with i18n translation keys via react-i18next: update the validator
return messages in validateUrl, validateEmail, and validateToken to return
translated strings (e.g., t('settings.jira.validation.required'),
t('settings.jira.validation.invalidUrl'), etc.), replace any hardcoded
placeholders and the summary banner text (the strings around lines referenced:
placeholders at ~100-101, summary at ~146, and other labels at ~161 and
~167-172) with t('settings.jira...') keys, and ensure the component imports and
uses the useTranslation hook to access t; add corresponding keys under
settings.jira in the translations resource files.
In
`@apps/desktop/src/renderer/components/settings/integrations/SourceControlSection.tsx`:
- Around line 2-5: Replace the deep-relative imports in SourceControlSection
with the configured TS path aliases: import UI components (Tabs, TabsList,
TabsTrigger, TabsContent) from "@/renderer/ui/tabs" (or "@/ui/tabs" per
convention) and import types (ProjectEnvConfig, GitHubSyncStatus,
GitLabSyncStatus, ProjectSettings) from "@shared/types"; also import
GitHubIntegration and GitLabIntegration using
"@/renderer/components/settings/integrations/GitHubIntegration" and
"@/renderer/components/settings/integrations/GitLabIntegration" (or the repo's
"@/..." shorthand for renderer code) so the module specifiers use the `@/`* and
`@shared/`* aliases instead of deep relative paths.
- Around line 33-34: The TabsTrigger labels in SourceControlSection.tsx are
hardcoded ("GitHub"/"GitLab") and must use i18n keys instead; replace the
literal strings in the TabsTrigger components (value="github"/"gitlab") with the
translated strings via your localization function (e.g.,
t('settings.sourceControl.github') and t('settings.sourceControl.gitlab')) and
ensure the corresponding keys are added to the locale files (e.g.,
en/settings.json and fr/settings.json under a "sourceControl" object with
"github" and "gitlab" entries).
In
`@apps/desktop/src/renderer/components/settings/integrations/VaultIntegration.tsx`:
- Around line 45-56: The component VaultIntegration is using hardcoded strings
for the Vault input placeholder and validation error fallback; update usages to
use react-i18next translation keys: replace the hardcoded input placeholder with
t('vault.pathPlaceholder') and when setting validationResult in the
vaultValidatePath flow (and in the catch block, and the other occurrences around
lines 94-96) use t('vault.pathInvalid') as the default/fallback error message
instead of hardcoded text—ensure setValidationResult({ valid: false, error:
result.error || t('vault.pathInvalid') }) and the catch branch uses error
instanceof Error ? error.message : t('vault.pathInvalid').
In `@apps/desktop/src/renderer/components/settings/ProjectSettingsContent.tsx`:
- Line 11: The import in ProjectSettingsContent.tsx currently uses a relative
path for the Project type; update the import to use the configured path alias
`@shared` by replacing the relative import of Project with the alias import (i.e.,
import type { Project } from '@shared/types') so the file
ProjectSettingsContent.tsx and the Project type symbol use the tsconfig path
alias.
In `@apps/desktop/src/renderer/stores/jira-store.ts`:
- Around line 162-176: The parseJiraIssue function currently uses unchecked
casts and should validate inputs at runtime; update parseJiraIssue to perform
explicit type guards (or use a small Zod/schema) to verify each field before
assignment: check typeof raw.key/summary/status/issueType/created/updated are
strings, description is string or undefined, assignee/priority are strings or
undefined, and labels is an array of strings via Array.isArray + every(item =>
typeof item === 'string'), then use the validated values or safe defaults;
reference the parseJiraIssue function and the JiraIssue shape when implementing
these guards so malformed backend responses can't silently produce invalid
state.
- Around line 226-236: The investigateJiraIssue function currently only sets a
'fetching' state in useJiraStore and leaves a TODO; implement the full IPC flow
by calling window.electronAPI.jiraInvestigateIssue(projectId, issueKey) from
investigateJiraIssue, await the result, and update the store via
useJiraStore.getState().setInvestigationStatus to reflect success (e.g., phase:
'completed' with any returned data) or failure (phase: 'error' with an error
message); ensure errors are caught and logged, and remove the placeholder
comment so InvestigationDialog no longer has to handle the entire flow.
- Around line 3-14: The JiraIssue interface in this file duplicates the type
defined elsewhere; extract the JiraIssue definition into a single shared types
module and replace the local interface with an import from that shared module
(remove the local JiraIssue declaration and import the shared JiraIssue type),
making sure the shared module exports the same fields (key, summary,
description?, status, assignee?, priority?, issueType, created, updated, labels)
so renderer and main code use the single source of truth.
- Around line 148-157: The buildJql function is vulnerable because projectKey is
interpolated directly into JQL; instead sanitize or escape projectKey before
interpolation (in function buildJql) by either validating it against a strict
whitelist/regex (e.g. only allow A-Z,0-9, hyphen/underscore) or by escaping any
double quotes/backslashes so injected JQL cannot break out of the quoted string;
update buildJql to use the sanitized/escaped value for the project clause and
keep the surrounding quotes and ordering intact.
In `@apps/desktop/src/shared/i18n/locales/fr/jira.json`:
- Around line 1-64: The new "jira" i18n namespace (file defining keys like
"title", "states", "detail", "investigation", "priority", etc.) violates the
project's allowed namespaces; either move all these keys into one of the
approved namespaces (e.g., merge into "tasks" or "common" and rename nested keys
accordingly) or update the project's namespace policy/lockfile to include "jira"
so the new namespace is allowed; locate the file introducing "jira" and perform
the namespace migration or policy update consistently for all keys (title,
states, header, filters, empty, notConnected, detail, investigation, priority).
- Around line 4-55: The French locale contains multiple spelling and diacritic
errors; update the strings under keys such as "open"/"closed",
"header.searchPlaceholder", "filters.*", "empty.noMatch" and
"empty.selectIssue",
"notConnected.title"/"notConnected.description"/"notConnected.openSettings", and
entries in "detail.*" and "investigation.*" to use correct French accents and
orthography (e.g. "Ouvert", "Fermé", "Rechercher des tickets…", "Aucun ticket ne
correspond à votre recherche", "Sélectionnez un ticket pour voir les détails",
"JIRA non connecté", "hôte", "e‑mail", "jeton API", "Créer une tâche", "Tâche
liée", "Aucune description fournie.", "Créer une tâche à partir du ticket",
etc.); ensure punctuation and capitalization follow existing locale conventions
and run a quick spellcheck on all values before committing.
---
Outside diff comments:
In `@apps/desktop/src/preload/api/agent-api.ts`:
- Around line 92-100: The exported type list in agent-api.ts omits the newly
added JiraAPI and VaultAPI types; update the export block that currently lists
RoadmapAPI, IdeationAPI, InsightsAPI, ChangelogAPI, LinearAPI, GitHubAPI,
GitLabAPI, and ShellAPI to also include JiraAPI and VaultAPI so those types are
re-exported for consumers who need them.
In `@apps/desktop/src/renderer/App.tsx`:
- Around line 913-925: The onOpenSettings handlers for the GitHubIssues and
GitLabIssues views currently call
setSettingsInitialProjectSection('source-control') which is inconsistent with
the JIRA view; update those handlers to call
setSettingsInitialProjectSection('issue-tracking') and still call
setIsSettingsDialogOpen(true). Locate the onOpenSettings in the GitHubIssues and
GitLabIssues component blocks in App.tsx and replace the 'source-control'
argument with 'issue-tracking' so all issue views route to the same Issue
Tracking settings section.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 372e183e-3bc4-4f29-85f0-875cebbc5a1a
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json,!**/package-lock.json
📒 Files selected for processing (48)
apps/desktop/src/main/ai/config/agent-configs.tsapps/desktop/src/main/ai/mcp/registry.tsapps/desktop/src/main/ai/mcp/types.tsapps/desktop/src/main/ipc-handlers/env-handlers.tsapps/desktop/src/main/ipc-handlers/index.tsapps/desktop/src/main/ipc-handlers/jira/index.tsapps/desktop/src/main/ipc-handlers/jira/investigation-handlers.tsapps/desktop/src/main/ipc-handlers/jira/issue-handlers.tsapps/desktop/src/main/ipc-handlers/jira/spec-utils.tsapps/desktop/src/main/ipc-handlers/jira/types.tsapps/desktop/src/main/ipc-handlers/jira/utils.tsapps/desktop/src/main/ipc-handlers/vault/index.tsapps/desktop/src/main/ipc-handlers/vault/types.tsapps/desktop/src/main/ipc-handlers/vault/utils.tsapps/desktop/src/main/ipc-handlers/vault/vault-handlers.tsapps/desktop/src/preload/api/agent-api.tsapps/desktop/src/preload/api/modules/index.tsapps/desktop/src/preload/api/modules/jira-api.tsapps/desktop/src/preload/api/modules/vault-api.tsapps/desktop/src/renderer/App.tsxapps/desktop/src/renderer/components/JiraIssues.tsxapps/desktop/src/renderer/components/Sidebar.tsxapps/desktop/src/renderer/components/jira-issues/components/InvestigationDialog.tsxapps/desktop/src/renderer/components/jira-issues/index.tsapps/desktop/src/renderer/components/settings/AppSettings.tsxapps/desktop/src/renderer/components/settings/ProjectSettingsContent.tsxapps/desktop/src/renderer/components/settings/integrations/IssueTrackingSection.tsxapps/desktop/src/renderer/components/settings/integrations/JiraIntegration.tsxapps/desktop/src/renderer/components/settings/integrations/MemoryContextSection.tsxapps/desktop/src/renderer/components/settings/integrations/SourceControlSection.tsxapps/desktop/src/renderer/components/settings/integrations/VaultIntegration.tsxapps/desktop/src/renderer/components/settings/integrations/index.tsapps/desktop/src/renderer/components/settings/sections/SectionRouter.tsxapps/desktop/src/renderer/lib/browser-mock.tsapps/desktop/src/renderer/stores/jira-store.tsapps/desktop/src/shared/constants/ipc.tsapps/desktop/src/shared/i18n/index.tsapps/desktop/src/shared/i18n/locales/en/jira.jsonapps/desktop/src/shared/i18n/locales/en/navigation.jsonapps/desktop/src/shared/i18n/locales/en/settings.jsonapps/desktop/src/shared/i18n/locales/fr/jira.jsonapps/desktop/src/shared/i18n/locales/fr/navigation.jsonapps/desktop/src/shared/i18n/locales/fr/settings.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
| /** JIRA MCP tools for issue tracking (when JIRA is configured) */ | ||
| export const JIRA_TOOLS = [ | ||
| 'mcp__jira__list_projects', | ||
| 'mcp__jira__get_issue', | ||
| 'mcp__jira__create_issue', | ||
| 'mcp__jira__update_issue', | ||
| 'mcp__jira__search_issues', | ||
| 'mcp__jira__add_comment', | ||
| 'mcp__jira__get_transitions', | ||
| 'mcp__jira__transition_issue', | ||
| ] as const; | ||
|
|
||
| /** Vault MCP tools for external vault access */ | ||
| export const VAULT_TOOLS = [ | ||
| 'mcp__vault__read_file', | ||
| 'mcp__vault__list_directory', | ||
| 'mcp__vault__search_files', | ||
| ] as const; | ||
|
|
There was a problem hiding this comment.
Add jira/vault to MCP server name normalization map.
You introduced new MCP integrations, but mapMcpServerName() cannot currently resolve jira or vault for agentMcpAdd/agentMcpRemove, so those overrides won’t work for the new servers.
🔧 Proposed fix
const MCP_SERVER_NAME_MAP: Record<string, string> = {
context7: 'context7',
'graphiti-memory': 'memory',
graphiti: 'memory',
memory: 'memory',
linear: 'linear',
+ jira: 'jira',
+ vault: 'vault',
electron: 'electron',
puppeteer: 'puppeteer',
'auto-claude': 'auto-claude',
};📝 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.
| /** JIRA MCP tools for issue tracking (when JIRA is configured) */ | |
| export const JIRA_TOOLS = [ | |
| 'mcp__jira__list_projects', | |
| 'mcp__jira__get_issue', | |
| 'mcp__jira__create_issue', | |
| 'mcp__jira__update_issue', | |
| 'mcp__jira__search_issues', | |
| 'mcp__jira__add_comment', | |
| 'mcp__jira__get_transitions', | |
| 'mcp__jira__transition_issue', | |
| ] as const; | |
| /** Vault MCP tools for external vault access */ | |
| export const VAULT_TOOLS = [ | |
| 'mcp__vault__read_file', | |
| 'mcp__vault__list_directory', | |
| 'mcp__vault__search_files', | |
| ] as const; | |
| const MCP_SERVER_NAME_MAP: Record<string, string> = { | |
| context7: 'context7', | |
| 'graphiti-memory': 'memory', | |
| graphiti: 'memory', | |
| memory: 'memory', | |
| linear: 'linear', | |
| jira: 'jira', | |
| vault: 'vault', | |
| electron: 'electron', | |
| puppeteer: 'puppeteer', | |
| 'auto-claude': 'auto-claude', | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/main/ai/config/agent-configs.ts` around lines 91 - 109,
mapMcpServerName() currently doesn't normalize the new MCP integrations, so
agentMcpAdd/agentMcpRemove won't match the JIRA_TOOLS and VAULT_TOOLS servers;
update the normalization map inside mapMcpServerName to include entries for
"jira" and "vault" (mapping whatever canonical server keys your codebase uses
for those integrations to the same normalized form) so that calls involving
JIRA_TOOLS and VAULT_TOOLS resolve correctly when agentMcpAdd or agentMcpRemove
is invoked.
| case 'jira': { | ||
| const jiraHost = options.jiraEnv?.JIRA_HOST ?? options.env?.JIRA_HOST; | ||
| if (!jiraHost) return null; | ||
| const jiraEnv = options.jiraEnv ?? { | ||
| JIRA_HOST: options.env?.JIRA_HOST ?? '', | ||
| JIRA_EMAIL: options.env?.JIRA_EMAIL ?? '', | ||
| JIRA_TOKEN: options.env?.JIRA_TOKEN ?? '', | ||
| }; | ||
| return createJiraServer(jiraEnv); | ||
| } |
There was a problem hiding this comment.
Require full JIRA credentials before returning MCP config.
Line 231 currently checks only JIRA_HOST. If JIRA_EMAIL or JIRA_TOKEN is missing, the server still starts and fails later at runtime. Validate all required fields up front.
🛡️ Proposed fix
case 'jira': {
- const jiraHost = options.jiraEnv?.JIRA_HOST ?? options.env?.JIRA_HOST;
- if (!jiraHost) return null;
- const jiraEnv = options.jiraEnv ?? {
- JIRA_HOST: options.env?.JIRA_HOST ?? '',
- JIRA_EMAIL: options.env?.JIRA_EMAIL ?? '',
- JIRA_TOKEN: options.env?.JIRA_TOKEN ?? '',
- };
+ const jiraHost = options.jiraEnv?.JIRA_HOST ?? options.env?.JIRA_HOST;
+ const jiraEmail = options.jiraEnv?.JIRA_EMAIL ?? options.env?.JIRA_EMAIL;
+ const jiraToken = options.jiraEnv?.JIRA_TOKEN ?? options.env?.JIRA_TOKEN;
+ if (!jiraHost || !jiraEmail || !jiraToken) return null;
+ const jiraEnv = {
+ JIRA_HOST: jiraHost,
+ JIRA_EMAIL: jiraEmail,
+ JIRA_TOKEN: jiraToken,
+ };
return createJiraServer(jiraEnv);
}📝 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.
| case 'jira': { | |
| const jiraHost = options.jiraEnv?.JIRA_HOST ?? options.env?.JIRA_HOST; | |
| if (!jiraHost) return null; | |
| const jiraEnv = options.jiraEnv ?? { | |
| JIRA_HOST: options.env?.JIRA_HOST ?? '', | |
| JIRA_EMAIL: options.env?.JIRA_EMAIL ?? '', | |
| JIRA_TOKEN: options.env?.JIRA_TOKEN ?? '', | |
| }; | |
| return createJiraServer(jiraEnv); | |
| } | |
| case 'jira': { | |
| const jiraHost = options.jiraEnv?.JIRA_HOST ?? options.env?.JIRA_HOST; | |
| const jiraEmail = options.jiraEnv?.JIRA_EMAIL ?? options.env?.JIRA_EMAIL; | |
| const jiraToken = options.jiraEnv?.JIRA_TOKEN ?? options.env?.JIRA_TOKEN; | |
| if (!jiraHost || !jiraEmail || !jiraToken) return null; | |
| const jiraEnv = { | |
| JIRA_HOST: jiraHost, | |
| JIRA_EMAIL: jiraEmail, | |
| JIRA_TOKEN: jiraToken, | |
| }; | |
| return createJiraServer(jiraEnv); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/main/ai/mcp/registry.ts` around lines 230 - 239, The MCP
Jira branch currently only verifies JIRA_HOST before returning createJiraServer;
update the validation in the 'jira' case to require JIRA_HOST, JIRA_EMAIL and
JIRA_TOKEN (from options.jiraEnv or fallback options.env) and return null if any
are missing. Locate the 'jira' case block (variables jiraHost, jiraEnv and the
call to createJiraServer) and perform a combined presence check for
jiraEnv.JIRA_HOST, jiraEnv.JIRA_EMAIL and jiraEnv.JIRA_TOKEN before calling
createJiraServer so the MCP config is only returned when all credentials are
present.
| // Independent issue tracking flags (separate from source control) | ||
| if (config.githubIssuesEnabled !== undefined) { | ||
| existingVars['GITHUB_ISSUES_ENABLED'] = config.githubIssuesEnabled ? 'true' : 'false'; | ||
| } | ||
| if (config.gitlabIssuesEnabled !== undefined) { | ||
| existingVars['GITLAB_ISSUES_ENABLED'] = config.gitlabIssuesEnabled ? 'true' : 'false'; | ||
| } |
There was a problem hiding this comment.
Persist GITHUB_ISSUES_ENABLED in the generated .env.
generateEnvContent() stores config.githubIssuesEnabled in existingVars, but the template never writes that key back out. Any save round-trip will silently drop the GitHub Issues toggle.
🐛 Proposed fix
# =============================================================================
# GITHUB INTEGRATION (OPTIONAL)
# =============================================================================
${existingVars['GITHUB_TOKEN'] ? `GITHUB_TOKEN=${existingVars['GITHUB_TOKEN']}` : '# GITHUB_TOKEN='}
${existingVars['GITHUB_REPO'] ? `GITHUB_REPO=${existingVars['GITHUB_REPO']}` : '# GITHUB_REPO=owner/repo'}
${existingVars['GITHUB_AUTO_SYNC'] !== undefined ? `GITHUB_AUTO_SYNC=${existingVars['GITHUB_AUTO_SYNC']}` : '# GITHUB_AUTO_SYNC=false'}
+${existingVars['GITHUB_ISSUES_ENABLED'] !== undefined ? `GITHUB_ISSUES_ENABLED=${existingVars['GITHUB_ISSUES_ENABLED']}` : '# GITHUB_ISSUES_ENABLED=false'}Also applies to: 238-247
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/main/ipc-handlers/env-handlers.ts` around lines 92 - 98,
generateEnvContent currently sets existingVars['GITHUB_ISSUES_ENABLED'] and
['GITLAB_ISSUES_ENABLED'] from config but the output template never emits those
keys, so the toggles are lost on save; update generateEnvContent to include
those keys in the generated .env output when present (e.g., ensure the
template/serializer writes existingVars['GITHUB_ISSUES_ENABLED'] and
existingVars['GITLAB_ISSUES_ENABLED'] as GITHUB_ISSUES_ENABLED=true/false
lines), and mirror the same fix for the other similar block that handles these
flags (the duplicate logic later in the function).
| // JIRA config | ||
| if (vars['JIRA_HOST']) { | ||
| config.jiraHost = vars['JIRA_HOST']; | ||
| // Enable by default if host exists and JIRA_ENABLED is not explicitly false | ||
| config.jiraEnabled = vars['JIRA_ENABLED']?.toLowerCase() !== 'false'; | ||
| } |
There was a problem hiding this comment.
Honor JIRA_ENABLED even when the host is still blank.
ENV_GET only restores jiraEnabled inside the JIRA_HOST branch. That means a user can enable JIRA, save, and have the toggle reset to false on the next load until they also fill in the host field.
🐛 Proposed fix
- if (vars['JIRA_HOST']) {
- config.jiraHost = vars['JIRA_HOST'];
- // Enable by default if host exists and JIRA_ENABLED is not explicitly false
- config.jiraEnabled = vars['JIRA_ENABLED']?.toLowerCase() !== 'false';
- }
+ if (vars['JIRA_ENABLED'] !== undefined) {
+ config.jiraEnabled = vars['JIRA_ENABLED'].toLowerCase() === 'true';
+ }
+ if (vars['JIRA_HOST']) {
+ config.jiraHost = vars['JIRA_HOST'];
+ if (vars['JIRA_ENABLED'] === undefined) {
+ config.jiraEnabled = true;
+ }
+ }📝 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.
| // JIRA config | |
| if (vars['JIRA_HOST']) { | |
| config.jiraHost = vars['JIRA_HOST']; | |
| // Enable by default if host exists and JIRA_ENABLED is not explicitly false | |
| config.jiraEnabled = vars['JIRA_ENABLED']?.toLowerCase() !== 'false'; | |
| } | |
| // JIRA config | |
| if (vars['JIRA_ENABLED'] !== undefined) { | |
| config.jiraEnabled = vars['JIRA_ENABLED'].toLowerCase() === 'true'; | |
| } | |
| if (vars['JIRA_HOST']) { | |
| config.jiraHost = vars['JIRA_HOST']; | |
| if (vars['JIRA_ENABLED'] === undefined) { | |
| config.jiraEnabled = true; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/main/ipc-handlers/env-handlers.ts` around lines 431 - 436,
The code only sets config.jiraEnabled inside the JIRA_HOST branch so the saved
JIRA_ENABLED toggle is lost when JIRA_HOST is blank; update the ENV_GET handling
so vars['JIRA_ENABLED'] is respected regardless of vars['JIRA_HOST'] by
assigning config.jiraEnabled from vars['JIRA_ENABLED'] (e.g., config.jiraEnabled
= vars['JIRA_ENABLED']?.toLowerCase() !== 'false') outside or before the
vars['JIRA_HOST'] branch while still keeping the existing assignment of
config.jiraHost within the vars['JIRA_HOST'] block.
| function sanitizeIssueKey(value: unknown): string { | ||
| if (typeof value !== 'string') return ''; | ||
| // JIRA keys follow pattern: PROJECT-NUMBER | ||
| const match = value.match(/^[A-Z][A-Z0-9_]+-\d+$/); | ||
| return match ? value : ''; | ||
| } |
There was a problem hiding this comment.
Issue key regex may be too strict.
The regex ^[A-Z][A-Z0-9_]+-\d+$ only matches uppercase project keys starting with a letter. While this is the standard JIRA format, some instances may have lowercase keys or keys starting with numbers after migration. Consider a case-insensitive match.
🔧 Suggested fix
function sanitizeIssueKey(value: unknown): string {
if (typeof value !== 'string') return '';
- // JIRA keys follow pattern: PROJECT-NUMBER
- const match = value.match(/^[A-Z][A-Z0-9_]+-\d+$/);
- return match ? value : '';
+ // JIRA keys follow pattern: PROJECT-NUMBER (case-insensitive)
+ const match = value.match(/^[A-Za-z][A-Za-z0-9_]+-\d+$/);
+ return match ? value.toUpperCase() : '';
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/main/ipc-handlers/jira/spec-utils.ts` around lines 161 -
166, The sanitizeIssueKey regex is too strict; update the pattern in
sanitizeIssueKey to allow keys that start with a letter or digit and to perform
a case-insensitive match so lowercase project keys are accepted. Replace the
current /^[A-Z][A-Z0-9_]+-\d+$/ check with a regex that permits an initial
letter or digit (e.g., /^[A-Z0-9][A-Z0-9_]+-\d+$/) and add the case-insensitive
flag (/i), keeping the same return behavior (return value if match else '').
| function buildJql(projectKey: string, state: 'open' | 'closed' | 'all'): string { | ||
| let jql = `project = "${projectKey}"`; | ||
| if (state === 'open') { | ||
| jql += ' AND statusCategory != Done'; | ||
| } else if (state === 'closed') { | ||
| jql += ' AND statusCategory = Done'; | ||
| } | ||
| jql += ' ORDER BY updated DESC'; | ||
| return jql; | ||
| } |
There was a problem hiding this comment.
JQL injection vulnerability: projectKey is interpolated unsafely.
The projectKey value comes from user/config input and is directly interpolated into the JQL string without escaping. JIRA JQL uses double quotes for string values, and a malicious project key like "test" OR project = "ADMIN" could manipulate the query.
🔒 Proposed fix with proper escaping
+/**
+ * Escape a value for safe use in JQL string literals.
+ * JQL uses backslash escaping for special characters within double-quoted strings.
+ */
+function escapeJqlString(value: string): string {
+ return value.replace(/\\/g, '\\\\').replace(/"/g, '\\"');
+}
+
function buildJql(projectKey: string, state: 'open' | 'closed' | 'all'): string {
- let jql = `project = "${projectKey}"`;
+ let jql = `project = "${escapeJqlString(projectKey)}"`;
if (state === 'open') {
jql += ' AND statusCategory != Done';
} else if (state === 'closed') {
jql += ' AND statusCategory = Done';
}
jql += ' ORDER BY updated DESC';
return jql;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/stores/jira-store.ts` around lines 148 - 157, The
buildJql function is vulnerable because projectKey is interpolated directly into
JQL; instead sanitize or escape projectKey before interpolation (in function
buildJql) by either validating it against a strict whitelist/regex (e.g. only
allow A-Z,0-9, hyphen/underscore) or by escaping any double quotes/backslashes
so injected JQL cannot break out of the quoted string; update buildJql to use
the sanitized/escaped value for the project clause and keep the surrounding
quotes and ordering intact.
| function parseJiraIssue(raw: Record<string, unknown>): JiraIssue { | ||
| // Backend already transforms the issue (flattened, no fields wrapper) | ||
| return { | ||
| key: (raw.key as string) || '', | ||
| summary: (raw.summary as string) || '', | ||
| description: typeof raw.description === 'string' ? raw.description : undefined, | ||
| status: (raw.status as string) || 'Unknown', | ||
| assignee: (raw.assignee as string) || undefined, | ||
| priority: (raw.priority as string) || undefined, | ||
| issueType: (raw.issueType as string) || 'Task', | ||
| created: (raw.created as string) || '', | ||
| updated: (raw.updated as string) || '', | ||
| labels: (raw.labels as string[]) || [] | ||
| }; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding runtime type guards for safer parsing.
parseJiraIssue uses unchecked type assertions. If the backend response shape changes or is malformed, this could produce invalid state silently. A type guard or validation library (e.g., Zod) would improve robustness.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/stores/jira-store.ts` around lines 162 - 176, The
parseJiraIssue function currently uses unchecked casts and should validate
inputs at runtime; update parseJiraIssue to perform explicit type guards (or use
a small Zod/schema) to verify each field before assignment: check typeof
raw.key/summary/status/issueType/created/updated are strings, description is
string or undefined, assignee/priority are strings or undefined, and labels is
an array of strings via Array.isArray + every(item => typeof item === 'string'),
then use the validated values or safe defaults; reference the parseJiraIssue
function and the JiraIssue shape when implementing these guards so malformed
backend responses can't silently produce invalid state.
| export function investigateJiraIssue(projectId: string, issueKey: string): void { | ||
| const store = useJiraStore.getState(); | ||
| store.setInvestigationStatus({ | ||
| phase: 'fetching', | ||
| issueKey, | ||
| progress: 0, | ||
| message: 'Starting investigation...' | ||
| }); | ||
|
|
||
| // TODO: Call window.electronAPI.jiraInvestigateIssue(projectId, issueKey) when IPC is available | ||
| // For now, this is a placeholder. The InvestigationDialog handles the full flow. |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Incomplete implementation: investigateJiraIssue only updates local state.
The TODO comment indicates the IPC call is not yet wired. The function sets the investigation status to 'fetching' but never completes the flow. Ensure this is tracked for completion before release.
Would you like me to open an issue to track this incomplete implementation?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/stores/jira-store.ts` around lines 226 - 236, The
investigateJiraIssue function currently only sets a 'fetching' state in
useJiraStore and leaves a TODO; implement the full IPC flow by calling
window.electronAPI.jiraInvestigateIssue(projectId, issueKey) from
investigateJiraIssue, await the result, and update the store via
useJiraStore.getState().setInvestigationStatus to reflect success (e.g., phase:
'completed' with any returned data) or failure (phase: 'error' with an error
message); ensure errors are caught and logged, and remove the placeholder
comment so InvestigationDialog no longer has to handle the entire flow.
| { | ||
| "title": "Tickets JIRA", | ||
| "states": { | ||
| "open": "Ouvert", | ||
| "closed": "Ferme" | ||
| }, | ||
| "header": { | ||
| "open": "ouvert", | ||
| "searchPlaceholder": "Rechercher des tickets..." | ||
| }, | ||
| "filters": { | ||
| "open": "Ouvert", | ||
| "closed": "Ferme", | ||
| "all": "Tous" | ||
| }, | ||
| "empty": { | ||
| "noMatch": "Aucun ticket ne correspond a votre recherche", | ||
| "selectIssue": "Selectionnez un ticket pour voir les details" | ||
| }, | ||
| "notConnected": { | ||
| "title": "JIRA non connecte", | ||
| "description": "Configurez votre hote JIRA, email et jeton API dans les parametres du projet pour synchroniser les tickets.", | ||
| "openSettings": "Ouvrir les parametres" | ||
| }, | ||
| "detail": { | ||
| "createTask": "Creer une tache", | ||
| "viewTask": "Voir la tache", | ||
| "taskLinked": "Tache liee", | ||
| "taskId": "ID de tache", | ||
| "description": "Description", | ||
| "noDescription": "Aucune description fournie.", | ||
| "assignee": "Responsable", | ||
| "priority": "Priorite", | ||
| "status": "Statut", | ||
| "type": "Type", | ||
| "labels": "Labels", | ||
| "created": "Cree le", | ||
| "updated": "Mis a jour le" | ||
| }, | ||
| "investigation": { | ||
| "title": "Creer une tache a partir du ticket", | ||
| "issuePrefix": "Ticket", | ||
| "description": "Creer une tache a partir de ce ticket JIRA. La tache sera ajoutee a votre tableau Kanban dans la colonne Backlog.", | ||
| "willInclude": "La tache inclura :", | ||
| "includeTitle": "Titre et description du ticket", | ||
| "includeLink": "Lien vers le ticket JIRA", | ||
| "includeLabels": "Labels et metadonnees du ticket", | ||
| "taskCreated": "Tache creee ! Consultez-la dans votre tableau Kanban.", | ||
| "creating": "Creation...", | ||
| "fetchingDetails": "Recuperation des details du ticket...", | ||
| "analyzingIssue": "Analyse du ticket...", | ||
| "creatingTask": "Creation de la tache...", | ||
| "cancel": "Annuler", | ||
| "done": "Termine", | ||
| "close": "Fermer" | ||
| }, | ||
| "priority": { | ||
| "highest": "La plus haute", | ||
| "high": "Haute", | ||
| "medium": "Moyenne", | ||
| "low": "Basse", | ||
| "lowest": "La plus basse" | ||
| } | ||
| } |
There was a problem hiding this comment.
Namespace policy mismatch for i18n file.
This introduces a new jira namespace file, but the documented allowed namespaces do not include jira. Please either migrate these keys into an allowed namespace or update the project’s namespace policy in lockstep with this change.
As per coding guidelines: Namespaces are: common, navigation, settings, dialogs, tasks, errors, onboarding, welcome.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/shared/i18n/locales/fr/jira.json` around lines 1 - 64, The
new "jira" i18n namespace (file defining keys like "title", "states", "detail",
"investigation", "priority", etc.) violates the project's allowed namespaces;
either move all these keys into one of the approved namespaces (e.g., merge into
"tasks" or "common" and rename nested keys accordingly) or update the project's
namespace policy/lockfile to include "jira" so the new namespace is allowed;
locate the file introducing "jira" and perform the namespace migration or policy
update consistently for all keys (title, states, header, filters, empty,
notConnected, detail, investigation, priority).
| "open": "Ouvert", | ||
| "closed": "Ferme" | ||
| }, | ||
| "header": { | ||
| "open": "ouvert", | ||
| "searchPlaceholder": "Rechercher des tickets..." | ||
| }, | ||
| "filters": { | ||
| "open": "Ouvert", | ||
| "closed": "Ferme", | ||
| "all": "Tous" | ||
| }, | ||
| "empty": { | ||
| "noMatch": "Aucun ticket ne correspond a votre recherche", | ||
| "selectIssue": "Selectionnez un ticket pour voir les details" | ||
| }, | ||
| "notConnected": { | ||
| "title": "JIRA non connecte", | ||
| "description": "Configurez votre hote JIRA, email et jeton API dans les parametres du projet pour synchroniser les tickets.", | ||
| "openSettings": "Ouvrir les parametres" | ||
| }, | ||
| "detail": { | ||
| "createTask": "Creer une tache", | ||
| "viewTask": "Voir la tache", | ||
| "taskLinked": "Tache liee", | ||
| "taskId": "ID de tache", | ||
| "description": "Description", | ||
| "noDescription": "Aucune description fournie.", | ||
| "assignee": "Responsable", | ||
| "priority": "Priorite", | ||
| "status": "Statut", | ||
| "type": "Type", | ||
| "labels": "Labels", | ||
| "created": "Cree le", | ||
| "updated": "Mis a jour le" | ||
| }, | ||
| "investigation": { | ||
| "title": "Creer une tache a partir du ticket", | ||
| "issuePrefix": "Ticket", | ||
| "description": "Creer une tache a partir de ce ticket JIRA. La tache sera ajoutee a votre tableau Kanban dans la colonne Backlog.", | ||
| "willInclude": "La tache inclura :", | ||
| "includeTitle": "Titre et description du ticket", | ||
| "includeLink": "Lien vers le ticket JIRA", | ||
| "includeLabels": "Labels et metadonnees du ticket", | ||
| "taskCreated": "Tache creee ! Consultez-la dans votre tableau Kanban.", | ||
| "creating": "Creation...", | ||
| "fetchingDetails": "Recuperation des details du ticket...", | ||
| "analyzingIssue": "Analyse du ticket...", | ||
| "creatingTask": "Creation de la tache...", | ||
| "cancel": "Annuler", | ||
| "done": "Termine", | ||
| "close": "Fermer" |
There was a problem hiding this comment.
French localization contains multiple user-facing spelling/diacritic errors.
Several strings are missing French accents/orthography (for example on Lines 5, 17-18, 21-23, 26, 41-43, 49-52), which degrades UI quality.
✏️ Suggested text corrections (sample patch)
- "closed": "Ferme"
+ "closed": "Fermé"
...
- "noMatch": "Aucun ticket ne correspond a votre recherche",
- "selectIssue": "Selectionnez un ticket pour voir les details"
+ "noMatch": "Aucun ticket ne correspond à votre recherche",
+ "selectIssue": "Sélectionnez un ticket pour voir les détails"
...
- "title": "JIRA non connecte",
- "description": "Configurez votre hote JIRA, email et jeton API dans les parametres du projet pour synchroniser les tickets.",
- "openSettings": "Ouvrir les parametres"
+ "title": "JIRA non connecté",
+ "description": "Configurez votre hôte JIRA, e-mail et jeton API dans les paramètres du projet pour synchroniser les tickets.",
+ "openSettings": "Ouvrir les paramètres"
...
- "createTask": "Creer une tache",
+ "createTask": "Créer une tâche",📝 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.
| "open": "Ouvert", | |
| "closed": "Ferme" | |
| }, | |
| "header": { | |
| "open": "ouvert", | |
| "searchPlaceholder": "Rechercher des tickets..." | |
| }, | |
| "filters": { | |
| "open": "Ouvert", | |
| "closed": "Ferme", | |
| "all": "Tous" | |
| }, | |
| "empty": { | |
| "noMatch": "Aucun ticket ne correspond a votre recherche", | |
| "selectIssue": "Selectionnez un ticket pour voir les details" | |
| }, | |
| "notConnected": { | |
| "title": "JIRA non connecte", | |
| "description": "Configurez votre hote JIRA, email et jeton API dans les parametres du projet pour synchroniser les tickets.", | |
| "openSettings": "Ouvrir les parametres" | |
| }, | |
| "detail": { | |
| "createTask": "Creer une tache", | |
| "viewTask": "Voir la tache", | |
| "taskLinked": "Tache liee", | |
| "taskId": "ID de tache", | |
| "description": "Description", | |
| "noDescription": "Aucune description fournie.", | |
| "assignee": "Responsable", | |
| "priority": "Priorite", | |
| "status": "Statut", | |
| "type": "Type", | |
| "labels": "Labels", | |
| "created": "Cree le", | |
| "updated": "Mis a jour le" | |
| }, | |
| "investigation": { | |
| "title": "Creer une tache a partir du ticket", | |
| "issuePrefix": "Ticket", | |
| "description": "Creer une tache a partir de ce ticket JIRA. La tache sera ajoutee a votre tableau Kanban dans la colonne Backlog.", | |
| "willInclude": "La tache inclura :", | |
| "includeTitle": "Titre et description du ticket", | |
| "includeLink": "Lien vers le ticket JIRA", | |
| "includeLabels": "Labels et metadonnees du ticket", | |
| "taskCreated": "Tache creee ! Consultez-la dans votre tableau Kanban.", | |
| "creating": "Creation...", | |
| "fetchingDetails": "Recuperation des details du ticket...", | |
| "analyzingIssue": "Analyse du ticket...", | |
| "creatingTask": "Creation de la tache...", | |
| "cancel": "Annuler", | |
| "done": "Termine", | |
| "close": "Fermer" | |
| "closed": "Fermé" | |
| }, | |
| "header": { | |
| "open": "ouvert", | |
| "searchPlaceholder": "Rechercher des tickets..." | |
| }, | |
| "filters": { | |
| "open": "Ouvert", | |
| "closed": "Fermé", | |
| "all": "Tous" | |
| }, | |
| "empty": { | |
| "noMatch": "Aucun ticket ne correspond à votre recherche", | |
| "selectIssue": "Sélectionnez un ticket pour voir les détails" | |
| }, | |
| "notConnected": { | |
| "title": "JIRA non connecté", | |
| "description": "Configurez votre hôte JIRA, e-mail et jeton API dans les paramètres du projet pour synchroniser les tickets.", | |
| "openSettings": "Ouvrir les paramètres" | |
| }, | |
| "detail": { | |
| "createTask": "Créer une tâche", | |
| "viewTask": "Voir la tâche", | |
| "taskLinked": "Tâche liée", | |
| "taskId": "ID de tâche", | |
| "description": "Description", | |
| "noDescription": "Aucune description fournie.", | |
| "assignee": "Responsable", | |
| "priority": "Priorité", | |
| "status": "Statut", | |
| "type": "Type", | |
| "labels": "Labels", | |
| "created": "Créé le", | |
| "updated": "Mis à jour le" | |
| }, | |
| "investigation": { | |
| "title": "Créer une tâche à partir du ticket", | |
| "issuePrefix": "Ticket", | |
| "description": "Créer une tâche à partir de ce ticket JIRA. La tâche sera ajoutée à votre tableau Kanban dans la colonne Backlog.", | |
| "willInclude": "La tâche inclura :", | |
| "includeTitle": "Titre et description du ticket", | |
| "includeLink": "Lien vers le ticket JIRA", | |
| "includeLabels": "Labels et métadonnées du ticket", | |
| "taskCreated": "Tâche créée ! Consultez-la dans votre tableau Kanban.", | |
| "creating": "Création...", | |
| "fetchingDetails": "Récupération des détails du ticket...", | |
| "analyzingIssue": "Analyse du ticket...", | |
| "creatingTask": "Création de la tâche...", | |
| "cancel": "Annuler", | |
| "done": "Terminé", | |
| "close": "Fermer" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/shared/i18n/locales/fr/jira.json` around lines 4 - 55, The
French locale contains multiple spelling and diacritic errors; update the
strings under keys such as "open"/"closed", "header.searchPlaceholder",
"filters.*", "empty.noMatch" and "empty.selectIssue",
"notConnected.title"/"notConnected.description"/"notConnected.openSettings", and
entries in "detail.*" and "investigation.*" to use correct French accents and
orthography (e.g. "Ouvert", "Fermé", "Rechercher des tickets…", "Aucun ticket ne
correspond à votre recherche", "Sélectionnez un ticket pour voir les détails",
"JIRA non connecté", "hôte", "e‑mail", "jeton API", "Créer une tâche", "Tâche
liée", "Aucune description fournie.", "Créer une tâche à partir du ticket",
etc.); ensure punctuation and capitalization follow existing locale conventions
and run a quick spellcheck on all values before committing.
The planning pipeline was writing blank/unhelpful error entries to task_logs.json. Root cause: AI SDK error events contain 'Error' as the message with the actual cause buried in nested properties. Fixes: - stream-handler.ts: Extract error cause from nested Error objects, fall back to stack trace or JSON serialization when message is 'Error' - task-log-writer.ts: Handle malformed error objects with null-safe access and JSON fallback for the error detail - worker.ts: Write errors to task_logs.json in session catch blocks and orchestrator error handler (previously only sent to main thread) Actual root cause of planning failures identified: "Model completed session without making any tool calls — expected files not created: context.json" — tracked in issue AndyMik90#1978.
|
@AndyMik90 Pushed a follow-up commit that fixes blank error messages in the planning phase logs. What was happening: Planning errors showed blank detail in the Logs tab - just red 'error' badges with no message. We traced the root cause:
What we fixed (3 files):
Actual planning failure root cause (separate issue):
This is tracked in #1978 - it appears to be a Vercel AI SDK Once #1978 is resolved, this PR's full JIRA integration can be end-to-end tested. |
| const resolvedFile = path.resolve(filePath); | ||
| if (!resolvedFile.startsWith(resolvedVault + path.sep)) { | ||
| continue; | ||
| } |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/ai/agent/worker.ts`:
- Around line 679-686: The error handler is casting the orchestrator phase into
a TaskLogWriter Phase unsafely; instead map the BuildOrchestrator phase using
the existing mapExecutionPhaseToPhase (or toLogPhase) helper and pass that
mappedPhase into logWriter.logText; update the orchestrator.on('error', ...)
block to compute mappedPhase = mapExecutionPhaseToPhase(phase) and call
logWriter.logText(error.message, mappedPhase, 'error') so qa_review/qa_fixing
map correctly.
In `@apps/desktop/src/main/ai/logging/task-log-writer.ts`:
- Around line 160-169: The error logging paths in task-log-writer.ts call
JSON.stringify directly on unknown-typed values (cause and err) which can throw
for circular structures or unsupported types; create a small safe serializer
(e.g., safeStringify(value): string) that tries JSON.stringify(value) in a
try/catch and returns String(value) on failure, then replace the direct
JSON.stringify(cause).slice(0,500) and JSON.stringify(err).slice(0,500) uses
with safeStringify(cause).slice(0,500) and safeStringify(err).slice(0,500)
inside the same error-handling branch so logging cannot throw and the task log
entry will still be persisted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8fda5581-ce06-4a9b-8305-f029b819a555
📒 Files selected for processing (3)
apps/desktop/src/main/ai/agent/worker.tsapps/desktop/src/main/ai/logging/task-log-writer.tsapps/desktop/src/main/ai/session/stream-handler.ts
| orchestrator.on('error', (error: Error, phase: string) => { | ||
| postLog(`Error in ${phase} phase: ${error.message}`); | ||
| const errorMsg = `Error in ${phase} phase: ${error.message}`; | ||
| postLog(errorMsg); | ||
| postError(errorMsg); | ||
| // Also write to task_logs.json so the UI shows the error detail | ||
| if (logWriter) { | ||
| logWriter.logText(error.message, phase as Phase, 'error'); | ||
| } |
There was a problem hiding this comment.
Map orchestrator phases before logging them.
The cast on Line 685 is unsafe: BuildOrchestrator emits phases like qa_review and qa_fixing, but TaskLogWriter.logText() only understands planning / coding / qa / spec. Those QA failures will hit toLogPhase()'s default branch and be written under coding. Reuse mapExecutionPhaseToPhase() here instead of casting.
🩹 Suggested fix
if (logWriter) {
- logWriter.logText(error.message, phase as Phase, 'error');
+ const logPhase = mapExecutionPhaseToPhase(phase as ExecutionPhase);
+ logWriter.logText(error.message, logPhase, 'error');
}📝 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.
| orchestrator.on('error', (error: Error, phase: string) => { | |
| postLog(`Error in ${phase} phase: ${error.message}`); | |
| const errorMsg = `Error in ${phase} phase: ${error.message}`; | |
| postLog(errorMsg); | |
| postError(errorMsg); | |
| // Also write to task_logs.json so the UI shows the error detail | |
| if (logWriter) { | |
| logWriter.logText(error.message, phase as Phase, 'error'); | |
| } | |
| orchestrator.on('error', (error: Error, phase: string) => { | |
| const errorMsg = `Error in ${phase} phase: ${error.message}`; | |
| postLog(errorMsg); | |
| postError(errorMsg); | |
| // Also write to task_logs.json so the UI shows the error detail | |
| if (logWriter) { | |
| const logPhase = mapExecutionPhaseToPhase(phase as ExecutionPhase); | |
| logWriter.logText(error.message, logPhase, 'error'); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/main/ai/agent/worker.ts` around lines 679 - 686, The error
handler is casting the orchestrator phase into a TaskLogWriter Phase unsafely;
instead map the BuildOrchestrator phase using the existing
mapExecutionPhaseToPhase (or toLogPhase) helper and pass that mappedPhase into
logWriter.logText; update the orchestrator.on('error', ...) block to compute
mappedPhase = mapExecutionPhaseToPhase(phase) and call
logWriter.logText(error.message, mappedPhase, 'error') so qa_review/qa_fixing
map correctly.
| } else if (cause instanceof Error) { | ||
| errorContent = cause.message || String(cause); | ||
| } else if (cause && typeof cause === 'object') { | ||
| errorContent = JSON.stringify(cause).slice(0, 500); | ||
| } else if (cause) { | ||
| errorContent = String(cause); | ||
| } else if (code && code !== 'error') { | ||
| errorContent = `Error code: ${code}`; | ||
| } else { | ||
| errorContent = `Error: ${JSON.stringify(err).slice(0, 500)}`; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists and get its structure
fd task-log-writer.tsRepository: AndyMik90/Aperant
Length of output: 112
🏁 Script executed:
# Read the full file to understand the processEvent context
cat -n apps/desktop/src/main/ai/logging/task-log-writer.ts | head -200Repository: AndyMik90/Aperant
Length of output: 7978
🏁 Script executed:
# Search for JSON.stringify usage patterns in similar logging/error contexts
rg "JSON.stringify" apps/desktop/src/main/ -B 2 -A 2 --max-count=20Repository: AndyMik90/Aperant
Length of output: 50374
🏁 Script executed:
# Find the StreamEvent type definition
fd -e ts -e tsx | xargs grep -l "type StreamEvent" | head -5Repository: AndyMik90/Aperant
Length of output: 102
🏁 Script executed:
# Look at the session/types.ts file to understand StreamEvent structure
rg "type StreamEvent|interface StreamEvent" -A 30 apps/desktop/src/main/ai/session/Repository: AndyMik90/Aperant
Length of output: 2029
🏁 Script executed:
# Get the full ErrorEvent definition
rg "interface ErrorEvent" -A 10 apps/desktop/src/main/ai/session/types.tsRepository: AndyMik90/Aperant
Length of output: 248
🏁 Script executed:
# Find the SessionError type definition
rg "type SessionError|interface SessionError" -A 15 apps/desktop/src/main/ai/session/types.tsRepository: AndyMik90/Aperant
Length of output: 518
Wrap JSON.stringify calls in error fallback with try-catch.
Lines 163 and 169 call JSON.stringify() directly on cause (typed unknown) and err respectively. Since cause is explicitly unknown per the SessionError type definition, it could contain circular references or BigInt, which would throw. Failure in this error-logging path would prevent the task log entry from being persisted. Add a safe serializer function that falls back to String() on stringify failure.
🔧 Suggested fix
const err = event.error;
+ const safeSerialize = (value: unknown): string => {
+ try {
+ return JSON.stringify(value);
+ } catch {
+ return String(value);
+ }
+ };
let errorContent = 'Unknown error';
if (err) {
const msg = err.message || '';
const code = err.code || '';
const cause = err.cause;
if (msg && msg !== 'error') {
errorContent = msg;
} else if (cause instanceof Error) {
errorContent = cause.message || String(cause);
- } else if (cause && typeof cause === 'object') {
- errorContent = JSON.stringify(cause).slice(0, 500);
+ } else if (cause && typeof cause === 'object') {
+ errorContent = safeSerialize(cause).slice(0, 500);
} else if (cause) {
errorContent = String(cause);
} else if (code && code !== 'error') {
errorContent = `Error code: ${code}`;
} else {
- errorContent = `Error: ${JSON.stringify(err).slice(0, 500)}`;
+ errorContent = `Error: ${safeSerialize(err).slice(0, 500)}`;
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/main/ai/logging/task-log-writer.ts` around lines 160 - 169,
The error logging paths in task-log-writer.ts call JSON.stringify directly on
unknown-typed values (cause and err) which can throw for circular structures or
unsupported types; create a small safe serializer (e.g., safeStringify(value):
string) that tries JSON.stringify(value) in a try/catch and returns
String(value) on failure, then replace the direct
JSON.stringify(cause).slice(0,500) and JSON.stringify(err).slice(0,500) uses
with safeStringify(cause).slice(0,500) and safeStringify(err).slice(0,500)
inside the same error-handling branch so logging cannot throw and the task log
entry will still be persisted.
| // DEBUG: Dump the raw error to console for diagnosis | ||
| console.warn('[StreamHandler] RAW ERROR:', JSON.stringify(part.error, Object.getOwnPropertyNames(part.error instanceof Error ? part.error : {}), 2).slice(0, 1000)); | ||
| console.warn('[StreamHandler] ERROR TYPE:', typeof part.error, part.error?.constructor?.name); | ||
| if (part.error instanceof Error) { | ||
| console.warn('[StreamHandler] ERROR CAUSE:', (part.error as { cause?: unknown }).cause); | ||
| console.warn('[StreamHandler] ERROR STACK:', part.error.stack?.slice(0, 500)); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
head -300 apps/desktop/src/main/ai/session/stream-handler.ts | tail -60Repository: AndyMik90/Aperant
Length of output: 2479
🏁 Script executed:
sed -n '250,300p' apps/desktop/src/main/ai/session/stream-handler.tsRepository: AndyMik90/Aperant
Length of output: 2102
🏁 Script executed:
git ls-files apps/desktop/src/main/ | head -20Repository: AndyMik90/Aperant
Length of output: 1199
🏁 Script executed:
# Check for any dev-only conditionals or Sentry usage in the handler
sed -n '250,310p' apps/desktop/src/main/ai/session/stream-handler.tsRepository: AndyMik90/Aperant
Length of output: 2383
🏁 Script executed:
# Verify if classifyError sanitizes and how it works
rg -A 10 "function classifyError" apps/desktop/src/main/ai/session/stream-handler.tsRepository: AndyMik90/Aperant
Length of output: 43
🏁 Script executed:
# Check if there's any dev-only flag or environment check in this file
rg "isDev|process.env.NODE_ENV|__DEV__" apps/desktop/src/main/ai/session/stream-handler.tsRepository: AndyMik90/Aperant
Length of output: 43
🏁 Script executed:
# Check imports at the top of the file
head -50 apps/desktop/src/main/ai/session/stream-handler.tsRepository: AndyMik90/Aperant
Length of output: 1869
🏁 Script executed:
# Look for classifyError definition anywhere in the codebase
rg "classifyError" apps/desktop/src/main/ai/ -A 5Repository: AndyMik90/Aperant
Length of output: 10800
🏁 Script executed:
# Check the full handleError function and what follows
sed -n '260,300p' apps/desktop/src/main/ai/session/stream-handler.tsRepository: AndyMik90/Aperant
Length of output: 1897
Remove unsanitized debug logs that leak sensitive error data.
Lines 269-275 log raw error, cause, and stack directly via console.warn() before classifyError() can sanitize credentials. This violates the guideline requiring production main-process code to use Sentry instead of console output. Additionally, the JSON.stringify() call can throw on circular references or BigInt values, causing the diagnostic path itself to fail. Wrap these debug dumps behind a development-only flag and remove them before shipping, or route them through a safe serializer that cannot fail.
|
Thanks @thameema could you let me know about this: Critical 1: Non-existent npm packages Both packages return 404 on npm:
The real filesystem MCP server is @modelcontextprotocol/server-filesystem (published by the MCP project, not |
- @anthropic-ai/jira-mcp-server -> @modelcontextprotocol/server-atlassian - @anthropic-ai/filesystem-mcp-server -> @modelcontextprotocol/server-filesystem Previous names were placeholders that do not exist on npm. The Atlassian server uses ATLASSIAN_SITE_URL/USER_EMAIL/API_TOKEN env vars. Note: JIRA functionality works without MCP - the IPC handlers in jira/issue-handlers.ts provide direct JIRA API access. The MCP server is optional, only used when AI agents need direct JIRA tool access.
|
Good catch @AndyMik90 - fixed in the latest commit. What happened: The MCP package names were placeholders generated during implementation. They should have been validated before commit. What we fixed:
Important context: The core JIRA integration does not depend on these MCP servers at all. All JIRA functionality (issue browsing, search, task creation, investigation) is implemented directly via IPC handlers that call the JIRA REST API v3. The MCP entries are only used when AI agents need optional direct JIRA tool access during autonomous sessions. Updated the env var names to match Are there any other findings from the review? |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/ai/mcp/registry.ts`:
- Around line 130-143: The function createJiraServer currently accepts a broad
Record<string,string>, hiding missing/optional keys; define a narrow JiraEnv
type (e.g. interface JiraEnv { JIRA_HOST?: string; JIRA_EMAIL?: string;
JIRA_TOKEN?: string } or Partial<Record<'JIRA_HOST'|'JIRA_EMAIL'|'JIRA_TOKEN',
string>>) and update the createJiraServer(env: JiraEnv): McpServerConfig
signature to use it, adjust any call sites and the other registry uses noted at
lines 184-185 to accept the same JiraEnv type, and run the TypeScript typecheck
(npm run typecheck) to ensure missing keys are caught or explicitly handled
before accessing env.JIRA_HOST / env.JIRA_EMAIL / env.JIRA_TOKEN.
- Around line 138-139: Pin MCP server package specifiers instead of invoking
unversioned packages with npx (update the args for the MCP invocations that
currently use 'npx -y `@modelcontextprotocol/server-atlassian`' and similar to
include explicit semver pins like '@modelcontextprotocol/server-atlassian@x.y.z'
and the filesystem server to a fixed version), correct the comment/usage for the
filesystem/Vault server in registry.ts (the comment claiming "read-only file
access" is wrong for `@modelcontextprotocol/server-filesystem` — either change the
comment to reflect write-capable tools or switch to a read-only server variant
if available), and tighten Jira credential validation in the code path that
enables the Jira MCP server (where JIRA_HOST is checked and the server is
enabled, also require non-empty JIRA_EMAIL and JIRA_TOKEN environment variables
before enabling and passing them into the MCP server configuration).
- Around line 149-166: The createVaultServer function currently runs
`@modelcontextprotocol/server-filesystem` which exposes write tools; change the
Vault MCP to a read-only integration by either (a) replacing the transport
command/args to a read-only fork (e.g., danielsuguimoto/readonly-filesystem-mcp)
or (b) run the filesystem server under a Docker invocation that mounts the
vaultPath read-only (use the container runtime with a ro mount) or (c)
patch/filter the exported tools from `@modelcontextprotocol/server-filesystem` to
remove write tools (write_file, edit_file, move_file, create_directory) before
registering the MCP; then update the top comment text in createVaultServer to
accurately state the actual access level (read-only or read-write depending on
chosen fix) and ensure the MCP id 'vault' and transport command reflect the
chosen approach.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4352cd3b-800a-455d-9d26-ad4a619b9f2f
📒 Files selected for processing (1)
apps/desktop/src/main/ai/mcp/registry.ts
| function createJiraServer(env: Record<string, string>): McpServerConfig { | ||
| return { | ||
| id: 'jira', | ||
| name: 'JIRA', | ||
| description: 'Issue tracking integration for JIRA/Atlassian', | ||
| enabledByDefault: false, | ||
| transport: { | ||
| type: 'stdio', | ||
| command: 'npx', | ||
| args: ['-y', '@modelcontextprotocol/server-atlassian'], | ||
| env: { | ||
| ATLASSIAN_SITE_URL: env.JIRA_HOST || '', | ||
| ATLASSIAN_USER_EMAIL: env.JIRA_EMAIL || '', | ||
| ATLASSIAN_API_TOKEN: env.JIRA_TOKEN || '', |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use a narrow JiraEnv shape instead of Record<string, string>.
Record<string, string> makes env.JIRA_HOST / env.JIRA_EMAIL / env.JIRA_TOKEN look like ordinary strings in the type system, even though callers can still omit them at runtime. An explicit interface or Partial<Record<'JIRA_HOST' | 'JIRA_EMAIL' | 'JIRA_TOKEN', string>>, paired with a required shape at createJiraServer(), would let strict mode catch this class of bug earlier. As per coding guidelines, "Use TypeScript strict mode. Perform type checking with npm run typecheck."
Also applies to: 184-185
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/main/ai/mcp/registry.ts` around lines 130 - 143, The
function createJiraServer currently accepts a broad Record<string,string>,
hiding missing/optional keys; define a narrow JiraEnv type (e.g. interface
JiraEnv { JIRA_HOST?: string; JIRA_EMAIL?: string; JIRA_TOKEN?: string } or
Partial<Record<'JIRA_HOST'|'JIRA_EMAIL'|'JIRA_TOKEN', string>>) and update the
createJiraServer(env: JiraEnv): McpServerConfig signature to use it, adjust any
call sites and the other registry uses noted at lines 184-185 to accept the same
JiraEnv type, and run the TypeScript typecheck (npm run typecheck) to ensure
missing keys are caught or explicitly handled before accessing env.JIRA_HOST /
env.JIRA_EMAIL / env.JIRA_TOKEN.
| command: 'npx', | ||
| args: ['-y', '@modelcontextprotocol/server-atlassian'], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/desktop/src/main/ai/mcp/registry.ts | head -250 | tail -150Repository: AndyMik90/Aperant
Length of output: 5816
🏁 Script executed:
wc -l apps/desktop/src/main/ai/mcp/registry.tsRepository: AndyMik90/Aperant
Length of output: 105
🏁 Script executed:
cat -n apps/desktop/src/main/ai/mcp/registry.ts | sed -n '130,250p'Repository: AndyMik90/Aperant
Length of output: 4700
🌐 Web query:
npx -y npm package install behavior latest dist-tag unversioned
💡 Result:
When running npx -y (unversioned package specifier) where the package is not found locally or globally, npx (npm exec) will install and execute the version associated with the 'latest' dist-tag from the npm registry. The -y (or --yes) flag bypasses the confirmation prompt for installation. If a matching package exists locally in the project's dependencies (exact name and version match if specifier provided, or any version for unversioned), it uses the local one first. Otherwise, it fetches from the registry using standard npm resolution, defaulting to 'latest' tag for unversioned names. This aligns with npm install behavior: unversioned installs the 'latest' tag (docs confirm npm install without specifier uses latest tag, and npx uses same package resolution).
Citations:
- 1: https://docs.npmjs.com/cli/v9/commands/npm-dist-tag
- 2: https://docs.npmjs.com/cli/v9/commands/npx/
- 3: https://docs.npmjs.com/cli/v9/commands/npm-exec/
- 4: https://stackoverflow.com/questions/39650856/how-to-install-the-latest-possible-version-of-an-npm-package
🌐 Web query:
@modelcontextprotocol/server-filesystem write capabilities read-only
💡 Result:
The @modelcontextprotocol/server-filesystem provides full read and write capabilities for filesystem operations within configured allowed directories. It is not read-only by default. Key write tools include: - write_file: Create new or overwrite existing files - edit_file: Selective edits to files - create_directory: Create directories - delete_file / delete_directory / move_file / move_directory (implied by features) Read-only tools (marked with readOnlyHint: true) include read_text_file, read_media_file, list_directory, etc. Tool annotations distinguish read-only from write-capable tools. Operations are restricted to specified directories via command-line args or MCP Roots protocol. Separate read-only variants exist (e.g., readonly-filesystem-mcp), but the official server supports writing. Mounts can use 'ro' flag for read-only access in Docker.
Citations:
- 1: https://github.com/modelcontextprotocol/servers/tree/main/src/filesystem
- 2: https://registry.npmjs.org/%40modelcontextprotocol%2Fserver-filesystem
- 3: https://www.npmjs.com/package/@modelcontextprotocol/server-filesystem
- 4: https://github.com/modelcontextprotocol/servers/blob/main/src/filesystem/README.md
Address supply-chain risk and security issues in MCP server invocations.
Three issues require fixing:
-
Pin MCP server versions instead of using
npx -ywith unversioned packages. Unversioned specifiers resolve to npm'slatestdist-tag, meaning these secret-bearing (Jira) and filesystem-bearing (Vault) integrations will download and execute the latest package version on each install—non-deterministic and widening supply-chain blast radius. Pinned versions (e.g.,@modelcontextprotocol/server-atlassian@1.0.0) are required. -
The Vault server is write-capable, not read-only as documented. The comment at line 152 claims "read-only file access," but
@modelcontextprotocol/server-filesystemexposes write tools (write_file,edit_file,create_directory,delete_file). Correct the documentation or restrict access by using a dedicated read-only variant. Using Dockerromounts is insufficient if the application controls the server directly. -
Jira credential validation is incomplete. Lines 236–244 enable the Jira server if only
JIRA_HOSTis present, but then passJIRA_EMAILandJIRA_TOKENwithout validating they exist. Missing credentials will be passed as empty strings (lines 141–143), causing runtime failure with cryptic MCP server errors. Validate all three credentials before enabling.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/main/ai/mcp/registry.ts` around lines 138 - 139, Pin MCP
server package specifiers instead of invoking unversioned packages with npx
(update the args for the MCP invocations that currently use 'npx -y
`@modelcontextprotocol/server-atlassian`' and similar to include explicit semver
pins like '@modelcontextprotocol/server-atlassian@x.y.z' and the filesystem
server to a fixed version), correct the comment/usage for the filesystem/Vault
server in registry.ts (the comment claiming "read-only file access" is wrong for
`@modelcontextprotocol/server-filesystem` — either change the comment to reflect
write-capable tools or switch to a read-only server variant if available), and
tighten Jira credential validation in the code path that enables the Jira MCP
server (where JIRA_HOST is checked and the server is enabled, also require
non-empty JIRA_EMAIL and JIRA_TOKEN environment variables before enabling and
passing them into the MCP server configuration).
| /** | ||
| * Vault MCP server - external vault/Obsidian integration. | ||
| * Conditionally enabled when vault path is configured. | ||
| * Provides read-only file access to vault directory for agent context. | ||
| * | ||
| * Uses the official @modelcontextprotocol/server-filesystem package. | ||
| */ | ||
| function createVaultServer(vaultPath: string): McpServerConfig { | ||
| return { | ||
| id: 'vault', | ||
| name: 'Vault', | ||
| description: 'External vault integration for context and learnings', | ||
| enabledByDefault: false, | ||
| transport: { | ||
| type: 'stdio', | ||
| command: 'npx', | ||
| args: ['-y', '@modelcontextprotocol/server-filesystem', vaultPath], | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the actual file to understand the current implementation
find . -name "registry.ts" -path "*apps/desktop/src/main/ai/mcp*" -type fRepository: AndyMik90/Aperant
Length of output: 103
🏁 Script executed:
# Check the specific file content around the mentioned lines
cat -n apps/desktop/src/main/ai/mcp/registry.ts | sed -n '140,175p'Repository: AndyMik90/Aperant
Length of output: 1400
🏁 Script executed:
# Look for any related MCP server configuration or tool handling
rg "createVaultServer|@modelcontextprotocol/server-filesystem" --type ts -A 5 -B 5Repository: AndyMik90/Aperant
Length of output: 2334
🏁 Script executed:
# Check package.json for version info and related dependencies
cat -n apps/desktop/package.json | grep -i "modelcontext\|mcp"Repository: AndyMik90/Aperant
Length of output: 290
🌐 Web query:
@modelcontextprotocol/server-filesystem read-only configuration options capabilities
💡 Result:
The @modelcontextprotocol/server-filesystem is a Node.js MCP (Model Context Protocol) server providing filesystem operations including read/write files, directory management, search, and metadata retrieval. All operations are restricted to explicitly allowed directories for security. No built-in read-only configuration option or flag exists in the official server. Write tools (create_directory, write_file, edit_file, etc.) are always available with readOnlyHint: false in tool annotations, allowing clients to distinguish them. Read-only access can be achieved via: 1. Docker mounts with 'ro' flag: Mount host directories as read-only inside the container. Example in claude_desktop_config.json: { "mcpServers": { "filesystem": { "command": "docker", "args": [ "run", "-i", "--rm", "--mount", "type=bind,src=/host/path,dst=/projects/allowed,ro", "mcp/filesystem", "/projects" ] } } } The 'ro' makes the mount read-only at the OS level, preventing writes despite server capabilities. [3,6,12] 2. Third-party read-only forks, e.g., danielsuguimoto/readonly-filesystem-mcp (clone without write tools). [5] Capabilities include read-only tools (readOnlyHint: true): read_text_file, read_media_file, read_multiple_files, list_directory, list_directory_with_sizes, directory_tree, search_files, get_file_info, list_allowed_directories. Write tools (readOnlyHint: false): create_directory (idempotent), write_file (destructive), edit_file (non-idempotent destructive), etc. Full list in official README. [1,2,3,6,7]
Citations:
- 1: https://registry.npmjs.org/%40modelcontextprotocol%2Fserver-filesystem
- 2: https://www.npmjs.com/package/%40modelcontextprotocol/server-filesystem
- 3: https://github.com/modelcontextprotocol/servers/tree/main/src/filesystem
- 4: https://github.com/danielsuguimoto/readonly-filesystem-mcp
- 5: https://github.com/modelcontextprotocol/servers/blob/main/src/filesystem/README.md
- 6: https://www.npmjs.com/package/@modelcontextprotocol/server-filesystem
- 7: https://playbooks.com/mcp/modelcontextprotocol-filesystem
- 8: https://github.com/modelcontextprotocol/servers/blob/main/src/filesystem/index.ts
🌐 Web query:
@modelcontextprotocol/server-filesystem write_file edit_file tools documentation
💡 Result:
@modelcontextprotocol/server-filesystem is a Node.js MCP (Model Context Protocol) server for secure filesystem operations, developed by Anthropic. It provides tools like write_file and edit_file for file manipulation within allowed directories. Key features: - Read/write files, manage directories, search files, get metadata. - Directory access control via command-line args or MCP Roots protocol. - Install via npm: npm install @modelcontextprotocol/server-filesystem (latest v2026.1.14 as of Jan 2026). - Run: npx @modelcontextprotocol/server-filesystem /path/to/allowed/dir write_file tool: - Inputs: path (string), content (string) - Overwrites existing files completely. Use with caution. edit_file tool: - Inputs: path (string), edits (array of {oldText: string, newText: string}), dryRun (boolean, default false) - Performs line-based, exact-match replacements with whitespace normalization and indentation preservation. - Supports multiple edits, git-style diff output. - dryRun previews changes without applying. - Not idempotent; re-applying may fail or double-apply. Configuration example for Claude Desktop (claude_desktop_config.json): { "mcpServers": { "filesystem": { "command": "npx", "args": ["-y", "@modelcontextprotocol/server-filesystem", "/Users/username/Desktop"] } } } Source code: https://github.com/modelcontextprotocol/servers/tree/main/src/filesystem NPM: https://www.npmjs.com/package/@modelcontextprotocol/server-filesystem Note: Early NPM versions lacked edit_file; use @latest or build from GitHub source if needed.
Citations:
- 1: https://www.npmjs.com/package/@modelcontextprotocol/server-filesystem
- 2: https://registry.npmjs.org/%40modelcontextprotocol%2Fserver-filesystem
- 3: https://github.com/modelcontextprotocol/servers/blob/main/src/filesystem/index.ts
- 4: https://github.com/modelcontextprotocol/servers/tree/main/src/filesystem
- 5: https://github.com/modelcontextprotocol/servers/blob/main/src/filesystem/README.md
- 6: https://aembit.io/blog/how-to-enable-filesystem-support-in-model-context-protocol-mcp/
🏁 Script executed:
# Check if `@modelcontextprotocol/server-filesystem` has any env vars or args for tool restriction
npm view `@modelcontextprotocol/server-filesystem` readme | head -100Repository: AndyMik90/Aperant
Length of output: 3915
🌐 Web query:
@modelcontextprotocol/server-filesystem environment variables configuration arguments restrict tools
💡 Result:
The @modelcontextprotocol/server-filesystem is a Node.js MCP server for secure filesystem operations, restricting access to specified directories. Configuration: - Command-line arguments: Pass allowed directories as positional args after the command, e.g., npx -y @modelcontextprotocol/server-filesystem /path/to/dir1 /path/to/dir2. These set initial allowed directories. Use exclusion syntax like /dir!.env,dist to disallow specific files/paths within allowed dirs (e.g., .git and node_modules excluded by default). Negate with !, e.g., /dir!.git to allow. - MCP config (e.g., claude_desktop_config.json or mcp.json): { "mcpServers": { "filesystem": { "command": "npx", "args": ["-y", "@modelcontextprotocol/server-filesystem", "/path/to/dir1", "/path/to/dir2"] } } } Docker mounts to /projects, args like ["/projects/dir"]. Environment variables: No standard env vars for config in current version (proposal in GitHub issue #1879 for ALLOWED_DIRS etc., dated 2025-05-22, not merged). General MCP configs support "env" field, e.g., {"env": {"DEBUG": "true"}} (seen in examples). Tool restrictions: All tools (read_text_file, write_file, list_directory, etc., ~11 tools) restricted to allowed directories via path validation. Use list_allowed_directories tool to query. Supports MCP Roots for dynamic updates if client capabilities.roots supported. Tool annotations provide hints (readOnlyHint, destructiveHint, etc.). No separate "restrict tools" config; access controlled by directories.
Citations:
- 1: Proposal: Environment Variable Configuration for Filesystem MCP Server modelcontextprotocol/servers#1879
- 2: https://registry.npmjs.org/%40modelcontextprotocol%2Fserver-filesystem
- 3: https://github.com/modelcontextprotocol/servers/tree/main/src/filesystem
- 4: https://github.com/modelcontextprotocol/servers/blob/main/src/filesystem/README.md
- 5: https://www.npmjs.com/package/@modelcontextprotocol/server-filesystem
- 6: Files and paths can be disallowed inside of directories modelcontextprotocol/servers#1012
- 7: https://github.com/modelcontextprotocol/servers/blob/main/src/filesystem/index.ts
Fix misleading documentation comment and remove write-enabled tools from Vault MCP.
The comment claiming "read-only file access" (line 152) is false. The @modelcontextprotocol/server-filesystem package exposes write tools (write_file, edit_file, move_file, create_directory) by default with no configuration option to disable them—read-only is only enforced via Docker ro mounts or third-party read-only forks. The current setup allows agents to modify vault files, creating data-integrity risk.
Use one of:
- Mount the vault directory read-only via Docker with
roflag - Switch to a read-only fork like
danielsuguimoto/readonly-filesystem-mcp - Modify
@modelcontextprotocol/server-filesystemto support tool filtering
Update the misleading comment at line 152 to accurately describe the access level once the fix is in place.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/main/ai/mcp/registry.ts` around lines 149 - 166, The
createVaultServer function currently runs
`@modelcontextprotocol/server-filesystem` which exposes write tools; change the
Vault MCP to a read-only integration by either (a) replacing the transport
command/args to a read-only fork (e.g., danielsuguimoto/readonly-filesystem-mcp)
or (b) run the filesystem server under a Docker invocation that mounts the
vaultPath read-only (use the container runtime with a ro mount) or (c)
patch/filter the exported tools from `@modelcontextprotocol/server-filesystem` to
remove write tools (write_file, edit_file, move_file, create_directory) before
registering the MCP; then update the top comment text in createVaultServer to
accurately state the actual access level (read-only or read-write depending on
chosen fix) and ensure the MCP id 'vault' and transport command reflect the
chosen approach.
|
@AndyMik90 Hi, just following up - the MCP package name issue has been fixed and pushed. All CI checks are passing. We also identified and reported the planning pipeline issue (#1978) where the model completes sessions without making tool calls. That's blocking end-to-end testing of the full JIRA flow. Would appreciate a re-review when you get a chance. Happy to address any other findings. |
|
@AndyMik90 Following up again - this PR has been waiting for review since Mar 22. We addressed your MCP package finding within hours and all CI checks pass. This PR adds significant value:
We also filed #1978 which blocks all task execution - not just our changes. Could you take a look at both when you get a chance? Happy to jump on a call if that would be easier to review. |
3 participants
Summary
Complete JIRA integration with full feature parity to GitHub/GitLab, plus Vault support and a consolidated settings UI. Migrated to the new TypeScript/Vercel AI SDK architecture per @AndyMik90's request.
JIRA Integration
Vault Integration
Consolidated Settings UI
Sidebar
Other
Related
Test Plan
Summary by CodeRabbit