feat: Python env paths redesign v2 with PowerShell fixes #1229
+9,954
−273
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add comprehensive conda environment support for isolated Python environments: **Conda Environment Features:** - Conda installation detection across OS-specific locations - Application-level environment at ~/miniconda3/envs/auto-claude - Project-level self-contained environments in .envs/<project-name>/ - Automatic terminal activation for configured projects - Cross-platform activation scripts (CMD, PowerShell, Bash) - VS Code workspace file generation with conda terminal profiles - Python version detection from requirements.txt, pyproject.toml, environment.yml - Setup wizard with multi-step progress tracking **Windows/PowerShell Compatibility Fixes:** - Add windowsHide: true to all spawn() calls to prevent console popups - Fix PowerShell command syntax (semicolons, $env:PATH, & call operator) - Platform-aware shell escaping (PowerShell vs bash quote handling) - Add pathWasModified flag to skip unnecessary PATH modifications - PowerShell-specific conda activation via init scripts **New Files (11):** - conda-detector.ts, conda-env-manager.ts, conda-workspace-generator.ts - conda-project-structure.ts, conda-handlers.ts, conda-api.ts - PythonEnvSettings.tsx, CondaSetupWizard.tsx, CondaDetectionDisplay.tsx - useCondaSetup.ts, conda.ts (types) **Modified Files (48):** - Terminal integration, Claude CLI integration, shell utilities - Settings UI components, IPC handlers, type definitions - i18n translations (en + fr) Co-Authored-By: Claude Opus 4.5 <[email protected]>
- GeneralSettings.tsx: Fix 6 hardcoded strings (validation messages, browse buttons, file dialog titles) - SectionRouter.tsx: Fix 2 hardcoded strings (section title/description) - python-env-manager.ts: Change console.log to console.warn for consistency - Add 7 new translation keys to EN and FR locale files Co-Authored-By: Claude Opus 4.5 <[email protected]>
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Backend Python fixes: - Add encoding="utf-8" to file operations in coder.py - Guard task_logger before logging, remove debug prints in feedback.py - Normalize provider_setting in git_provider.py - Add platform-aware glab resolution in worktree.py - Fix api_get params and add_comment docstring in gitlab_provider.py - Sort imports in providers/__init__.py and factory.py Frontend platform abstraction: - Replace process.platform checks with isWindows()/getPathDelimiter() - Add isMac(), isLinux() helpers to python-path-utils.ts - Fix agent-queue.ts, agent-process.ts, conda-detector.ts - Fix oauth-handlers, claude-code-handlers, version-suggester - Add timeout and GLAB_NO_PROMPT to gitlab utils - Convert metadata-handlers to async fs operations Frontend i18n/UI fixes: - Localize ProviderIntegration, TaskConfiguration, TaskDetailModal - Localize CondaDetectionDisplay, IntegrationSettings, Insights - Fix factory.ts visibility mapping and error i18n - Add functional state updates to GeneralSettings - Fix slider.tsx disabled styling (data-[disabled]:) - Remove debug console.log statements Security and quality: - Add sanitizeShellPath for command injection prevention - Guard OAuth logs in GitLabIntegration - Add caching to detectProjectStructure - Fix TOCTOU race condition in conda-workspace-generator - Tighten types in factory.ts and task-api.ts Bug fix: - Add PowerShell (.ps1) support to getEnvironmentPathFromScript Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Add user_feedback.py stub module for coder.py import - Remove incompatible feature files (TaskConfiguration, TaskFeedback, ProviderIntegration, metadata-handlers) that require missing infrastructure - Fix TypeScript type errors in test mocks - Remove pythonActivationScript references (not in AppSettings type) - Fix task path resolution in shared.ts - Fix unnecessary regex escapes in python-detector.ts Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Add pythonActivationScript property to AppSettings type - Restore Python Activation Script field with browse button in GeneralSettings - Add FolderOpen icon import for browse button - Add i18n translations (en/fr) for pythonActivationScript labels Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Add PythonPackageValidator component for validating and installing required Python packages from the Paths settings section - Add IPC handlers for package validation, installation, environment validation, and environment reinstallation with progress events - Fix ESM compatibility by adding __dirname definition to python-env-manager - Fix platform marker filtering in parseRequirementsTxt() to skip packages like secretstorage on Windows (sys_platform == "linux") - Add missing i18n translation keys for package validation UI - Fix incorrect translation key references (general.* -> python.*) Co-Authored-By: Claude Opus 4.5 <[email protected]>
The buildPathPrefix function was generating cmd.exe syntax (set "PATH=..." &&) even when running in PowerShell. PowerShell 5.1 doesn't support the && operator. - Add shellType parameter to buildPathPrefix function - Use $env:PATH = "..."; syntax for PowerShell - Keep set "PATH=..." && syntax for cmd.exe - Update all call sites to pass terminal.shellType Co-Authored-By: Claude Opus 4.5 <[email protected]>
PowerShell interprets `--` as the decrement operator, causing errors like: "Unexpected token 'dangerously-skip-permissions' in expression or statement" - Add shellType parameter to escapeShellCommand function - Use `& "path"` syntax for PowerShell (call operator) - Keep `"path"` syntax for cmd.exe - Update all call sites to pass terminal.shellType Co-Authored-By: Claude Opus 4.5 <[email protected]>
PowerShell's Set-Location (cd alias) doesn't support the /d flag which is specific to cmd.exe for changing drives. - Use `cd 'path'; ` for PowerShell (single quotes to avoid $ expansion) - Keep `cd /d "path" && ` for cmd.exe - Escape embedded single quotes by doubling them for PowerShell Co-Authored-By: Claude Opus 4.5 <[email protected]>
Contributor
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds comprehensive Conda-based environment detection, creation, validation, and activation across frontend and IPC; introduces multi-provider Git support (auto-detect, GitHub, GitLab) in backend; broad cross-platform spawn/shell fixes (Windows hiding, shell-aware escaping); many new UI components, types, and i18n entries for Python/Conda workflows. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Renderer as Renderer
participant Main as Main Process
participant FileSystem as File System
participant CondaCLI as Conda/Python
User->>Renderer: Click "Detect Conda"
Renderer->>Main: IPC: CONDA_DETECT
Main->>FileSystem: Scan OS-specific paths
FileSystem-->>Main: Candidate paths
Main->>CondaCLI: Validate executable & version
CondaCLI-->>Main: Version/type
Main->>Main: Cache detection result
Main-->>Renderer: IPC: Detection result
Renderer->>Renderer: Update UI
sequenceDiagram
actor User
participant Renderer as Renderer
participant Main as Main Process
participant IPC as Conda IPC handlers
participant FS as File System
participant Terminal as Terminal/PTY
User->>Renderer: Start Project Env Setup
Renderer->>Main: IPC: CONDA_SETUP_PROJECT_ENV
Main->>IPC: createEnvironment stream (progress events)
IPC->>FS: Create env, install Python, write activation scripts
IPC-->>Main: CONDA_SETUP_PROGRESS (updates)
Main-->>Renderer: CONDA_SETUP_PROGRESS (forwarded)
Renderer->>Terminal: request terminal activation (condaEnvPath)
Terminal->>Terminal: injectCondaActivation into PTY
Terminal-->>Renderer: activation result
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 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 |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| timestamp = fb.get("timestamp", "") | ||
| # Use actual_idx so agent knows the correct index for mark_feedback_read tool | ||
| feedback_messages.append(f"[Index {actual_idx}] [{timestamp}]\n{msg}") | ||
| feedback_text = "\n\n".join(feedback_messages) |
Check warning
Code scanning / CodeQL
Variable defined multiple times Warning
| # 1. Try shutil.which (works if glab is in PATH) | ||
| glab_path = shutil.which("glab") | ||
| if glab_path: | ||
| _cached_glab_path = glab_path |
Check notice
Code scanning / CodeQL
Unused global variable Note
| for path in common_paths: | ||
| try: | ||
| if os.path.isfile(path): | ||
| _cached_glab_path = path |
Check notice
Code scanning / CodeQL
Unused global variable Note
| if result.returncode == 0 and result.stdout.strip(): | ||
| found_path = result.stdout.strip().split("\n")[0].strip() | ||
| if found_path and os.path.isfile(found_path): | ||
| _cached_glab_path = found_path |
Check notice
Code scanning / CodeQL
Unused global variable Note
| if found_path and os.path.isfile(found_path): | ||
| _cached_glab_path = found_path | ||
| return found_path | ||
| except (subprocess.TimeoutExpired, OSError): |
Check notice
Code scanning / CodeQL
Empty except Note
| pass | ||
|
|
||
| # Default fallback - let subprocess handle it (may fail) | ||
| _cached_glab_path = "glab" |
Check notice
Code scanning / CodeQL
Unused global variable Note
10 tasks
| const args = [specRunnerPath, '--task', taskDescription, '--project-dir', projectPath]; | ||
| // Write task description to a temp file to avoid command-line quoting issues | ||
| const tempTaskFile = path.join(os.tmpdir(), `task-${Date.now()}-${Math.random().toString(36).substring(7)}.txt`); | ||
| writeFileSync(tempTaskFile, taskDescription, 'utf-8'); |
Check failure
Code scanning / CodeQL
Insecure temporary file High
| const args = [specRunnerPath, '--task', taskDescription, '--project-dir', projectPath]; | ||
| // Write task description to a temp file to avoid command-line quoting issues | ||
| const tempTaskFile = path.join(os.tmpdir(), `task-${Date.now()}-${Math.random().toString(36).substring(7)}.txt`); | ||
| writeFileSync(tempTaskFile, taskDescription, 'utf-8'); |
Check warning
Code scanning / CodeQL
Network data written to file Medium
| onProgress?.('Installation complete'); | ||
| resolve(); | ||
| } else { | ||
| reject(new Error(`pip install failed with code ${code}`)); |
Check warning
Code scanning / CodeQL
Indirect uncontrolled command line Medium
| } | ||
|
|
||
| stepsCompleted.push(`Found conda: ${condaExe}`); | ||
|
|
Check warning
Code scanning / CodeQL
Indirect uncontrolled command line Medium
| import { projectStore } from '../project-store'; | ||
| import { getClaudeProfileManager } from '../claude-profile-manager'; | ||
| import { parsePythonCommand, validatePythonPath } from '../python-detector'; | ||
| import { parsePythonCommand, validatePythonPath, isValidActivationScript } from '../python-detector'; |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
| * Sanitize and validate shell/command interpreter paths from environment variables. | ||
| * Prevents command injection through malicious COMSPEC or SHELL values. | ||
| */ | ||
| function sanitizeShellPath(envValue: string | undefined, defaultValue: string): string { |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
| import { getToolInfo, configureTools, sortNvmVersionDirs, getClaudeDetectionPaths, type ExecFileAsyncOptionsWithVerbatim } from '../cli-tool-manager'; | ||
| import { readSettingsFile, writeSettingsFile } from '../settings-utils'; | ||
| import { isSecurePath } from '../utils/windows-paths'; | ||
| import { isWindows, isMac, isLinux } from '../python-path-utils'; |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
Contributor
Summary of ChangesHello @jasonnator, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the application's Python environment management capabilities by integrating robust Conda support and addressing critical PowerShell compatibility issues. It also expands Git integration to include GitLab and refines the agent's interaction with user feedback and progress reporting. The changes aim to provide a more stable, flexible, and user-friendly experience for Python-based projects across different operating systems. 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
|
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Comment on lines
+848
to
+850
| if (trimmed.includes('python_version < "3.11"') || trimmed.includes("python_version < '3.11'")) { | ||
| continue; // Skip packages only for Python < 3.11 | ||
| } |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces significant enhancements, including a comprehensive Python environment management system using Conda, support for GitLab as a git provider, and numerous fixes for PowerShell compatibility. The changes are well-structured, with new modules for conda detection and management, a provider factory pattern for git integrations, and extensive test coverage for the new logic. The PowerShell fixes, such as using correct command separators and escaping, are consistently applied across the codebase, which will greatly improve the user experience on Windows. My feedback includes a few suggestions for improving code consistency and robustness.
| print_status("Implementation plan synced to main project", "success") | ||
|
|
||
| # Show plan statistics | ||
| from implementation_plan import ImplementationPlan |
Contributor
There was a problem hiding this comment.
For better code organization and to follow standard Python conventions (PEP 8), it's recommended to move this import to the top of the file with the other imports. If this is a local import to avoid a circular dependency, please add a comment explaining why.
Comment on lines
+1148
to
+1196
| // Step 2: Find conda executable (cross-platform) | ||
| onProgress?.('Finding conda executable', 1, 3); | ||
|
|
||
| let condaPaths: string[]; | ||
| if (process.platform === 'win32') { | ||
| // Windows: look for conda.bat in common locations | ||
| condaPaths = [ | ||
| path.join(process.env.USERPROFILE || '', 'miniconda3', 'condabin', 'conda.bat'), | ||
| path.join(process.env.USERPROFILE || '', 'anaconda3', 'condabin', 'conda.bat'), | ||
| path.join(process.env.LOCALAPPDATA || '', 'miniconda3', 'condabin', 'conda.bat'), | ||
| path.join(process.env.LOCALAPPDATA || '', 'anaconda3', 'condabin', 'conda.bat'), | ||
| process.env.CONDA_EXE || '' | ||
| ]; | ||
| } else { | ||
| // Linux/macOS: look for conda in common locations | ||
| const homeDir = process.env.HOME || ''; | ||
| condaPaths = [ | ||
| path.join(homeDir, 'miniconda3', 'bin', 'conda'), | ||
| path.join(homeDir, 'anaconda3', 'bin', 'conda'), | ||
| path.join('/opt', 'miniconda3', 'bin', 'conda'), | ||
| path.join('/opt', 'anaconda3', 'bin', 'conda'), | ||
| path.join('/usr', 'local', 'miniconda3', 'bin', 'conda'), | ||
| path.join('/usr', 'local', 'anaconda3', 'bin', 'conda'), | ||
| process.env.CONDA_EXE || '' | ||
| ]; | ||
| } | ||
|
|
||
| let condaExe: string | null = null; | ||
| for (const condaPath of condaPaths) { | ||
| if (condaPath && existsSync(condaPath)) { | ||
| condaExe = condaPath; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| // If not found in specific paths, try PATH | ||
| if (!condaExe) { | ||
| try { | ||
| const { stdout } = await execAsync(process.platform === 'win32' ? 'where conda' : 'which conda', { | ||
| timeout: 5000 | ||
| }); | ||
| const foundPath = stdout.trim().split('\n')[0]; | ||
| if (foundPath && existsSync(foundPath)) { | ||
| condaExe = foundPath; | ||
| } | ||
| } catch (error) { | ||
| // conda not in PATH | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
This logic for finding the conda executable seems to duplicate the functionality of the new conda-detector.ts module. For consistency and to reduce code duplication, consider using the detectCondaInstallations function from conda-detector.ts here. It provides a more robust and cached way to find the preferred conda installation.
// Step 2: Find conda executable (cross-platform)
onProgress?.('Finding conda executable', 1, 3);
const { detectCondaInstallations } = await import('../conda-detector');
const detection = await detectCondaInstallations();
if (!detection.found || !detection.preferred) {
return {
success: false,
environmentPath,
pythonVersion: null,
error: 'Could not find conda executable. Please ensure conda is installed and in PATH.',
stepsCompleted
};
}
const condaExe = detection.preferred.condaExe;
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 64
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
apps/backend/prompts/planner.md (1)
783-795: Fix contradictory commit instructions.Phase 6 says spec/plan files must NOT be committed (Line 783-795), but the ending section instructs to “Commit planning files” (Line 945-948). This contradiction will confuse agents and could cause policy violations. Please remove or correct the commit instruction.
Also applies to: 945-948
apps/frontend/src/main/ipc-handlers/mcp-handlers.ts (2)
419-427: Use platform abstraction function instead of directprocess.platformcheck.Same issue as above—the new
windowsHideaddition on line 426 and the existingshelloption on line 425 use directprocess.platformchecks instead of the platform abstraction module.Suggested refactor using platform abstraction
// On Windows, use shell: true to properly handle .cmd/.bat scripts like npx const proc = spawn(server.command!, args, { stdio: ['pipe', 'pipe', 'pipe'], timeout: 15000, // OS-level timeout for reliable process termination - shell: process.platform === 'win32', // Required for Windows to run npx.cmd - ...(process.platform === 'win32' && { windowsHide: true }) + shell: isWindows(), // Required for Windows to run npx.cmd + ...(isWindows() && { windowsHide: true }) });Based on coding guidelines and retrieved learnings.
57-61: Pre-existingprocess.platformusage also violates coding guidelines.While not part of this PR's changes, this existing direct platform check at line 57 should also be refactored to use
isWindows()for consistency with the rest of the codebase.Suggested refactor
- if (process.platform === 'win32') { + if (isWindows()) { if (args.some(arg => SHELL_METACHARACTERS.some(char => arg.includes(char)))) { return false; } }Based on coding guidelines.
apps/frontend/src/main/ipc-handlers/gitlab/utils.ts (1)
6-6: Remove unused importexecSync.
execSyncis imported but never used in this file. OnlyexecFileSyncis used.Suggested fix
-import { execSync, execFileSync } from 'child_process'; +import { execFileSync } from 'child_process';apps/frontend/src/shared/utils/shell-escape.ts (1)
59-65: Docs don’t match the actual escaping behavior.PowerShell uses single quotes in
buildCdCommand, andescapeShellArgWindowsis for cmd.exe, not PowerShell. Please update the comments to reflect current behavior.✍️ Doc correction
- * - Windows PowerShell: double quotes with `;` separator (&&` not valid in PS 5.1) + * - Windows PowerShell: single quotes with `;` separator (&&` not valid in PS 5.1) @@ - * Escape a string for safe use as a PowerShell argument. + * Escape a string for safe use as a cmd.exe argument.Also applies to: 96-104
apps/frontend/src/main/memory-service.ts (1)
211-217: Avoid directprocess.platformchecks here.Use the platform abstraction helper instead of
process.platform === 'win32'.
As per coding guidelines, use platform module helpers for OS checks.♻️ Proposed fix
+import { isWindows } from './platform'; @@ - ...(process.platform === 'win32' && { windowsHide: true }) + ...(isWindows() && { windowsHide: true }) @@ - ...(process.platform === 'win32' && { windowsHide: true }) + ...(isWindows() && { windowsHide: true })Also applies to: 355-360
apps/frontend/src/main/ipc-handlers/memory-handlers.ts (1)
311-317: Use platform helpers instead of process.platform.
Directprocess.platformchecks violate the platform abstraction rule; switch toisWindows()and add the import. As per coding guidelines, ...♻️ Proposed fix
-import { ipcMain, app } from 'electron'; +import { ipcMain, app } from 'electron'; +import { isWindows } from '../platform';- ...(process.platform === 'win32' && { windowsHide: true }) + ...(isWindows() && { windowsHide: true })- ...(process.platform === 'win32' && { windowsHide: true }) + ...(isWindows() && { windowsHide: true })Also applies to: 786-794
🤖 Fix all issues with AI agents
In `@apps/backend/agents/coder.py`:
- Around line 252-275: get_unread_feedback expects a string path but spec_dir is
a Path; change the call to pass str(spec_dir) (or otherwise convert to a string)
when invoking get_unread_feedback so the helper receives a str, e.g., replace
get_unread_feedback(spec_dir) with get_unread_feedback(str(spec_dir)); also
ensure any other calls nearby (e.g., get_task_logger) remain compatible or are
similarly converted if they require a str.
In `@apps/backend/agents/tools_pkg/tools/feedback.py`:
- Around line 98-136: The file I/O for reading and writing task metadata should
explicitly use UTF-8 and preserve Unicode: update the open(...) calls that use
metadata_file in the block around the read (where metadata = json.load(...)) and
the write (where json.dump(...) is called) to pass encoding="utf-8"; also pass
ensure_ascii=False to json.dump so non-ASCII feedback is preserved. Ensure you
only change the open calls and the json.dump call in this function handling
feedback_list and marked_count.
In `@apps/backend/core/worktree.py`:
- Around line 1184-1192: The subprocess.run call that executes glab (identified
by the glab_args usage) hardcodes timeout=30 while the module uses the shared
GH_CLI_TIMEOUT constant for GitHub CLI; change the literal 30 to GH_CLI_TIMEOUT
so the GitLab MR timeout is aligned with GH_CLI_TIMEOUT, and ensure
GH_CLI_TIMEOUT is in scope (import or reference the module-level constant) where
subprocess.run is invoked.
- Around line 39-98: The get_glab_executable function currently uses os.name and
hardcoded Windows paths; replace that platform-specific probing with the project
platform-abstraction module’s executable/path helper (call that helper to locate
"glab" instead of using os.name, common_paths, and subprocess.where), keep
caching to _cached_glab_path and return its value when found, and retain the
final fallback of "glab" if the abstraction helper fails; update
get_glab_executable to call the abstraction helper (e.g.,
platform_abstraction.find_executable or the repo's provided which/find API),
remove the os.name block and hardcoded paths/subprocess.run logic, and ensure
exceptions are handled and the result is cached in _cached_glab_path before
returning.
In `@apps/backend/prompts/planner.md`:
- Around line 9-49: The docs/validator are inconsistent: the global validation
requires at least one phase with subtasks but the "SIMPLE workflow" description
says it uses only subtasks (no phases), making SIMPLE plans invalid; fix by
either updating the validation logic to allow SIMPLE workflow to accept a flat
subtasks-only plan (bypass the "phases array non-empty" check when workflow_type
=== "SIMPLE") or by changing the SIMPLE workflow wording/plan generation to
produce a single phase wrapper (e.g., create a phase with id "phase-1" and move
subtasks into it); update the validator that checks the phases array and the
SIMPLE workflow documentation/plan generator to be consistent so that either
SIMPLE is exempted from the phase requirement or SIMPLE produces one phase
containing the subtasks.
In `@apps/backend/runners/gitlab/providers/gitlab_provider.py`:
- Around line 321-334: The three placeholder methods get_repository_info,
get_default_branch, and check_permissions should not return misleading defaults;
update each to raise a NotImplementedError (or call the real implementation on
glab_client) so callers detect the missing implementation: replace the current
return {} in get_repository_info, "main" in get_default_branch, and "read" in
check_permissions with raising NotImplementedError (or delegate to glab_client
methods if available) and include a brief message referencing the method name
for clarity.
- Around line 135-141: The fetch_prs method currently silently returns an empty
list which hides real merge requests; update the fetch_prs function to raise
NotImplementedError (or implement and call glab_client.list_mr) instead of
returning [] so callers can explicitly handle the unimplemented behavior;
reference the async method fetch_prs and the potential glab_client.list_mr
integration when making the change.
- Around line 286-315: The four label-related methods (apply_labels,
remove_labels, create_label, list_labels) are currently no-ops; update each to
raise NotImplementedError with a clear message (e.g., "Label operations not
implemented: requires glab_client support") so callers are not misled—replace
the pass/empty return in apply_labels, remove_labels, create_label, and
list_labels with raising NotImplementedError (include the function name or brief
context in the error string).
- Around line 217-231: The close_pr method currently always returns False after
optionally posting a note, which misreports failure; update close_pr in
gitlab_provider.py to either raise NotImplementedError to signal it’s not
implemented (instead of returning False) or implement proper MR closing by
delegating to a glab_client method (e.g., call a new
glab_client.close_mr(pr_number) or similar) and return its success boolean;
ensure any posted note still occurs before raising/returning and reference
close_pr and glab_client.post_mr_note/glab_client.close_mr when making the
change.
In `@apps/backend/runners/providers/__init__.py`:
- Around line 1-11: Export the convenience initializer create_provider_from_env
from the package by adding it to the module's public API: update the __all__
list in __init__.py (which already exports ProviderConfig and create_provider)
to include "create_provider_from_env" so users can import it directly from the
package without referencing .factory; ensure the name matches the function
defined in factory.py (create_provider_from_env).
In `@apps/backend/user_feedback.py`:
- Around line 11-22: get_unread_feedback currently returns an empty list,
disabling feedback flow; implement it to read the real feedback store under
spec_dir, parse unread items into List[Tuple[int, Dict[str, Any]]] and return
them (index should be a stable integer identifier and dict the feedback
payload), and mark those items as read (or provide a flag to leave unread) so
callers receive actual data; update get_unread_feedback to locate the feedback
file(s) (e.g., feedback.json or similar in spec_dir), deserialize entries,
filter by unread flag, convert to the expected tuple form, and persist the
updated read state.
In `@apps/frontend/src/main/__tests__/conda-project-structure.test.ts`:
- Around line 123-148: The mockFileSystem helper uses lax any types for mocked
implementations; replace those with explicit fs types: import the fs types and
change the existsSync mock to have signature (filePath: string): boolean => ...,
the readdirSync mock to (dirPath: string): string[] => ..., and the statSync
mock to (filePath: string): fs.Stats => ({ isDirectory: () => ..., isFile: () =>
... } as unknown as fs.Stats) (or construct a Partial<fs.Stats> and cast to
fs.Stats) so you avoid any; update the vi.mocked(..).mockImplementation calls in
mockFileSystem to use these concrete return types for existsSync, readdirSync,
and statSync.
- Around line 74-90: The test file defines local helpers isWindows, isMacOS, and
isLinux that directly read process.platform instead of using the platform
detection functions exported by the module under test; replace usages of these
local helpers with the exported platform helper functions from the
conda-project-structure module (or, if these helpers are meant only for test
internals, rename them to _isWindows/_isMacOS/_isLinux to signal they are
test-only) and update the test assertions (the ones around the self-check in the
test file) to import and assert the actual exported functions so the tests
exercise the production platform abstraction rather than local copies.
- Line 542: The test callback destructures an unused variable projectRoot in the
describe.each(platforms) block which triggers ESLint; update the destructuring
to mark it unused (e.g., rename to _projectRoot) or remove projectRoot from the
parameter list in the describe.each callback (the describe.each line where
platforms is iterated) so only used variables (like platform) remain.
In `@apps/frontend/src/main/agent/agent-process.test.ts`:
- Around line 49-77: There are two vi.mock('fs') blocks causing the latter to
override the former; consolidate them into one vi.mock('fs') factory that merges
the existsSync and readFileSync behaviors (keep the blacklist logic for .env and
terminal.cmd in existsSync and the realistic return strings for .env and
terminal.cmd in readFileSync) and remove the duplicate mock block so both
behaviors run from a single mock definition for fs.
In `@apps/frontend/src/main/agent/agent-process.ts`:
- Around line 19-21: Replace the direct import of isWindows from
'../python-path-utils' with the platform abstraction: remove that import and
instead import the OS helper from the platform module (the project's platform
helper) and update any uses of isWindows in agent-process.ts to call the
platform helper (e.g., platform.isWindows or the named export from the platform
module) so agent-process.ts uses the platform abstraction rather than
process.platform; keep other imports (parsePythonCommand, validatePythonPath,
isValidActivationScript, pythonEnvManager, getConfiguredPythonPath) unchanged.
- Around line 30-89: The sanitizeShellPath function currently rejects
parentheses because the dangerousChars regex includes '(' and ')', which
incorrectly blocks valid Windows paths like "C:\Program Files (x86)\..."; update
the dangerousChars pattern in sanitizeShellPath to remove parentheses from the
exclusion set (e.g. remove \( and \) from /[;&|`$<>(){}[\]!*?~#]/) or replace it
with a whitelist of true command-separator characters (such as ; & | ` $ < >) so
parentheses are allowed while still rejecting real shell metacharacters; keep
all other validations in sanitizeShellPath (Windows extension check, path
format, newline check, Unix absolute-path checks) unchanged.
In `@apps/frontend/src/main/changelog/generator.ts`:
- Around line 145-151: Replace direct process.platform === 'win32' checks with
the platform abstraction isWindows() imported from python-path-utils; update the
spawn call in generate() (where pythonCommand, pythonBaseArgs and childProcess
are created) to compute isWindows via isWindows() instead of process.platform,
and do the same inside buildSpawnEnvironment() so any conditional
platform-specific options (e.g., { windowsHide: true }) use isWindows()
consistently; ensure you add the import for isWindows() if missing and remove
the direct process.platform comparisons.
In `@apps/frontend/src/main/claude-cli-utils.ts`:
- Line 4: Update the import to use the canonical platform abstraction: replace
the import source for isWindows and getPathDelimiter so the module imports
isWindows and getPathDelimiter from './platform' instead of
'./python-path-utils'; locate the import line that currently references
'./python-path-utils' in claude-cli-utils.ts and change its module specifier to
'./platform' to match other files like env-utils.ts and python-env-manager.ts.
In `@apps/frontend/src/main/conda-detector.ts`:
- Around line 35-70: CONDA_SEARCH_PATHS currently includes hardcoded OS-specific
absolute paths; replace them with platform-aware discovery by deriving candidate
roots from environment variables (e.g., process.env.HOME,
process.env.PROGRAMDATA, process.env.LOCALAPPDATA) and by probing for
conda/miniconda executables (use a platform executable-finder or spawn
which/where) to infer base install directories instead of literals; update the
CONDA_SEARCH_PATHS construction (and the LOCALAPPDATA spread) to build paths
from those discovered roots and fallback to only non-hardcoded common locations
so detection works across custom installs and platforms.
- Around line 245-251: The getPlatformKey function currently returns
process.platform directly; replace that direct check with the platform module
helpers (e.g., use isMacOS() and isLinux() instead of process.platform) and
ensure the isWindows import comes from the same platform helper module; update
getPlatformKey to return 'darwin' when isMacOS() is true, 'linux' when isLinux()
is true, 'win32' when isWindows() is true, and a sensible default (e.g.,
'linux') otherwise so no direct process.platform usage remains.
- Around line 128-165: The version regex in getCondaVersion and
getCondaVersionAsync only matches strict x.y.z and misses valid suffixes like
.post0 or rc1; update both functions' regex from /conda\s+(\d+\.\d+\.\d+)/i to a
pattern that accepts optional suffixes, e.g.
/conda\s+(\d+\.\d+\.\d+(?:[.-][0-9A-Za-z]+)*)/i, so the full version (including
.post, rc, etc.) is captured and returned instead of null.
In `@apps/frontend/src/main/conda-env-manager.ts`:
- Around line 439-466: Before calling fsPromises.rm on envPath (after
existsSync), add a defensive validation that rejects empty/undefined paths, path
roots (path.parse(envPath).root or envPath === '/' or Windows drive roots like
'C:\\'), and any well-known system directories; also ensure envPath is within
the expected conda environments base directory (or matches an allowed base
prefix) and has a minimum path length to avoid short-path mistakes. If
validation fails, yield the 'error' step with a clear userMessage and do not
call fsPromises.rm. Apply the same guard around the other rm invocation at the
later block referenced (lines ~874-885) so both fsPromises.rm usages validate
envPath before deletion.
- Around line 398-603: The progress/error messages in the createEnvironment
async generator are user-facing and must be converted to i18n keys and params
rather than raw strings; update every yielded SetupProgress object inside
createEnvironment (e.g., places emitting step 'detecting', 'creating',
'installing-python', 'verifying-python', 'generating-scripts', 'finalizing',
'complete' and all error yields) to emit structured translation info (for
example messageKey and messageParams or code and params) instead of literal
English text and details, and adjust error branches (removal errors, mkdir,
spawnCommand, runCommand, generateActivationScripts) to include an appropriate
key plus dynamic params (like envPath, pythonVersion, error text) so the
renderer can localize; apply the same changes to the other similar blocks noted
in the comment (the ranges mentioned) to ensure all user-facing strings are
replaced with translation keys and parameters.
- Around line 955-1020: The checkDependencyCompatibility function currently
always calls pip with --dry-run which fails on pip <22.2; detect pip version
first by calling the pip executable from getCondaPipPath (use
spawnCommand(pipExe, ['--version']) or similar), parse the semantic version from
stdout, compare it to 22.2, and if the installed pip is older than 22.2 avoid
using --dry-run (fallback: skip the dry-run check and return a result that notes
"pip too old for --dry-run" in issues or mark compatibility as "not supported");
otherwise proceed with the existing spawnCommand(...
['install','-r',requirementsPath,'--dry-run']) flow; update error messages in
the catch paths to reference pipExe and include version parsing failures where
appropriate.
In `@apps/frontend/src/main/conda-project-structure.ts`:
- Around line 11-13: Replace direct use of Node's path.join in
apps/frontend/src/main/conda-project-structure.ts with the platform abstraction
helper (e.g., joinPaths): add an import for joinPaths from the platform
utilities module, remove or stop using the default path import for joins, and
update every occurrence of path.join in this file to call joinPaths so path
operations in functions like any project-scanning utilities use the platform
helper consistently; keep other fs imports (existsSync, readdirSync, statSync)
as-is.
In `@apps/frontend/src/main/env-utils.ts`:
- Around line 520-525: Replace the direct platform check in the return object
(currently using process.platform === 'win32') with the platform helper
isWindows(); import isWindows from the platform helper module, and use it to
gate the windowsHide spread so the code becomes ...(isWindows() && {
windowsHide: baseOptions?.windowsHide ?? true }); ensure the import is added
alongside other imports and no other behavior changes to shouldUseShell or
baseOptions.
In `@apps/frontend/src/main/ipc-handlers/conda-handlers.ts`:
- Around line 181-203: The progress/error messages in the conda setup flow are
hardcoded English; update the emitting code to send i18n keys and params instead
of raw strings so the renderer can localize them: replace the message/detail
fields in the detectingProgress and errorProgress SetupProgress objects (and any
other direct user-facing strings sent via mainWindow?.webContents.send to
IPC_CHANNELS.CONDA_SETUP_PROGRESS) with translation keys and optional params
(e.g., messageKey, messageParams or similar shape your renderer expects), and
ensure detectCondaInstallations consumers still receive the same semantic info;
keep the object shape consistent (use the existing
detectingProgress/errorProgress variable names and the same
IPC_CHANNELS.CONDA_SETUP_PROGRESS send calls).
In `@apps/frontend/src/main/ipc-handlers/github/oauth-handlers.ts`:
- Around line 245-250: Replace the direct platform check using process.platform
=== 'win32' with the platform abstraction function isWindows(): import isWindows
from the platform abstraction module at the top of the file, then replace the
local isWindows const and its usage in the spawn options (the ghProcess spawn
call and the isWindows variable) to call isWindows() so the spawn options use
...(isWindows() && { windowsHide: true }) instead of checking process.platform
directly.
In `@apps/frontend/src/main/ipc-handlers/github/utils/subprocess-runner.ts`:
- Line 116: Replace the direct process.platform check with the platform
abstraction: import isWindows from the platform helper (e.g., add import {
isWindows } from '../../../platform';) and change the spread conditional that
currently uses (process.platform === 'win32' && { windowsHide: true }) to use
isWindows() instead (e.g., ...(isWindows() && { windowsHide: true })). Update
any related references in subprocess-runner.ts so all platform checks use
isWindows().
In `@apps/frontend/src/main/ipc-handlers/gitlab/oauth-handlers.ts`:
- Around line 100-134: The console.log and console.warn calls in the glab
version-detection block (around execFileSync calls using glabPath and
getAugmentedEnv) are leaking verbose output to production; replace those calls
with the project's debug logging (e.g., debugLog) or wrap them in the DEBUG
guard so only debug-mode emits them—use debugLog for the two raw version outputs
and the parsed version log, and replace the console.warn fallback with a
debug-level or guarded warning (e.g., debugLog.warn or a guarded console.warn)
to keep logs scoped.
In `@apps/frontend/src/main/ipc-handlers/gitlab/utils.ts`:
- Around line 136-139: The catch block that currently does
console.error('[GitLab] Failed to authenticate glab CLI:', error) (the one
wrapping the execFileSync call used to authenticate glab CLI) may leak tokens;
replace this direct error log with a sanitized error output: extract only
non-sensitive fields (e.g., error.name, error.code) and a scrubbed error.message
(remove or mask any token-like substrings or remove stdin/command content), or
log a generic failure message with those safe fields instead of the full
error/stack, so the execFileSync-related error is not printed verbatim.
In `@apps/frontend/src/main/ipc-handlers/mcp-handlers.ts`:
- Around line 196-200: Replace direct process.platform checks with the platform
helper: import and use isWindows() instead of checking process.platform ===
'win32' in the code that sets command and spawn options; update the command
selection (currently using 'where' vs 'which') to use isWindows() and set the
spawn options to include windowsHide: true via isWindows() (while keeping
timeout and server.command usage intact) so both the command variable and the
spawn options reference isWindows() rather than process.platform.
In `@apps/frontend/src/main/ipc-handlers/settings-handlers.ts`:
- Around line 793-869: The handlers use hardcoded user-facing strings (e.g., the
progress step "Starting environment reinstall" sent via
IPC_CHANNELS.PYTHON_REINSTALL_PROGRESS and error texts returned in the catch
blocks of IPC handlers for PYTHON_VALIDATE_PACKAGES,
PYTHON_INSTALL_REQUIREMENTS, PYTHON_VALIDATE_ENVIRONMENT,
PYTHON_REINSTALL_ENVIRONMENT) — replace these with i18n translation keys or
structured error codes; update event.sender.send calls (e.g., for
IPC_CHANNELS.PYTHON_REINSTALL_PROGRESS, PYTHON_VALIDATION_PROGRESS,
PYTHON_INSTALL_PROGRESS, PYTHON_REINSTALL_PROGRESS) to send objects like { key:
'settings.reinstall.starting', params: { completed:0, total:3 } } and change
returned error/result payloads to { success: false, errorKey:
'settings.error.extract_env_path' } or { success: true, data: { resultKey:
'settings.reinstall.success', details } } so the renderer can localize; ensure
functions referenced (getEnvironmentPathFromScript, validatePythonPackages,
installPythonRequirements, validatePythonEnvironment,
reinstallPythonEnvironment) still receive the same values and only the IPC
messages / return payloads switch to keys or codes.
In `@apps/frontend/src/main/ipc-handlers/task/shared.ts`:
- Line 1: Replace direct Node path usage with the platform abstraction: import
joinPaths from ./platform and use joinPaths(...) instead of path.join(...)
inside the getSpecDir function; update the import list to remove or stop using
the path module and ensure getSpecDir calls joinPaths with the same arguments
previously passed to path.join.
In `@apps/frontend/src/main/python-detector.ts`:
- Around line 479-502: The Windows-only patterns in isValidActivationScript
(CONDA_ACTIVATION_PATTERNS) currently only allow activate\.bat; update those
Windows regexes used in isValidActivationScript to accept both .bat and .ps1
extensions (e.g., allow activate\.bat or activate\.ps1) so PowerShell activation
scripts generated by conda-env-manager.ts are considered valid; keep the same
path structure and case-insensitive flag (/i) and ensure all Windows patterns
that reference Scripts\\activate\.bat are changed to match either extension.
In `@apps/frontend/src/main/python-env-manager.ts`:
- Around line 291-296: Replace direct process.platform checks with the module's
platform abstraction: change the spread expression that currently uses
(process.platform === 'win32' && { windowsHide: true }) in the spawn(...) call
that creates `proc` (spawn(systemPython, ['-m', 'venv', venvPath], {...})) to
use isWindows() instead (e.g. ...(isWindows() && { windowsHide: true })). Apply
the same replacement to the other spawn usages in this file that use the
process.platform === 'win32' pattern (the spawn calls creating subprocesses
around the venv and related commands) so all Windows-only options consistently
use isWindows().
- Around line 1015-1111: The code in validatePythonEnvironment uses direct
process.platform checks (see pythonExeName and pythonPath calculations) which
bypass the app's platform abstraction; replace those checks with the shared
platform helper functions from apps/frontend/src/main (import the existing
helpers and use them to decide the executable name and bin directory instead of
process.platform), update the pythonExeName and pythonPath computations to call
the helpers (e.g., use the helper that detects Windows vs POSIX and the helper
that returns the proper bin directory for the environment), and keep all
existing error/return logic intact so getEnvironmentPathFromScript and execAsync
usage remain unchanged.
In `@apps/frontend/src/main/python-path-utils.ts`:
- Around line 8-85: Remove the duplicated platform helpers and stop reading
process.platform directly: import the platform abstraction (the exported
isWindows/isMac/isLinux functions) and use isWindows() in getPathDelimiter,
getCondaPythonPath, getVenvPythonPath, getCondaPipPath and getVenvPipPath
instead of process.platform checks; then delete the local
isWindows/isMac/isLinux implementations (or re-export them from the platform
module) so all platform detection comes from the shared platform abstraction and
only the path-builder functions (getPathDelimiter, getCondaPythonPath,
getVenvPythonPath, getCondaPipPath, getVenvPipPath) remain in this file.
In `@apps/frontend/src/main/terminal-name-generator.ts`:
- Around line 181-182: Replace the direct platform check in the spawn options
that uses (process.platform === 'win32' && { windowsHide: true }) with a call to
the platform abstraction isWindows(); add an import for isWindows from
'./python-path-utils' at the top of the module and change the spread to
...(isWindows() && { windowsHide: true }) to follow the project's
platform-detection guideline and match other files like title-generator.ts and
version-suggester.ts.
In `@apps/frontend/src/main/terminal/claude-integration-handler.ts`:
- Around line 72-93: buildClaudeShellCommand must accept and forward the
shellType used by buildPathPrefix/escapeShellCommand so Windows PowerShell 5.1
gets compatible syntax; add a shellType parameter to buildClaudeShellCommand and
pass it into buildPathPrefix and escapeShellCommand calls, then branch when
shellType === 'powershell' to (1) use ';' instead of '&&' for command chaining,
(2) invoke temp-file scripts with PowerShell invocation operator (&) instead of
cmd.exe's call, and (3) remove temp files with a PowerShell-safe command (e.g.,
Remove-Item -Force or the equivalent), while retaining existing cmd.exe behavior
when shellType is not 'powershell'.
- Around line 80-87: The PowerShell PATH-escaping logic in the shellType ===
'powershell' block omits escaping double quotes; update the psEscapedPath
transformation (used when building the `$env:PATH = "...";` assignment) to also
replace double quotes by their PowerShell-escaped form (i.e., add a .replace for
`"` -> backtick-quote) so any PATH entries containing " are properly escaped
before returning the string.
In `@apps/frontend/src/main/terminal/pty-manager.ts`:
- Around line 506-535: The writeCondaWarning function uses direct
process.platform checks; replace them with the project's platform abstraction by
importing and using isWindows() (and corresponding non-Windows checks if
available) instead of process.platform === 'win32', and apply the same change
for the other conda activation helper blocks referenced; update the top-of-file
imports to pull isWindows from the platform helper module, then condition the
PowerShell vs Unix ptyProcess.write branches on isWindows() (negating it for the
Unix branch) so platform detection is centralized and consistent.
In `@apps/frontend/src/main/terminal/terminal-lifecycle.ts`:
- Around line 23-48: computeCondaEnvPath duplicates the conda-env derivation
logic used elsewhere (notably TerminalManager.create and other spots), leading
to drift; extract the shared rules into a single helper (e.g.,
getCondaEnvPathForProject) placed in the conda-project-structure or a terminal
utility and have computeCondaEnvPath, TerminalManager.create, and the other call
sites call that helper. The helper should accept the project identifier (path or
project object), read project.settings, apply the exact rule (useCondaEnv is
true AND condaAutoActivate !== false), and internally call
getPythonEnvPath(projectPath, projectName) to return the env path or undefined;
update all referenced locations to use this helper so restore/create behavior
stays consistent.
In `@apps/frontend/src/renderer/components/project-settings/GeneralSettings.tsx`:
- Around line 79-81: The fallback envPath uses string concatenation which can
produce mixed separators on Windows; update the logic around
getProjectPaths/pathsResult and envPath to use the platform-aware path join
helper (e.g., joinPaths) or return early when pathsResult indicates failure:
call joinPaths(project.path, ".envs", projectName) for the fallback instead of
`${project.path}/.envs/${projectName}` and ensure getProjectPaths errors are
handled so we don't rely on a platform-incorrect fallback.
- Around line 126-146: The handleDeleteEnv function currently swallows errors
but always sets useCondaEnv to false and resets UI state even if deletion
failed; update it to check the IPC results from condaApi.deleteProjectEnv and
condaApi.deleteActivationScripts, surface errors (e.g., via setState or a
visible error dialog) and only call setSettings(..., useCondaEnv: false) and
onUseCondaEnvChange(false) after both deletions succeed; keep the cleanup of
dialog state (setShowDeleteEnvDialog, setEnvPathToDelete,
setDeleteActivationScripts) conditional on success or perform partial cleanup
with clear user-visible error if any operation failed; reference
handleDeleteEnv, condaApi.deleteProjectEnv, condaApi.deleteActivationScripts,
setSettings, and onUseCondaEnvChange when making these changes.
In
`@apps/frontend/src/renderer/components/project-settings/IntegrationSettings.tsx`:
- Around line 539-550: Replace the hardcoded provider labels inside the
SelectItem components in IntegrationSettings.tsx (the SelectItem value="github"
and value="gitlab" blocks) with calls to the i18n translation function used in
this file (e.g., t('...')) and add corresponding keys (like
"integrations.github" and "integrations.gitlab" or whatever naming convention is
used) to the settings.json locale files; ensure the JSX uses the translation
function (same import/usage pattern as other strings in this component) so both
"GitHub" and "GitLab" are pulled from the locale files rather than hardcoded.
In
`@apps/frontend/src/renderer/components/project-settings/PythonEnvSettings.tsx`:
- Around line 52-295: The component currently uses un-prefixed keys like
t('python.*') and hardcoded strings (e.g. "Python {version}") — update all t()
calls to use the settings namespace (e.g. t('settings:python.selectVersion'),
t('settings:python.statusReady'), etc.) and remove inline concatenation by
replacing hardcoded labels inside SelectItem, SelectValue, the detectedPython
display, and the renderStatusIndicator outputs with translation keys that accept
interpolation (e.g. t('settings:python.versionLabel', { version })) so versions,
package counts and source strings come from i18n; update placeholders
(selectVersionPlaceholder) likewise, and add the corresponding keys to the
translations JSON/TS files. Ensure you modify every usage site (SelectItem
mapping, SelectValue placeholder, the envStatus blocks, detectedPython output,
and any inline messages in handleOpenInVsCode/handleShowInFolder UI text) to
reference the new settings-prefixed keys.
- Around line 60-135: hasInitializedVersionRef is never reset when the user
switches projects, causing setSelectedPythonVersion to be skipped for the new
project; add a short effect that watches the project identity (e.g.,
project.path or project.name) and on change reset
hasInitializedVersionRef.current = false and clear the selection with
setSelectedPythonVersion('') (optionally setRecommendedVersion to a sensible
default) so the next loadEnvStatus()/listPythonVersions run can set the new
project's recommended version; reference hasInitializedVersionRef,
setSelectedPythonVersion, setRecommendedVersion and the
loadEnvStatus/listPythonVersions flow when adding this useEffect.
In `@apps/frontend/src/renderer/components/settings/AppSettings.tsx`:
- Around line 140-153: The effect that syncs useCondaEnvState currently skips
updates when newValue is undefined, so switching to a project without an
explicit useCondaEnv leaves the previous value in state; modify the useEffect
(the hook referencing hookUseCondaEnv, selectedProject, useCondaEnvState,
setUseCondaEnvState) to always call setUseCondaEnvState(newValue) instead of
only when newValue !== undefined (or explicitly set to false/undefined when no
value is present), so the state resets correctly when the new project has no
explicit setting.
In `@apps/frontend/src/renderer/components/settings/CondaDetectionDisplay.tsx`:
- Around line 41-126: All t(...) calls in CondaDetectionDisplay must use the
settings namespace; update every occurrence of t('python.*') to
t('settings:python.*') (e.g. t('python.detecting') →
t('settings:python.detecting'), t('python.notDetected') →
t('settings:python.notDetected'), t('python.installMiniconda') →
t('settings:python.installMiniconda'), etc.), and update the dynamic conda type
key and its defaultValue to t(`settings:python.condaTypes.${preferred.type}` , {
defaultValue: t('settings:python.condaTypes.unknown') }); keep usage of
useTranslation('settings') and the rest of the logic in CondaDetectionDisplay,
detection and preferred unchanged.
In `@apps/frontend/src/renderer/components/settings/CondaSetupWizard.tsx`:
- Around line 245-251: The step header div in CondaSetupWizard.tsx is only
mouse-interactive; make it keyboard-accessible by either replacing the div with
a semantic button or adding role="button", tabIndex={0}, and an onKeyDown
handler that triggers toggleStepExpanded(stepInfo.id) when Enter or Space is
pressed (respecting the hasLogs guard), and keep the existing onClick behavior;
reference the div that uses hasLogs and toggleStepExpanded(stepInfo.id) to
locate and update the element.
In `@apps/frontend/src/renderer/components/settings/PythonPackageValidator.tsx`:
- Around line 155-197: The reinstall flow in handleReinstallEnvironment may
throw from window.electronAPI.reinstallPythonEnvironment and skip cleanup; wrap
the async call and related logic in try/catch/finally so unsubscribe(),
setReinstalling(false) and setReinstallProgress(null) always run; in the try
block await window.electronAPI.reinstallPythonEnvironment(...) and handle
success logic (setEnvStatus, setEnvValidation, await validateEnvironment()), in
catch set the error via setError(result/error fallback) and
setEnvStatus('invalid'), and in finally call unsubscribe(),
setReinstalling(false) and setReinstallProgress(null).
- Around line 61-120: The progress listeners and state resets in checkPackages
and installRequirements must be executed even if validatePythonPackages or
installPythonRequirements throws: wrap the async call and its surrounding logic
in try/catch/finally blocks so that unsubscribe() is always called in finally
and state resets (e.g., setValidationProgress(null), setInstalling(false),
setInstallProgress, and any spinner/error state updates) happen there; keep
result handling in try (or after awaiting the call) and set error in catch using
the thrown error, referencing the existing functions checkPackages and
installRequirements and the unsubscribe variable returned from
window.electronAPI.onPythonValidationProgress/onPythonInstallProgress.
- Around line 91-352: Several user-facing strings in this component are
hardcoded; update all UI and error strings used in installRequirements,
validateEnvironment, handleReinstallEnvironment, the loading overlay
(isInitialValidation text), the Error panel title, confirm dialog text,
reinstall copy, "Install location", and any dynamic messages to use the
translation function with the settings: namespace (e.g.,
t('settings:python.validatingEnvironment'), t('settings:python.error'), etc.),
and convert string concatenations like 'Failed to validate Python environment: '
+ String(error) to interpolation-based translations (e.g.,
t('settings:python.envValidationFailed', { error: String(error) })); ensure
dynamic values (package names, counts, version, steps) use interpolation keys
and that all new keys are added to the i18n resource files under settings.
In `@apps/frontend/src/renderer/components/ui/slider.tsx`:
- Around line 21-24: The current Slider implementation renders a single
SliderPrimitive.Thumb which prevents multi-value (range) sliders; update the
Slider component (the forwardRef React component that wraps
SliderPrimitive.Root) to determine thumbCount from props.value or
props.defaultValue (e.g., value?.length ?? defaultValue?.length ?? 1) and render
that many SliderPrimitive.Thumb elements by mapping/Array.from so each value in
the array gets a thumb; keep all existing className and props passed to each
Thumb and ensure a stable key (index) for each rendered thumb.
- Line 6: The project imports `@radix-ui/react-slider` (see the import of
SliderPrimitive in the slider component) but that package is not declared in
package.json; add "@radix-ui/react-slider" to the apps/frontend dependencies
with a version compatible with the other `@radix-ui` packages, run your package
manager install to update node_modules and the lockfile, and verify the import
resolves by rebuilding or running the frontend dev server.
In `@apps/frontend/src/renderer/hooks/useCondaSetup.ts`:
- Around line 93-140: The progress listener (handleProgress) keeps updating
state after cancelSetup is called because cancelSetup only flips local state;
fix by adding a cancellation guard and/or unsubscribing the listener when
cancelSetup runs and re-creating the subscription on new starts: modify
cancelSetup to call the cleanup function stored in cleanupRef (and null it), add
a cancelled boolean/ref checked at the top of handleProgress (or reuse
isRunning/isMountedRef) to early-return when cancelled, and ensure the effect
that sets up cleanupRef resets that cancelled flag when a new start begins so
progress updates resume on subsequent runs.
In `@apps/frontend/src/shared/i18n/locales/en/settings.json`:
- Around line 396-454: The locale JSON contains a duplicate key
"pathPlaceholder" (appears twice); remove the redundant entry and keep a single
"pathPlaceholder" with the intended value (e.g., "C:\\path\\to\\conda") so keys
are not silently overwritten—locate both "pathPlaceholder" entries in the
settings JSON and delete one, leaving only one canonical "pathPlaceholder" line.
In `@apps/frontend/src/shared/i18n/locales/fr/tasks.json`:
- Around line 246-283: Several French strings in the tasks locale are missing
accents; update the values for keys like "title", "descriptionRunning",
"descriptionNotRunning", "taskRunningWarning", "changesApplyFrom",
"currentPhaseComplete", "pendingChanges", "pendingChangesHint", "profile",
"resetToDefaults", "customProfile", "customProfileDescription",
"defaultProfile", "defaultProfileDescription", "phaseConfiguration",
"phaseConfigurationHint", "phaseLabel", "model", "thinkingLevel", the entries
under "thinkingLabels" ("none","low","medium","high","ultra"), and any other
affected keys to use proper French diacritics (e.g., tâche, modèle, réflexion,
paramètres, défaut, personnalisé, réinitialiser) so the UI reads correctly;
locate and replace the affected string literals in
apps/frontend/src/shared/i18n/locales/fr/tasks.json accordingly.
In `@apps/frontend/src/shared/utils/shell-escape.ts`:
- Around line 34-39: The platform check in escapeShellArg currently uses
process.platform === 'win32'; replace that conditional with the isWindows()
helper (imported from ../platform) to follow the platform abstraction guideline,
i.e., call isWindows() inside escapeShellArg and keep the existing Windows
escaping logic intact; ensure the file imports isWindows and do not change
behavior for cmd.exe contexts—route those to escapeShellArgWindows() or
buildCdCommand(shellType) if/when added.
In `@CLAUDE.md`:
- Around line 268-275: Replace the bare URLs under the "GitHub" and "GitLab"
sections with Markdown link syntax: change "More info: https://cli.github.com/"
to "More info: [GitHub CLI](https://cli.github.com/)" (or similar descriptive
text) and change "Download: https://gitlab.com/gitlab-org/cli/-/releases" to
"Download: [GitLab CLI releases](https://gitlab.com/gitlab-org/cli/-/releases)";
keep the surrounding bullets and commands intact and apply the same Markdown
link style for any other bare URLs in those "GitHub" and "GitLab" lines.
In `@PR_DESCRIPTION.md`:
- Around line 1-3: Change the first heading from a level-2 to a level-1 heading
so the file starts with a top-level heading; replace the existing "## Summary"
line with "# Summary" (search for the "## Summary" heading in PR_DESCRIPTION.md
and update it to "# Summary") to satisfy Markdownlint MD041.
- Around line 36-49: The PR_DESCRIPTION.md headings lack surrounding blank lines
required by markdownlint MD022; add a blank line before and after each top-level
and subsection heading (e.g., "New Files (11)", the list block beneath it, and
"Modified Files (48)") so each heading is separated from adjacent text or lists,
ensuring every heading has an empty line above and below throughout the file.
Comment on lines
+252
to
+275
| # Check for unread user feedback before starting this subtask | ||
| unread_feedback = get_unread_feedback(spec_dir) | ||
| feedback_text = None | ||
| if unread_feedback: | ||
| # Combine all unread feedback messages with their ACTUAL indices from full list | ||
| # unread_feedback is now a list of (actual_index, feedback_dict) tuples | ||
| feedback_messages = [] | ||
| for actual_idx, fb in unread_feedback: | ||
| msg = fb.get("message", "") | ||
| timestamp = fb.get("timestamp", "") | ||
| # Use actual_idx so agent knows the correct index for mark_feedback_read tool | ||
| feedback_messages.append(f"[Index {actual_idx}] [{timestamp}]\n{msg}") | ||
| feedback_text = "\n\n".join(feedback_messages) | ||
|
|
||
| # Log feedback detection to task_logs.json (visible in UI) | ||
| task_logger = get_task_logger(spec_dir) | ||
| if task_logger: | ||
| task_logger.log( | ||
| content=f"USER FEEDBACK DETECTED - {len(unread_feedback)} unread feedback item(s) will be incorporated into this subtask", | ||
| entry_type=LogEntryType.INFO, | ||
| phase=LogPhase.CODING, | ||
| print_to_console=True, | ||
| ) | ||
|
|
Contributor
There was a problem hiding this comment.
Pass a string path into get_unread_feedback.
The helper signature expects str, but spec_dir is a Path. Convert to str (or update the helper signature) to avoid type/behavior mismatches once the stub is implemented.
🛠️ Suggested fix
- unread_feedback = get_unread_feedback(spec_dir)
+ unread_feedback = get_unread_feedback(str(spec_dir))🤖 Prompt for AI Agents
In `@apps/backend/agents/coder.py` around lines 252 - 275, get_unread_feedback
expects a string path but spec_dir is a Path; change the call to pass
str(spec_dir) (or otherwise convert to a string) when invoking
get_unread_feedback so the helper receives a str, e.g., replace
get_unread_feedback(spec_dir) with get_unread_feedback(str(spec_dir)); also
ensure any other calls nearby (e.g., get_task_logger) remain compatible or are
similarly converted if they require a str.
Comment on lines
+98
to
+136
| try: | ||
| with open(metadata_file) as f: | ||
| metadata = json.load(f) | ||
|
|
||
| feedback_list = metadata.get("feedback", []) | ||
| if not feedback_list: | ||
| return { | ||
| "content": [ | ||
| { | ||
| "type": "text", | ||
| "text": "No feedback entries found in task_metadata.json", | ||
| } | ||
| ] | ||
| } | ||
|
|
||
| # Mark specified feedback items as read | ||
| marked_count = 0 | ||
| invalid_indices = [] | ||
| for idx in feedback_indices: | ||
| if 0 <= idx < len(feedback_list): | ||
| feedback_list[idx]["read"] = True | ||
| marked_count += 1 | ||
| else: | ||
| invalid_indices.append(idx) | ||
|
|
||
| if invalid_indices: | ||
| return { | ||
| "content": [ | ||
| { | ||
| "type": "text", | ||
| "text": f"Error: Invalid feedback indices: {invalid_indices}. " | ||
| f"Valid range is 0-{len(feedback_list) - 1}", | ||
| } | ||
| ] | ||
| } | ||
|
|
||
| # Save updated metadata | ||
| with open(metadata_file, "w") as f: | ||
| json.dump(metadata, f, indent=2) |
Contributor
There was a problem hiding this comment.
Read/write task_metadata.json with explicit UTF‑8 encoding.
On Windows or non‑UTF‑8 locales, non‑ASCII feedback can fail to load, causing the tool to error even though the JSON is valid. Use UTF‑8 on read/write and preserve Unicode output.
✅ Proposed fix
- with open(metadata_file) as f:
+ with open(metadata_file, encoding="utf-8") as f:
metadata = json.load(f)
@@
- with open(metadata_file, "w") as f:
- json.dump(metadata, f, indent=2)
+ with open(metadata_file, "w", encoding="utf-8") as f:
+ json.dump(metadata, f, indent=2, ensure_ascii=False)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| try: | |
| with open(metadata_file) as f: | |
| metadata = json.load(f) | |
| feedback_list = metadata.get("feedback", []) | |
| if not feedback_list: | |
| return { | |
| "content": [ | |
| { | |
| "type": "text", | |
| "text": "No feedback entries found in task_metadata.json", | |
| } | |
| ] | |
| } | |
| # Mark specified feedback items as read | |
| marked_count = 0 | |
| invalid_indices = [] | |
| for idx in feedback_indices: | |
| if 0 <= idx < len(feedback_list): | |
| feedback_list[idx]["read"] = True | |
| marked_count += 1 | |
| else: | |
| invalid_indices.append(idx) | |
| if invalid_indices: | |
| return { | |
| "content": [ | |
| { | |
| "type": "text", | |
| "text": f"Error: Invalid feedback indices: {invalid_indices}. " | |
| f"Valid range is 0-{len(feedback_list) - 1}", | |
| } | |
| ] | |
| } | |
| # Save updated metadata | |
| with open(metadata_file, "w") as f: | |
| json.dump(metadata, f, indent=2) | |
| try: | |
| with open(metadata_file, encoding="utf-8") as f: | |
| metadata = json.load(f) | |
| feedback_list = metadata.get("feedback", []) | |
| if not feedback_list: | |
| return { | |
| "content": [ | |
| { | |
| "type": "text", | |
| "text": "No feedback entries found in task_metadata.json", | |
| } | |
| ] | |
| } | |
| # Mark specified feedback items as read | |
| marked_count = 0 | |
| invalid_indices = [] | |
| for idx in feedback_indices: | |
| if 0 <= idx < len(feedback_list): | |
| feedback_list[idx]["read"] = True | |
| marked_count += 1 | |
| else: | |
| invalid_indices.append(idx) | |
| if invalid_indices: | |
| return { | |
| "content": [ | |
| { | |
| "type": "text", | |
| "text": f"Error: Invalid feedback indices: {invalid_indices}. " | |
| f"Valid range is 0-{len(feedback_list) - 1}", | |
| } | |
| ] | |
| } | |
| # Save updated metadata | |
| with open(metadata_file, "w", encoding="utf-8") as f: | |
| json.dump(metadata, f, indent=2, ensure_ascii=False) |
🤖 Prompt for AI Agents
In `@apps/backend/agents/tools_pkg/tools/feedback.py` around lines 98 - 136, The
file I/O for reading and writing task metadata should explicitly use UTF-8 and
preserve Unicode: update the open(...) calls that use metadata_file in the block
around the read (where metadata = json.load(...)) and the write (where
json.dump(...) is called) to pass encoding="utf-8"; also pass ensure_ascii=False
to json.dump so non-ASCII feedback is preserved. Ensure you only change the open
calls and the json.dump call in this function handling feedback_list and
marked_count.
Comment on lines
+39
to
+98
| def get_glab_executable() -> str: | ||
| """Find the glab CLI executable, with platform-specific fallbacks. | ||
| Returns the path to glab executable. On Windows, checks multiple sources: | ||
| 1. shutil.which (if glab is in PATH) | ||
| 2. Common installation locations (scoop, chocolatey) | ||
| Caches the result after first successful find. | ||
| """ | ||
| global _cached_glab_path | ||
|
|
||
| # Return cached result if available | ||
| if _cached_glab_path is not None: | ||
| return _cached_glab_path | ||
|
|
||
| # 1. Try shutil.which (works if glab is in PATH) | ||
| glab_path = shutil.which("glab") | ||
| if glab_path: | ||
| _cached_glab_path = glab_path | ||
| return glab_path | ||
|
|
||
| # 2. Windows-specific: check common installation locations | ||
| if os.name == "nt": | ||
| common_paths = [ | ||
| # Scoop installation | ||
| os.path.expandvars(r"%USERPROFILE%\scoop\shims\glab.exe"), | ||
| # Chocolatey installation | ||
| os.path.expandvars(r"%PROGRAMDATA%\chocolatey\bin\glab.exe"), | ||
| # Manual installation in Program Files | ||
| os.path.expandvars(r"%PROGRAMFILES%\glab\glab.exe"), | ||
| os.path.expandvars(r"%LOCALAPPDATA%\Programs\glab\glab.exe"), | ||
| ] | ||
| for path in common_paths: | ||
| try: | ||
| if os.path.isfile(path): | ||
| _cached_glab_path = path | ||
| return path | ||
| except OSError: | ||
| continue | ||
|
|
||
| # 3. Try 'where' command with shell=True (more reliable on Windows) | ||
| try: | ||
| result = subprocess.run( | ||
| "where glab", | ||
| capture_output=True, | ||
| text=True, | ||
| timeout=5, | ||
| shell=True, | ||
| ) | ||
| if result.returncode == 0 and result.stdout.strip(): | ||
| found_path = result.stdout.strip().split("\n")[0].strip() | ||
| if found_path and os.path.isfile(found_path): | ||
| _cached_glab_path = found_path | ||
| return found_path | ||
| except (subprocess.TimeoutExpired, OSError): | ||
| pass | ||
|
|
||
| # Default fallback - let subprocess handle it (may fail) | ||
| _cached_glab_path = "glab" | ||
| return "glab" |
Contributor
There was a problem hiding this comment.
Use the platform abstraction module for executable discovery (avoid hardcoded Windows paths).
This helper directly checks os.name and hardcodes Windows install paths, which conflicts with the backend platform abstraction requirements and the “no hardcoded platform paths” rule. Please route discovery through the platform abstraction module’s executable/path helpers instead. As per coding guidelines, ...
🤖 Prompt for AI Agents
In `@apps/backend/core/worktree.py` around lines 39 - 98, The get_glab_executable
function currently uses os.name and hardcoded Windows paths; replace that
platform-specific probing with the project platform-abstraction module’s
executable/path helper (call that helper to locate "glab" instead of using
os.name, common_paths, and subprocess.where), keep caching to _cached_glab_path
and return its value when found, and retain the final fallback of "glab" if the
abstraction helper fails; update get_glab_executable to call the abstraction
helper (e.g., platform_abstraction.find_executable or the repo's provided
which/find API), remove the os.name block and hardcoded paths/subprocess.run
logic, and ensure exceptions are handled and the result is cached in
_cached_glab_path before returning.
Comment on lines
+1184
to
+1192
| result = subprocess.run( | ||
| glab_args, | ||
| cwd=info.path, | ||
| capture_output=True, | ||
| text=True, | ||
| encoding="utf-8", | ||
| errors="replace", | ||
| timeout=30, # Same timeout as GitHub | ||
| ) |
Contributor
There was a problem hiding this comment.
Align GitLab MR timeout with the GitHub CLI timeout (or fix the comment).
The comment says “Same timeout as GitHub,” but GH_CLI_TIMEOUT is 60 while this uses 30. Prefer using the shared constant to avoid drift.
🛠️ Suggested fix
- timeout=30, # Same timeout as GitHub
+ timeout=self.GH_CLI_TIMEOUT,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| result = subprocess.run( | |
| glab_args, | |
| cwd=info.path, | |
| capture_output=True, | |
| text=True, | |
| encoding="utf-8", | |
| errors="replace", | |
| timeout=30, # Same timeout as GitHub | |
| ) | |
| result = subprocess.run( | |
| glab_args, | |
| cwd=info.path, | |
| capture_output=True, | |
| text=True, | |
| encoding="utf-8", | |
| errors="replace", | |
| timeout=self.GH_CLI_TIMEOUT, | |
| ) |
🤖 Prompt for AI Agents
In `@apps/backend/core/worktree.py` around lines 1184 - 1192, The subprocess.run
call that executes glab (identified by the glab_args usage) hardcodes timeout=30
while the module uses the shared GH_CLI_TIMEOUT constant for GitHub CLI; change
the literal 30 to GH_CLI_TIMEOUT so the GitLab MR timeout is aligned with
GH_CLI_TIMEOUT, and ensure GH_CLI_TIMEOUT is in scope (import or reference the
module-level constant) where subprocess.run is invoked.
Comment on lines
+9
to
+49
| ## CRITICAL REQUIREMENT: SUBTASKS ARE MANDATORY | ||
|
|
||
| **YOUR PLAN MUST INCLUDE AT LEAST ONE PHASE WITH AT LEAST ONE SUBTASK.** | ||
|
|
||
| **This is NOT optional.** An empty implementation plan will cause the build to fail immediately. The validation system will reject your plan if: | ||
| - You create zero phases | ||
| - You create phases with zero subtasks | ||
| - Your phases array is empty | ||
|
|
||
| **If you cannot determine detailed subtasks**, create a minimal plan with basic work units: | ||
| - At minimum: "Research", "Implementation", "Testing" subtasks | ||
| - Break work down into the smallest units you can identify | ||
| - Use the existing codebase patterns to guide your breakdown | ||
| - When in doubt, create MORE subtasks rather than fewer | ||
|
|
||
| **Example MINIMUM acceptable plan:** | ||
|
|
||
| ```json | ||
| { | ||
| "feature": "Feature name", | ||
| "workflow_type": "feature", | ||
| "phases": [ | ||
| { | ||
| "id": "phase-1", | ||
| "name": "Implementation", | ||
| "type": "implementation", | ||
| "subtasks": [ | ||
| { | ||
| "id": "1.1", | ||
| "description": "Implement the core feature", | ||
| "service": "backend", | ||
| "status": "pending" | ||
| } | ||
| ] | ||
| } | ||
| ] | ||
| } | ||
| ``` | ||
|
|
||
| **NEVER create an empty plan. The build will fail and you will waste tokens.** | ||
|
|
Contributor
There was a problem hiding this comment.
Resolve SIMPLE workflow vs mandatory phase requirement.
The new requirement mandates at least one phase with subtasks (Line 9-49), but the SIMPLE workflow explicitly says “just subtasks, no phases” (Line 241-244). This makes SIMPLE workflow plans invalid by definition. Please align these rules (e.g., allow a single-phase SIMPLE plan or exempt SIMPLE from the mandatory-phase check).
Also applies to: 241-244
🤖 Prompt for AI Agents
In `@apps/backend/prompts/planner.md` around lines 9 - 49, The docs/validator are
inconsistent: the global validation requires at least one phase with subtasks
but the "SIMPLE workflow" description says it uses only subtasks (no phases),
making SIMPLE plans invalid; fix by either updating the validation logic to
allow SIMPLE workflow to accept a flat subtasks-only plan (bypass the "phases
array non-empty" check when workflow_type === "SIMPLE") or by changing the
SIMPLE workflow wording/plan generation to produce a single phase wrapper (e.g.,
create a phase with id "phase-1" and move subtasks into it); update the
validator that checks the phases array and the SIMPLE workflow
documentation/plan generator to be consistent so that either SIMPLE is exempted
from the phase requirement or SIMPLE produces one phase containing the subtasks.
Comment on lines
+246
to
+283
| "title": "Configuration de la tache", | ||
| "descriptionRunning": "Configurez le modele et les niveaux de reflexion par phase. Les modifications s'appliqueront aux phases suivantes.", | ||
| "descriptionNotRunning": "Configurez le modele et les niveaux de reflexion par phase. Les modifications sont enregistrees automatiquement.", | ||
| "taskRunningWarning": "Tache en cours (phase {{phase}})", | ||
| "changesApplyFrom": "Les modifications s'appliqueront a partir de la phase {{phase}}", | ||
| "currentPhaseComplete": "La phase actuelle se terminera avec les parametres d'origine", | ||
| "pendingChanges": "Modifications en attente", | ||
| "pendingChangesHint": "Cliquez sur \"Appliquer les modifications\" pour enregistrer. Les modifications prendront effet a la prochaine phase.", | ||
| "profile": "Profil : {{name}}", | ||
| "resetToDefaults": "Reinitialiser aux valeurs par defaut", | ||
| "customProfile": "Personnalise", | ||
| "customProfileDescription": "Configuration par phase", | ||
| "defaultProfile": "Par defaut", | ||
| "defaultProfileDescription": "Utilisation des valeurs par defaut du systeme", | ||
| "phaseConfiguration": "Configuration des phases", | ||
| "phaseConfigurationHint": "Ajustez le modele et l'effort de reflexion pour chaque phase. Des niveaux de reflexion plus eleves utilisent plus de jetons mais fournissent une analyse plus approfondie.", | ||
| "phases": { | ||
| "spec": "Spec", | ||
| "planning": "Planification", | ||
| "coding": "Codage", | ||
| "qa": "QA" | ||
| }, | ||
| "phaseLabel": "Phase {{phase}}", | ||
| "currentPhase": "Actuelle", | ||
| "willApplyChanges": "Appliquera les modifications", | ||
| "model": "Modele", | ||
| "thinkingLevel": "Niveau de reflexion", | ||
| "thinkingLabels": { | ||
| "none": "Aucun", | ||
| "low": "Faible", | ||
| "medium": "Moyen", | ||
| "high": "Eleve", | ||
| "ultra": "Ultra" | ||
| }, | ||
| "tokens": "{{count}} jetons", | ||
| "tokensK": "{{count}}K jetons", | ||
| "applyChanges": "Appliquer les modifications", | ||
| "cancel": "Annuler" |
Contributor
There was a problem hiding this comment.
Fix missing French accents/diacritics in the new configuration strings.
Several new UI labels omit accents (tâche, modèle, réflexion, paramètres, défaut, etc.), which reads unpolished in the UI. Consider updating to standard French spelling.
✍️ Suggested corrections
- "title": "Configuration de la tache",
- "descriptionRunning": "Configurez le modele et les niveaux de reflexion par phase. Les modifications s'appliqueront aux phases suivantes.",
- "descriptionNotRunning": "Configurez le modele et les niveaux de reflexion par phase. Les modifications sont enregistrees automatiquement.",
- "taskRunningWarning": "Tache en cours (phase {{phase}})",
- "changesApplyFrom": "Les modifications s'appliqueront a partir de la phase {{phase}}",
- "currentPhaseComplete": "La phase actuelle se terminera avec les parametres d'origine",
+ "title": "Configuration de la tâche",
+ "descriptionRunning": "Configurez le modèle et les niveaux de réflexion par phase. Les modifications s'appliqueront aux phases suivantes.",
+ "descriptionNotRunning": "Configurez le modèle et les niveaux de réflexion par phase. Les modifications sont enregistrées automatiquement.",
+ "taskRunningWarning": "Tâche en cours (phase {{phase}})",
+ "changesApplyFrom": "Les modifications s'appliqueront à partir de la phase {{phase}}",
+ "currentPhaseComplete": "La phase actuelle se terminera avec les paramètres d'origine",
@@
- "resetToDefaults": "Reinitialiser aux valeurs par defaut",
- "customProfile": "Personnalise",
- "defaultProfile": "Par defaut",
- "defaultProfileDescription": "Utilisation des valeurs par defaut du systeme",
+ "resetToDefaults": "Réinitialiser aux valeurs par défaut",
+ "customProfile": "Personnalisé",
+ "defaultProfile": "Par défaut",
+ "defaultProfileDescription": "Utilisation des valeurs par défaut du système",
@@
- "phaseConfigurationHint": "Ajustez le modele et l'effort de reflexion pour chaque phase. Des niveaux de reflexion plus eleves utilisent plus de jetons mais fournissent une analyse plus approfondie.",
+ "phaseConfigurationHint": "Ajustez le modèle et l'effort de réflexion pour chaque phase. Des niveaux de réflexion plus élevés utilisent plus de jetons mais fournissent une analyse plus approfondie.",
@@
- "model": "Modele",
- "thinkingLevel": "Niveau de reflexion",
+ "model": "Modèle",
+ "thinkingLevel": "Niveau de réflexion",
@@
- "high": "Eleve",
+ "high": "Élevé",🤖 Prompt for AI Agents
In `@apps/frontend/src/shared/i18n/locales/fr/tasks.json` around lines 246 - 283,
Several French strings in the tasks locale are missing accents; update the
values for keys like "title", "descriptionRunning", "descriptionNotRunning",
"taskRunningWarning", "changesApplyFrom", "currentPhaseComplete",
"pendingChanges", "pendingChangesHint", "profile", "resetToDefaults",
"customProfile", "customProfileDescription", "defaultProfile",
"defaultProfileDescription", "phaseConfiguration", "phaseConfigurationHint",
"phaseLabel", "model", "thinkingLevel", the entries under "thinkingLabels"
("none","low","medium","high","ultra"), and any other affected keys to use
proper French diacritics (e.g., tâche, modèle, réflexion, paramètres, défaut,
personnalisé, réinitialiser) so the UI reads correctly; locate and replace the
affected string literals in apps/frontend/src/shared/i18n/locales/fr/tasks.json
accordingly.
Comment on lines
34
to
+39
| export function escapeShellArg(arg: string): string { | ||
| // Replace single quotes with: end quote, escaped quote, start quote | ||
| if (process.platform === 'win32') { | ||
| // PowerShell: escape single quotes by doubling them | ||
| const escaped = arg.replace(/'/g, "''"); | ||
| return `'${escaped}'`; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/frontend/src/shared/utils/shell-escape.tsRepository: AndyMik90/Auto-Claude
Length of output: 10966
🏁 Script executed:
# Find the platform abstraction module
fd -t f "platform" apps/frontend/src --type f | head -20Repository: AndyMik90/Auto-Claude
Length of output: 160
🏁 Script executed:
# Search for escapeShellArg usage
rg -n "escapeShellArg" --type ts --type tsx -C 2Repository: AndyMik90/Auto-Claude
Length of output: 92
🏁 Script executed:
# Look for platform module exports and available functions
find apps/frontend/src -name "*platform*" -type fRepository: AndyMik90/Auto-Claude
Length of output: 160
🏁 Script executed:
# Search for escapeShellArg call sites with context
rg -n "escapeShellArg\(" --type ts -C 3Repository: AndyMik90/Auto-Claude
Length of output: 16605
🏁 Script executed:
# Check the platform module to confirm available functions
cat -n apps/frontend/src/shared/platform.tsRepository: AndyMik90/Auto-Claude
Length of output: 2080
🏁 Script executed:
# Look for any cmd.exe usage patterns in the codebase
rg -n "cmd.exe|cmd\.exe|shellType.*cmd" --type ts -C 2Repository: AndyMik90/Auto-Claude
Length of output: 25538
Replace direct process.platform check with platform abstraction.
The direct process.platform === 'win32' check at line 35 violates the platform abstraction guideline. Switch to isWindows() (already imported from ../platform) for consistency with the codebase standards.
The function is well-designed for its purpose—it provides platform-aware escaping appropriate to the runtime environment. Current usage is safe: all Unix/macOS call sites are in explicit else branches, and file drop insertion works correctly cross-platform. However, if new call sites are added in cmd.exe contexts, route them to escapeShellArgWindows() or buildCdCommand() with appropriate shellType parameter, which already handle shell-specific escaping.
Fix
- if (process.platform === 'win32') {
+ if (isWindows()) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function escapeShellArg(arg: string): string { | |
| // Replace single quotes with: end quote, escaped quote, start quote | |
| if (process.platform === 'win32') { | |
| // PowerShell: escape single quotes by doubling them | |
| const escaped = arg.replace(/'/g, "''"); | |
| return `'${escaped}'`; | |
| } | |
| export function escapeShellArg(arg: string): string { | |
| if (isWindows()) { | |
| // PowerShell: escape single quotes by doubling them | |
| const escaped = arg.replace(/'/g, "''"); | |
| return `'${escaped}'`; | |
| } |
🤖 Prompt for AI Agents
In `@apps/frontend/src/shared/utils/shell-escape.ts` around lines 34 - 39, The
platform check in escapeShellArg currently uses process.platform === 'win32';
replace that conditional with the isWindows() helper (imported from ../platform)
to follow the platform abstraction guideline, i.e., call isWindows() inside
escapeShellArg and keep the existing Windows escaping logic intact; ensure the
file imports isWindows and do not change behavior for cmd.exe contexts—route
those to escapeShellArgWindows() or buildCdCommand(shellType) if/when added.
Comment on lines
+268
to
+275
| - **GitHub**: Requires `gh` CLI installed and authenticated | ||
| - Install: `brew install gh` (macOS), `scoop install gh` (Windows), `sudo apt install gh` (Linux) | ||
| - Authenticate: `gh auth login` | ||
| - More info: https://cli.github.com/ | ||
| - **GitLab**: Requires `glab` CLI installed and authenticated | ||
| - Install: `brew install glab` (macOS), `scoop install glab` (Windows) | ||
| - Download: https://gitlab.com/gitlab-org/cli/-/releases | ||
| - Authenticate: `glab auth login` |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use Markdown link syntax for URLs.
The bare URLs should be converted to proper Markdown links for consistency with documentation standards.
📝 Suggested fix
- **GitHub**: Requires `gh` CLI installed and authenticated
- Install: `brew install gh` (macOS), `scoop install gh` (Windows), `sudo apt install gh` (Linux)
- Authenticate: `gh auth login`
- - More info: https://cli.github.com/
+ - More info: [GitHub CLI](https://cli.github.com/)
- **GitLab**: Requires `glab` CLI installed and authenticated
- Install: `brew install glab` (macOS), `scoop install glab` (Windows)
- - Download: https://gitlab.com/gitlab-org/cli/-/releases
+ - Download: [GitLab CLI releases](https://gitlab.com/gitlab-org/cli/-/releases)
- Authenticate: `glab auth login`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - **GitHub**: Requires `gh` CLI installed and authenticated | |
| - Install: `brew install gh` (macOS), `scoop install gh` (Windows), `sudo apt install gh` (Linux) | |
| - Authenticate: `gh auth login` | |
| - More info: https://cli.github.com/ | |
| - **GitLab**: Requires `glab` CLI installed and authenticated | |
| - Install: `brew install glab` (macOS), `scoop install glab` (Windows) | |
| - Download: https://gitlab.com/gitlab-org/cli/-/releases | |
| - Authenticate: `glab auth login` | |
| - **GitHub**: Requires `gh` CLI installed and authenticated | |
| - Install: `brew install gh` (macOS), `scoop install gh` (Windows), `sudo apt install gh` (Linux) | |
| - Authenticate: `gh auth login` | |
| - More info: [GitHub CLI](https://cli.github.com/) | |
| - **GitLab**: Requires `glab` CLI installed and authenticated | |
| - Install: `brew install glab` (macOS), `scoop install glab` (Windows) | |
| - Download: [GitLab CLI releases](https://gitlab.com/gitlab-org/cli/-/releases) | |
| - Authenticate: `glab auth login` |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
271-271: Bare URL used
(MD034, no-bare-urls)
274-274: Bare URL used
(MD034, no-bare-urls)
🤖 Prompt for AI Agents
In `@CLAUDE.md` around lines 268 - 275, Replace the bare URLs under the "GitHub"
and "GitLab" sections with Markdown link syntax: change "More info:
https://cli.github.com/" to "More info: [GitHub CLI](https://cli.github.com/)"
(or similar descriptive text) and change "Download:
https://gitlab.com/gitlab-org/cli/-/releases" to "Download: [GitLab CLI
releases](https://gitlab.com/gitlab-org/cli/-/releases)"; keep the surrounding
bullets and commands intact and apply the same Markdown link style for any other
bare URLs in those "GitHub" and "GitLab" lines.
Comment on lines
+1
to
+3
| ## Summary | ||
|
|
||
| This PR implements comprehensive Conda environment management for Auto Claude, enabling isolated Python environments at both application and project levels. It also includes significant Windows/PowerShell compatibility fixes to ensure reliable terminal integration across platforms. |
Contributor
There was a problem hiding this comment.
Use an H1 as the first heading.
Markdownlint MD041: first line should be a top-level heading.
✏️ Suggested fix
-## Summary
+# Summary📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ## Summary | |
| This PR implements comprehensive Conda environment management for Auto Claude, enabling isolated Python environments at both application and project levels. It also includes significant Windows/PowerShell compatibility fixes to ensure reliable terminal integration across platforms. | |
| # Summary | |
| This PR implements comprehensive Conda environment management for Auto Claude, enabling isolated Python environments at both application and project levels. It also includes significant Windows/PowerShell compatibility fixes to ensure reliable terminal integration across platforms. |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🤖 Prompt for AI Agents
In `@PR_DESCRIPTION.md` around lines 1 - 3, Change the first heading from a
level-2 to a level-1 heading so the file starts with a top-level heading;
replace the existing "## Summary" line with "# Summary" (search for the "##
Summary" heading in PR_DESCRIPTION.md and update it to "# Summary") to satisfy
Markdownlint MD041.
Comment on lines
+36
to
+49
| ### New Files (11) | ||
| - `conda-detector.ts` - Conda installation detection service | ||
| - `conda-env-manager.ts` - Environment creation and management | ||
| - `conda-workspace-generator.ts` - VS Code workspace file generation | ||
| - `conda-project-structure.ts` - Project structure detection (pure-python vs mixed) | ||
| - `conda-handlers.ts` - IPC handlers for conda operations | ||
| - `conda-api.ts` - Preload API for renderer access | ||
| - `PythonEnvSettings.tsx` - Project-level Python environment settings UI | ||
| - `CondaSetupWizard.tsx` - Multi-step setup wizard with progress | ||
| - `CondaDetectionDisplay.tsx` - Conda detection status display | ||
| - `useCondaSetup.ts` - React hook for setup progress | ||
| - `conda.ts` - TypeScript type definitions | ||
|
|
||
| ### Modified Files (48) |
Contributor
There was a problem hiding this comment.
Add blank lines around section headings.
Markdownlint MD022 expects blank lines before/after headings.
✏️ Suggested fix
-### New Files (11)
+### New Files (11)
...
-### Modified Files (48)
+### Modified Files (48)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ### New Files (11) | |
| - `conda-detector.ts` - Conda installation detection service | |
| - `conda-env-manager.ts` - Environment creation and management | |
| - `conda-workspace-generator.ts` - VS Code workspace file generation | |
| - `conda-project-structure.ts` - Project structure detection (pure-python vs mixed) | |
| - `conda-handlers.ts` - IPC handlers for conda operations | |
| - `conda-api.ts` - Preload API for renderer access | |
| - `PythonEnvSettings.tsx` - Project-level Python environment settings UI | |
| - `CondaSetupWizard.tsx` - Multi-step setup wizard with progress | |
| - `CondaDetectionDisplay.tsx` - Conda detection status display | |
| - `useCondaSetup.ts` - React hook for setup progress | |
| - `conda.ts` - TypeScript type definitions | |
| ### Modified Files (48) | |
| ### New Files (11) | |
| - `conda-detector.ts` - Conda installation detection service | |
| - `conda-env-manager.ts` - Environment creation and management | |
| - `conda-project-structure.ts` - Project structure detection (pure-python vs mixed) | |
| - `conda-handlers.ts` - IPC handlers for conda operations | |
| - `conda-api.ts` - Preload API for renderer access | |
| - `PythonEnvSettings.tsx` - Project-level Python environment settings UI | |
| - `CondaSetupWizard.tsx` - Multi-step setup wizard with progress | |
| - `CondaDetectionDisplay.tsx` - Conda detection status display | |
| - `useCondaSetup.ts` - React hook for setup progress | |
| - `conda.ts` - TypeScript type definitions | |
| ### Modified Files (48) | |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
36-36: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
49-49: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
In `@PR_DESCRIPTION.md` around lines 36 - 49, The PR_DESCRIPTION.md headings lack
surrounding blank lines required by markdownlint MD022; add a blank line before
and after each top-level and subsection heading (e.g., "New Files (11)", the
list block beneath it, and "Modified Files (48)") so each heading is separated
from adjacent text or lists, ensuring every heading has an empty line above and
below throughout the file.
Comment on lines
+135
to
+141
| async def fetch_prs(self, filters: PRFilters | None = None) -> list[PRData]: | ||
| """ | ||
| Fetch merge requests with optional filters. | ||
| """ | ||
| # For now, return empty list - would need to implement list_mr in glab_client | ||
| # This is a placeholder for the full implementation | ||
| return [] |
Contributor
There was a problem hiding this comment.
Avoid silently returning an empty list for fetch_prs.
Returning [] will hide merge requests and look like there are none. If this isn't implemented yet, raise NotImplementedError (or implement list_mr) so callers can handle it explicitly.
🛠️ Suggested change
- return []
+ raise NotImplementedError("fetch_prs not yet implemented for GitLab")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async def fetch_prs(self, filters: PRFilters | None = None) -> list[PRData]: | |
| """ | |
| Fetch merge requests with optional filters. | |
| """ | |
| # For now, return empty list - would need to implement list_mr in glab_client | |
| # This is a placeholder for the full implementation | |
| return [] | |
| async def fetch_prs(self, filters: PRFilters | None = None) -> list[PRData]: | |
| """ | |
| Fetch merge requests with optional filters. | |
| """ | |
| # For now, return empty list - would need to implement list_mr in glab_client | |
| # This is a placeholder for the full implementation | |
| raise NotImplementedError("fetch_prs not yet implemented for GitLab") |
🤖 Prompt for AI Agents
In `@apps/backend/runners/gitlab/providers/gitlab_provider.py` around lines 135 -
141, The fetch_prs method currently silently returns an empty list which hides
real merge requests; update the fetch_prs function to raise NotImplementedError
(or implement and call glab_client.list_mr) instead of returning [] so callers
can explicitly handle the unimplemented behavior; reference the async method
fetch_prs and the potential glab_client.list_mr integration when making the
change.
Comment on lines
+217
to
+231
| async def close_pr( | ||
| self, | ||
| pr_number: int, | ||
| comment: str | None = None, | ||
| ) -> bool: | ||
| """ | ||
| Close a merge request without merging. | ||
| GitLab doesn't have a direct close endpoint - would need to add to glab_client. | ||
| """ | ||
| if comment: | ||
| self.glab_client.post_mr_note(pr_number, comment) | ||
| # TODO: Implement MR closing in glab_client | ||
| return False | ||
|
|
Contributor
There was a problem hiding this comment.
close_pr is effectively unimplemented.
It always returns False even after posting a note, which can be mistaken for a real failure. Prefer raising NotImplementedError until a close endpoint exists, or wire it through glab_client.
🛠️ Suggested change
- if comment:
- self.glab_client.post_mr_note(pr_number, comment)
- # TODO: Implement MR closing in glab_client
- return False
+ if comment:
+ self.glab_client.post_mr_note(pr_number, comment)
+ # TODO: Implement MR closing in glab_client
+ raise NotImplementedError("close_pr not yet implemented for GitLab")🤖 Prompt for AI Agents
In `@apps/backend/runners/gitlab/providers/gitlab_provider.py` around lines 217 -
231, The close_pr method currently always returns False after optionally posting
a note, which misreports failure; update close_pr in gitlab_provider.py to
either raise NotImplementedError to signal it’s not implemented (instead of
returning False) or implement proper MR closing by delegating to a glab_client
method (e.g., call a new glab_client.close_mr(pr_number) or similar) and return
its success boolean; ensure any posted note still occurs before
raising/returning and reference close_pr and
glab_client.post_mr_note/glab_client.close_mr when making the change.
Comment on lines
+286
to
+315
| async def apply_labels( | ||
| self, | ||
| issue_or_pr_number: int, | ||
| labels: list[str], | ||
| ) -> None: | ||
| """Apply labels to an issue or MR.""" | ||
| # TODO: Implement label operations in glab_client | ||
| pass | ||
|
|
||
| async def remove_labels( | ||
| self, | ||
| issue_or_pr_number: int, | ||
| labels: list[str], | ||
| ) -> None: | ||
| """Remove labels from an issue or MR.""" | ||
| # TODO: Implement label operations in glab_client | ||
| pass | ||
|
|
||
| async def create_label( | ||
| self, | ||
| label: LabelData, | ||
| ) -> None: | ||
| """Create a label in the repository.""" | ||
| # TODO: Implement label operations in glab_client | ||
| pass | ||
|
|
||
| async def list_labels(self) -> list[LabelData]: | ||
| """List all labels in the repository.""" | ||
| # TODO: Implement label operations in glab_client | ||
| return [] |
Contributor
There was a problem hiding this comment.
Label operations are no-ops—prefer explicit NotImplemented.
apply_labels, remove_labels, create_label, and list_labels currently succeed without doing anything, which can mislead callers. Raise NotImplementedError until the client support exists.
🛠️ Suggested change
- # TODO: Implement label operations in glab_client
- pass
+ # TODO: Implement label operations in glab_client
+ raise NotImplementedError("Label operations not yet implemented for GitLab")
...
- # TODO: Implement label operations in glab_client
- pass
+ # TODO: Implement label operations in glab_client
+ raise NotImplementedError("Label operations not yet implemented for GitLab")
...
- # TODO: Implement label operations in glab_client
- pass
+ # TODO: Implement label operations in glab_client
+ raise NotImplementedError("Label operations not yet implemented for GitLab")
...
- # TODO: Implement label operations in glab_client
- return []
+ # TODO: Implement label operations in glab_client
+ raise NotImplementedError("Label operations not yet implemented for GitLab")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async def apply_labels( | |
| self, | |
| issue_or_pr_number: int, | |
| labels: list[str], | |
| ) -> None: | |
| """Apply labels to an issue or MR.""" | |
| # TODO: Implement label operations in glab_client | |
| pass | |
| async def remove_labels( | |
| self, | |
| issue_or_pr_number: int, | |
| labels: list[str], | |
| ) -> None: | |
| """Remove labels from an issue or MR.""" | |
| # TODO: Implement label operations in glab_client | |
| pass | |
| async def create_label( | |
| self, | |
| label: LabelData, | |
| ) -> None: | |
| """Create a label in the repository.""" | |
| # TODO: Implement label operations in glab_client | |
| pass | |
| async def list_labels(self) -> list[LabelData]: | |
| """List all labels in the repository.""" | |
| # TODO: Implement label operations in glab_client | |
| return [] | |
| async def apply_labels( | |
| self, | |
| issue_or_pr_number: int, | |
| labels: list[str], | |
| ) -> None: | |
| """Apply labels to an issue or MR.""" | |
| # TODO: Implement label operations in glab_client | |
| raise NotImplementedError("Label operations not yet implemented for GitLab") | |
| async def remove_labels( | |
| self, | |
| issue_or_pr_number: int, | |
| labels: list[str], | |
| ) -> None: | |
| """Remove labels from an issue or MR.""" | |
| # TODO: Implement label operations in glab_client | |
| raise NotImplementedError("Label operations not yet implemented for GitLab") | |
| async def create_label( | |
| self, | |
| label: LabelData, | |
| ) -> None: | |
| """Create a label in the repository.""" | |
| # TODO: Implement label operations in glab_client | |
| raise NotImplementedError("Label operations not yet implemented for GitLab") | |
| async def list_labels(self) -> list[LabelData]: | |
| """List all labels in the repository.""" | |
| # TODO: Implement label operations in glab_client | |
| raise NotImplementedError("Label operations not yet implemented for GitLab") |
🤖 Prompt for AI Agents
In `@apps/backend/runners/gitlab/providers/gitlab_provider.py` around lines 286 -
315, The four label-related methods (apply_labels, remove_labels, create_label,
list_labels) are currently no-ops; update each to raise NotImplementedError with
a clear message (e.g., "Label operations not implemented: requires glab_client
support") so callers are not misled—replace the pass/empty return in
apply_labels, remove_labels, create_label, and list_labels with raising
NotImplementedError (include the function name or brief context in the error
string).
Comment on lines
+321
to
+334
| async def get_repository_info(self) -> dict[str, Any]: | ||
| """Get repository information.""" | ||
| # TODO: Implement in glab_client | ||
| return {} | ||
|
|
||
| async def get_default_branch(self) -> str: | ||
| """Get the default branch name.""" | ||
| # TODO: Implement in glab_client | ||
| return "main" | ||
|
|
||
| async def check_permissions(self, username: str) -> str: | ||
| """Check a user's permission level on the repository.""" | ||
| # TODO: Implement in glab_client | ||
| return "read" |
Contributor
There was a problem hiding this comment.
Repository info/default branch/permissions placeholders can mislead callers.
Returning {}, "main", or "read" may mask missing implementations. Prefer explicit NotImplemented (or implement via glab_client) so callers can handle the gap.
🤖 Prompt for AI Agents
In `@apps/backend/runners/gitlab/providers/gitlab_provider.py` around lines 321 -
334, The three placeholder methods get_repository_info, get_default_branch, and
check_permissions should not return misleading defaults; update each to raise a
NotImplementedError (or call the real implementation on glab_client) so callers
detect the missing implementation: replace the current return {} in
get_repository_info, "main" in get_default_branch, and "read" in
check_permissions with raising NotImplementedError (or delegate to glab_client
methods if available) and include a brief message referencing the method name
for clarity.
Comment on lines
+35
to
+70
| const CONDA_SEARCH_PATHS: Record<string, string[]> = { | ||
| win32: [ | ||
| path.join(os.homedir(), 'miniconda3'), | ||
| path.join(os.homedir(), 'Miniconda3'), | ||
| 'C:\\miniconda3', | ||
| 'C:\\Miniconda3', | ||
| path.join(os.homedir(), 'anaconda3'), | ||
| path.join(os.homedir(), 'Anaconda3'), | ||
| 'C:\\anaconda3', | ||
| 'C:\\Anaconda3', | ||
| 'C:\\ProgramData\\miniconda3', | ||
| 'C:\\ProgramData\\Anaconda3', | ||
| path.join(os.homedir(), 'mambaforge'), | ||
| path.join(os.homedir(), 'miniforge3'), | ||
| // Only include LOCALAPPDATA path if the env var is defined (prevents relative path search) | ||
| ...(process.env.LOCALAPPDATA ? [path.join(process.env.LOCALAPPDATA, 'miniconda3')] : []), | ||
| ], | ||
| darwin: [ | ||
| path.join(os.homedir(), 'miniconda3'), | ||
| path.join(os.homedir(), 'anaconda3'), | ||
| path.join(os.homedir(), 'mambaforge'), | ||
| path.join(os.homedir(), 'miniforge3'), | ||
| '/opt/miniconda3', | ||
| '/opt/anaconda3', | ||
| '/opt/homebrew/Caskroom/miniconda/base', | ||
| '/usr/local/Caskroom/miniconda/base', | ||
| ], | ||
| linux: [ | ||
| path.join(os.homedir(), 'miniconda3'), | ||
| path.join(os.homedir(), 'anaconda3'), | ||
| path.join(os.homedir(), 'mambaforge'), | ||
| path.join(os.homedir(), 'miniforge3'), | ||
| '/opt/conda', | ||
| '/opt/miniconda3', | ||
| '/opt/anaconda3', | ||
| ], |
Contributor
There was a problem hiding this comment.
Replace hardcoded platform paths with platform-aware discovery.
The absolute path list hardcodes OS-specific locations (e.g., C:\..., /opt/..., /usr/local/...), which violates the repo rule against hardcoded platform paths and risks missing custom installs. Prefer deriving roots from env vars (HOME/PROGRAMDATA/LOCALAPPDATA) or using the platform abstraction’s executable discovery to infer the base path. As per coding guidelines, avoid hardcoded platform-specific paths.
🤖 Prompt for AI Agents
In `@apps/frontend/src/main/conda-detector.ts` around lines 35 - 70,
CONDA_SEARCH_PATHS currently includes hardcoded OS-specific absolute paths;
replace them with platform-aware discovery by deriving candidate roots from
environment variables (e.g., process.env.HOME, process.env.PROGRAMDATA,
process.env.LOCALAPPDATA) and by probing for conda/miniconda executables (use a
platform executable-finder or spawn which/where) to infer base install
directories instead of literals; update the CONDA_SEARCH_PATHS construction (and
the LOCALAPPDATA spread) to build paths from those discovered roots and fallback
to only non-hardcoded common locations so detection works across custom installs
and platforms.
Comment on lines
+41
to
+126
| const { t } = useTranslation('settings'); | ||
|
|
||
| // Loading state | ||
| if (isLoading) { | ||
| return ( | ||
| <div className="text-sm text-muted-foreground flex items-center gap-2"> | ||
| <RefreshCw className="h-4 w-4 animate-spin" /> | ||
| <span>{t('python.detecting')}</span> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| // Not found state | ||
| if (!detection || !detection.found || !detection.preferred) { | ||
| return ( | ||
| <div className="space-y-3"> | ||
| <div className="flex items-center gap-2 text-amber-500"> | ||
| <AlertTriangle className="h-4 w-4" /> | ||
| <span className="text-sm">{t('python.notDetected')}</span> | ||
| </div> | ||
|
|
||
| <div className="flex items-center gap-2"> | ||
| <Button | ||
| variant="outline" | ||
| size="sm" | ||
| onClick={openMinicondaInstallPage} | ||
| className="gap-1.5" | ||
| > | ||
| {t('python.installMiniconda')} | ||
| <ExternalLink className="h-3.5 w-3.5" /> | ||
| </Button> | ||
| <Button | ||
| variant="outline" | ||
| size="sm" | ||
| onClick={onRefresh} | ||
| className="gap-1.5" | ||
| > | ||
| <RefreshCw className="h-3.5 w-3.5" /> | ||
| {t('python.refresh')} | ||
| </Button> | ||
| </div> | ||
|
|
||
| {onManualPathChange && ( | ||
| <div className="flex items-center gap-2"> | ||
| <span className="text-sm text-muted-foreground whitespace-nowrap"> | ||
| {t('python.specifyPath')}: | ||
| </span> | ||
| <Input | ||
| value={manualPath || ''} | ||
| onChange={(e) => onManualPathChange(e.target.value)} | ||
| placeholder={t('python.pathPlaceholder')} | ||
| className="flex-1 h-8 text-sm" | ||
| /> | ||
| {onBrowse && ( | ||
| <Button | ||
| variant="outline" | ||
| size="sm" | ||
| onClick={onBrowse} | ||
| className="gap-1.5" | ||
| > | ||
| <FolderOpen className="h-3.5 w-3.5" /> | ||
| {t('python.browse')} | ||
| </Button> | ||
| )} | ||
| </div> | ||
| )} | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| // Found state | ||
| const { preferred } = detection; | ||
| const condaType = t(`python.condaTypes.${preferred.type}`, { defaultValue: t('python.condaTypes.unknown') }); | ||
| const versionDisplay = preferred.version ? ` (v${preferred.version})` : ''; | ||
|
|
||
| return ( | ||
| <div className="flex items-center gap-2 text-sm"> | ||
| <CheckCircle2 className="h-4 w-4 text-green-500" /> | ||
| <span className="text-muted-foreground"> | ||
| {t('python.detected')}: | ||
| </span> | ||
| <code className="bg-muted px-1.5 py-0.5 rounded text-xs"> | ||
| {preferred.path} | ||
| </code> | ||
| <span className="text-muted-foreground"> | ||
| {condaType}{versionDisplay} |
Contributor
There was a problem hiding this comment.
Use namespace‑prefixed i18n keys
The new strings use t('python.*') without the required settings: namespace. Please update keys to the namespace:section.key format. As per coding guidelines.
💡 Example update
- <span>{t('python.detecting')}</span>
+ <span>{t('settings:python.detecting')}</span>
...
- {t('python.installMiniconda')}
+ {t('settings:python.installMiniconda')}🤖 Prompt for AI Agents
In `@apps/frontend/src/renderer/components/settings/CondaDetectionDisplay.tsx`
around lines 41 - 126, All t(...) calls in CondaDetectionDisplay must use the
settings namespace; update every occurrence of t('python.*') to
t('settings:python.*') (e.g. t('python.detecting') →
t('settings:python.detecting'), t('python.notDetected') →
t('settings:python.notDetected'), t('python.installMiniconda') →
t('settings:python.installMiniconda'), etc.), and update the dynamic conda type
key and its defaultValue to t(`settings:python.condaTypes.${preferred.type}` , {
defaultValue: t('settings:python.condaTypes.unknown') }); keep usage of
useTranslation('settings') and the rest of the logic in CondaDetectionDisplay,
detection and preferred unchanged.
Comment on lines
+61
to
+120
| const checkPackages = async () => { | ||
| setStatus('checking'); | ||
| setValidationProgress(null); | ||
| setError(''); | ||
|
|
||
| // Listen for validation progress | ||
| const unsubscribe = window.electronAPI.onPythonValidationProgress((progress) => { | ||
| setValidationProgress({ | ||
| current: progress.current, | ||
| total: progress.total, | ||
| packageName: progress.packageName | ||
| }); | ||
| }); | ||
|
|
||
| const result = await window.electronAPI.validatePythonPackages({ | ||
| pythonPath, | ||
| activationScript | ||
| }); | ||
|
|
||
| unsubscribe(); | ||
|
|
||
| if (result.success && result.data) { | ||
| setStatus(result.data.allInstalled ? 'valid' : 'invalid'); | ||
| setMissingPackages(result.data.missingPackages || []); | ||
| setInstallLocation(result.data.installLocation || ''); | ||
| setError(''); | ||
| } else { | ||
| setStatus('invalid'); | ||
| setMissingPackages([]); | ||
| setInstallLocation(''); | ||
| setError(result.error || 'Failed to validate packages. Check that your Python path points to a Python executable (python.exe), not a directory.'); | ||
| } | ||
|
|
||
| setValidationProgress(null); | ||
| }; | ||
|
|
||
| const installRequirements = async () => { | ||
| setInstalling(true); | ||
| setInstallProgress(t('python.installing')); | ||
| setError(''); | ||
|
|
||
| const unsubscribe = window.electronAPI.onPythonInstallProgress((progress) => { | ||
| setInstallProgress(progress); | ||
| }); | ||
|
|
||
| const result = await window.electronAPI.installPythonRequirements({ | ||
| pythonPath, | ||
| activationScript | ||
| }); | ||
|
|
||
| unsubscribe(); | ||
| setInstalling(false); | ||
|
|
||
| if (result.success) { | ||
| setError(''); | ||
| await checkPackages(); | ||
| } else { | ||
| setError(result.error || 'Installation failed. Check the progress output above for details.'); | ||
| } | ||
| }; |
Contributor
There was a problem hiding this comment.
Ensure progress listeners are always cleaned up on failure
If validatePythonPackages or installPythonRequirements throws, unsubscribe() and state resets won’t run, leaving dangling listeners and stuck spinners. Wrap these calls in try/catch/finally and move cleanup into finally.
🐛 Suggested fix
- const unsubscribe = window.electronAPI.onPythonValidationProgress((progress) => {
+ const unsubscribe = window.electronAPI.onPythonValidationProgress((progress) => {
setValidationProgress({
current: progress.current,
total: progress.total,
packageName: progress.packageName
});
});
-
- const result = await window.electronAPI.validatePythonPackages({
- pythonPath,
- activationScript
- });
-
- unsubscribe();
-
- if (result.success && result.data) {
- setStatus(result.data.allInstalled ? 'valid' : 'invalid');
- setMissingPackages(result.data.missingPackages || []);
- setInstallLocation(result.data.installLocation || '');
- setError('');
- } else {
- setStatus('invalid');
- setMissingPackages([]);
- setInstallLocation('');
- setError(result.error || 'Failed to validate packages. Check that your Python path points to a Python executable (python.exe), not a directory.');
- }
-
- setValidationProgress(null);
+ try {
+ const result = await window.electronAPI.validatePythonPackages({
+ pythonPath,
+ activationScript
+ });
+
+ if (result.success && result.data) {
+ setStatus(result.data.allInstalled ? 'valid' : 'invalid');
+ setMissingPackages(result.data.missingPackages || []);
+ setInstallLocation(result.data.installLocation || '');
+ setError('');
+ } else {
+ setStatus('invalid');
+ setMissingPackages([]);
+ setInstallLocation('');
+ setError(result.error || 'Failed to validate packages. Check that your Python path points to a Python executable (python.exe), not a directory.');
+ }
+ } catch (error) {
+ setStatus('invalid');
+ setMissingPackages([]);
+ setInstallLocation('');
+ setError(String(error));
+ } finally {
+ unsubscribe();
+ setValidationProgress(null);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const checkPackages = async () => { | |
| setStatus('checking'); | |
| setValidationProgress(null); | |
| setError(''); | |
| // Listen for validation progress | |
| const unsubscribe = window.electronAPI.onPythonValidationProgress((progress) => { | |
| setValidationProgress({ | |
| current: progress.current, | |
| total: progress.total, | |
| packageName: progress.packageName | |
| }); | |
| }); | |
| const result = await window.electronAPI.validatePythonPackages({ | |
| pythonPath, | |
| activationScript | |
| }); | |
| unsubscribe(); | |
| if (result.success && result.data) { | |
| setStatus(result.data.allInstalled ? 'valid' : 'invalid'); | |
| setMissingPackages(result.data.missingPackages || []); | |
| setInstallLocation(result.data.installLocation || ''); | |
| setError(''); | |
| } else { | |
| setStatus('invalid'); | |
| setMissingPackages([]); | |
| setInstallLocation(''); | |
| setError(result.error || 'Failed to validate packages. Check that your Python path points to a Python executable (python.exe), not a directory.'); | |
| } | |
| setValidationProgress(null); | |
| }; | |
| const installRequirements = async () => { | |
| setInstalling(true); | |
| setInstallProgress(t('python.installing')); | |
| setError(''); | |
| const unsubscribe = window.electronAPI.onPythonInstallProgress((progress) => { | |
| setInstallProgress(progress); | |
| }); | |
| const result = await window.electronAPI.installPythonRequirements({ | |
| pythonPath, | |
| activationScript | |
| }); | |
| unsubscribe(); | |
| setInstalling(false); | |
| if (result.success) { | |
| setError(''); | |
| await checkPackages(); | |
| } else { | |
| setError(result.error || 'Installation failed. Check the progress output above for details.'); | |
| } | |
| }; | |
| const checkPackages = async () => { | |
| setStatus('checking'); | |
| setValidationProgress(null); | |
| setError(''); | |
| // Listen for validation progress | |
| const unsubscribe = window.electronAPI.onPythonValidationProgress((progress) => { | |
| setValidationProgress({ | |
| current: progress.current, | |
| total: progress.total, | |
| packageName: progress.packageName | |
| }); | |
| }); | |
| try { | |
| const result = await window.electronAPI.validatePythonPackages({ | |
| pythonPath, | |
| activationScript | |
| }); | |
| if (result.success && result.data) { | |
| setStatus(result.data.allInstalled ? 'valid' : 'invalid'); | |
| setMissingPackages(result.data.missingPackages || []); | |
| setInstallLocation(result.data.installLocation || ''); | |
| setError(''); | |
| } else { | |
| setStatus('invalid'); | |
| setMissingPackages([]); | |
| setInstallLocation(''); | |
| setError(result.error || 'Failed to validate packages. Check that your Python path points to a Python executable (python.exe), not a directory.'); | |
| } | |
| } catch (error) { | |
| setStatus('invalid'); | |
| setMissingPackages([]); | |
| setInstallLocation(''); | |
| setError(String(error)); | |
| } finally { | |
| unsubscribe(); | |
| setValidationProgress(null); | |
| } | |
| }; | |
| const installRequirements = async () => { | |
| setInstalling(true); | |
| setInstallProgress(t('python.installing')); | |
| setError(''); | |
| const unsubscribe = window.electronAPI.onPythonInstallProgress((progress) => { | |
| setInstallProgress(progress); | |
| }); | |
| const result = await window.electronAPI.installPythonRequirements({ | |
| pythonPath, | |
| activationScript | |
| }); | |
| unsubscribe(); | |
| setInstalling(false); | |
| if (result.success) { | |
| setError(''); | |
| await checkPackages(); | |
| } else { | |
| setError(result.error || 'Installation failed. Check the progress output above for details.'); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In `@apps/frontend/src/renderer/components/settings/PythonPackageValidator.tsx`
around lines 61 - 120, The progress listeners and state resets in checkPackages
and installRequirements must be executed even if validatePythonPackages or
installPythonRequirements throws: wrap the async call and its surrounding logic
in try/catch/finally blocks so that unsubscribe() is always called in finally
and state resets (e.g., setValidationProgress(null), setInstalling(false),
setInstallProgress, and any spinner/error state updates) happen there; keep
result handling in try (or after awaiting the call) and set error in catch using
the thrown error, referencing the existing functions checkPackages and
installRequirements and the unsubscribe variable returned from
window.electronAPI.onPythonValidationProgress/onPythonInstallProgress.
Comment on lines
+91
to
+352
| setError(result.error || 'Failed to validate packages. Check that your Python path points to a Python executable (python.exe), not a directory.'); | ||
| } | ||
|
|
||
| setValidationProgress(null); | ||
| }; | ||
|
|
||
| const installRequirements = async () => { | ||
| setInstalling(true); | ||
| setInstallProgress(t('python.installing')); | ||
| setError(''); | ||
|
|
||
| const unsubscribe = window.electronAPI.onPythonInstallProgress((progress) => { | ||
| setInstallProgress(progress); | ||
| }); | ||
|
|
||
| const result = await window.electronAPI.installPythonRequirements({ | ||
| pythonPath, | ||
| activationScript | ||
| }); | ||
|
|
||
| unsubscribe(); | ||
| setInstalling(false); | ||
|
|
||
| if (result.success) { | ||
| setError(''); | ||
| await checkPackages(); | ||
| } else { | ||
| setError(result.error || 'Installation failed. Check the progress output above for details.'); | ||
| } | ||
| }; | ||
|
|
||
| const validateEnvironment = async () => { | ||
| if (!activationScript) { | ||
| checkPackages(); | ||
| return; | ||
| } | ||
|
|
||
| setEnvStatus('checking'); | ||
| setError(''); | ||
|
|
||
| try { | ||
| const result = await window.electronAPI.validatePythonEnvironment({ | ||
| activationScript | ||
| }); | ||
|
|
||
| if (result.success && result.data) { | ||
| setEnvValidation(result.data); | ||
| setEnvStatus(result.data.valid ? 'valid' : 'invalid'); | ||
|
|
||
| // If environment is valid, proceed to check packages | ||
| if (result.data.valid) { | ||
| await checkPackages(); | ||
| } | ||
| } else { | ||
| setEnvStatus('invalid'); | ||
| setError(result.error || 'Failed to validate Python environment'); | ||
| } | ||
| } catch (error) { | ||
| console.error('Error validating Python environment:', error); | ||
| setEnvStatus('invalid'); | ||
| setError('Failed to validate Python environment: ' + String(error)); | ||
| } | ||
| }; | ||
|
|
||
| const handleReinstallEnvironment = async () => { | ||
| if (!activationScript) { | ||
| setError('No activation script configured'); | ||
| return; | ||
| } | ||
|
|
||
| if (!confirm('⚠️ WARNING: This will DELETE the existing Python environment and reinstall Python 3.12.\n\nAll installed packages will be removed.\n\nContinue?')) { | ||
| return; | ||
| } | ||
|
|
||
| setReinstalling(true); | ||
| setError(''); | ||
| setReinstallProgress({ step: 'Starting...', completed: 0, total: 3 }); | ||
|
|
||
| const unsubscribe = window.electronAPI.onPythonReinstallProgress((progress) => { | ||
| setReinstallProgress(progress); | ||
| }); | ||
|
|
||
| const result = await window.electronAPI.reinstallPythonEnvironment({ | ||
| activationScript, | ||
| pythonVersion: '3.12' | ||
| }); | ||
|
|
||
| unsubscribe(); | ||
| setReinstalling(false); | ||
| setReinstallProgress(null); | ||
|
|
||
| if (result.success && result.data?.success) { | ||
| setError(''); | ||
| setEnvStatus('valid'); | ||
| setEnvValidation({ | ||
| valid: true, | ||
| pythonPath: result.data.environmentPath, | ||
| version: result.data.pythonVersion, | ||
| error: null, | ||
| status: 'valid' | ||
| }); | ||
| // Re-validate environment and packages after reinstall | ||
| await validateEnvironment(); | ||
| } else { | ||
| setError(result.data?.error || result.error || 'Environment reinstall failed'); | ||
| setEnvStatus('invalid'); | ||
| } | ||
| }; | ||
|
|
||
| // Check if initial validation is in progress | ||
| const isInitialValidation = (envStatus === 'checking' && !envValidation) || | ||
| (status === 'checking' && !missingPackages.length && !installLocation); | ||
|
|
||
| // Notify parent component of validation state changes | ||
| useEffect(() => { | ||
| onValidationStateChange?.(isInitialValidation); | ||
| }, [isInitialValidation, onValidationStateChange]); | ||
|
|
||
| return ( | ||
| <div className="space-y-4 relative"> | ||
| {/* Loading Overlay - Show during initial validation */} | ||
| {isInitialValidation && ( | ||
| <div className="absolute inset-0 bg-background/80 backdrop-blur-sm z-10 flex flex-col items-center justify-center rounded-lg"> | ||
| <Loader2 className="h-8 w-8 animate-spin text-primary mb-3" /> | ||
| <p className="text-sm font-medium text-foreground">Validating Python environment...</p> | ||
| <p className="text-xs text-muted-foreground mt-1">This may take a few seconds</p> | ||
| </div> | ||
| )} | ||
|
|
||
| {/* Error Display */} | ||
| {error && ( | ||
| <div className="p-3 bg-destructive/10 border border-destructive/20 rounded-md"> | ||
| <p className="text-sm font-medium text-destructive mb-1">Error</p> | ||
| <p className="text-xs text-destructive/80">{error}</p> | ||
| </div> | ||
| )} | ||
|
|
||
| {/* Python Environment Section (only show if checking or invalid) */} | ||
| {activationScript && envStatus !== 'idle' && envStatus !== 'valid' && ( | ||
| <div className="space-y-3 p-4 border border-border rounded-md bg-muted/30"> | ||
| {/* Environment Status Display */} | ||
| {envStatus === 'checking' && ( | ||
| <div className="flex items-center gap-2 text-sm text-muted-foreground"> | ||
| <Loader2 className="h-4 w-4 animate-spin" /> | ||
| <span>Validating Python environment...</span> | ||
| </div> | ||
| )} | ||
|
|
||
| {envStatus === 'invalid' && envValidation && ( | ||
| <div className="space-y-3"> | ||
| <div className="flex items-center gap-2 text-sm text-destructive"> | ||
| {envValidation.status === 'missing' && <XCircle className="h-4 w-4" />} | ||
| {envValidation.status === 'wrong_version' && <AlertTriangle className="h-4 w-4" />} | ||
| {envValidation.status === 'error' && <XCircle className="h-4 w-4" />} | ||
| <span> | ||
| {envValidation.status === 'missing' && 'Python not found'} | ||
| {envValidation.status === 'wrong_version' && `Wrong version: ${envValidation.version}`} | ||
| {envValidation.status === 'error' && 'Validation error'} | ||
| </span> | ||
| </div> | ||
|
|
||
| <p className="text-xs text-muted-foreground"> | ||
| {envValidation.status === 'missing' && | ||
| 'Python 3.12+ is required but not found. Click below to install it.'} | ||
| {envValidation.status === 'wrong_version' && | ||
| 'Python 3.12+ is required. Click below to reinstall with the correct version.'} | ||
| {envValidation.status === 'error' && | ||
| envValidation.error} | ||
| </p> | ||
|
|
||
| <Button | ||
| onClick={handleReinstallEnvironment} | ||
| disabled={reinstalling} | ||
| size="sm" | ||
| variant="destructive" | ||
| > | ||
| {reinstalling && <Loader2 className="mr-2 h-4 w-4 animate-spin" />} | ||
| Reinstall Python Environment | ||
| </Button> | ||
|
|
||
| {/* Reinstall Progress */} | ||
| {reinstallProgress && ( | ||
| <div className="space-y-2"> | ||
| <div className="flex items-center gap-2 text-xs text-muted-foreground"> | ||
| <Loader2 className="h-3 w-3 animate-spin" /> | ||
| <span>{reinstallProgress.step}</span> | ||
| </div> | ||
| <div className="text-xs text-muted-foreground"> | ||
| Step {reinstallProgress.completed} of {reinstallProgress.total} | ||
| </div> | ||
| </div> | ||
| )} | ||
| </div> | ||
| )} | ||
| </div> | ||
| )} | ||
|
|
||
| {/* Installation Location */} | ||
| {installLocation && ( | ||
| <div className="text-xs text-muted-foreground"> | ||
| <span className="font-medium">Install location:</span>{' '} | ||
| <code className="bg-muted px-1 py-0.5 rounded text-xs">{installLocation}</code> | ||
| </div> | ||
| )} | ||
|
|
||
| {status === 'checking' && ( | ||
| <div className="flex flex-col gap-1"> | ||
| <div className="flex items-center gap-2 text-sm text-muted-foreground"> | ||
| <Loader2 className="h-4 w-4 animate-spin" /> | ||
| <span> | ||
| {validationProgress | ||
| ? `${validationProgress.packageName} (${validationProgress.current}/${validationProgress.total})` | ||
| : t('python.validatingPackages')} | ||
| </span> | ||
| </div> | ||
| </div> | ||
| )} | ||
|
|
||
| {status === 'valid' && ( | ||
| <div className="flex items-center gap-2 text-sm text-green-600 dark:text-green-500"> | ||
| <CheckCircle2 className="h-4 w-4" /> | ||
| <span>{t('python.allPackagesInstalled')}</span> | ||
| </div> | ||
| )} | ||
|
|
||
| {status === 'invalid' && ( | ||
| <div className="space-y-3"> | ||
| <div className="flex items-center gap-2 text-sm text-destructive"> | ||
| <XCircle className="h-4 w-4" /> | ||
| <span>{t('python.packagesMissing')}</span> | ||
| </div> | ||
|
|
||
| {missingPackages.length > 0 && ( | ||
| <div className="space-y-1"> | ||
| <p className="text-xs font-medium text-foreground"> | ||
| Missing packages ({missingPackages.length}): | ||
| </p> | ||
| <div className="max-h-32 overflow-y-auto bg-muted/50 rounded p-2 space-y-1"> | ||
| {missingPackages.map((pkg) => ( | ||
| <div key={pkg} className="text-xs text-muted-foreground font-mono"> | ||
| • {pkg} | ||
| </div> | ||
| ))} | ||
| </div> | ||
| </div> | ||
| )} | ||
|
|
||
| <Button | ||
| onClick={installRequirements} | ||
| disabled={installing} | ||
| size="sm" | ||
| > | ||
| {installing && <Loader2 className="mr-2 h-4 w-4 animate-spin" />} | ||
| {t('python.installMissing')} | ||
| </Button> | ||
|
|
||
| {(installing || installProgress) && ( | ||
| <div className="space-y-2"> | ||
| <p className="text-xs font-medium">Installation Output:</p> | ||
| <div className="text-xs text-muted-foreground whitespace-pre-wrap max-h-48 overflow-y-auto bg-muted/50 rounded p-3 font-mono border border-border"> | ||
| {installProgress || 'Starting installation...'} | ||
| </div> |
Contributor
There was a problem hiding this comment.
Localize newly introduced UI and error strings
There are multiple hardcoded user-facing strings (overlay text, “Error”, confirm text, reinstall copy, “Install location”, etc.), and some keys omit the settings: namespace. Please move these to translation keys and use interpolation for dynamic content (including error concatenation). As per coding guidelines.
💡 Example update
- setError('No activation script configured');
+ setError(t('settings:python.noActivationScript'));
...
- if (!confirm('⚠️ WARNING: This will DELETE the existing Python environment and reinstall Python 3.12.\n\nAll installed packages will be removed.\n\nContinue?')) {
+ if (!confirm(t('settings:python.reinstallConfirm'))) {
return;
}
...
- <p className="text-sm font-medium text-foreground">Validating Python environment...</p>
+ <p className="text-sm font-medium text-foreground">
+ {t('settings:python.validatingEnv')}
+ </p>🤖 Prompt for AI Agents
In `@apps/frontend/src/renderer/components/settings/PythonPackageValidator.tsx`
around lines 91 - 352, Several user-facing strings in this component are
hardcoded; update all UI and error strings used in installRequirements,
validateEnvironment, handleReinstallEnvironment, the loading overlay
(isInitialValidation text), the Error panel title, confirm dialog text,
reinstall copy, "Install location", and any dynamic messages to use the
translation function with the settings: namespace (e.g.,
t('settings:python.validatingEnvironment'), t('settings:python.error'), etc.),
and convert string concatenations like 'Failed to validate Python environment: '
+ String(error) to interpolation-based translations (e.g.,
t('settings:python.envValidationFailed', { error: String(error) })); ensure
dynamic values (package names, counts, version, steps) use interpolation keys
and that all new keys are added to the i18n resource files under settings.
Comment on lines
+155
to
+197
| const handleReinstallEnvironment = async () => { | ||
| if (!activationScript) { | ||
| setError('No activation script configured'); | ||
| return; | ||
| } | ||
|
|
||
| if (!confirm('⚠️ WARNING: This will DELETE the existing Python environment and reinstall Python 3.12.\n\nAll installed packages will be removed.\n\nContinue?')) { | ||
| return; | ||
| } | ||
|
|
||
| setReinstalling(true); | ||
| setError(''); | ||
| setReinstallProgress({ step: 'Starting...', completed: 0, total: 3 }); | ||
|
|
||
| const unsubscribe = window.electronAPI.onPythonReinstallProgress((progress) => { | ||
| setReinstallProgress(progress); | ||
| }); | ||
|
|
||
| const result = await window.electronAPI.reinstallPythonEnvironment({ | ||
| activationScript, | ||
| pythonVersion: '3.12' | ||
| }); | ||
|
|
||
| unsubscribe(); | ||
| setReinstalling(false); | ||
| setReinstallProgress(null); | ||
|
|
||
| if (result.success && result.data?.success) { | ||
| setError(''); | ||
| setEnvStatus('valid'); | ||
| setEnvValidation({ | ||
| valid: true, | ||
| pythonPath: result.data.environmentPath, | ||
| version: result.data.pythonVersion, | ||
| error: null, | ||
| status: 'valid' | ||
| }); | ||
| // Re-validate environment and packages after reinstall | ||
| await validateEnvironment(); | ||
| } else { | ||
| setError(result.data?.error || result.error || 'Environment reinstall failed'); | ||
| setEnvStatus('invalid'); | ||
| } |
Contributor
There was a problem hiding this comment.
Apply the same try/finally cleanup to reinstall flow
reinstallPythonEnvironment can throw, which would skip unsubscribe() and leave reinstalling stuck. Use try/catch/finally here as well.
🐛 Suggested fix
- const unsubscribe = window.electronAPI.onPythonReinstallProgress((progress) => {
+ const unsubscribe = window.electronAPI.onPythonReinstallProgress((progress) => {
setReinstallProgress(progress);
});
-
- const result = await window.electronAPI.reinstallPythonEnvironment({
- activationScript,
- pythonVersion: '3.12'
- });
-
- unsubscribe();
- setReinstalling(false);
- setReinstallProgress(null);
-
- if (result.success && result.data?.success) {
- setError('');
- setEnvStatus('valid');
- setEnvValidation({
- valid: true,
- pythonPath: result.data.environmentPath,
- version: result.data.pythonVersion,
- error: null,
- status: 'valid'
- });
- // Re-validate environment and packages after reinstall
- await validateEnvironment();
- } else {
- setError(result.data?.error || result.error || 'Environment reinstall failed');
- setEnvStatus('invalid');
- }
+ try {
+ const result = await window.electronAPI.reinstallPythonEnvironment({
+ activationScript,
+ pythonVersion: '3.12'
+ });
+
+ if (result.success && result.data?.success) {
+ setError('');
+ setEnvStatus('valid');
+ setEnvValidation({
+ valid: true,
+ pythonPath: result.data.environmentPath,
+ version: result.data.pythonVersion,
+ error: null,
+ status: 'valid'
+ });
+ // Re-validate environment and packages after reinstall
+ await validateEnvironment();
+ } else {
+ setError(result.data?.error || result.error || 'Environment reinstall failed');
+ setEnvStatus('invalid');
+ }
+ } finally {
+ unsubscribe();
+ setReinstalling(false);
+ setReinstallProgress(null);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const handleReinstallEnvironment = async () => { | |
| if (!activationScript) { | |
| setError('No activation script configured'); | |
| return; | |
| } | |
| if (!confirm('⚠️ WARNING: This will DELETE the existing Python environment and reinstall Python 3.12.\n\nAll installed packages will be removed.\n\nContinue?')) { | |
| return; | |
| } | |
| setReinstalling(true); | |
| setError(''); | |
| setReinstallProgress({ step: 'Starting...', completed: 0, total: 3 }); | |
| const unsubscribe = window.electronAPI.onPythonReinstallProgress((progress) => { | |
| setReinstallProgress(progress); | |
| }); | |
| const result = await window.electronAPI.reinstallPythonEnvironment({ | |
| activationScript, | |
| pythonVersion: '3.12' | |
| }); | |
| unsubscribe(); | |
| setReinstalling(false); | |
| setReinstallProgress(null); | |
| if (result.success && result.data?.success) { | |
| setError(''); | |
| setEnvStatus('valid'); | |
| setEnvValidation({ | |
| valid: true, | |
| pythonPath: result.data.environmentPath, | |
| version: result.data.pythonVersion, | |
| error: null, | |
| status: 'valid' | |
| }); | |
| // Re-validate environment and packages after reinstall | |
| await validateEnvironment(); | |
| } else { | |
| setError(result.data?.error || result.error || 'Environment reinstall failed'); | |
| setEnvStatus('invalid'); | |
| } | |
| const handleReinstallEnvironment = async () => { | |
| if (!activationScript) { | |
| setError('No activation script configured'); | |
| return; | |
| } | |
| if (!confirm('⚠️ WARNING: This will DELETE the existing Python environment and reinstall Python 3.12.\n\nAll installed packages will be removed.\n\nContinue?')) { | |
| return; | |
| } | |
| setReinstalling(true); | |
| setError(''); | |
| setReinstallProgress({ step: 'Starting...', completed: 0, total: 3 }); | |
| const unsubscribe = window.electronAPI.onPythonReinstallProgress((progress) => { | |
| setReinstallProgress(progress); | |
| }); | |
| try { | |
| const result = await window.electronAPI.reinstallPythonEnvironment({ | |
| activationScript, | |
| pythonVersion: '3.12' | |
| }); | |
| if (result.success && result.data?.success) { | |
| setError(''); | |
| setEnvStatus('valid'); | |
| setEnvValidation({ | |
| valid: true, | |
| pythonPath: result.data.environmentPath, | |
| version: result.data.pythonVersion, | |
| error: null, | |
| status: 'valid' | |
| }); | |
| // Re-validate environment and packages after reinstall | |
| await validateEnvironment(); | |
| } else { | |
| setError(result.data?.error || result.error || 'Environment reinstall failed'); | |
| setEnvStatus('invalid'); | |
| } | |
| } finally { | |
| unsubscribe(); | |
| setReinstalling(false); | |
| setReinstallProgress(null); | |
| } |
🤖 Prompt for AI Agents
In `@apps/frontend/src/renderer/components/settings/PythonPackageValidator.tsx`
around lines 155 - 197, The reinstall flow in handleReinstallEnvironment may
throw from window.electronAPI.reinstallPythonEnvironment and skip cleanup; wrap
the async call and related logic in try/catch/finally so unsubscribe(),
setReinstalling(false) and setReinstallProgress(null) always run; in the try
block await window.electronAPI.reinstallPythonEnvironment(...) and handle
success logic (setEnvStatus, setEnvValidation, await validateEnvironment()), in
catch set the error via setError(result/error fallback) and
setEnvStatus('invalid'), and in finally call unsubscribe(),
setReinstalling(false) and setReinstallProgress(null).
Comment on lines
+93
to
+140
| useEffect(() => { | ||
| // Reset mounted ref on each effect run (important for React 18 StrictMode) | ||
| isMountedRef.current = true; | ||
|
|
||
| const handleProgress = (progressData: SetupProgress) => { | ||
| if (!isMountedRef.current) return; | ||
|
|
||
| // Update current state | ||
| setStep(progressData.step); | ||
| setMessage(progressData.message); | ||
| setDetail(progressData.detail || ''); | ||
| setProgress(progressData.progress); | ||
|
|
||
| // Accumulate log entry (limit to last 50 entries to prevent memory issues) | ||
| setLogs((prevLogs) => { | ||
| const newLogs = [ | ||
| ...prevLogs, | ||
| { | ||
| step: progressData.step, | ||
| message: progressData.message, | ||
| detail: progressData.detail, | ||
| }, | ||
| ]; | ||
| // Keep only the last 50 entries | ||
| return newLogs.length > 50 ? newLogs.slice(-50) : newLogs; | ||
| }); | ||
|
|
||
| // Check for completion or error states | ||
| if (progressData.step === 'complete' || progressData.step === 'error') { | ||
| setIsRunning(false); | ||
| if (progressData.step === 'error') { | ||
| setError(progressData.message); | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| // Subscribe to progress events | ||
| cleanupRef.current = window.electronAPI.conda.onSetupProgress(handleProgress); | ||
|
|
||
| // Cleanup on unmount | ||
| return () => { | ||
| isMountedRef.current = false; | ||
| if (cleanupRef.current) { | ||
| cleanupRef.current(); | ||
| cleanupRef.current = null; | ||
| } | ||
| }; | ||
| }, []); |
Contributor
There was a problem hiding this comment.
Prevent progress updates after cancel.
cancelSetup only flips local state; the progress listener continues to update state and can overwrite the cancelled UI. Add a cancellation guard (or unsubscribe on cancel) and reset it on new starts.
🛠️ Suggested guard
- const cleanupRef = useRef<(() => void) | null>(null);
+ const cleanupRef = useRef<(() => void) | null>(null);
+ const isCancelledRef = useRef<boolean>(false);
@@
- const handleProgress = (progressData: SetupProgress) => {
- if (!isMountedRef.current) return;
+ const handleProgress = (progressData: SetupProgress) => {
+ if (!isMountedRef.current || isCancelledRef.current) return;
@@
- const startSetup = useCallback(async () => {
+ const startSetup = useCallback(async () => {
+ isCancelledRef.current = false;
@@
- const cancelSetup = useCallback(() => {
+ const cancelSetup = useCallback(() => {
+ isCancelledRef.current = true;
setIsRunning(false);
setMessage('Setup cancelled');
setStep('error');
setError('Setup was cancelled');
}, []);Also applies to: 192-197
🤖 Prompt for AI Agents
In `@apps/frontend/src/renderer/hooks/useCondaSetup.ts` around lines 93 - 140, The
progress listener (handleProgress) keeps updating state after cancelSetup is
called because cancelSetup only flips local state; fix by adding a cancellation
guard and/or unsubscribing the listener when cancelSetup runs and re-creating
the subscription on new starts: modify cancelSetup to call the cleanup function
stored in cleanupRef (and null it), add a cancelled boolean/ref checked at the
top of handleProgress (or reuse isRunning/isMountedRef) to early-return when
cancelled, and ensure the effect that sets up cleanupRef resets that cancelled
flag when a new start begins so progress updates resume on subsequent runs.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/backend/core/git_provider.py`:
- Around line 65-66: The except clause in the function that detects the git
provider currently catches subprocess.TimeoutExpired, FileNotFoundError, and
subprocess.CalledProcessError even though subprocess.CalledProcessError won't be
raised because subprocess.run is called without check=True; update the exception
tuple in that except block to remove subprocess.CalledProcessError so it only
catches subprocess.TimeoutExpired and FileNotFoundError (leaving the return
GitProvider.GITHUB fail-safe unchanged), or if you prefer defensive coding, add
a comment explaining why subprocess.CalledProcessError is intentionally included
for future check=True changes.
- Around line 62-63: The failing ruff format check is due to formatting on the
final return line in the Git detection function that returns GitProvider.GITHUB;
fix it by running the auto-formatter (e.g., run `ruff format apps/backend/` or
`cd apps/backend && ruff format core/git_provider.py`) or manually reformat the
return line to conform to ruff/PEP8 style so the file passes the linter.
Comment on lines
+62
to
+63
| # GitHub detection patterns (default) | ||
| return GitProvider.GITHUB |
Contributor
There was a problem hiding this comment.
Fix formatting to resolve pipeline failure.
The pipeline reports a ruff format check failure on this line. Run ruff format apps/backend/ to auto-fix.
#!/bin/bash
# Check what ruff format wants to change
cd apps/backend && ruff format --diff core/git_provider.py🧰 Tools
🪛 GitHub Actions: Lint
[error] 62-62: ruff format check failed. 1 file would be reformatted. Apply formatting changes (e.g., run 'ruff format apps/backend/').
🤖 Prompt for AI Agents
In `@apps/backend/core/git_provider.py` around lines 62 - 63, The failing ruff
format check is due to formatting on the final return line in the Git detection
function that returns GitProvider.GITHUB; fix it by running the auto-formatter
(e.g., run `ruff format apps/backend/` or `cd apps/backend && ruff format
core/git_provider.py`) or manually reformat the return line to conform to
ruff/PEP8 style so the file passes the linter.
Comment on lines
+65
to
+66
| except (subprocess.TimeoutExpired, FileNotFoundError, subprocess.CalledProcessError): | ||
| return GitProvider.GITHUB # Fail-safe default |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor: CalledProcessError won't be raised here.
CalledProcessError is only raised when check=True is passed to subprocess.run, which isn't the case here (returncode is checked manually on line 43). Including it is harmless but slightly misleading. Consider removing it for accuracy, or keep it for defensive coding if check=True might be added later.
🤖 Prompt for AI Agents
In `@apps/backend/core/git_provider.py` around lines 65 - 66, The except clause in
the function that detects the git provider currently catches
subprocess.TimeoutExpired, FileNotFoundError, and subprocess.CalledProcessError
even though subprocess.CalledProcessError won't be raised because subprocess.run
is called without check=True; update the exception tuple in that except block to
remove subprocess.CalledProcessError so it only catches
subprocess.TimeoutExpired and FileNotFoundError (leaving the return
GitProvider.GITHUB fail-safe unchanged), or if you prefer defensive coding, add
a comment explaining why subprocess.CalledProcessError is intentionally included
for future check=True changes.
AndyMik90
requested changes
Jan 17, 2026
Owner
AndyMik90
left a comment
AndyMik90
left a comment
There was a problem hiding this comment.
🤖 Auto Claude PR Review
Merge Verdict: 🔴 BLOCKED
🔴 Blocked - 7 CI check(s) failing. Fix CI before merge.
Blocked: 7 CI check(s) failing. Fix CI before merge.
Risk Assessment
| Factor | Level | Notes |
|---|---|---|
| Complexity | High | Based on lines changed |
| Security Impact | Low | Based on security findings |
| Scope Coherence | Good | Based on structural review |
🚨 Blocking Issues (Must Fix)
- Branch Out of Date: PR branch is behind the base branch and needs to be updated
- CI Failed: CI Complete
- CI Failed: CodeQL
- CI Failed: Lint Complete
- CI Failed: test-frontend (windows-latest)
- CI Failed: test-frontend (ubuntu-latest)
- CI Failed: Python (Ruff)
- CI Failed: test-frontend (macos-latest)
Findings Summary
- High: 3 issue(s)
- Medium: 5 issue(s)
- Low: 4 issue(s)
Generated by Auto Claude PR Review
Findings (12 selected of 12 total)
🟠 [b573886b04bd] [HIGH] Incorrect pip path for Conda environments on Windows
📁 apps/frontend/src/main/python-path-utils.ts:71
The function getCondaPipPath returns path.join(envPath, 'pip.exe') for Windows, but in Conda environments pip.exe is located in the Scripts subdirectory (Scripts/pip.exe), not at the environment root. This will cause all pip operations to fail on Windows.
Suggested fix:
Change to `return path.join(envPath, 'Scripts', 'pip.exe');`
🟠 [e3dd917f00b4] [HIGH] Hardcoded English strings violate i18n requirement
📁 apps/frontend/src/renderer/components/settings/PythonPackageValidator.tsx:161
PythonPackageValidator.tsx contains multiple hardcoded English strings in confirm dialogs and status messages (lines 161, 215-217, 223-224, 246-258). CLAUDE.md explicitly states: 'CRITICAL: Always use i18n translation keys for all user-facing text in the frontend.' Other settings components use t() properly.
Suggested fix:
Add translation keys to settings.json and use t('settings:python.confirmDeleteEnv'), t('settings:python.errorTitle'), etc.
🟠 [e2d1b04cead1] [HIGH] Linux terminal command escaping missing for bash -c argument
📁 apps/frontend/src/main/ipc-handlers/claude-code-handlers.ts:685
For Linux terminals (xfce4-terminal, lxterminal, mate-terminal, terminator), commands are constructed as -e "bash -c \"${bashCommand}\"" without proper escaping of double quotes in the original command. If the command contains double quotes (e.g., echo "hello"), they are not escaped, causing parsing errors or potential injection.
Suggested fix:
Use proper escaping for the bashCommand string, or use array form like gnome-terminal: `['--', 'bash', '-c', bashCommand]`
🟡 [0f94c4a31139] [MEDIUM] Shell command injection via unsanitized Python command in getPythonVersion
📁 apps/frontend/src/main/python-detector.ts:118
The getPythonVersion function uses execSync with string interpolation: execSync(\${pythonCmd} --version`)`. If pythonCmd contains shell metacharacters, they would be executed. On Windows, candidates include 'py -3' which is passed directly to shell. While validatePythonPath exists, getPythonVersion is called with system-detected paths that bypass validation.
Suggested fix:
Use execFileSync instead of execSync with parsePythonCommand: `const [cmd, args] = parsePythonCommand(pythonCmd); execFileSync(cmd, [...args, '--version'], { shell: false })`
🟡 [9e6b75b4b154] [MEDIUM] Temp file with predictable name pattern enables TOCTOU race condition
📁 apps/frontend/src/main/agent/agent-manager.ts:140
Temp file is created with predictable pattern using Date.now() and Math.random().toString(36). An attacker with local access could predict the filename and create a symlink before the file is written (symlink attack). Math.random() is not cryptographically secure.
Suggested fix:
Use crypto.randomUUID() for unpredictable names, or use fs.mkdtemp for secure temp directory creation
🟡 [82a7569e54be] [MEDIUM] Missing UTF-8 encoding for file read/write operations
📁 apps/backend/agents/tools_pkg/tools/feedback.py:99
File operations on lines 99 and 135 use open(metadata_file) and open(metadata_file, 'w') without explicit encoding. On Windows, default encoding may not be UTF-8, causing UnicodeDecodeError with non-ASCII characters in feedback JSON.
Suggested fix:
Add encoding='utf-8' to both: `open(metadata_file, encoding='utf-8')` and `open(metadata_file, 'w', encoding='utf-8')`
🟡 [4b20e9c9364b] [MEDIUM] GitLab detection pattern may cause false positives
📁 apps/backend/core/git_provider.py:54
The pattern /gitlab/ at line 54 will match any URL containing 'gitlab' in the path. A GitHub repo like github.com/myorg/gitlab-tools would incorrectly detect as GitLab because the lowercased URL contains '/gitlab'.
Suggested fix:
Remove the `/gitlab/` pattern. The other patterns (`gitlab.com`, `@gitlab.`, `://gitlab.`) are sufficient for actual GitLab hosts.
🟡 [ddda9ec67baf] [MEDIUM] Uses os.name directly instead of platform abstraction module
📁 apps/backend/core/worktree.py:61
CLAUDE.md documents centralized platform abstraction in apps/backend/core/platform/ with is_windows(). The worktree.py file uses os.name == 'nt' directly instead of the established pattern, violating the documented cross-platform development guidelines.
Suggested fix:
Import from core.platform: `from core.platform import is_windows` and use `if is_windows():`
🔵 [dc0d05e5698a] [LOW] Unused import and dead code: isValidActivationScript and sanitizeShellPath
📁 apps/frontend/src/main/agent/agent-process.ts:19
The import isValidActivationScript (line 19) is never called. The function sanitizeShellPath (lines 34-92) is defined but never called anywhere. Comments indicate activation script support is not implemented (lines 553-555). This is dead code left from incomplete implementation.
Suggested fix:
Remove unused import. Either integrate sanitizeShellPath into the spawn logic or remove it if the feature won't be implemented.
🔵 [b8743b16c270] [LOW] Unused import: isLinux
📁 apps/frontend/src/main/ipc-handlers/claude-code-handlers.ts:22
The import isLinux from '../python-path-utils' is imported but never used. Only isWindows and isMac are called in the openTerminalWithCommand function.
Suggested fix:
Change to `import { isWindows, isMac } from '../python-path-utils';`
🔵 [0dbfb10d6ec0] [LOW] Shell command execution uses shell=True unnecessarily
📁 apps/backend/core/worktree.py:81
The get_glab_executable function runs subprocess with shell=True for 'where glab' command. While the command is hardcoded and not immediately exploitable, using shell=True is discouraged as a defense-in-depth measure.
Suggested fix:
Use shell=False with arguments as list: `subprocess.run(['where', 'glab'], capture_output=True, text=True, timeout=5)`
🔵 [fc3683be95b0] [LOW] File defines isWindows() helper but uses process.platform directly
📁 apps/frontend/src/main/python-path-utils.ts:48
The python-path-utils.ts file defines isWindows(), isMac(), isLinux() helper functions (lines 14-32) but then uses process.platform === 'win32' directly in lines 48, 59, 69, 80 instead of calling its own helpers. This internal inconsistency affects testability.
Suggested fix:
Replace `process.platform === 'win32'` with `isWindows()` throughout the file for consistency
This review was generated by Auto Claude.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Changes
Python Package Validation
PythonPackageValidatorcomponent for validating/installing packagessecretstorageon Windows)PowerShell Terminal Fixes
$env:PATH = "...";instead ofset "PATH=..." &&& "path"for executable invocationcd 'path';instead ofcd /d "path" &&Other Fixes
__dirnamecompatibility in python-env-manager.tsTest Plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.