Feature/japanese localization 15675166926112915977 #793
Conversation
- Created Japanese locale files in `apps/frontend/src/shared/i18n/locales/ja/`
- Translated all UI strings to Japanese, following the guidelines:
- Technical terms in Katakana (e.g., プルリクエスト, マージ)
- Concise UI style
- Registered Japanese locale in `apps/frontend/src/shared/i18n/index.ts` and set it as default.
- Verified build with `npm run typecheck`
- Verified frontend with Playwright screenshot.
- Added complete Japanese translation files in `apps/frontend/src/shared/i18n/locales/ja/`. - Configured Japanese (`ja`) as the default language in `apps/frontend/src/shared/i18n/index.ts`. - Verified translations follow concise UI style and use Katakana for technical terms. - Verified Windows build process using `npm run package:win`: - Successfully generated unpacked executable in `apps/frontend/dist/win-unpacked`. - Installer packaging (NSIS) skipped due to missing `wine` in environment, but build configuration is verified correct. - Verified application build with `npm run typecheck` and `npm test`.
…5675166926112915977
|
|
|
Caution Review failedFailed to post review comments 📝 WalkthroughWalkthroughAdds Japanese i18n resources and performs widespread synchronous/simplification refactors across frontend and backend: removes async variants and worktree/PR-worktree managers, replaces git helper wrappers with subprocess calls, consolidates various flows (CLI/tool detection, dotenv loading, Claude CLI paths), and updates many localization, tests, and docs. Changes
Sequence Diagram(s)(Skipped — changes are widespread across many subsystems and do not form a single coherent new sequence worth a single diagram.) Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly Related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 @Utakata, 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 integrates full Japanese localization into the frontend, enabling the application to display its user interface in Japanese. This involves adding numerous new translation files and updating the core internationalization setup to support and utilize these new language resources. 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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🎉 Thanks for your first PR!
A maintainer will review it soon. Please make sure:
- Your branch is synced with
develop - CI checks pass
- You've followed our contribution guide
Welcome to the Auto Claude community!
![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 Japanese localization by adding translation files and configuring i18next. The changes are generally good, but I've identified a critical issue where the default language is hardcoded to Japanese, which would negatively impact non-Japanese users. I've also found a minor translation issue in the GitLab-related strings that could lead to awkward phrasing in the UI. My review includes suggestions to correct both of these points.
| .init({ | ||
| resources, | ||
| lng: 'en', // Default language (will be overridden by settings) | ||
| lng: 'ja', // Default language (will be overridden by settings) |
There was a problem hiding this comment.
The default language lng has been changed to 'ja'. This is likely for testing purposes, but it should be reverted to 'en' before merging. Hardcoding the default language to Japanese will result in a poor user experience for non-Japanese users on their first visit, before any user-specific settings are applied. The default language should be English, which also serves as the fallback.
| lng: 'ja', // Default language (will be overridden by settings) | |
| lng: 'en', // Default language (will be overridden by settings) |
| "newCommits": "件の新しいコミット", | ||
| "newCommitsPlural": "件の新しいコミット", |
There was a problem hiding this comment.
The translation for newCommits and newCommitsPlural is 件の新しいコミット. This phrase is grammatically incomplete in Japanese and translates roughly to "of new commit(s)". Assuming the number of commits is displayed separately (as suggested by the English and French translations), a more appropriate translation for "new commit" / "new commits" would be just "新しいコミット". This would form a more coherent phrase when combined with the count.
"newCommits": "新しいコミット",
"newCommitsPlural": "新しいコミット"
|
日本語を追加希望です。 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @apps/frontend/src/shared/i18n/index.ts:
- Line 85: The default language was changed to 'ja' in the i18n initialization
(the lng option in the i18n.init call) which forces Japanese for all users;
revert lng back to 'en' to restore the prior default, and if you want
locale-based defaults instead, implement a small detection using
navigator.language (e.g., extract the language code, check against a
supportedLanguages array like ['en','fr','ja'], and set lng to the detected code
only if supported, otherwise 'en'); update the lng value used in the i18n.init
configuration accordingly.
In @apps/frontend/src/shared/i18n/locales/ja/gitlab.json:
- Around line 1-208: The en/gitlab.json is missing eight mrReview.severity keys
present in ja/gitlab.json (critical, criticalDesc, high, highDesc, medium,
mediumDesc, low, lowDesc); add these eight keys under the mrReview.severity
object in en/gitlab.json with appropriate English values (e.g., "critical":
"Critical", "criticalDesc": "Requires immediate fix", etc.), and also ensure
fr/gitlab.json contains the same keys (with French translations) so all locales
have the same mrReview.severity set.
In @apps/frontend/src/shared/i18n/locales/ja/taskReview.json:
- Around line 1-7: The Japanese translation file nests keys under "terminal" but
other locales place them at the root; update
apps/frontend/src/shared/i18n/locales/ja/taskReview.json by removing the
"terminal" object and promoting "openTerminal", "openInbuilt", and
"openExternal" to the top-level so the JSON matches the structure used by
en/taskReview.json and fr/taskReview.json.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (11)
apps/frontend/src/shared/i18n/index.tsapps/frontend/src/shared/i18n/locales/ja/common.jsonapps/frontend/src/shared/i18n/locales/ja/dialogs.jsonapps/frontend/src/shared/i18n/locales/ja/gitlab.jsonapps/frontend/src/shared/i18n/locales/ja/navigation.jsonapps/frontend/src/shared/i18n/locales/ja/onboarding.jsonapps/frontend/src/shared/i18n/locales/ja/settings.jsonapps/frontend/src/shared/i18n/locales/ja/taskReview.jsonapps/frontend/src/shared/i18n/locales/ja/tasks.jsonapps/frontend/src/shared/i18n/locales/ja/terminal.jsonapps/frontend/src/shared/i18n/locales/ja/welcome.json
🧰 Additional context used
📓 Path-based instructions (4)
apps/frontend/src/shared/i18n/locales/**/*.json
📄 CodeRabbit inference engine (CLAUDE.md)
apps/frontend/src/shared/i18n/locales/**/*.json: Store translation strings in namespace-organized JSON files atapps/frontend/src/shared/i18n/locales/{lang}/*.jsonfor each supported language
When implementing new frontend features, add translation keys to all language files (minimum: en/.json and fr/.json)
Files:
apps/frontend/src/shared/i18n/locales/ja/taskReview.jsonapps/frontend/src/shared/i18n/locales/ja/onboarding.jsonapps/frontend/src/shared/i18n/locales/ja/dialogs.jsonapps/frontend/src/shared/i18n/locales/ja/settings.jsonapps/frontend/src/shared/i18n/locales/ja/tasks.jsonapps/frontend/src/shared/i18n/locales/ja/navigation.jsonapps/frontend/src/shared/i18n/locales/ja/common.jsonapps/frontend/src/shared/i18n/locales/ja/terminal.jsonapps/frontend/src/shared/i18n/locales/ja/gitlab.jsonapps/frontend/src/shared/i18n/locales/ja/welcome.json
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/shared/i18n/index.ts
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/shared/i18n/index.ts
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/shared/i18n/index.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: MikeeBuilds
Repo: AndyMik90/Auto-Claude PR: 661
File: apps/frontend/src/renderer/components/onboarding/OllamaModelSelector.tsx:176-189
Timestamp: 2026-01-04T23:59:45.209Z
Learning: In the AndyMik90/Auto-Claude repository, pre-existing i18n issues (hardcoded user-facing strings that should be localized) can be deferred to future i18n cleanup passes rather than requiring immediate fixes in PRs that don't introduce new i18n violations.
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/shared/i18n/locales/**/*.json : When implementing new frontend features, add translation keys to all language files (minimum: en/*.json and fr/*.json)
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/shared/i18n/locales/**/*.json : Store translation strings in namespace-organized JSON files at `apps/frontend/src/shared/i18n/locales/{lang}/*.json` for each supported language
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/**/*.{ts,tsx,jsx} : Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/shared/i18n/locales/**/*.json : When implementing new frontend features, add translation keys to all language files (minimum: en/*.json and fr/*.json)
Applied to files:
apps/frontend/src/shared/i18n/locales/ja/taskReview.jsonapps/frontend/src/shared/i18n/locales/ja/onboarding.jsonapps/frontend/src/shared/i18n/locales/ja/dialogs.jsonapps/frontend/src/shared/i18n/locales/ja/settings.jsonapps/frontend/src/shared/i18n/locales/ja/tasks.jsonapps/frontend/src/shared/i18n/locales/ja/navigation.jsonapps/frontend/src/shared/i18n/locales/ja/common.jsonapps/frontend/src/shared/i18n/locales/ja/terminal.jsonapps/frontend/src/shared/i18n/locales/ja/gitlab.jsonapps/frontend/src/shared/i18n/locales/ja/welcome.jsonapps/frontend/src/shared/i18n/index.ts
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/shared/i18n/locales/**/*.json : Store translation strings in namespace-organized JSON files at `apps/frontend/src/shared/i18n/locales/{lang}/*.json` for each supported language
Applied to files:
apps/frontend/src/shared/i18n/locales/ja/onboarding.jsonapps/frontend/src/shared/i18n/locales/ja/dialogs.jsonapps/frontend/src/shared/i18n/locales/ja/settings.jsonapps/frontend/src/shared/i18n/locales/ja/tasks.jsonapps/frontend/src/shared/i18n/locales/ja/navigation.jsonapps/frontend/src/shared/i18n/locales/ja/common.jsonapps/frontend/src/shared/i18n/locales/ja/terminal.jsonapps/frontend/src/shared/i18n/locales/ja/gitlab.jsonapps/frontend/src/shared/i18n/locales/ja/welcome.jsonapps/frontend/src/shared/i18n/index.ts
📚 Learning: 2026-01-04T23:59:45.209Z
Learnt from: MikeeBuilds
Repo: AndyMik90/Auto-Claude PR: 661
File: apps/frontend/src/renderer/components/onboarding/OllamaModelSelector.tsx:176-189
Timestamp: 2026-01-04T23:59:45.209Z
Learning: In the AndyMik90/Auto-Claude repository, pre-existing i18n issues (hardcoded user-facing strings that should be localized) can be deferred to future i18n cleanup passes rather than requiring immediate fixes in PRs that don't introduce new i18n violations.
Applied to files:
apps/frontend/src/shared/i18n/locales/ja/onboarding.jsonapps/frontend/src/shared/i18n/locales/ja/navigation.json
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/**/*.{ts,tsx,jsx} : Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Applied to files:
apps/frontend/src/shared/i18n/locales/ja/common.jsonapps/frontend/src/shared/i18n/index.ts
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/frontend/src/**/*.{ts,tsx} : Use `useTranslation()` hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Applied to files:
apps/frontend/src/shared/i18n/index.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CodeQL (javascript-typescript)
- GitHub Check: CodeQL (python)
🔇 Additional comments (10)
apps/frontend/src/shared/i18n/locales/ja/welcome.json (1)
1-17: English and French translation files already exist with matching keys.The verification confirms that
apps/frontend/src/shared/i18n/locales/en/welcome.jsonandapps/frontend/src/shared/i18n/locales/fr/welcome.jsonboth exist and contain all the same translation keys as the Japanese file, satisfying the coding guideline requirement.apps/frontend/src/shared/i18n/locales/ja/terminal.json (1)
1-36: The English and French translation files already exist with all required keys matching the Japanese file. No action is needed.apps/frontend/src/shared/i18n/locales/ja/tasks.json (1)
1-119: Add missing translation keys to Japanese locale file.The Japanese translation file (
apps/frontend/src/shared/i18n/locales/ja/tasks.json) is missing thenotificationsandwizardkeys that are present in both the English and French files. Per coding guidelines, all language files must contain matching translation keys.apps/frontend/src/shared/i18n/locales/ja/common.json (1)
1-278: Translation files are properly aligned across all supported languages.Verified that
apps/frontend/src/shared/i18n/locales/en/common.jsonandapps/frontend/src/shared/i18n/locales/fr/common.jsoncontain all the same top-level translation keys as the Japanese file, meeting the coding guidelines requirement for multilingual consistency.apps/frontend/src/shared/i18n/index.ts (2)
67-78: Japanese resources block is well-structured.The ja resource block follows the same pattern as en and fr, maintaining consistency across language configurations. The namespace organization matches the expected structure.
28-38: All Japanese translation files are present and correctly imported.Verification confirms that all 10 Japanese translation files referenced in the imports exist:
- ja/common.json ✓
- ja/navigation.json ✓
- ja/settings.json ✓
- ja/tasks.json ✓
- ja/welcome.json ✓
- ja/onboarding.json ✓
- ja/dialogs.json ✓
- ja/gitlab.json ✓
- ja/taskReview.json ✓
- ja/terminal.json ✓
apps/frontend/src/shared/i18n/locales/ja/onboarding.json (1)
1-196: Japanese onboarding translations are comprehensive and consistent.The translation file provides thorough coverage of the onboarding flow with appropriate Japanese localization. The use of Katakana for technical terms follows common Japanese UI conventions. The key structure is fully consistent across all language files—en/onboarding.json, fr/onboarding.json, and ja/onboarding.json all contain the same 134 keys with no gaps or discrepancies.
apps/frontend/src/shared/i18n/locales/ja/navigation.json (1)
1-51: Verify key structure matches en/navigation.json and fr/navigation.jsonThe translation file provides good coverage of navigation UI elements with clean namespace organization. However, the key structure consistency across all supported languages (en, fr, ja) needs to be verified before approval to ensure complete translation coverage.
apps/frontend/src/shared/i18n/locales/ja/dialogs.json (1)
1-137: Translation structure is consistent and complete across all supported languages.Verification confirms that ja/dialogs.json maintains perfect structural consistency with en/dialogs.json and fr/dialogs.json. All top-level sections and nested keys match identically across all three language files, fulfilling the coding guideline requirement to include translation keys in all language files.
apps/frontend/src/shared/i18n/locales/ja/settings.json (1)
1-564: Translation keys are complete and fully consistent with the English version.The verification confirms that all 416 keys present in
en/settings.jsonare also present in this Japanese translation file, with no missing or extraneous keys. The JSON structure is valid, translations use appropriate Japanese (kanji, hiragana, katakana, and technical terms in katakana), and the file is ready for integration.
| { | ||
| "title": "GitLab Issue", | ||
| "states": { | ||
| "opened": "オープン", | ||
| "closed": "クローズ済み" | ||
| }, | ||
| "complexity": { | ||
| "simple": "単純", | ||
| "standard": "標準", | ||
| "complex": "複雑" | ||
| }, | ||
| "header": { | ||
| "open": "オープン", | ||
| "searchPlaceholder": "Issue を検索..." | ||
| }, | ||
| "filters": { | ||
| "opened": "オープン", | ||
| "closed": "クローズ済み", | ||
| "all": "すべて" | ||
| }, | ||
| "empty": { | ||
| "noMatch": "検索に一致する Issue はありません", | ||
| "selectIssue": "詳細を表示するには Issue を選択してください" | ||
| }, | ||
| "notConnected": { | ||
| "title": "GitLab 未接続", | ||
| "description": "Issue を同期するには、プロジェクト設定で GitLab トークンとプロジェクトを設定してください。", | ||
| "openSettings": "設定を開く" | ||
| }, | ||
| "detail": { | ||
| "notes": "ノート", | ||
| "viewTask": "タスクを表示", | ||
| "createTask": "タスクを作成", | ||
| "taskLinked": "タスクがリンクされました", | ||
| "taskId": "タスク ID", | ||
| "description": "説明", | ||
| "noDescription": "説明はありません。", | ||
| "assignees": "担当者", | ||
| "milestone": "マイルストーン" | ||
| }, | ||
| "investigation": { | ||
| "title": "Issue からタスクを作成", | ||
| "issuePrefix": "Issue", | ||
| "description": "この GitLab Issue からタスクを作成します。タスクはカンバンボードのバックログ列に追加されます。", | ||
| "selectNotes": "含めるノートを選択", | ||
| "deselectAll": "すべて選択解除", | ||
| "selectAll": "すべて選択", | ||
| "willInclude": "タスクに含まれる内容:", | ||
| "includeTitle": "Issue のタイトルと説明", | ||
| "includeLink": "GitLab Issue へのリンク", | ||
| "includeLabels": "Issue のラベルとメタデータ", | ||
| "noNotes": "ノートなし (この Issue にはノートがありません)", | ||
| "failedToLoadNotes": "ノートの読み込みに失敗しました", | ||
| "taskCreated": "タスクが作成されました！カンバンボードで確認してください。", | ||
| "creating": "作成中...", | ||
| "cancel": "キャンセル", | ||
| "done": "完了", | ||
| "close": "閉じる" | ||
| }, | ||
| "settings": { | ||
| "enableIssues": "GitLab Issue を有効化", | ||
| "enableIssuesDescription": "GitLab から Issue を同期し、タスクを自動的に作成します", | ||
| "instance": "GitLab インスタンス", | ||
| "instanceDescription": "https://gitlab.com またはセルフホストインスタンスの URL を使用します", | ||
| "connectedVia": "GitLab CLI 経由で接続済み", | ||
| "authenticatedAs": "認証済みユーザー:", | ||
| "useDifferentToken": "別のトークンを使用", | ||
| "authentication": "GitLab 認証", | ||
| "useManualToken": "手動トークンを使用", | ||
| "authenticating": "glab CLI で認証中...", | ||
| "browserWindow": "ログイン用のブラウザウィンドウが開きます。", | ||
| "personalAccessToken": "パーソナルアクセストークン", | ||
| "useOAuth": "OAuth を使用", | ||
| "tokenScope": "トークン作成時のスコープ:", | ||
| "scopeApi": "api", | ||
| "scopeFrom": "スコープ元:", | ||
| "gitlabSettings": "GitLab 設定", | ||
| "project": "プロジェクト", | ||
| "enterManually": "手動入力", | ||
| "loadingProjects": "プロジェクトを読み込み中...", | ||
| "selectProject": "プロジェクトを選択...", | ||
| "searchProjects": "プロジェクトを検索...", | ||
| "noMatchingProjects": "一致するプロジェクトはありません", | ||
| "noProjectsFound": "プロジェクトが見つかりません", | ||
| "selected": "選択済み", | ||
| "projectFormat": "形式:", | ||
| "projectFormatExample": "(例: gitlab-org/gitlab)", | ||
| "connectionStatus": "接続ステータス", | ||
| "checking": "確認中...", | ||
| "connectedTo": "接続先:", | ||
| "notConnected": "未接続", | ||
| "issuesAvailable": "利用可能な Issue", | ||
| "issuesAvailableDescription": "サイドバーから GitLab Issue にアクセスして、Issue の表示、調査、タスク作成を行います。", | ||
| "defaultBranch": "デフォルトブランチ", | ||
| "defaultBranchDescription": "タスクワークツリーを作成するためのベースブランチ", | ||
| "loadingBranches": "ブランチを読み込み中...", | ||
| "autoDetect": "自動検出 (main/master)", | ||
| "searchBranches": "ブランチを検索...", | ||
| "noMatchingBranches": "一致するブランチはありません", | ||
| "noBranchesFound": "ブランチが見つかりません", | ||
| "branchFromNote": "すべての新規タスクは以下からブランチされます:", | ||
| "autoSyncOnLoad": "ロード時に自動同期", | ||
| "autoSyncDescription": "プロジェクトロード時に Issue を自動的に取得します", | ||
| "cli": { | ||
| "required": "GitLab CLI が必要です", | ||
| "notInstalled": "OAuth 認証には GitLab CLI (glab) が必要です。「OAuth を使用」オプションを利用するにはインストールしてください。", | ||
| "installButton": "glab をインストール", | ||
| "installing": "インストール中...", | ||
| "installSuccess": "インストールがターミナルで開始されました。完了したら「更新」をクリックしてください。", | ||
| "refresh": "更新", | ||
| "learnMore": "詳細を見る", | ||
| "installed": "GitLab CLI インストール済み:" | ||
| } | ||
| }, | ||
| "mergeRequests": { | ||
| "title": "GitLab Merge Request", | ||
| "newMR": "新規 Merge Request", | ||
| "selectMR": "詳細を表示するには Merge Request を選択してください", | ||
| "states": { | ||
| "opened": "オープン", | ||
| "closed": "クローズ済み", | ||
| "merged": "マージ済み", | ||
| "locked": "ロック済み" | ||
| }, | ||
| "filters": { | ||
| "opened": "オープン", | ||
| "closed": "クローズ済み", | ||
| "merged": "マージ済み", | ||
| "all": "すべて" | ||
| } | ||
| }, | ||
| "mrReview": { | ||
| "runReview": "AIレビューを実行", | ||
| "reviewing": "レビュー中...", | ||
| "followupReview": "フォローアップレビュー", | ||
| "newCommits": "件の新しいコミット", | ||
| "newCommitsPlural": "件の新しいコミット", | ||
| "cancel": "キャンセル", | ||
| "postFindings": "指摘事項を投稿", | ||
| "posting": "投稿中...", | ||
| "postedTo": "GitLab に投稿しました", | ||
| "approve": "承認", | ||
| "approving": "承認中...", | ||
| "merge": "MRをマージ", | ||
| "merging": "マージ中...", | ||
| "aiReviewResult": "AIレビュー結果", | ||
| "followupReviewResult": "フォローアップレビュー", | ||
| "description": "説明", | ||
| "noDescription": "説明はありません。", | ||
| "labels": "ラベル", | ||
| "status": { | ||
| "notReviewed": "未レビュー", | ||
| "notReviewedDesc": "この MR を分析するために AI レビューを実行します", | ||
| "reviewComplete": "レビュー完了", | ||
| "reviewCompleteDesc": "件の指摘が見つかりました。選択して GitLab に投稿してください。", | ||
| "waitingForChanges": "変更待ち", | ||
| "waitingForChangesDesc": "件の指摘を投稿しました。コントリビューターによる対応待ちです。", | ||
| "readyToMerge": "マージ可能", | ||
| "readyToMergeDesc": "ブロッキングな問題は見つかりませんでした。この MR はマージ可能です。", | ||
| "needsAttention": "要確認", | ||
| "needsAttentionDesc": "件の指摘を GitLab に投稿する必要があります。", | ||
| "readyForFollowup": "フォローアップ可能", | ||
| "readyForFollowupDesc": "レビュー以降。問題が解決されたか確認するためにフォローアップを実行してください。", | ||
| "blockingIssues": "ブロッキングな問題", | ||
| "blockingIssuesDesc": "件のブロッキングな問題が未解決です。" | ||
| }, | ||
| "overallStatus": { | ||
| "approve": "承認", | ||
| "requestChanges": "変更要求", | ||
| "comment": "コメント" | ||
| }, | ||
| "resolution": { | ||
| "resolved": "解決済み", | ||
| "stillOpen": "未解決", | ||
| "newIssue": "新しい問題", | ||
| "newIssues": "新しい問題" | ||
| } | ||
| }, | ||
| "findings": { | ||
| "summary": "選択済み", | ||
| "selectCriticalHigh": "ブロッカー/必須を選択", | ||
| "selectAll": "すべて選択", | ||
| "clear": "クリア", | ||
| "noIssues": "問題は見つかりませんでした！コードは良好です。", | ||
| "suggestedFix": "修正案:", | ||
| "posted": "投稿済み", | ||
| "severity": { | ||
| "critical": "ブロッカー", | ||
| "criticalDesc": "修正必須", | ||
| "high": "必須", | ||
| "highDesc": "修正すべき", | ||
| "medium": "推奨", | ||
| "mediumDesc": "品質向上", | ||
| "low": "提案", | ||
| "lowDesc": "検討事項" | ||
| }, | ||
| "category": { | ||
| "security": "セキュリティ", | ||
| "quality": "品質", | ||
| "style": "スタイル", | ||
| "test": "テスト", | ||
| "docs": "ドキュメント", | ||
| "pattern": "パターン", | ||
| "performance": "パフォーマンス", | ||
| "logic": "ロジック" | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify gitlab.json structure consistency
echo "=== Comparing gitlab.json top-level keys ==="
for lang in en fr ja; do
file="apps/frontend/src/shared/i18n/locales/${lang}/gitlab.json"
if [ -f "$file" ]; then
echo "${lang}/gitlab.json keys:"
jq -r 'keys | .[]' "$file" | sort
echo ""
fi
done
echo "=== Checking gitlab.json nested structure sample ==="
for lang in en fr ja; do
file="apps/frontend/src/shared/i18n/locales/${lang}/gitlab.json"
if [ -f "$file" ]; then
echo "${lang}/gitlab.json nested paths (first 30):"
jq -r 'paths(scalars) | join(".")' "$file" | head -30
echo ""
fi
done
echo "=== Comparing total key count ==="
for lang in en fr ja; do
file="apps/frontend/src/shared/i18n/locales/${lang}/gitlab.json"
if [ -f "$file" ]; then
count=$(jq -r 'paths(scalars) | join(".")' "$file" | wc -l)
echo "${lang}/gitlab.json: $count keys"
fi
doneRepository: AndyMik90/Auto-Claude
Length of output: 2467
🏁 Script executed:
#!/bin/bash
# Compare keys in detail
echo "=== Keys in en but not in fr or ja ==="
jq -r 'paths(scalars) | join(".")' apps/frontend/src/shared/i18n/locales/en/gitlab.json | sort > /tmp/en_keys.txt
jq -r 'paths(scalars) | join(".")' apps/frontend/src/shared/i18n/locales/fr/gitlab.json | sort > /tmp/fr_keys.txt
jq -r 'paths(scalars) | join(".")' apps/frontend/src/shared/i18n/locales/ja/gitlab.json | sort > /tmp/ja_keys.txt
echo "Keys in fr but not in en:"
comm -13 /tmp/en_keys.txt /tmp/fr_keys.txt
echo ""
echo "Keys in en but not in ja:"
comm -23 /tmp/en_keys.txt /tmp/ja_keys.txt
echo ""
echo "Keys in ja but not in en:"
comm -13 /tmp/en_keys.txt /tmp/ja_keys.txtRepository: AndyMik90/Auto-Claude
Length of output: 393
Add missing translation keys to en/gitlab.json for consistency.
The verification reveals an inconsistency: the French translation file contains 8 additional keys in mrReview.severity that are missing from the English file, while the Japanese file aligns perfectly with English (166 keys each). The missing keys in en/gitlab.json are:
mrReview.severity.criticalandmrReview.severity.criticalDescmrReview.severity.highandmrReview.severity.highDescmrReview.severity.mediumandmrReview.severity.mediumDescmrReview.severity.lowandmrReview.severity.lowDesc
These keys exist in the ja/gitlab.json file being reviewed. Add them to en/gitlab.json and fr/gitlab.json to maintain consistency across all supported languages.
🤖 Prompt for AI Agents
In @apps/frontend/src/shared/i18n/locales/ja/gitlab.json around lines 1 - 208,
The en/gitlab.json is missing eight mrReview.severity keys present in
ja/gitlab.json (critical, criticalDesc, high, highDesc, medium, mediumDesc, low,
lowDesc); add these eight keys under the mrReview.severity object in
en/gitlab.json with appropriate English values (e.g., "critical": "Critical",
"criticalDesc": "Requires immediate fix", etc.), and also ensure fr/gitlab.json
contains the same keys (with French translations) so all locales have the same
mrReview.severity set.
| { | ||
| "terminal": { | ||
| "openTerminal": "ターミナルを開く", | ||
| "openInbuilt": "内蔵ターミナルで開く", | ||
| "openExternal": "外部ターミナルで開く" | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that English and French locale files contain matching translation keys
echo "=== Checking English translations ==="
if [ -f "apps/frontend/src/shared/i18n/locales/en/taskReview.json" ]; then
echo "✓ en/taskReview.json exists"
cat apps/frontend/src/shared/i18n/locales/en/taskReview.json | jq '.terminal // "MISSING terminal namespace"'
else
echo "✗ en/taskReview.json NOT FOUND"
fi
echo ""
echo "=== Checking French translations ==="
if [ -f "apps/frontend/src/shared/i18n/locales/fr/taskReview.json" ]; then
echo "✓ fr/taskReview.json exists"
cat apps/frontend/src/shared/i18n/locales/fr/taskReview.json | jq '.terminal // "MISSING terminal namespace"'
else
echo "✗ fr/taskReview.json NOT FOUND"
fi
echo ""
echo "=== Checking Japanese file for reference ==="
if [ -f "apps/frontend/src/shared/i18n/locales/ja/taskReview.json" ]; then
echo "✓ ja/taskReview.json exists"
cat apps/frontend/src/shared/i18n/locales/ja/taskReview.json | jq '.'
else
echo "✗ ja/taskReview.json NOT FOUND"
fiRepository: AndyMik90/Auto-Claude
Length of output: 670
Fix structural inconsistency in translation files.
The English and French translation files structure keys at the root level, but the Japanese file nests them under a terminal namespace. Ensure consistent structure across all language files:
en/taskReview.jsonandfr/taskReview.jsonhave keys at root:{"openTerminal": "...", "openInbuilt": "...", "openExternal": "..."}ja/taskReview.jsonhas keys nested:{"terminal": {"openTerminal": "...", "openInbuilt": "...", "openExternal": "..."}}
Align the Japanese file structure to match the English and French format.
🤖 Prompt for AI Agents
In @apps/frontend/src/shared/i18n/locales/ja/taskReview.json around lines 1 - 7,
The Japanese translation file nests keys under "terminal" but other locales
place them at the root; update
apps/frontend/src/shared/i18n/locales/ja/taskReview.json by removing the
"terminal" object and promoting "openTerminal", "openInbuilt", and
"openExternal" to the top-level so the JSON matches the structure used by
en/taskReview.json and fr/taskReview.json.
- Added Japanese translation files (`ja/*.json`) with concise UI style. - Updated `apps/frontend/src/shared/i18n/index.ts` to detect system locale (fallback to 'en') and force 'en' in test environment. - Verified Windows build process (`npm run package:win`) and output. - Fixed `newCommits` translation in `gitlab.json` to "新しいコミット" as requested. - Verified passing tests for `ModelSearchableSelect` and `ProfileEditDialog`.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| if "/" in upstream: | ||
| return upstream.split("/", 1)[1] | ||
| return upstream | ||
| except subprocess.CalledProcessError: |
Check notice
Code scanning / CodeQL
Empty except Note
| # Try to import tree-sitter - it's optional but recommended | ||
| TREE_SITTER_AVAILABLE = False | ||
| try: | ||
| import tree_sitter # noqa: F401 |
Check notice
Code scanning / CodeQL
Unused import Note
| import tree_sitter_python as tspython | ||
|
|
||
| LANGUAGES_AVAILABLE[".py"] = tspython.language() | ||
| except ImportError: |
Check notice
Code scanning / CodeQL
Empty except Note
|
|
||
| LANGUAGES_AVAILABLE[".js"] = tsjs.language() | ||
| LANGUAGES_AVAILABLE[".jsx"] = tsjs.language() | ||
| except ImportError: |
Check notice
Code scanning / CodeQL
Empty except Note
|
|
||
| LANGUAGES_AVAILABLE[".ts"] = tsts.language_typescript() | ||
| LANGUAGES_AVAILABLE[".tsx"] = tsts.language_tsx() | ||
| except ImportError: |
Check notice
Code scanning / CodeQL
Empty except Note
There was a problem hiding this comment.
Actionable comments posted: 57
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (15)
apps/frontend/src/main/ipc-handlers/settings-handlers.ts (1)
132-146: Update the issue link placeholder.The migration logic is correct and handles cross-platform path cleanup appropriately. However, the issue link in the comment on line 135 contains a placeholder:
// See: https://github.com/AndyMik90/Auto-Claude/issues/XXXConsider updating
XXXwith the actual issue number, or remove the comment if no specific issue exists.Proposed fix
// Migration: Clear CLI tool paths that are from a different platform // Fixes issue where Windows paths persisted on macOS (and vice versa) // when settings were synced/transferred between platforms - // See: https://github.com/AndyMik90/Auto-Claude/issues/XXXapps/frontend/src/main/ipc-handlers/memory-handlers.ts (1)
219-230: Incorrect macOS installation method — macOS should use the official .dmg installer, not curl | sh.The current code treats macOS and Linux identically by using
curl | sh, but the official Ollama documentation specifies different methods:
- macOS: Download the .dmg file, mount it, and drag
Ollama.appto/Applications. This is the official graphical installation method, not the curl script.- Linux: The
curl -fsSL https://ollama.com/install.sh | shmethod is correct.Update the macOS branch to return an appropriate message or method that guides users to download and install the .dmg file. Additionally, per coding guidelines, these hardcoded shell commands are user-facing strings that should use i18n translation keys rather than being hardcoded.
apps/backend/spec/compaction.py (1)
16-87: Consider adding type hints for improved type safety.While the function has proper async patterns and good error handling, adding type hints for the response stream would improve type safety and code clarity per Python best practices:
from typing import AsyncIterator, Any # In the function: async for msg in client.receive_response(): # type: Any if hasattr(msg, "content"): for block in msg.content: if hasattr(block, "text"): response_text += block.textThis is especially important given the relaxed type checking introduced in this change.
Based on coding guidelines emphasizing Python best practices and type hints.
apps/backend/init.py (1)
23-59: Add error handling for file I/O operations.The function performs file read and write operations without error handling. Operations like
read_text()andwrite_text()can raiseIOError,PermissionError, orUnicodeDecodeError. Since the function returns a boolean indicating success, it should handle these exceptions gracefully or document that they may be raised.As per coding guidelines, proper error handling is required for Python backend code.
🛡️ Add error handling
def ensure_gitignore_entry(project_dir: Path, entry: str = ".auto-claude/") -> bool: """ Ensure an entry exists in the project's .gitignore file. Creates .gitignore if it doesn't exist. Args: project_dir: The project root directory entry: The gitignore entry to add (default: ".auto-claude/") Returns: True if entry was added, False if it already existed + + Raises: + IOError: If file operations fail + PermissionError: If insufficient permissions """ gitignore_path = project_dir / ".gitignore" - # Check if .gitignore exists and if entry is already present - if gitignore_path.exists(): - content = gitignore_path.read_text() + try: + # Check if .gitignore exists and if entry is already present + if gitignore_path.exists(): + content = gitignore_path.read_text() + else: + content = "" + except (IOError, PermissionError, UnicodeDecodeError) as e: + raise IOError(f"Failed to read .gitignore at {gitignore_path}: {e}") from e + + if content: lines = content.splitlines() # Check if entry already exists (exact match or with trailing newline variations) entry_normalized = entry.rstrip("/") for line in lines: line_stripped = line.strip() # Match both ".auto-claude" and ".auto-claude/" if ( line_stripped == entry or line_stripped == entry_normalized or line_stripped == entry_normalized + "/" ): return False # Already exists # Entry doesn't exist, append it # Ensure file ends with newline before adding our entry if content and not content.endswith("\n"): content += "\n" # Add a comment and the entry content += "\n# Auto Claude data directory\n" content += entry + "\n" - - gitignore_path.write_text(content) - return True else: # Create new .gitignore with the entry content = "# Auto Claude data directory\n" content += entry + "\n" + + try: + gitignore_path.write_text(content) + except (IOError, PermissionError) as e: + raise IOError(f"Failed to write .gitignore at {gitignore_path}: {e}") from e - gitignore_path.write_text(content) - return True + return Trueapps/frontend/src/main/ipc-handlers/insights-handlers.ts (1)
33-49: Remove unnecessaryasynckeyword and add error handling for pre-try/catch operations.The handler is marked
asyncbut doesn'tawaitanything. SinceipcMain.onis one-way communication, theasynckeyword is misleading and serves no purpose.Additionally, lines 122–140 (session loading/creation and message processing) execute outside the try/catch block in
sendMessageand could throw uncaught errors. While the executor's async work is properly caught (lines 162–188) and error events are forwarded to the renderer (lines 282–287), synchronous errors fromcancelSession,loadSession,createNewSession,generateTitle, orsaveSessionwould not be caught.Remove the
asynckeyword from the handler and consider wrapping pre-try/catch operations or ensuring all upstream methods handle errors gracefully.apps/backend/ollama_model_detector.py (1)
131-146: Consider logging or narrowing the generic exception handler.The bare
except Exception:on line 144 silently swallows all unexpected errors and returnsNone, which could make debugging difficult when unexpected failures occur.♻️ Improve error visibility
Add logging or narrow the exception types:
def fetch_ollama_api(base_url: str, endpoint: str, timeout: int = 5) -> dict | None: """Fetch data from Ollama API.""" url = f"{base_url.rstrip('/')}/{endpoint}" try: req = urllib.request.Request(url) req.add_header("Content-Type", "application/json") with urllib.request.urlopen(req, timeout=timeout) as response: return json.loads(response.read().decode()) except urllib.error.URLError as e: return None except json.JSONDecodeError: return None - except Exception: + except Exception as e: + # Log unexpected errors for debugging + print(f"Unexpected error fetching {url}: {e}", file=sys.stderr) return Noneapps/backend/agents/tools_pkg/tools/memory.py (1)
32-100: Essential refactor: Keep function async when all callers are async.Both call sites for this function are already async functions (
record_discoveryat line 127 andrecord_gotchaat line 198). Converting an async operation into a sync wrapper that internally manages event loops is an anti-pattern when the callers are already in async contexts. This approach:
- Adds unnecessary complexity with event loop detection and fire-and-forget logic
- Makes error handling more difficult (as seen with the unhandled exception issue)
- Violates async/await best practices
- Makes the code harder to understand and maintain
Recommendation: Restore the async version of this function and let the async callers simply
awaitit. If fire-and-forget behavior is desired in specific cases, the caller can useasyncio.create_task()explicitly where needed, making the intent clearer.Based on coding guidelines requiring focus on Python best practices and async patterns.
♻️ Refactor to async pattern
-def _save_to_graphiti_sync( +async def _save_to_graphiti_async( spec_dir: Path, project_dir: Path, save_type: str, data: dict, ) -> bool: """ - Save data to Graphiti/LadybugDB (synchronous wrapper for async operation). + Save data to Graphiti/LadybugDB. Args: spec_dir: Spec directory for GraphitiMemory initialization project_dir: Project root directory save_type: Type of save - 'discovery', 'gotcha', or 'pattern' data: Data to save Returns: True if save succeeded, False otherwise """ try: # Check if Graphiti is enabled from graphiti_config import is_graphiti_enabled if not is_graphiti_enabled(): return False from integrations.graphiti.queries_pkg.graphiti import GraphitiMemory - async def _async_save(): - memory = GraphitiMemory(spec_dir, project_dir) - try: - if save_type == "discovery": - # Save as codebase discovery - # Format: {file_path: description} - result = await memory.save_codebase_discoveries( - {data["file_path"]: data["description"]} - ) - elif save_type == "gotcha": - # Save as gotcha - gotcha_text = data["gotcha"] - if data.get("context"): - gotcha_text += f" (Context: {data['context']})" - result = await memory.save_gotcha(gotcha_text) - elif save_type == "pattern": - # Save as pattern - result = await memory.save_pattern(data["pattern"]) - else: - result = False - return result - finally: - await memory.close() - - # Run async operation in event loop - try: - asyncio.get_running_loop() - # If we're already in an async context, schedule the task - # Don't block - just fire and forget for the Graphiti save - # The file-based save is the primary, Graphiti is supplementary - asyncio.ensure_future(_async_save()) - return False # Can't confirm async success, file-based is source of truth - except RuntimeError: - # No running loop, create one - return asyncio.run(_async_save()) + memory = GraphitiMemory(spec_dir, project_dir) + try: + if save_type == "discovery": + # Save as codebase discovery + # Format: {file_path: description} + result = await memory.save_codebase_discoveries( + {data["file_path"]: data["description"]} + ) + elif save_type == "gotcha": + # Save as gotcha + gotcha_text = data["gotcha"] + if data.get("context"): + gotcha_text += f" (Context: {data['context']})" + result = await memory.save_gotcha(gotcha_text) + elif save_type == "pattern": + # Save as pattern + result = await memory.save_pattern(data["pattern"]) + else: + result = False + return result + finally: + await memory.close() except ImportError as e: logger.debug(f"Graphiti not available for memory tools: {e}") return False except Exception as e: logger.warning(f"Failed to save to Graphiti: {e}") return FalseThen update the call sites:
# SECONDARY: Also save to Graphiti/LadybugDB (for Memory UI) - saved_to_graphiti = _save_to_graphiti_sync( + saved_to_graphiti = await _save_to_graphiti_async( spec_dir, project_dir, "discovery",And similarly for line 226 in
record_gotcha.apps/frontend/src/main/claude-profile/profile-storage.ts (1)
36-73: Verify that synchronous I/O is acceptable in the main process.The change from async to synchronous file operations (
readFileSync/writeFileSync) will block the Electron main process during I/O. While profile data files are likely small, this pattern can cause UI freezes if:
- Files grow larger over time
- Disk I/O is slow (network drives, older hardware)
- Multiple rapid saves occur
Please confirm that blocking I/O is acceptable for this use case, or consider whether the async pattern should be retained for better responsiveness.
Based on learnings: The codebase has synchronous and asynchronous patterns; ensure this aligns with the project's architectural direction for main process I/O.
apps/backend/runners/github/services/sdk_utils.py (1)
238-250: Fix inconsistent block type checking in text collection.Lines 201 and 283 use
block_type = type(block).__name__to check ToolUseBlock and ToolResultBlock types strictly, but line 239 useshasattr(block, "text")without type verification. While this works in practice (only TextBlock has a text attribute), it breaks the consistency pattern established within the same function. Change line 239 to:if block_type == "TextBlock" and hasattr(block, "text"):or keep
block_typefrom line 201's context and reuse it here to maintain the established type-checking pattern.apps/backend/core/workspace.py (1)
772-799: Text-based copy assumes decoded content and ignoresgit addfailuresBoth the “new files” copy path and the “simple copy” path now:
- Assume
_get_file_content_from_ref(...)returns decoded text suitable forPath.write_text(...).- Invoke
git addand ignore the return code.If
_get_file_content_from_refever returns bytes (e.g., for misclassified/binary files),write_textwill raise; and ifgit addfails, these files are still added toresolved_files, which can misreport merge success.Consider:
- Ensuring
_get_file_content_from_refeither always returnsstrfor text files orNonefor anything else, and documenting that contract here.- At least logging non‑zero
git addexit codes so failed staging doesn’t silently look like a resolved file.Example: log `git add` failures
- target_path.write_text(content, encoding="utf-8") - subprocess.run( - ["git", "add", target_file_path], - cwd=project_dir, - capture_output=True, - ) + target_path.write_text(content, encoding="utf-8") + add_result = subprocess.run( + ["git", "add", target_file_path], + cwd=project_dir, + capture_output=True, + text=True, + ) + if add_result.returncode != 0: + debug_warning( + MODULE, + f"git add failed for {target_file_path}: {add_result.stderr.strip()}", + ) + continueAlso applies to: 1120-1135
apps/backend/runners/github/services/parallel_followup_reviewer.py (1)
149-223: Worktree creation/cleanup flow is solid; consider minor diagnosticsThe new
_create_pr_worktree/_cleanup_pr_worktreehelpers:
- Correctly validate
head_shavia_validate_git_refbefore using it ingitcommands.- Use argument arrays for
subprocess.run, avoiding shell injection.- Provide a reasonable cleanup fallback chain (
git worktree remove→shutil.rmtree+git worktree prune).Two small robustness improvements you might consider:
- If
git fetch origin <head_sha>fails (non‑zeroreturncode), log that explicitly before attemptinggit worktree addso diagnosing fork/remote issues is easier.- In
_cleanup_pr_worktree, when the path is missing you currently return early and never rungit worktree prune; optionally pruning even in that case would clear stale worktree metadata.Also applies to: 224-263
apps/backend/runners/github/services/parallel_orchestrator_reviewer.py (1)
1019-1053: Parallel orchestrator verdict generation is intentionally simplified but lacks critical blocking logic from main orchestrator.The
_generate_verdictmethod in parallel orchestrator only considers finding severity, intentionally removing merge conflict and CI checks. However:
CI status is never checked: Main orchestrator receives and validates
ci_status(failed checks, awaiting approval), but parallel orchestrator has no access toget_pr_checks_comprehensive()results. PRContext doesn't include ci_status, and parallel review returns frompr_review_enginewithout checking CI afterward.Verification failures and redundancy issues are not detected: Main orchestrator explicitly flags
VERIFICATION_FAILEDand high/criticalREDUNDANCYcategories as blockers. The parallel orchestrator agents are not prompted to categorize findings this way, only severity-based blocking.Structural issues and AI triages are excluded: These additional blocker types exist in main orchestrator but are absent from parallel verdict logic.
For initial reviews using
use_parallel_orchestrator=true, these checks are completely bypassed. The follow-up review path has a fallback CI override (lines 758–778 of orchestrator.py), but initial parallel reviews lack this safety mechanism.Consider: Either pass
ci_statusto parallel orchestrator verdict generation, or document that parallel reviews intentionally skip these checks and recommend the multi-pass flow for production use.apps/backend/runners/spec_runner.py (1)
84-92: Remove invalid fallback path and add warning when no environment file is loaded.The
dev_env_filepath (apps/dev/auto-claude/.env) does not exist in the repository and will never be loaded. Additionally, if neither.envfile exists, the code silently continues without warning—this violates the project's error handling guidelines and can lead to confusing runtime errors when required environment variables are missing (e.g.,CLAUDE_CODE_OAUTH_TOKEN).Recommended changes:
- Remove the unreachable fallback path since
apps/dev/auto-claude/directory does not exist- Add error logging when no environment file is found to aid troubleshooting
💡 Suggested refactor
# Load .env file from dotenv import load_dotenv env_file = Path(__file__).parent.parent / ".env" -dev_env_file = Path(__file__).parent.parent.parent / "dev" / "auto-claude" / ".env" if env_file.exists(): load_dotenv(env_file) -elif dev_env_file.exists(): - load_dotenv(dev_env_file) +else: + import sys + print(f"Warning: No .env file found at {env_file}", file=sys.stderr) + print("Required environment variables may be missing. See .env.example for configuration.", file=sys.stderr)apps/frontend/src/main/utils/windows-paths.ts (2)
82-115: Consider the accumulated latency of synchronous file system checks.While individual
existsSynccalls are typically fast, iterating through multiple path patterns with synchronous file system checks can accumulate latency, especially on slower storage or when antivirus software is active.For consistency with Electron best practices and to avoid any potential UI freezing, consider whether an async implementation would be more appropriate here as well.
168-171: Consider logging errors for debugging purposes.The empty catch block silently swallows all errors, including timeouts, permission issues, or other unexpected failures. While returning
nullis correct for "not found" cases, logging the actual error would aid debugging.🔍 Proposed addition for better observability
} catch (error) { // 'where' returns exit code 1 if not found, which throws an error + // Log other errors for debugging (timeouts, permissions, etc.) + if (error instanceof Error && !error.message.includes('Command failed')) { + console.warn(`${logPrefix} Error executing where: ${error.message}`); + } return null; }
🤖 Fix all issues with AI agents
In @apps/backend/agents/session.py:
- Around line 448-449: The change removed the intentional error-guard around
blocked-command detection causing false positives; restore the original
conditional so the blocked check only runs when is_error is true (e.g., change
the condition back to require is_error and "blocked" in
str(result_content).lower()), and ensure the control flow still uses the blocked
check before the subsequent elif is_error branch (update the conditional
surrounding result_content handling in session.py where is_error and
result_content are evaluated).
In @apps/backend/agents/tools_pkg/tools/memory.py:
- Around line 84-93: The fire-and-forget scheduling of _async_save via
asyncio.ensure_future leaves exceptions uncaught; modify the scheduling path so
the created Task from asyncio.ensure_future(_async_save()) is assigned to a
variable and given a done callback that logs or handles exceptions (inspect
task.exception() and log them using the module's logger or processLogger) to
avoid silent failures; ensure you still return False (file save is source of
truth) but attach the callback to the Task to capture and report any exceptions
from _async_save when run in an existing event loop.
- Line 59: The internal coroutine _async_save should include an explicit return
type annotation; update its definition to annotate it as async def _async_save()
-> bool and ensure any return paths inside _async_save return boolean values to
match the outer function's return type and typing expectations.
In @apps/backend/analysis/insight_extractor.py:
- Around line 425-428: The reconstructed code-block content assigned to variable
text is not trimmed, which can leave extra whitespace/newlines that break
downstream JSON parsing; after joining lines (variable lines), apply a final
strip() to text (i.e., set text = "\n".join(lines).strip()) so the extracted
code/JSON has no leading/trailing whitespace before parsing, while keeping the
preceding logic that removes the trailing "```" line intact.
- Around line 394-395: Replace the fragile hasattr(block, "text") check with an
explicit block type check used elsewhere: first obtain block_type (e.g., from
block.type or the same property used in surrounding code) and only append
block.text when block_type == "TextBlock" and hasattr(block, "text") to ensure
you only process text blocks; also change any backend client construction that
calls create_simple_client() to use create_client() from
apps/backend/core/client.py (update imports and the call site) so the extractor
uses the standard SDK client creation pattern.
In @apps/backend/commit_message.py:
- Around line 234-235: The code appends block.text when hasattr(block, "text")
but other files use an explicit type check; update the conditional in the loop
that builds response_text to require block_type == "TextBlock" in addition to
hasattr(block, "text"), i.e., change the if that references block, block_type,
and response_text so it only concatenates when the block is a TextBlock and has
a text attribute.
In @apps/backend/core/workspace/git_utils.py:
- Around line 242-247: The subprocess.run call that executes ["git", "show",
f"{ref}:{file_path}"] and assigns to result should explicitly set
encoding="utf-8" and errors="replace" alongside cwd=project_dir,
capture_output=True, text=True to ensure non-ASCII file contents are decoded
consistently; update the subprocess.run invocation in the git_utils.py code path
(the line creating result from subprocess.run) to include those two parameters.
- Around line 271-276: The subprocess.run call that sets result in git_utils.py
currently uses capture_output=True and text=True but omits explicit encoding;
update the subprocess.run invocation (the one that executes ["git", "diff",
"--name-status", f"{base_branch}...{spec_branch}"] and assigns to result) to
include encoding="utf-8" and errors="replace" so output is decoded consistently
and malformed bytes are handled safely.
- Around line 494-499: In create_conflict_file_with_git update the
subprocess.run call that invokes ["git", "merge-file", "-p", main_path,
base_path, wt_path] to include encoding="utf-8" and errors="replace" so its
signature matches other git invocations; locate the subprocess.run invocation
storing its result in the variable result and add those two keyword args to
ensure consistent text decoding and error handling.
- Around line 205-211: The subprocess.run call that determines the current
branch (the call returning result and using result.stdout.strip()) must include
encoding="utf-8" and errors="replace" to match the pattern in worktree.py;
update that subprocess.run invocation to pass encoding="utf-8" and
errors="replace" so stdout is decoded consistently and invalid bytes are
replaced.
- Around line 194-200: The subprocess.run call that checks git status should
include encoding="utf-8" and errors="replace" (matching the usage in
worktree.py) to ensure non-ASCII git output is handled; update the
subprocess.run invocation that produces the variable result in git_utils.py (the
git status --porcelain call) to add those two keyword args so result.stdout is
reliably decoded.
In @apps/backend/init.py:
- Around line 30-40: Replace the multi-branch comparison with a single
normalized equality check: normalize both the candidate line and the target
entry by stripping trailing slashes (e.g., use line_stripped.rstrip("/") and
entry.rstrip("/")) and return False if they are equal; this removes
entry_normalized and the separate comparisons while preserving behavior for
".auto-claude" vs ".auto-claude/" matches.
In @apps/backend/integrations/graphiti/config.py:
- Around line 625-627: get_graphiti_status currently sets status["available"]
solely via config.is_valid(), which no longer verifies that required packages
are installed; restore the dependency import check so available reflects real
install status. Update get_graphiti_status (or add a small helper called e.g.
_graphiti_dependencies_installed) to attempt importing the runtime dependencies
mentioned in the docstring (for example graphiti_core, real_ladybug, etc.),
catch ImportError/Exception, and set status["available"] = config.is_valid() and
that import check; include the import error message in status or a log entry so
missing dependencies are observable.
In @apps/backend/integrations/graphiti/queries_pkg/client.py:
- Around line 37-38: Replace the silent except ImportError: pass around the
real_ladybug import with explicit logging: ensure a logger (e.g., logger =
logging.getLogger(__name__)) is available in client.py and change the handler to
except ImportError as e: logger.debug("Failed to import real_ladybug; falling
back to kuzu: %s", e) (or logger.exception if you prefer stack traces) so the
ImportError details are recorded while still allowing the fallback to native
kuzu to proceed.
In @apps/backend/merge/ai_resolver/claude_client.py:
- Around line 85-86: Replace the loose hasattr check when concatenating
block.text with an explicit TextBlock-type guard: determine the block's type (as
other modules do) and only append to response_text when the block is a TextBlock
and has a text attribute (i.e., use the same pattern as in
apps/backend/task_logger/capture.py, apps/backend/spec/pipeline/agent_runner.py,
and apps/backend/agents/session.py) so ThinkingBlock or ToolUseBlock with text
fields are excluded; update the logic around the block iteration in
claude_client.py to perform the block_type == "TextBlock" check before using
block.text.
In @apps/backend/merge/file_evolution/modification_tracker.py:
- Around line 274-289: The empty except around the subprocess.run that invokes
["git", "rev-parse", "--abbrev-ref", "--symbolic-full-name", "@{u}"] is
unnecessary because the code already checks result.returncode and result.stdout;
remove the try/except (delete the try: and the except
subprocess.CalledProcessError: pass) and unindent the subprocess.run block so
unexpected exceptions will propagate, leaving the existing returncode/stdout
handling for normal command failures (references: subprocess.run(...) call,
upstream variable).
In @apps/backend/merge/semantic_analysis/js_analyzer.py:
- Around line 65-68: The comment about arrow functions is ambiguous: update the
comment where child.type == "arrow_function" to state explicitly that the
analyzer only captures arrow functions assigned to named variables (via variable
declarations) and thus intentionally ignores standalone or anonymous arrow
functions (e.g., IIFEs or arrow functions passed as args); mention this
limitation and rationale so future readers know this is by design and not a bug
(refer to the "arrow_function" branch handling and the variable-declaration
extraction logic that captures assigned arrow functions).
- Around line 11-14: The import fallback sets Node = None which breaks
annotations and runtime usage; update js_analyzer.py so any function/method
signatures that currently type-annotate or require Node accept Optional[Node]
(or remove the annotation) and add a runtime guard before using Node-backed
APIs: check if Node is None and raise a clear ImportError or skip
tree-sitter-dependent code paths. Ensure you import typing.Optional if used and
update all references to Node in function signatures and internal
isinstance/type checks to handle the None case.
In @apps/backend/merge/semantic_analysis/python_analyzer.py:
- Around line 67-79: The analyzer currently only recurses into class bodies so
nested functions inside other functions are missed; update the function handling
for nodes with type "function_definition" to also recurse into their body using
the full_name as the new parent. After creating the ExtractedElement for the
current function (see function_definition, full_name, ExtractedElement),
traverse the function's body node (or its child nodes) and call the same
extraction routine/logic with parent=full_name so inner function_definition
nodes are discovered and added with names like "function:parent.inner". Ensure
the recursive call preserves existing metadata extraction (start_line, end_line,
content) for nested functions.
- Around line 99-104: The recursive call inside the decorated_definition branch
is passing the wrong node: change the call in the block handling decorated
functions/classes so that extract_python_elements is invoked with sub (the inner
function/class node) instead of child, i.e. call extract_python_elements(sub,
elements, get_text, get_line, parent) to correctly recurse into the
function_definition or class_definition node.
In @apps/backend/merge/semantic_analyzer.py:
- Around line 266-272: get_line currently treats the tree-sitter byte offset as
a character index by slicing the Unicode string `source[:byte_pos]`; for
multi-byte UTF-8 this yields wrong line numbers. Change get_line to operate on
the raw bytes (use `source_bytes[:byte_pos]`) and count b'\n' (bytes) to compute
the 1-indexed line number, ensuring you use the tree-sitter byte offset directly
and guard against out-of-range byte_pos if necessary.
- Around line 72-93: The empty except blocks around the Tree-sitter imports are
swallowing ImportError details; update the three except ImportError blocks that
follow the imports for tree_sitter_python, tree_sitter_javascript, and
tree_sitter_typescript to log a debug-level message (via the module logger or
existing logging facility) including the exception info and which language
extension(s) were not registered in LANGUAGES_AVAILABLE, so failures are visible
while preserving graceful degradation.
- Around line 204-254: The loop in _analyze_with_tree_sitter unconditionally
adds change.line_end - change.line_start + 1 to
FileAnalysis.total_lines_changed, which can go negative if a SemanticChange has
inverted ranges; validate or clamp the range before adding (e.g., compute delta
= max(0, change.line_end - change.line_start + 1) or skip the change if line_end
< line_start) and then add that non-negative delta to
analysis.total_lines_changed; update references to ChangeType handling remain
the same while applying this range check for each change.
- Around line 56-68: Remove the unused top-level import "import tree_sitter"
(and its trailing noqa comment) in the try block so only the explicit imports
"from tree_sitter import Language, Node, Parser, Tree" remain; keep the rest of
the logic (setting TREE_SITTER_AVAILABLE, logger.info, and the except block
assigning Tree/Node = None) unchanged to preserve the availability check and
fallback behavior.
In @apps/backend/runners/github/services/parallel_orchestrator_reviewer.py:
- Line 148: The worktree name currently uses only 8 hex chars from uuid.uuid4()
(worktree_name = f"pr-{pr_number}-{uuid.uuid4().hex[:8]}"), which is low entropy
and risks collisions; modify that expression to slice 12 hex chars (e.g.,
uuid.uuid4().hex[:12]) so worktree_name gains 48 bits of uniqueness, leaving the
rest of the logic unchanged.
- Around line 286-292: The subprocess.run call to ["git", "worktree", "list",
"--porcelain"] in the stale worktree cleanup should have error handling: wrap
the call in try/except for subprocess.SubprocessError (and handle
TimeoutExpired), check result.returncode and only parse result.stdout into the
registered set if returncode == 0; on any error log the failure (including
result.stderr or exception) and abort/skip the cleanup to avoid removing valid
worktrees. Update the code around the subprocess.run invocation (the call that
uses self.project_dir and populates registered) to implement these checks and
early return on failure.
- Line 233: The conditional currently uses truthiness on the Path object (if not
worktree_path or not worktree_path.exists()) which conflates None with falsy
Path values; change it to an explicit None check by replacing the condition with
a check like "if worktree_path is None or not worktree_path.exists()" so that
the variable worktree_path is only treated as missing when actually None, and
still verify existence via worktree_path.exists().
- Line 268: Replace the silent deletion call shutil.rmtree(worktree_path,
ignore_errors=True) with explicit error handling: call
shutil.rmtree(worktree_path) inside a try/except that catches Exception as e and
logs the failure (including worktree_path and exception details) via the
module/logger used in this file (e.g., logger.error or process_logger.error) so
cleanup failures (permission, disk, etc.) are visible for troubleshooting; keep
the removal attempt but ensure errors are logged and optionally
handled/re-raised as appropriate for the surrounding function.
In @apps/backend/runners/github/services/review_tools.py:
- Around line 143-144: The block text extraction uses only hasattr(block,
"text") which can include non-text blocks; update the three functions
spawn_security_review, spawn_quality_review, and spawn_deep_analysis to
explicitly check the block type before appending text by using the same pattern
as the codebase: ensure you test block_type == "TextBlock" and hasattr(block,
"text") when iterating blocks (the variable names block and block_type in those
functions) so only TextBlock contents are added before passing to
_parse_findings_from_response().
In @apps/backend/runners/gitlab/services/mr_review_engine.py:
- Around line 237-238: The loop that concatenates text uses hasattr(block,
"text") without type hints or explicit block type checks; update the loop in
mr_review_engine (the code handling msg/content and building result_text) to add
type hints for msg and block, determine block_type = type(block).__name__, and
only access block.text when block_type == "TextBlock" and hasattr(block,
"text"); mirror the pattern used in commit_message.py and pr_review_engine.py so
the code defensively checks msg_type == "AssistantMessage" and block_type ==
"TextBlock" before appending to result_text.
In @apps/backend/spec/compaction.py:
- Around line 76-79: The current loop appends content blocks without checking
the message type; change it to follow the established pattern by computing
msg_type = type(msg).__name__ and only proceed if msg_type == "AssistantMessage"
and hasattr(msg, "content"), then iterate msg.content and append block.text when
hasattr(block, "text") to response_text; update the block that references msg
and response_text accordingly so behavior matches commit_message.py and other
modules.
In @apps/backend/task_logger/capture.py:
- Around line 95-96: The code uses hard-coded magic numbers for truncation (50
and 47) when shortening fp; introduce module-level constants (e.g.,
MAX_TOOL_INPUT_DISPLAY_LENGTH = 50 and TRUNCATED_DISPLAY_LENGTH = 47) near the
top of the file and replace the literal checks and slicing in the truncation
logic (the block using fp and the other occurrences at the indicated locations)
to reference these constants so the thresholds are centralized and clear.
- Around line 95-104: The branch handling "path" in capture.py should apply the
same truncation used for file_path: inspect the variable inp["path"] (used in
the elif "path" branch) and if its length exceeds 50 characters set tool_input
to "..." + the last 47 characters, rather than the full path; keep the existing
truncation behavior for fp/file_path and the command truncation logic intact so
presentation is consistent across file-like inputs.
In @apps/frontend/scripts/download-python.cjs:
- Around line 705-708: Before returning success after validating the Python
binary (the early return that returns { success: true, pythonPath: pythonBin,
sitePackagesPath: sitePackagesDir }), add a defensive check that the
sitePackagesDir actually exists and is a directory (e.g., using fs.existsSync or
fs.statSync). If the directory is missing or invalid, treat it like a broken
installation: log a message, remove platformDir (same cleanup as in the catch
block), and continue with re-download/installation flow instead of returning
success with a bad sitePackagesPath.
In @apps/frontend/src/main/agent/agent-queue.ts:
- Around line 208-210: Duplicate cwd resolution appears in spawnIdeationProcess
and spawnRoadmapProcess; extract it into a private helper (e.g., getProcessCwd)
that calls this.processManager.getAutoBuildSourcePath() and returns
autoBuildSource || process.cwd(), then replace the inline cwd assignments in
both spawnIdeationProcess and spawnRoadmapProcess with a call to
getProcessCwd().
In @apps/frontend/src/main/claude-profile/profile-storage.ts:
- Around line 68-70: The catch block in ProfileStorage.loadProfiles logs errors
without the storePath context; update the catch to include the storePath value
in the console.error call (e.g., include `storePath` alongside the error) so
logs show which profile store failed and optionally include error.message or the
error object for full details.
In @apps/frontend/src/main/ipc-handlers/agent-events-handlers.ts:
- Around line 166-180: The current execution-progress handler repeatedly scans
all projects and their tasks (using projectStore.getProjects and
projectStore.getTasks) to find a task by taskId and persist via getPlanPath +
persistPlanStatusSync; add a simple taskId→project (or projectId) cache that is
updated whenever projects or tasks change and use that cache to directly locate
the owning project for a given taskId, falling back to the existing loop only on
cache miss and updating the cache when you find the mapping; ensure cache
invalidation on project/task add/remove/update so getPlanPath and
persistPlanStatusSync receive the correct project and task.
In @apps/frontend/src/main/ipc-handlers/roadmap/transformers.ts:
- Line 110: The transformer is using unsafe casts like (raw.status as
RoadmapFeature['status']) which skip runtime validation; add runtime type-guard
functions (e.g., isValidFeatureStatus, isValidPhaseStatus, isValidPriority,
isValidComplexity, isValidImpact) that check the value against the allowed
string unions and use them when assigning fields in the transformer (replace
casts at RoadmapPhase status, RoadmapFeature priority/complexity/impact,
RoadmapFeature status, and Roadmap status); if a guard returns false, assign the
appropriate safe default (e.g., 'under_review' for feature status) or handle the
error, so only validated values flow out of the transformer.
In @apps/frontend/src/main/ipc-handlers/terminal-handlers.ts:
- Around line 54-59: The ipcMain.on handler for
IPC_CHANNELS.TERMINAL_INVOKE_CLAUDE calls terminalManager.invokeClaude in a
fire-and-forget way and currently allows synchronous throws or rejected promises
to go unhandled; wrap the invocation in a defensive try-catch and also attach a
.catch() if invokeClaude returns a promise so both sync exceptions and async
rejections are logged (use the app logger or console.error) to prevent crashing
the main process and to surface errors.
- Around line 635-640: The ipcMain handler for
IPC_CHANNELS.TERMINAL_RESUME_CLAUDE is a fire-and-forget that calls
terminalManager.resumeClaude(id, sessionId) without any error handling; wrap the
body of the ipcMain.on callback in a try/catch around
terminalManager.resumeClaude and handle failures by logging the error (e.g.,
processLogger.error or similar) and/or sending an IPC error response back to the
renderer (using the event or a reply channel), so synchronous throws from
resumeClaude are caught and surfaced instead of crashing the handler.
In @apps/frontend/src/main/project-store.ts:
- Around line 363-383: Replace the hardcoded startsWith('investigate') check
with a case-insensitive regex that matches a set of investigative prefixes
(e.g., investigate|analyze|research|study|review) applied to firstLine (use
something like /^ *(investigate|analyze|research|study|review)\b/i); keep the
existing split-on-paragraphs logic (sections = taskDesc.split('\n\n')) and
iterate as before to pick the first meaningful paragraph into description, and
change the includes('\n\n') guard to a more robust check like sections.length >
1 so other formatting variants still trigger paragraph extraction.
In @apps/frontend/src/main/python-env-manager.ts:
- Around line 132-139: The fallback check in PythonEnvManager
(hasBundledPackages) is too permissive: it uses existsSync(claudeSdkPath) ||
existsSync(dotenvPath) so bundled mode may be chosen when only one package is
present; change the condition to require both packages (use logical AND) so it
returns true only when both claude_agent_sdk and dotenv exist, and update the
log message to reflect that both key packages were found; ensure you reference
claudeSdkPath and dotenvPath in the revised check and keep the early return
behavior intact.
In @apps/frontend/src/main/task-log-service.ts:
- Around line 251-252: The polling loop in startWatching closes over the initial
worktreeSpecDir so newly created worktrees are never discovered; update
startWatching so each poll re-computes/discovers the current worktreeSpecDir
(don’t rely on the captured worktreeSpecDir variable) and pass the freshly
discovered specDir into loadLogs, or alternatively add a re-discovery step that
refreshes watchedPaths before each loadLogs call; specifically modify the
closure that references worktreeSpecDir, the poll loop in startWatching, and the
loadLogs invocation so they use a newly-resolved worktree spec directory (or
make loadLogs accept a resolved path argument) to ensure dynamically created
worktrees are detected.
In @apps/frontend/src/main/terminal/claude-integration-handler.ts:
- Around line 286-299: Duplicate post-invocation logic that updates
terminal.title, sends IPC via IPC_CHANNELS.TERMINAL_TITLE_CHANGE, calls
SessionHandler.persistSession, and invokes onSessionCapture should be extracted
into a single helper (e.g., finalizeInvoke) to avoid repetition across the three
code paths (blocks around lines showing activeProfile.name usage and other
branches). Implement finalizeInvoke(terminal: TerminalProcess, profileName?:
string, getWindow: WindowGetter, projectPath?: string, onSessionCapture:
(id:string, path:string, start:number)=>void, startTime:number) and replace the
repeated sequences with a single call; ensure the helper sets terminal.title
(use profileName when present), obtains win via getWindow() and sends
win.webContents.send(..., terminal.id, title), calls
SessionHandler.persistSession(terminal) when terminal.projectPath exists, and
calls onSessionCapture(terminal.id, projectPath, startTime) when projectPath is
provided.
In @apps/frontend/src/main/utils/windows-paths.ts:
- Line 12: The code currently uses the synchronous execFileSync which blocks the
Electron main process; restore and use the async implementations
getWindowsExecutablePathsAsync and findWindowsExecutableViaWhereAsync (which
should internally use child_process.execFile or execFilePromise instead of
execFileSync) and update all call sites to use them asynchronously —
specifically replace sync calls in cli-tool-manager.ts by invoking
detectGitAsync() (or the async equivalents) so detection awaits the async path
resolution; ensure exported function names are getWindowsExecutablePathsAsync
and findWindowsExecutableViaWhereAsync and that callers are updated to await
these promises.
In @tests/test_github_pr_review.py:
- Around line 104-110: The test uses the deprecated
asyncio.get_event_loop().run_until_complete pattern in test_save_and_load (and
the two other similar tests) — replace it by either making the test an async
test with async def and decorating it with @pytest.mark.asyncio and awaiting
sample_review_result.save(temp_github_dir), or keep it sync but call
asyncio.run(sample_review_result.save(temp_github_dir)) instead; update the
three occurrences (including the blocks around test_save_and_load, the block at
340-347, and 469-481) to use the chosen modern approach and remove
get_event_loop().run_until_complete to avoid deprecated APIs and align with
pytest async best practices.
- Around line 104-110: The test test_save_and_load uses deprecated
asyncio.get_event_loop().run_until_complete; update it to use
asyncio.run(sample_review_result.save(temp_github_dir)) or convert the test to
an async test decorated with pytest.mark.asyncio and await
sample_review_result.save(temp_github_dir) directly; modify the call site in
test_save_and_load to call asyncio.run(...) or make the test async and await
sample_review_result.save(...) so it is compatible with Python 3.12+.
- Around line 340-347: The test test_skip_returns_existing_review uses the
deprecated asyncio.get_event_loop().run_until_complete pattern; update it to use
a modern alternative such as calling
asyncio.run(sample_review_result.save(temp_github_dir)) or convert the test to
an async test using the @pytest.mark.asyncio decorator and await
sample_review_result.save(temp_github_dir) instead, replacing the get_event_loop
call in this and other similar tests.
- Around line 469-481: The test_posted_findings_serialization uses the
deprecated asyncio.get_event_loop() pattern; convert this test into an async
test using pytest-asyncio: add @pytest.mark.asyncio to the test, change its
signature to async def test_posted_findings_serialization(...), and replace
asyncio.get_event_loop().run_until_complete(sample_review_result.save(...)) with
await sample_review_result.save(temp_github_dir); ensure pytest is imported at
the top if not already and apply the same pattern consistently to the other
async tests in the file.
| # Check if command was blocked by security hook | ||
| if "blocked" in str(result_content).lower(): |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, check the current state of the file at lines 448-449
echo "=== Current state of lines 440-460 in session.py ==="
if [ -f "apps/backend/agents/session.py" ]; then
sed -n '440,460p' apps/backend/agents/session.py
else
echo "File not found at expected path"
fi
# Check git status and see recent changes
echo -e "\n=== Git log for session.py (recent commits) ==="
git log --oneline -5 -- apps/backend/agents/session.py 2>/dev/null || echo "Git history unavailable"
# Try to see the git diff for this file
echo -e "\n=== Git diff for session.py ==="
git diff HEAD -- apps/backend/agents/session.py 2>/dev/null | head -100 || echo "Git diff unavailable"
# Check for Japanese localization files in the codebase
echo -e "\n=== Looking for localization/Japanese files ==="
find . -type f \( -name "*ja*" -o -name "*japanese*" -o -name "*locali*" \) 2>/dev/null | head -20
# Check if there's a package.json or i18n configuration
echo -e "\n=== Checking for i18n/locale configuration files ==="
find . -type f -name "*.json" 2>/dev/null | xargs grep -l "ja" 2>/dev/null | head -10Repository: AndyMik90/Auto-Claude
Length of output: 1606
🏁 Script executed:
#!/bin/bash
# Get the most recent commit details
echo "=== Recent commit details ==="
git show --stat HEAD 2>/dev/null | head -50
# Look at the full diff of this commit
echo -e "\n=== Full diff of session.py in latest commit ==="
git show HEAD -- apps/backend/agents/session.py 2>/dev/null | head -200
# Or check the diff from develop if HEAD hasn't been pushed
echo -e "\n=== Alternative: Check diff from develop ==="
git diff develop -- apps/backend/agents/session.py 2>/dev/null | head -200
# Check git log -p for session.py to see what changed
echo -e "\n=== Recent changes to session.py (limited) ==="
git log -p --max-count=1 -- apps/backend/agents/session.py 2>/dev/null | head -300
# Look for is_error in the context around line 448
echo -e "\n=== Context: is_error usage around line 448 ==="
sed -n '440,470p' apps/backend/agents/session.py | cat -n
# Check if there's an 'is_error' reference after the blocked check
echo -e "\n=== Full blocked command handling block ==="
sed -n '448,480p' apps/backend/agents/session.py | cat -nRepository: AndyMik90/Auto-Claude
Length of output: 15518
This change is unrelated to Japanese localization and removes a critical guard condition.
This PR is titled "Add Japanese localization, Windows build support, and test fixes" with no mention of session.py logic changes. However, this commit removed the is_error guard from blocked-command detection, changing from:
# Check if this is an error (not just content containing "blocked")
if is_error and "blocked" in str(result_content).lower():to:
if "blocked" in str(result_content).lower():This change is problematic:
- Removes intentional guard: The original comment "not just content containing 'blocked'" shows this guard was deliberate protection against false positives.
- False positives: Any tool result containing "blocked" (case-insensitive) will now trigger blocked-command handling, regardless of error status. Legitimate outputs like
grep "blocked" server.logor file content mentioning blocked IPs will be misclassified. - Weakens error path distinction: The code explicitly splits error handling into two paths (
if "blocked"followed byelif is_error), indicating intentional separation. Removing the guard breaks this logic. - Security logic without justification: This changes security enforcement without explanation and appears to be an accidental inclusion in an unrelated PR.
Consider reverting this change or moving it to a separate PR with proper context and justification.
🤖 Prompt for AI Agents
In @apps/backend/agents/session.py around lines 448 - 449, The change removed
the intentional error-guard around blocked-command detection causing false
positives; restore the original conditional so the blocked check only runs when
is_error is true (e.g., change the condition back to require is_error and
"blocked" in str(result_content).lower()), and ensure the control flow still
uses the blocked check before the subsequent elif is_error branch (update the
conditional surrounding result_content handling in session.py where is_error and
result_content are evaluated).
| from integrations.graphiti.queries_pkg.graphiti import GraphitiMemory | ||
|
|
||
| memory = GraphitiMemory(spec_dir, project_dir) | ||
| async def _async_save(): |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add return type hint to internal async function.
The internal _async_save coroutine is missing a return type annotation. It should be annotated as -> bool for consistency with the outer function's return type and Python best practices.
📝 Add type hint
- async def _async_save():
+ async def _async_save() -> bool:
memory = GraphitiMemory(spec_dir, project_dir)📝 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.
| async def _async_save(): | |
| async def _async_save() -> bool: |
🤖 Prompt for AI Agents
In @apps/backend/agents/tools_pkg/tools/memory.py at line 59, The internal
coroutine _async_save should include an explicit return type annotation; update
its definition to annotate it as async def _async_save() -> bool and ensure any
return paths inside _async_save return boolean values to match the outer
function's return type and typing expectations.
| try: | ||
| if save_type == "discovery": | ||
| # Save as codebase discovery | ||
| # Format: {file_path: description} | ||
| result = await memory.save_codebase_discoveries( | ||
| {data["file_path"]: data["description"]} | ||
| ) | ||
| elif save_type == "gotcha": | ||
| # Save as gotcha | ||
| gotcha_text = data["gotcha"] | ||
| if data.get("context"): | ||
| gotcha_text += f" (Context: {data['context']})" | ||
| result = await memory.save_gotcha(gotcha_text) | ||
| elif save_type == "pattern": | ||
| # Save as pattern | ||
| result = await memory.save_pattern(data["pattern"]) | ||
| else: | ||
| result = False | ||
| return result | ||
| finally: | ||
| await memory.close() | ||
| asyncio.get_running_loop() | ||
| # If we're already in an async context, schedule the task | ||
| # Don't block - just fire and forget for the Graphiti save | ||
| # The file-based save is the primary, Graphiti is supplementary | ||
| asyncio.ensure_future(_async_save()) | ||
| return False # Can't confirm async success, file-based is source of truth | ||
| except RuntimeError: | ||
| # No running loop, create one | ||
| return asyncio.run(_async_save()) |
There was a problem hiding this comment.
Return value semantics are inconsistent between fire-and-forget and sync paths.
The function returns False in two different scenarios with different meanings:
- Line 90 (fire-and-forget):
Falsemeans "cannot confirm success yet, operation is pending" - Line 93 (sync execution):
Falsemeans "operation completed and failed"
This inconsistency makes it difficult for callers to interpret the return value. The current implementation prioritizes file-based storage (which is reasonable), but the boolean return type doesn't clearly communicate the "pending" state vs "failed" state distinction.
Consider documenting this behavior more explicitly in the docstring, or return a more descriptive type (e.g., Optional[bool] where None indicates "pending/unknown").
Critical: Unhandled exceptions in fire-and-forget path can cause silent failures.
When asyncio.ensure_future(_async_save()) schedules the task (line 89), exceptions raised during task execution will not be caught by the outer try-except block (lines 98-100). The outer exception handler only catches errors during the scheduling phase, not from the asynchronous execution of _async_save(). This can lead to silent failures where Graphiti saves fail without the caller being aware, with only warnings logged as unhandled task exceptions.
🔧 Add exception handling for scheduled task
One approach is to add an exception callback:
try:
asyncio.get_running_loop()
# If we're already in an async context, schedule the task
# Don't block - just fire and forget for the Graphiti save
# The file-based save is the primary, Graphiti is supplementary
- asyncio.ensure_future(_async_save())
+ task = asyncio.ensure_future(_async_save())
+ # Add error handler to prevent silent failures
+ task.add_done_callback(
+ lambda t: logger.warning(f"Graphiti save failed: {t.exception()}")
+ if t.exception()
+ else None
+ )
return False # Can't confirm async success, file-based is source of truthCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @apps/backend/agents/tools_pkg/tools/memory.py around lines 84 - 93, The
fire-and-forget scheduling of _async_save via asyncio.ensure_future leaves
exceptions uncaught; modify the scheduling path so the created Task from
asyncio.ensure_future(_async_save()) is assigned to a variable and given a done
callback that logs or handles exceptions (inspect task.exception() and log them
using the module's logger or processLogger) to avoid silent failures; ensure you
still return False (file save is source of truth) but attach the callback to the
Task to capture and report any exceptions from _async_save when run in an
existing event loop.
| # Remove last line if it's `` | ||
| if lines and lines[-1].strip() == "```": | ||
| lines = lines[:-1] | ||
| text = "\n".join(lines).strip() | ||
|
|
||
| # Check again after removing code blocks | ||
| if not text: | ||
| logger.warning( | ||
| "Cannot parse insights: response contained only markdown code block markers with no content" | ||
| ) | ||
| return None | ||
| text = "\n".join(lines) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Review whitespace handling in code block parsing.
The removal of whitespace stripping from the reassembled code-block content could leave extra newlines that may cause JSON parsing issues. Consider whether the JSON parser needs trimmed input.
💡 Consider adding a final strip
if lines and lines[-1].strip() == "```":
lines = lines[:-1]
- text = "\n".join(lines)
+ text = "\n".join(lines).strip()🤖 Prompt for AI Agents
In @apps/backend/analysis/insight_extractor.py around lines 425 - 428, The
reconstructed code-block content assigned to variable text is not trimmed, which
can leave extra whitespace/newlines that break downstream JSON parsing; after
joining lines (variable lines), apply a final strip() to text (i.e., set text =
"\n".join(lines).strip()) so the extracted code/JSON has no leading/trailing
whitespace before parsing, while keeping the preceding logic that removes the
trailing "```" line intact.
| if hasattr(block, "text"): | ||
| response_text += block.text |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/backend/commit_message.py | sed -n '225,245p'Repository: AndyMik90/Auto-Claude
Length of output: 960
🏁 Script executed:
# Check imports and how blocks are used in the file
head -50 apps/backend/commit_message.pyRepository: AndyMik90/Auto-Claude
Length of output: 1141
🏁 Script executed:
# Search for block type definitions and imports related to claude-agent-sdk
rg -n -C2 'TextBlock|ContentBlock|from claude|import' apps/backend/commit_message.py | head -100Repository: AndyMik90/Auto-Claude
Length of output: 999
🏁 Script executed:
# Find other places where blocks are processed or content is handled
rg -n 'msg\.content|block\.|TextBlock|ContentBlock' apps/backend/ --type=pyRepository: AndyMik90/Auto-Claude
Length of output: 10850
🏁 Script executed:
# Check the simple_client implementation to understand the SDK usage
cat -n apps/backend/core/simple_client.py | head -100Repository: AndyMik90/Auto-Claude
Length of output: 4508
🏁 Script executed:
# Look for any SDK imports and block type handling
rg -n 'from claude|ClaudeSDK|content|AssistantMessage' apps/backend/ --type=py | head -50Repository: AndyMik90/Auto-Claude
Length of output: 4183
🏁 Script executed:
# Look at specific examples of how blocks are checked in the codebase
cat -n apps/backend/task_logger/capture.py | sed -n '75,95p'Repository: AndyMik90/Auto-Claude
Length of output: 1204
🏁 Script executed:
# Check if there are any non-TextBlock blocks that have a text attribute
rg -B5 'hasattr\(block.*text\)' apps/backend/ --type=py | grep -A2 -B2 'block_type\|TextBlock\|ToolUseBlock'Repository: AndyMik90/Auto-Claude
Length of output: 47
🏁 Script executed:
# Look at what block types are actually handled to understand the SDK structure
rg -n 'ToolUseBlock|TextBlock|ToolBlock' apps/backend/ --type=py | head -30Repository: AndyMik90/Auto-Claude
Length of output: 3343
🏁 Script executed:
# Check which files use the unsafe pattern (hasattr without type check)
rg -n 'for block in msg\.content.*\n.*hasattr\(block.*text\)' apps/backend/ --type=py -A2 | head -80Repository: AndyMik90/Auto-Claude
Length of output: 256
🏁 Script executed:
# Get more context on files using unsafe pattern - check them specifically
cat -n apps/backend/spec/compaction.py | sed -n '75,82p'Repository: AndyMik90/Auto-Claude
Length of output: 498
🏁 Script executed:
# Check the merge resolver file
cat -n apps/backend/merge/ai_resolver/claude_client.py | sed -n '82,90p'Repository: AndyMik90/Auto-Claude
Length of output: 570
🏁 Script executed:
# Check github services that use unsafe pattern
cat -n apps/backend/runners/gitlab/services/mr_review_engine.py | sed -n '234,242p'Repository: AndyMik90/Auto-Claude
Length of output: 557
🏁 Script executed:
# Let's see if there are any other block types besides TextBlock and ToolUseBlock
rg -n 'block_type.*=' apps/backend/ --type=py | grep -v 'TextBlock\|ToolUseBlock\|msg_type' | head -20Repository: AndyMik90/Auto-Claude
Length of output: 2076
🏁 Script executed:
# Check how many files use the unsafe pattern vs the safe pattern
echo "=== Files using unsafe hasattr(block, 'text') without type check ==="
rg -l 'for block in msg\.content' apps/backend/ --type=py | while read f; do
if grep -q 'hasattr(block.*text)' "$f" && ! grep -q 'block_type.*TextBlock' "$f"; then
echo "$f"
fi
done | wc -l
echo ""
echo "=== Files using safe pattern with type check ==="
rg -l 'block_type.*TextBlock' apps/backend/ --type=py | wc -lRepository: AndyMik90/Auto-Claude
Length of output: 187
🏁 Script executed:
# Count the specific patterns
echo "=== Unsafe pattern (hasattr only) ==="
rg 'if hasattr\(block, "text"\):' apps/backend/ --type=py | wc -l
echo ""
echo "=== Safe pattern (type check + hasattr) ==="
rg 'if block_type == "TextBlock" and hasattr\(block, "text"\)' apps/backend/ --type=py | wc -lRepository: AndyMik90/Auto-Claude
Length of output: 153
🏁 Script executed:
# Let me check if there's a particular reason or pattern for why some files use the unsafe pattern
# Check the git history of commit_message.py to see if this was intentional
git log --oneline -10 apps/backend/commit_message.pyRepository: AndyMik90/Auto-Claude
Length of output: 137
🏁 Script executed:
# Check all files using the unsafe pattern to see if there's a pattern
echo "Files with unsafe hasattr(block, 'text') pattern:"
rg -l 'if hasattr\(block, "text"\)' apps/backend/ --type=pyRepository: AndyMik90/Auto-Claude
Length of output: 570
🏁 Script executed:
# Check if the coding guidelines or any documentation mentions this pattern
rg -i 'textblock|hasattr.*text|attribute.*check' apps/backend/ --type=py docs/ 2>/dev/null | grep -i guideRepository: AndyMik90/Auto-Claude
Length of output: 47
🏁 Script executed:
# Check if there's a recent PR or refactoring that explains the pattern
git log --all --oneline --grep="block\|content\|text" -- apps/backend/ | head -10Repository: AndyMik90/Auto-Claude
Length of output: 595
🏁 Script executed:
# Check the PR/commit that introduced the current code at line 234-235
git log -p --follow -S 'hasattr(block, "text")' -- apps/backend/commit_message.py | head -100Repository: AndyMik90/Auto-Claude
Length of output: 47
🏁 Script executed:
# Let's verify what SDK types we're actually dealing with by checking type() calls
rg -n 'type\(block\).__name__' apps/backend/ --type=py -A1 -B1 | head -50Repository: AndyMik90/Auto-Claude
Length of output: 3339
🏁 Script executed:
# Check if ToolResultBlock or other blocks might have a text attribute
rg -n 'ToolResultBlock' apps/backend/ --type=py -A5 -B2 | head -80Repository: AndyMik90/Auto-Claude
Length of output: 5117
🏁 Script executed:
# Look at what attributes are accessed on each block type
rg -n '\bblock\.(text|name|input|content)\b' apps/backend/ --type=py | cut -d: -f1 | sort -u | wc -lRepository: AndyMik90/Auto-Claude
Length of output: 67
🏁 Script executed:
# Check specifically what's accessed on blocks to understand their structure
echo "=== TextBlock attributes accessed ==="
rg -B3 'block\.text' apps/backend/ --type=py | grep -E 'TextBlock|block\.' | head -20
echo ""
echo "=== ToolResultBlock attributes accessed ==="
rg -B3 'block\.content|ToolResultBlock' apps/backend/ --type=py -A2 | head -30Repository: AndyMik90/Auto-Claude
Length of output: 4468
Add explicit type check for consistency with codebase patterns.
The code correctly collects text only from blocks with a text attribute, which works because only TextBlock from the SDK has this attribute. However, most other files in the codebase use an explicit type check (block_type == "TextBlock") before accessing block.text. Adding this check improves clarity and consistency:
if block_type == "TextBlock" and hasattr(block, "text"):
response_text += block.text🤖 Prompt for AI Agents
In @apps/backend/commit_message.py around lines 234 - 235, The code appends
block.text when hasattr(block, "text") but other files use an explicit type
check; update the conditional in the loop that builds response_text to require
block_type == "TextBlock" in addition to hasattr(block, "text"), i.e., change
the if that references block, block_type, and response_text so it only
concatenates when the block is a TextBlock and has a text attribute.
| // Handle version migration | ||
| if (data.version === 1) { | ||
| // Migrate v1 to v2: add usage and rateLimitEvents fields | ||
| data.version = STORE_VERSION; | ||
| data.autoSwitch = DEFAULT_AUTO_SWITCH_SETTINGS; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/frontend/src/main/claude-profile/profile-storage.ts | head -100Repository: AndyMik90/Auto-Claude
Length of output: 3149
🏁 Script executed:
# Search for all version-related code in profile storage
rg -n "version" apps/frontend/src/main/claude-profile/profile-storage.ts -B 2 -A 2Repository: AndyMik90/Auto-Claude
Length of output: 643
🏁 Script executed:
# Check git history for STORE_VERSION changes
git log --oneline --all -- apps/frontend/src/main/claude-profile/profile-storage.ts | head -20Repository: AndyMik90/Auto-Claude
Length of output: 294
🏁 Script executed:
# Search for version 2 references in the codebase
rg -n "version.*2|version.*=.*2" apps/frontend/src/main/claude-profile/ --type tsRepository: AndyMik90/Auto-Claude
Length of output: 167
🏁 Script executed:
# Look for how readProfilesSync is used/called
rg -n "readProfilesSync" apps/frontend/ --type ts -B 2 -A 3Repository: AndyMik90/Auto-Claude
Length of output: 47
🏁 Script executed:
# Search for writeFileSync or save operations after readProfilesSync
rg -n "writeFileSync|writeFile" apps/frontend/src/main/claude-profile/ --type ts -B 2 -A 2Repository: AndyMik90/Auto-Claude
Length of output: 1064
🏁 Script executed:
# Check git history for all STORE_VERSION values
git log -p --all -- apps/frontend/src/main/claude-profile/profile-storage.ts | grep -A 3 -B 3 "STORE_VERSION" | head -60Repository: AndyMik90/Auto-Claude
Length of output: 47
🏁 Script executed:
# Search for where loadProfileStore is called and how result is used
rg -n "loadProfileStore" apps/frontend/ --type ts -B 3 -A 5Repository: AndyMik90/Auto-Claude
Length of output: 2943
🏁 Script executed:
# Check for any initialization code that might set version to 1 or 2
rg -n "version.*=|version:" apps/frontend/src/main/claude-profile/ --type ts -B 2 -A 2Repository: AndyMik90/Auto-Claude
Length of output: 1512
🏁 Script executed:
# Look for ProfileStoreData initialization
rg -n "ProfileStoreData|activeProfileId" apps/frontend/ --type ts -B 2 -A 2 | head -80Repository: AndyMik90/Auto-Claude
Length of output: 6400
🏁 Script executed:
# Check the full context of how load() is used in claude-profile-manager
cat -n apps/frontend/src/main/claude-profile-manager.ts | sed -n '76,110p'Repository: AndyMik90/Auto-Claude
Length of output: 1128
🏁 Script executed:
# Verify if there's ever a call to saveProfileStore after loadProfileStore
rg -n "loadProfileStore|saveProfileStore" apps/frontend/src/main/claude-profile-manager.ts -B 2 -A 8Repository: AndyMik90/Auto-Claude
Length of output: 1053
Handle version 2 data and persist migration immediately.
Three issues with this version migration:
-
Version 2 data loss: If a store has
version: 2, the code returnsnull(neither condition on lines 43 or 49 matches), causing the caller to discard all data and create a default profile. This results in complete data loss. -
Migration not persisted: After migrating version 1 in-memory (lines 45–46), the changes are never written back to disk. The original file remains at version 1, and the next load will repeat the migration. While the caller eventually saves the migrated data, this creates unnecessary re-migration and correctness issues.
-
Misleading comment: Line 44 claims to migrate "v1 to v2", but the code sets
data.version = STORE_VERSION(3), causing confusion about the actual target version.
🔧 Suggested fix
// Handle version migration
- if (data.version === 1) {
- // Migrate v1 to v2: add usage and rateLimitEvents fields
+ if (data.version === 1 || data.version === 2) {
+ // Migrate v1/v2 to v3: initialize autoSwitch settings
data.version = STORE_VERSION;
- data.autoSwitch = DEFAULT_AUTO_SWITCH_SETTINGS;
+ data.autoSwitch = data.autoSwitch || DEFAULT_AUTO_SWITCH_SETTINGS;
+ // Persist migration immediately to avoid repeated migrations
+ writeFileSync(storePath, JSON.stringify(data, null, 2), 'utf-8');
}📝 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.
| // Handle version migration | |
| if (data.version === 1) { | |
| // Migrate v1 to v2: add usage and rateLimitEvents fields | |
| data.version = STORE_VERSION; | |
| data.autoSwitch = DEFAULT_AUTO_SWITCH_SETTINGS; | |
| } | |
| // Handle version migration | |
| if (data.version === 1 || data.version === 2) { | |
| // Migrate v1/v2 to v3: initialize autoSwitch settings | |
| data.version = STORE_VERSION; | |
| data.autoSwitch = data.autoSwitch || DEFAULT_AUTO_SWITCH_SETTINGS; | |
| // Persist migration immediately to avoid repeated migrations | |
| writeFileSync(storePath, JSON.stringify(data, null, 2), 'utf-8'); | |
| } |
| } catch (error) { | ||
| console.error('[ProfileStorage] Error loading profiles:', error); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider enriching error context.
The error handling works correctly, but including the storePath in the log message would aid debugging when multiple profile stores are involved or when diagnosing user-reported issues.
💡 Suggested enhancement
} catch (error) {
- console.error('[ProfileStorage] Error loading profiles:', error);
+ console.error('[ProfileStorage] Error loading profiles from', storePath, ':', error);
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @apps/frontend/src/main/claude-profile/profile-storage.ts around lines 68 -
70, The catch block in ProfileStorage.loadProfiles logs errors without the
storePath context; update the catch to include the storePath value in the
console.error call (e.g., include `storePath` alongside the error) so logs show
which profile store failed and optionally include error.message or the error
object for full details.
| phaseId: raw.phase_id || raw.phaseId || '', | ||
| dependencies: raw.dependencies || [], | ||
| status: normalizeFeatureStatus(raw.status), | ||
| status: (raw.status as RoadmapFeature['status']) || 'under_review', |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, check if the file exists and examine the code around line 110
echo "=== File location check ==="
fd transformers.ts apps/frontend/src
# Read the transformers.ts file to see the context
echo -e "\n=== transformers.ts content around line 110 ==="
wc -l apps/frontend/src/main/ipc-handlers/roadmap/transformers.ts
cat -n apps/frontend/src/main/ipc-handlers/roadmap/transformers.ts | sed -n '100,120p'
# Look for RoadmapFeature type definition
echo -e "\n=== Search for RoadmapFeature type definition ==="
rg -n --type=ts 'type RoadmapFeature|interface RoadmapFeature' apps/frontend/srcRepository: AndyMik90/Auto-Claude
Length of output: 1729
🏁 Script executed:
#!/bin/bash
# Look for where RoadmapFeature is defined (likely in a types file)
echo "=== Find RoadmapFeature definition ==="
fd -e ts -e tsx apps/frontend/src | xargs rg -l 'RoadmapFeature' | head -20Repository: AndyMik90/Auto-Claude
Length of output: 1496
🏁 Script executed:
#!/bin/bash
# Search for the RoadmapFeature type in type definition files
echo "=== Check types directory ==="
fd 'types|types\.ts' apps/frontend/src
# Look for the actual status type definition
echo -e "\n=== Search for status validation or type definitions ==="
rg -n 'RoadmapFeature.*status|status.*=.*' apps/frontend/src --type=ts | head -30Repository: AndyMik90/Auto-Claude
Length of output: 4409
🏁 Script executed:
#!/bin/bash
# Get the full RoadmapFeature interface and type definitions
echo "=== RoadmapFeature type definition ==="
cat -n apps/frontend/src/shared/types/roadmap.ts | sed -n '65,120p'
# Look for the RawRoadmap or input data type
echo -e "\n=== Search for RawRoadmap or input types ==="
rg -n 'type RawRoadmap|interface RawRoadmap' apps/frontend/src/main/ipc-handlers/roadmap/transformers.ts -B2 -A10
# Check if this file was recently modified to remove STATUS_MAP
echo -e "\n=== Full transformers.ts file context ==="
cat -n apps/frontend/src/main/ipc-handlers/roadmap/transformers.ts | head -30Repository: AndyMik90/Auto-Claude
Length of output: 3755
🏁 Script executed:
#!/bin/bash
# Look for STATUS_MAP or normalizeFeatureStatus in git history or comments
echo "=== Search for any removed normalization logic ==="
rg -n 'STATUS_MAP|normalizeFeatureStatus|normalize.*Status' apps/frontend/src/main/ipc-handlers/roadmap/
# Check where transformFeature is called from
echo -e "\n=== Usage of transformFeature ==="
rg -n 'transformFeature' apps/frontend/src/main/ipc-handlers/roadmap/ -B2 -A2
# Look for any test files
echo -e "\n=== Test files for transformers ==="
fd -e test.ts -e spec.ts apps/frontend/src/main/ipc-handlers/roadmap/
</parameter>Repository: AndyMik90/Auto-Claude
Length of output: 1768
🏁 Script executed:
#!/bin/bash
# Check if raw.status could come from untrusted sources
echo "=== Check transformRoadmapFromSnakeCase function ==="
cat -n apps/frontend/src/main/ipc-handlers/roadmap/transformers.ts | sed -n '118,143p'
# Look for where this transformation is called
echo -e "\n=== Where transformRoadmapFromSnakeCase is called ==="
rg -n 'transformRoadmapFromSnakeCase' apps/frontend/src --type=ts -B2 -A5Repository: AndyMik90/Auto-Claude
Length of output: 3761
🏁 Script executed:
#!/bin/bash
# Check if there are any integration tests or Python code that shows what status values are generated
echo "=== Search for Python roadmap generation ==="
fd -type f -name '*.py' | xargs rg -l 'roadmap|status' 2>/dev/null | head -10
# Look for any test files that might show expected status values
echo -e "\n=== Search for roadmap test files ==="
fd -type f 'roadmap.*test|test.*roadmap' apps/
# Check agent-queue context to see what data it actually receives
echo -e "\n=== Check if there's validation after parsing ==="
cat -n apps/frontend/src/main/agent/agent-queue.ts | sed -n '710,730p'Repository: AndyMik90/Auto-Claude
Length of output: 2172
🏁 Script executed:
#!/bin/bash
# Look for any runtime validation utilities that might be used elsewhere
echo "=== Search for validation utilities ==="
rg -n 'isValidStatus|validateStatus|isStatus' apps/frontend/src --type=ts
# Check if there's any type guard or assert function used elsewhere
echo -e "\n=== Search for type guards ==="
rg -n 'as const|satisfies' apps/frontend/src/main/ipc-handlers/roadmap/ --type=tsRepository: AndyMik90/Auto-Claude
Length of output: 136
🏁 Script executed:
#!/bin/bash
# Check Python test for roadmap validation to see if validation happens on the Python side
echo "=== Check Python roadmap validation test ==="
if [ -f tests/test_roadmap_validation.py ]; then
head -100 tests/test_roadmap_validation.py
fi
# Look for where Python generates the roadmap JSON
echo -e "\n=== Search for Python roadmap generation code ==="
fd -type f -name '*.py' | xargs grep -l 'status.*under_review\|status.*planned\|RoadmapFeature' 2>/dev/null | head -5
# Check if there's any schema validation in the codebase
echo -e "\n=== Search for schema or zod validation ==="
rg -n 'z\.|ZodSchema|JSONSchema|schema.*roadmap' apps/frontend/src --type=tsRepository: AndyMik90/Auto-Claude
Length of output: 5079
🏁 Script executed:
#!/bin/bash
# Look at the complete RawRoadmapFeature interface and RawRoadmapPhase/Milestone
echo "=== Complete RawRoadmapFeature interface ==="
cat -n apps/frontend/src/main/ipc-handlers/roadmap/transformers.ts | sed -n '27,48p'
# Check if there are other similar unsafe type casts in the codebase
echo -e "\n=== Check for similar unsafe casting patterns in other transformers ==="
rg -n 'as \w+\[.*\].*\|\|' apps/frontend/src/main/ipc-handlers/ --type=tsRepository: AndyMik90/Auto-Claude
Length of output: 2046
Unsafe type casting bypasses runtime validation for enum-like status fields.
The type assertion as RoadmapFeature['status'] performs no runtime check. Any truthy value in raw.status will pass through, and the fallback || 'under_review' only catches falsy values, not invalid truthy ones like 'in_progress_new' or other unexpected strings from the data source.
This pattern is systemic across enum-like fields:
- Line 93: RoadmapPhase status
- Lines 105–107: RoadmapFeature priority, complexity, impact
- Line 110: RoadmapFeature status
- Line 139: Roadmap status
While validation may exist on the Python side, the TypeScript transformer has no defensive check after JSON.parse(). Consider adding a runtime type guard or validation layer to ensure data integrity at the system boundary:
function isValidStatus(value: unknown): value is RoadmapFeatureStatus {
return ['under_review', 'planned', 'in_progress', 'done'].includes(value);
}Then use it before assignment to catch and handle invalid inputs gracefully.
🤖 Prompt for AI Agents
In @apps/frontend/src/main/ipc-handlers/roadmap/transformers.ts at line 110, The
transformer is using unsafe casts like (raw.status as RoadmapFeature['status'])
which skip runtime validation; add runtime type-guard functions (e.g.,
isValidFeatureStatus, isValidPhaseStatus, isValidPriority, isValidComplexity,
isValidImpact) that check the value against the allowed string unions and use
them when assigning fields in the transformer (replace casts at RoadmapPhase
status, RoadmapFeature priority/complexity/impact, RoadmapFeature status, and
Roadmap status); if a guard returns false, assign the appropriate safe default
(e.g., 'under_review' for feature status) or handle the error, so only validated
values flow out of the transformer.
| // Extract a clean summary from task_description (first line or first ~200 chars) | ||
| const taskDesc = requirements.task_description; | ||
| const firstLine = taskDesc.split('\n')[0].trim(); | ||
| // If the first line is a title like "Investigate GitHub Issue #36", use the next meaningful line | ||
| if (firstLine.toLowerCase().startsWith('investigate') && taskDesc.includes('\n\n')) { | ||
| const sections = taskDesc.split('\n\n'); | ||
| // Find the first paragraph that's not a title | ||
| for (const section of sections) { | ||
| const trimmed = section.trim(); | ||
| // Skip headers and short lines | ||
| if (trimmed.startsWith('#') || trimmed.length < 20) continue; | ||
| // Skip the "Please analyze" instruction at the end | ||
| if (trimmed.startsWith('Please analyze')) continue; | ||
| description = trimmed.substring(0, 200).split('\n')[0]; | ||
| break; | ||
| } | ||
| } | ||
| // If still no description, use a shortened version of task_description | ||
| if (!description) { | ||
| description = firstLine.substring(0, 150); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider making the investigative title detection more extensible.
The logic for extracting meaningful descriptions is sound. However, the check at line 367 only handles task descriptions starting with "investigate". Other similar prefixes (e.g., "Analyze", "Research", "Study", "Review") might also benefit from this paragraph extraction logic.
💡 Optional: Make pattern matching more flexible
- if (firstLine.toLowerCase().startsWith('investigate') && taskDesc.includes('\n\n')) {
+ const investigativePatterns = ['investigate', 'analyze', 'research', 'study', 'review'];
+ const isInvestigativeTitle = investigativePatterns.some(p => firstLine.toLowerCase().startsWith(p));
+ if (isInvestigativeTitle && taskDesc.includes('\n\n')) {📝 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.
| // Extract a clean summary from task_description (first line or first ~200 chars) | |
| const taskDesc = requirements.task_description; | |
| const firstLine = taskDesc.split('\n')[0].trim(); | |
| // If the first line is a title like "Investigate GitHub Issue #36", use the next meaningful line | |
| if (firstLine.toLowerCase().startsWith('investigate') && taskDesc.includes('\n\n')) { | |
| const sections = taskDesc.split('\n\n'); | |
| // Find the first paragraph that's not a title | |
| for (const section of sections) { | |
| const trimmed = section.trim(); | |
| // Skip headers and short lines | |
| if (trimmed.startsWith('#') || trimmed.length < 20) continue; | |
| // Skip the "Please analyze" instruction at the end | |
| if (trimmed.startsWith('Please analyze')) continue; | |
| description = trimmed.substring(0, 200).split('\n')[0]; | |
| break; | |
| } | |
| } | |
| // If still no description, use a shortened version of task_description | |
| if (!description) { | |
| description = firstLine.substring(0, 150); | |
| } | |
| // Extract a clean summary from task_description (first line or first ~200 chars) | |
| const taskDesc = requirements.task_description; | |
| const firstLine = taskDesc.split('\n')[0].trim(); | |
| // If the first line is a title like "Investigate GitHub Issue #36", use the next meaningful line | |
| const investigativePatterns = ['investigate', 'analyze', 'research', 'study', 'review']; | |
| const isInvestigativeTitle = investigativePatterns.some(p => firstLine.toLowerCase().startsWith(p)); | |
| if (isInvestigativeTitle && taskDesc.includes('\n\n')) { | |
| const sections = taskDesc.split('\n\n'); | |
| // Find the first paragraph that's not a title | |
| for (const section of sections) { | |
| const trimmed = section.trim(); | |
| // Skip headers and short lines | |
| if (trimmed.startsWith('#') || trimmed.length < 20) continue; | |
| // Skip the "Please analyze" instruction at the end | |
| if (trimmed.startsWith('Please analyze')) continue; | |
| description = trimmed.substring(0, 200).split('\n')[0]; | |
| break; | |
| } | |
| } | |
| // If still no description, use a shortened version of task_description | |
| if (!description) { | |
| description = firstLine.substring(0, 150); | |
| } |
🤖 Prompt for AI Agents
In @apps/frontend/src/main/project-store.ts around lines 363 - 383, Replace the
hardcoded startsWith('investigate') check with a case-insensitive regex that
matches a set of investigative prefixes (e.g.,
investigate|analyze|research|study|review) applied to firstLine (use something
like /^ *(investigate|analyze|research|study|review)\b/i); keep the existing
split-on-paragraphs logic (sections = taskDesc.split('\n\n')) and iterate as
before to pick the first meaningful paragraph into description, and change the
includes('\n\n') guard to a more robust check like sections.length > 1 so other
formatting variants still trigger paragraph extraction.
| // Fallback: check if key packages exist | ||
| // This handles cases where the marker might be missing but packages are there | ||
| const claudeSdkPath = path.join(sitePackagesPath, 'claude_agent_sdk'); | ||
| const dotenvPath = path.join(sitePackagesPath, 'dotenv'); | ||
| if (existsSync(claudeSdkPath) || existsSync(dotenvPath)) { | ||
| console.log(`[PythonEnvManager] Found key packages, using bundled packages`); | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Fallback check is too permissive—may cause runtime failures.
The OR logic allows hasBundledPackages() to return true when only one of the key packages exists. If, for example, dotenv exists but claude_agent_sdk is missing, the initialization will proceed with bundled mode (skipping venv creation and pip install), and the application will fail at runtime when attempting to import claude_agent_sdk.
Consider requiring both packages for the fallback to be valid:
🔧 Proposed fix
// Fallback: check if key packages exist
// This handles cases where the marker might be missing but packages are there
const claudeSdkPath = path.join(sitePackagesPath, 'claude_agent_sdk');
const dotenvPath = path.join(sitePackagesPath, 'dotenv');
- if (existsSync(claudeSdkPath) || existsSync(dotenvPath)) {
+ if (existsSync(claudeSdkPath) && existsSync(dotenvPath)) {
console.log(`[PythonEnvManager] Found key packages, using bundled packages`);
return 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.
| // Fallback: check if key packages exist | |
| // This handles cases where the marker might be missing but packages are there | |
| const claudeSdkPath = path.join(sitePackagesPath, 'claude_agent_sdk'); | |
| const dotenvPath = path.join(sitePackagesPath, 'dotenv'); | |
| if (existsSync(claudeSdkPath) || existsSync(dotenvPath)) { | |
| console.log(`[PythonEnvManager] Found key packages, using bundled packages`); | |
| return true; | |
| } | |
| // Fallback: check if key packages exist | |
| // This handles cases where the marker might be missing but packages are there | |
| const claudeSdkPath = path.join(sitePackagesPath, 'claude_agent_sdk'); | |
| const dotenvPath = path.join(sitePackagesPath, 'dotenv'); | |
| if (existsSync(claudeSdkPath) && existsSync(dotenvPath)) { | |
| console.log(`[PythonEnvManager] Found key packages, using bundled packages`); | |
| return true; | |
| } |
🤖 Prompt for AI Agents
In @apps/frontend/src/main/python-env-manager.ts around lines 132 - 139, The
fallback check in PythonEnvManager (hasBundledPackages) is too permissive: it
uses existsSync(claudeSdkPath) || existsSync(dotenvPath) so bundled mode may be
chosen when only one package is present; change the condition to require both
packages (use logical AND) so it returns true only when both claude_agent_sdk
and dotenv exist, and update the log message to reflect that both key packages
were found; ensure you reference claudeSdkPath and dotenvPath in the revised
check and keep the early return behavior intact.
2 participants
Base Branch
developbranch (required for all feature/fix PRs)main(hotfix only - maintainers)Description
Related Issue
Closes #
Type of Change
Area
Commit Message Format
Follow conventional commits:
<type>: <subject>Types: feat, fix, docs, style, refactor, test, chore
Example:
feat: add user authentication systemChecklist
developbranchCI/Testing Requirements
Screenshots
Feature Toggle
use_feature_nameBreaking Changes
Breaking: Yes / No
Details:
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.