fix(linux): add secretstorage to platform-critical packages (ACS-310)

[StillKnotKnown]

Summary

Linux binary installations were missing the secretstorage package, causing OAuth token storage via Freedesktop.org Secret Service (gnome-keyring/kwallet) to fail silently. The build script cache was created before secretstorage was added to requirements.txt (January 16, 2026), resulting in Linux binaries shipping without the required package.

Root Cause

The Linux binary build process caches Python dependencies in apps/frontend/python-runtime. When the .bundled marker file exists, the build script skips reinstallation. The cache was created before secretstorage was added to requirements.txt, and the validation only checked for hardcoded "critical packages" (claude_agent_sdk, dotenv, pydantic_core) which did not include secretstorage.

Changes

Build System

• apps/frontend/scripts/download-python.cjs: Added platform-aware critical packages validation at two locations (lines ~712 and ~815)
• apps/frontend/src/main/python-env-manager.ts: Mirrored the platform-aware critical packages logic

Backend

• apps/backend/core/dependency_validator.py: Added Linux secretstorage validation with helpful warning message

Tests

• tests/test_dependency_validator.py: Added comprehensive tests for Linux secretstorage validation

Pattern

This fix follows the same pattern as the Windows pywin32 fix (ACS-306):

• Platform-specific packages are only validated on their target platform
• Linux: secretstorage (OAuth token storage via keyring)
• Windows: pywintypes (MCP library dependency)
• macOS: No platform-specific packages

Behavior

The Linux validation emits a warning (not a blocking error) since the app gracefully falls back to .env file storage when secretstorage is unavailable. This is intentional because secretstorage is optional for app functionality, unlike pywin32 which is required on Windows.

Testing

• All existing tests pass (29/29)
• New tests cover all platform scenarios
• Ruff linting passes
• ESLint passes

Related

• Fixes: ACS-310 Linux Binary Missing secretstorage Dependency #1204
• Related: ACS-293 (added secretstorage dependency)
• Related: ACS-306 (Windows pywin32 fix)

Type of Change

• Bug fix
• Feature
• Breaking change
• Documentation update

Checklist

• All tests passing
• Code follows project style guidelines
• Self-documented (comments where needed)
• Cross-platform compatible (macOS, Windows, Linux)

Screenshots

N/A (backend/build system change)

Breaking Changes

None

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Improved platform-specific dependency validation with informative warnings instead of hard failures on Linux systems
  • Enhanced terminal reliability and cross-platform CLI invocation compatibility
  • Strengthened security path validation for Claude CLI detection

• New Features

  • Better OAuth token storage support across platforms with improved fallback mechanisms

• UI Improvements

  • Reorganized worktree selector with clearer visual grouping and improved deletion workflow
  • Streamlined terminal integration with simplified command execution

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR refactors platform-specific dependency handling by adding Linux secretstorage validation with non-fatal warnings, consolidating Claude command construction to use pre-escaped config values, removing CI verdict re-evaluation logic in the review orchestrator, and reorganizing worktree UI selection with improved delete flows.

Changes

Cohort / File(s)	Summary
Platform-specific dependency validation apps/backend/core/dependency_validator.py, apps/frontend/scripts/download-python.cjs, apps/frontend/src/main/python-env-manager.ts	Added Linux-specific secretstorage validation with non-fatal warnings; replaced sys.platform checks with is_windows()/is_linux(); added support for single-file Python modules (.py files) alongside traditional packages with __init__.py; extended critical package mapping to include platform-specific entries (pywintypes for Windows, secretstorage for Linux).
Claude CLI integration and command construction apps/frontend/src/main/terminal/claude-integration-handler.ts, apps/backend/core/client.py, apps/backend/core/simple_client.py	Replaced platform-specific shell-command helpers with centralized config-driven escaping (escapedTempFile, escapedConfigDir); introduced platform-aware Claude CLI path detection with Windows cmd.exe handling; added _is_secure_path() helper for path validation; expanded NVM-based Node.js discovery with version sorting; unified invocation flow across sync/async paths; removed public escapeShellCommand export and updated ClaudeCommandConfig type signature.
Backend CI/review orchestration apps/backend/runners/github/orchestrator.py, apps/backend/runners/github/services/parallel_orchestrator_reviewer.py, apps/backend/runners/github/services/pr_review_engine.py, apps/backend/runners/github/services/sdk_utils.py	Removed CI status comparison and verdict re-evaluation logic from no-change review path; simplified file-count logging to debug-only output; eliminated resolve_model_id import; refactored tool logging with StructuredOutput handling, removed progress-tracking boilerplate, and condensed debug/non-debug logging semantics.
Security and shell validation apps/backend/security/shell_validators.py, apps/frontend/src/main/ipc-handlers/claude-code-handlers.ts	Simplified nested shell command validation by removing dangerous-pattern checks for non--c invocations; removed Windows-specific path security validation (isSecurePath); eliminated windowsVerbatimArguments exec option usage.
Type definitions and imports apps/frontend/src/main/cli-tool-manager.ts, apps/frontend/src/main/env-utils.ts	Converted previously exported type aliases ExecFileSyncOptionsWithVerbatim and ExecFileAsyncOptionsWithVerbatim to non-exported internal types; removed platform helper imports from env-utils.
UI and component restructuring apps/frontend/src/renderer/components/terminal/WorktreeSelector.tsx	Reorganized worktree dropdown with dedicated separators and sections for Terminal and Task Worktrees; added per-item delete buttons with confirmation flow via AlertDialog; improved visual grouping and branch name display.
Localization apps/frontend/src/shared/i18n/locales/en/common.json, apps/frontend/src/shared/i18n/locales/fr/common.json	Removed unused "issues" translation block from ideation section in both English and French locales.
Test coverage and mocking tests/test_dependency_validator.py, apps/frontend/src/__tests__/integration/terminal-copy-paste.test.ts, apps/frontend/src/main/terminal/__tests__/claude-integration-handler.test.ts	Added tests for Linux secretstorage warning behavior across platforms; expanded Claude integration tests to cover temp-file token flow, profile resolution, and CLI invocation paths; added global requestAnimationFrame mock for synchronous test execution.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~65 minutes

Possibly related issues

• Linux Binary Missing secretstorage Dependency #1204: Directly addresses missing secretstorage dependency handling on Linux, adding validation checks, non-fatal warnings, and frontend build/packaging support across multiple backend and frontend modules.

Possibly related PRs

• fix(python-bundling): verify critical packages exist, not just marker file (#416) #774: Modifies the same Python bundle verification logic (download-python.cjs, python-env-manager.ts) to add critical-package checks; this PR extends that work with platform-aware validation for secretstorage and single-file module support.
• fix: windows #1056: Introduces the platform abstraction layer (is_windows(), is_linux()) and platform-specific path handling that this PR directly utilizes for dependency and CLI discovery.
• fix(terminal): add scroll area to worktree dropdown to prevent overflow #1146: Updates WorktreeSelector.tsx to reorganize worktree dropdown UI and add per-item delete controls, matching the component refactoring in this PR.

Suggested labels

size/M, 🔄 Checking

Suggested reviewers

• AndyMik90
• AlexMadera

Poem

🐰 In platforms far and wide we'd roam,
Secrets stored in every home,
Tempfiles escaped, commands refined,
Worktrees sorted, UI designed—
This rabbit hops through code so clean,
The finest refactor ever seen! ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the main change: adding secretstorage as a platform-critical package for Linux systems, with reference to the issue (ACS-310). It is concise, specific, and accurately reflects the primary objective of the pull request.
Docstring Coverage	✅ Passed	Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[gemini-code-assist]

Summary of Changes

Hello @StillKnotKnown, I'm Gemini Code Assist¹! 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 resolves a critical issue affecting Linux binary installations where the secretstorage package was missing, leading to silent failures in secure OAuth token storage. The changes introduce robust, platform-aware dependency validation into both the build process and the application's runtime, ensuring that essential packages like secretstorage on Linux and pywintypes on Windows are correctly identified and handled. Additionally, the frontend's session persistence logic has been simplified by removing unnecessary asynchronous operations, and a circular import in the backend has been addressed, contributing to overall system stability and maintainability.

Highlights

• Linux Dependency Validation: Implemented platform-aware validation for the secretstorage package on Linux, addressing a bug (ACS-310) where its absence caused OAuth token storage failures. The build system and runtime now check for this critical package.
• Build System Enhancement: The download-python.cjs and python-env-manager.ts scripts were updated to include secretstorage in the list of platform-critical packages for Linux, and pywintypes for Windows. The package existence check was also improved to correctly identify single-file modules.
• Frontend Session Persistence Refactor: Simplified the TerminalSessionStore by removing complex asynchronous saving mechanisms (saveAsync, saveSessionAsync, pendingDelete, writeInProgress, writePending). Session persistence is now handled synchronously, streamlining the code and removing potential race conditions.
• Circular Import Resolution: Resolved a circular import issue in apps/backend/spec/pipeline/agent_runner.py by lazy-loading the create_client function, improving module dependency management.
• Comprehensive Testing: Added new, comprehensive tests for the Linux secretstorage validation, ensuring correct behavior and warning messages. Existing tests were updated to use platform-agnostic checks.

Ignored Files

• Ignored by pattern: .github/workflows/** (1)
  • .github/workflows/test-azure-auth.yml

Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[sentry]

Codecov Report

❌ Patch coverage is 29.62963% with 38 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
apps/backend/core/client.py	12.50%	21 Missing ⚠️
apps/backend/runners/github/services/sdk_utils.py	0.00%	15 Missing ⚠️
.../github/services/parallel_orchestrator_reviewer.py	0.00%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!

[gemini-code-assist]

Code Review

This pull request addresses the missing secretstorage dependency on Linux, which is a valuable fix. The changes in dependency_validator.py and the corresponding tests are well-implemented.

However, this PR also includes a significant and unrelated refactoring of the terminal session persistence logic, changing it from asynchronous to synchronous writes. This is a major architectural change that can introduce performance bottlenecks by blocking the main UI thread, and it is not mentioned in the pull request description. Such a significant change should be in a separate pull request with a detailed explanation of the rationale and performance impact analysis. Mixing these two unrelated changes makes the PR difficult to review and increases risk. I strongly recommend splitting this PR into two: one for the secretstorage fix and another for the session persistence refactoring.

I am having trouble creating individual review comments. Click here to see my feedback.

apps/frontend/src/main/terminal-session-store.ts (78-95)

This change removes the asynchronous session saving logic and replaces it with synchronous writes (writeFileSync). This is a major architectural change that is not documented in the PR description. Synchronous file I/O on the main Electron process can block the UI thread, leading to a frozen or unresponsive application, especially if the session file is large or disk I/O is slow. This change should be reverted from this PR and moved to a separate one with a clear justification and performance analysis.

apps/frontend/scripts/download-python.cjs (720-726)

The logic for validating packages has been weakened. The previous implementation correctly checked for the existence of an __init__.py file within package directories, which is a more reliable way to verify a package. The new logic only checks for the directory's existence, which could lead to accepting corrupted or incomplete package directories as valid. This is a regression. The validation should check for either a valid package directory (with __init__.py) or a single module file.

      const missingPackages = criticalPackages.filter(pkg => {
        const pkgPath = path.join(sitePackagesDir, pkg);
        const initFile = path.join(pkgPath, '__init__.py');
        const moduleFile = path.join(sitePackagesDir, pkg + '.py');
        // Package is considered missing if it's not a valid directory package AND not a single module file.
        return !(fs.existsSync(pkgPath) && fs.existsSync(initFile)) && !fs.existsSync(moduleFile);
      });

apps/frontend/src/main/python-env-manager.ts (143-150)

Similar to the issue in download-python.cjs, the package validation logic here has been weakened. The previous implementation correctly checked for an __init__.py file, which is a more robust way to verify a package directory. The new logic only checks for the directory's existence, which is a regression. The validation should check for either a valid package directory (with __init__.py) or a single module file.

    const missingPackages = criticalPackages.filter((pkg) => {
      const pkgPath = path.join(sitePackagesPath, pkg);
      const initPath = path.join(pkgPath, '__init__.py');
      const moduleFile = path.join(sitePackagesPath, `${pkg}.py`);
      // Package is considered missing if it's not a valid directory package AND not a single module file.
      return !existsSync(pkgPath) && !existsSync(initPath) && !existsSync(moduleFile);
    });

apps/backend/core/dependency_validator.py (64)

The function name _exit_with_secretstorage_warning is a bit misleading, as the function intentionally does not exit the program. A more descriptive name like _show_secretstorage_warning would better reflect its behavior, which is to print a warning to stderr and continue execution.

def _show_secretstorage_warning() -> None:

apps/frontend/scripts/download-python.cjs (714-719)

The logic to define criticalPackages is duplicated starting at line 825. This increases maintenance overhead and the risk of inconsistencies. This logic should be extracted into a reusable function within this script to ensure it's defined only once and called from both locations.

apps/frontend/src/main/python-env-manager.ts (131-140)

The logic for defining criticalPackages is duplicated from apps/frontend/scripts/download-python.cjs. The comment even notes that they should be kept in sync. This duplication increases maintenance overhead. This logic should be centralized in a shared utility or configuration file to avoid inconsistencies.

[coderabbitai]

Actionable comments posted: 9

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

apps/frontend/src/main/terminal-session-store.ts (1)

177-207: Consider async I/O for session saves to prevent Electron main thread blocking.

saveSession() uses writeFileSync() for disk writes on the main thread. While current call frequency (every 30 seconds via persistAllSessions() plus discrete lifecycle events) is moderate, synchronous I/O can still cause temporary UI freezes. Consider migrating to writeFile() or batching writes with a debounce mechanism similar to session-persistence.ts.

🤖 Fix all issues with AI agents

In `@apps/backend/core/dependency_validator.py`:
- Around line 64-96: Rename the misleading function
_exit_with_secretstorage_warning to a name that reflects it does not exit
(suggest _warn_missing_secretstorage or _emit_secretstorage_warning), update all
references (e.g., calls from validate_platform_dependencies()), and update tests
in tests/test_dependency_validator.py accordingly; also add a flush of
sys.stderr after writing the warning so the message is immediately visible (call
sys.stderr.flush() right after the sys.stderr.write in the renamed function).

In `@apps/backend/spec/pipeline/agent_runner.py`:
- Around line 23-26: The module-level comment about lazy importing to avoid a
circular import should be moved (or duplicated with a short cross-reference) so
it's adjacent to the actual lazy import inside run_agent(); locate the lazy
import of create_client within the run_agent() function and place the
explanatory comment directly above that import (or leave a one-line pointer here
referencing run_agent() where the real import occurs) so future maintainers see
the rationale next to the code it documents.

In `@apps/frontend/scripts/download-python.cjs`:
- Around line 712-726: The package-existence check (building missingPackages
using platformCriticalPackages, criticalPackages, sitePackagesDir, pkgPath and
moduleFile) currently treats a directory as valid even if it lacks __init__.py;
change the condition so a package dir is only considered present if
path.join(pkgPath, '__init__.py') exists, while still allowing single-file
modules by checking pkg + '.py'; update both the pre-check (where
missingPackages is computed) and the equivalent post-install check to require
the __init__.py file for directory packages.

In `@apps/frontend/src/main/python-env-manager.ts`:
- Around line 143-149: The filter for missingPackages currently ignores the
presence of __init__.py (it only checks pkgPath and moduleFile); update the
predicate so a package is considered present only if its initPath exists or a
single-file module exists: replace the current return condition with one that
checks existsSync(initPath) or existsSync(moduleFile) (i.e., mark missing when
both are absent). Reference variables: missingPackages, criticalPackages,
pkgPath, initPath, moduleFile, and existsSync.
- Around line 131-140: The code currently indexes platformCriticalPackages using
process.platform in the criticalPackages initializer; replace that direct
platform access by using the platform helpers isWindows() and isLinux() to pick
the platform-specific arrays: use platformCriticalPackages['win32'] when
isWindows() is true and platformCriticalPackages['linux'] when isLinux() is true
(fall back to empty array otherwise) so that the constants
platformCriticalPackages and criticalPackages (referenced in this file) no
longer depend on process.platform directly; import isWindows and isLinux from
the platform module and use them in the criticalPackages construction.

In `@apps/frontend/src/main/terminal/session-handler.ts`:
- Around line 148-170: persistSession currently overwrites createdAt every time
it saves; modify persistSession(terminal: TerminalProcess) to check the existing
session in getTerminalSessionStore() (e.g., via store.getSession or equivalent
lookup by terminal.id) and if a session exists reuse its createdAt value when
building the TerminalSession object, otherwise set createdAt to new
Date().toISOString(); then call store.saveSession(session) as before so
createdAt is preserved for existing sessions.

In `@tests/test_dependency_validator.py`:
- Around line 192-216: The test test_linux_with_secretstorage_missing_warns
fails on Windows CI because it doesn't mock core.dependency_validator.is_windows
to False; update the test to patch core.dependency_validator.is_windows
returning False alongside the existing patch for is_linux so the Windows pywin32
path is skipped when calling validate_platform_dependencies(), ensuring the code
only exercises the Linux secretstorage branch and asserts
_exit_with_secretstorage_warning was called.
- Around line 218-233: The test
test_linux_with_secretstorage_installed_continues currently patches is_linux but
not is_windows, which can make validate_platform_dependencies() run the Windows
pywin32 check on Windows hosts; update the test to also patch
core.dependency_validator.is_windows to return False inside the same
with-context so validate_platform_dependencies() sees Linux=True and
Windows=False, ensuring the pywin32 branch is not executed and the test is
deterministic.
- Around line 235-253: Tests test_windows_skips_secretstorage_validation and
test_macos_skips_secretstorage_validation only patch is_linux but
validate_platform_dependencies() first queries is_windows(), so on Windows CI
the pywin32 check can run and break the test; update both tests to also patch
core.dependency_validator.is_windows to return False (mock is_windows) so
validate_platform_dependencies() skips the pywin32 branch, keeping the intent of
skipping secretstorage validation intact and preventing unintended pywin32
checks.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bb34c25 and 7029e37.

📒 Files selected for processing (11)

• .github/workflows/test-azure-auth.yml
• apps/backend/core/dependency_validator.py
• apps/backend/spec/pipeline/agent_runner.py
• apps/frontend/scripts/download-python.cjs
• apps/frontend/src/main/python-env-manager.ts
• apps/frontend/src/main/terminal-session-store.ts
• apps/frontend/src/main/terminal/claude-integration-handler.ts
• apps/frontend/src/main/terminal/session-handler.ts
• apps/frontend/src/main/terminal/terminal-lifecycle.ts
• apps/frontend/src/main/terminal/terminal-manager.ts
• tests/test_dependency_validator.py

💤 Files with no reviewable changes (1)

• .github/workflows/test-azure-auth.yml

🧰 Additional context used

📓 Path-based instructions (8)

apps/frontend/src/**/*.{tsx,ts}

📄 CodeRabbit inference engine (CLAUDE.md)

apps/frontend/src/**/*.{tsx,ts}: All user-facing text in the frontend MUST use i18n translation keys from react-i18next, not hardcoded strings
Use translation key format namespace:section.key (e.g., navigation:items.githubPRs) when referencing translations in code
For error messages with dynamic content, use i18n interpolation with syntax like t('errors:task.parseError', { error: errorMessage })

Files:

• apps/frontend/src/main/python-env-manager.ts
• apps/frontend/src/main/terminal/terminal-lifecycle.ts
• apps/frontend/src/main/terminal/terminal-manager.ts
• apps/frontend/src/main/terminal/claude-integration-handler.ts
• apps/frontend/src/main/terminal/session-handler.ts
• apps/frontend/src/main/terminal-session-store.ts

**/*.{ts,tsx,js,py}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx,js,py}: Do not check process.platform directly in code - always import platform detection functions from the platform abstraction module
Never hardcode platform-specific paths like C:\Program Files\, /opt/homebrew/bin/, or /usr/local/bin/ directly in code

Files:

• apps/frontend/src/main/python-env-manager.ts
• apps/frontend/src/main/terminal/terminal-lifecycle.ts
• apps/backend/spec/pipeline/agent_runner.py
• apps/frontend/src/main/terminal/terminal-manager.ts
• tests/test_dependency_validator.py
• apps/frontend/src/main/terminal/claude-integration-handler.ts
• apps/frontend/src/main/terminal/session-handler.ts
• apps/backend/core/dependency_validator.py
• apps/frontend/src/main/terminal-session-store.ts

apps/frontend/src/main/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use platform abstraction functions like isWindows(), isMacOS(), isLinux(), getPathDelimiter(), getExecutableExtension(), findExecutable(), joinPaths() from the platform module instead of hardcoding paths or platform checks

Files:

• apps/frontend/src/main/python-env-manager.ts
• apps/frontend/src/main/terminal/terminal-lifecycle.ts
• apps/frontend/src/main/terminal/terminal-manager.ts
• apps/frontend/src/main/terminal/claude-integration-handler.ts
• apps/frontend/src/main/terminal/session-handler.ts
• apps/frontend/src/main/terminal-session-store.ts

apps/frontend/**/*.{ts,tsx,js}

📄 CodeRabbit inference engine (CLAUDE.md)

Frontend code must be built with Electron, React, and TypeScript

Files:

• apps/frontend/src/main/python-env-manager.ts
• apps/frontend/src/main/terminal/terminal-lifecycle.ts
• apps/frontend/src/main/terminal/terminal-manager.ts
• apps/frontend/src/main/terminal/claude-integration-handler.ts
• apps/frontend/src/main/terminal/session-handler.ts
• apps/frontend/src/main/terminal-session-store.ts

apps/frontend/**/*.{ts,tsx}

⚙️ CodeRabbit configuration file

apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.

Files:

• apps/frontend/src/main/python-env-manager.ts
• apps/frontend/src/main/terminal/terminal-lifecycle.ts
• apps/frontend/src/main/terminal/terminal-manager.ts
• apps/frontend/src/main/terminal/claude-integration-handler.ts
• apps/frontend/src/main/terminal/session-handler.ts
• apps/frontend/src/main/terminal-session-store.ts

**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Use the Claude Agent SDK (claude-agent-sdk package) for all AI interactions - NEVER use the Anthropic API directly

Files:

• apps/backend/spec/pipeline/agent_runner.py
• tests/test_dependency_validator.py
• apps/backend/core/dependency_validator.py

apps/backend/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

apps/backend/**/*.py: Always import and use create_client() from core.client instead of instantiating anthropic.Anthropic() directly for LLM client creation
Use the platform abstraction module for path handling and executable detection in Python backend code
All Python backend code must be Python 3.12+ compatible
Use Graphiti as the primary memory system for cross-session context, configured in apps/backend/graphiti_config.py with multi-provider support (OpenAI, Anthropic, Azure, Ollama, Google AI)

Files:

• apps/backend/spec/pipeline/agent_runner.py
• apps/backend/core/dependency_validator.py

---

⚙️ CodeRabbit configuration file

apps/backend/**/*.py: Focus on Python best practices, type hints, and async patterns.
Check for proper error handling and security considerations.
Verify compatibility with Python 3.12+.

Files:

• apps/backend/spec/pipeline/agent_runner.py
• apps/backend/core/dependency_validator.py

tests/**

⚙️ CodeRabbit configuration file

tests/**: Ensure tests are comprehensive and follow pytest conventions.
Check for proper mocking and test isolation.

Files:

• tests/test_dependency_validator.py

🧠 Learnings (11)

📓 Common learnings

Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-16T09:10:31.702Z
Learning: When fixing platform-specific bugs, ensure the fix doesn't break other platforms and rely on multi-platform CI to validate (Windows, macOS, Linux)

📚 Learning: 2026-01-16T09:10:31.701Z

Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-16T09:10:31.701Z
Learning: Applies to **/*.{ts,tsx,js,py} : Do not check `process.platform` directly in code - always import platform detection functions from the platform abstraction module

Applied to files:

• apps/frontend/src/main/python-env-manager.ts

📚 Learning: 2026-01-16T09:10:31.701Z

Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-16T09:10:31.701Z
Learning: Applies to apps/frontend/src/main/**/*.{ts,tsx} : Use platform abstraction functions like `isWindows()`, `isMacOS()`, `isLinux()`, `getPathDelimiter()`, `getExecutableExtension()`, `findExecutable()`, `joinPaths()` from the platform module instead of hardcoding paths or platform checks

Applied to files:

• apps/frontend/src/main/python-env-manager.ts
• apps/frontend/src/main/terminal-session-store.ts

📚 Learning: 2026-01-16T09:10:31.701Z

Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-16T09:10:31.701Z
Learning: Applies to **/*.{ts,tsx,js,py} : Never hardcode platform-specific paths like `C:\Program Files\`, `/opt/homebrew/bin/`, or `/usr/local/bin/` directly in code

Applied to files:

• apps/frontend/src/main/python-env-manager.ts
• apps/frontend/src/main/terminal-session-store.ts

📚 Learning: 2026-01-16T09:10:31.701Z

Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-16T09:10:31.701Z
Learning: Applies to **/*.test.{ts,tsx,js,py} : Write tests for platform-specific code that mock `process.platform` or platform detection functions to test all three platforms (Windows, macOS, Linux)

Applied to files:

• apps/frontend/src/main/python-env-manager.ts

📚 Learning: 2026-01-16T09:10:31.702Z

Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-16T09:10:31.702Z
Learning: When fixing platform-specific bugs, ensure the fix doesn't break other platforms and rely on multi-platform CI to validate (Windows, macOS, Linux)

Applied to files:

• apps/frontend/src/main/python-env-manager.ts

📚 Learning: 2026-01-16T09:10:31.701Z

Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-16T09:10:31.701Z
Learning: Applies to apps/backend/**/*.py : Always import and use `create_client()` from `core.client` instead of instantiating `anthropic.Anthropic()` directly for LLM client creation

Applied to files:

• apps/backend/spec/pipeline/agent_runner.py

📚 Learning: 2026-01-16T09:10:31.701Z

Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-16T09:10:31.701Z
Learning: Applies to apps/backend/core/client.py : Agent-specific tool permissions are configured through `create_client()` based on agent type (planner, coder, qa_reviewer, qa_fixer)

Applied to files:

• apps/backend/spec/pipeline/agent_runner.py

📚 Learning: 2026-01-16T09:10:31.701Z

Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-16T09:10:31.701Z
Learning: Applies to apps/backend/**/*.py : Use the platform abstraction module for path handling and executable detection in Python backend code

Applied to files:

• tests/test_dependency_validator.py

📚 Learning: 2026-01-11T10:48:12.867Z

Learnt from: g1331
Repo: AndyMik90/Auto-Claude PR: 912
File: tests/test_issue_884_plan_schema.py:287-296
Timestamp: 2026-01-11T10:48:12.867Z
Learning: In test files under tests/, prefer keeping small helper classes (e.g., DummyClient) local to individual test functions when duplication is minimal, tests are integration-style and benefit from self-containment, and tests/conftest.py is already large. Extract helpers to conftest only when duplication across tests is significant or when shared setup is truly reusable across many tests.

Applied to files:

• tests/test_dependency_validator.py

📚 Learning: 2026-01-16T09:10:31.701Z

Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-16T09:10:31.701Z
Learning: Applies to apps/frontend/**/*.{ts,tsx,js} : Frontend code must be built with Electron, React, and TypeScript

Applied to files:

• apps/frontend/src/main/terminal-session-store.ts

🧬 Code graph analysis (4)

apps/frontend/src/main/python-env-manager.ts (1)

apps/frontend/scripts/download-python.cjs (1)

• path (20-20)

apps/backend/spec/pipeline/agent_runner.py (1)

apps/backend/core/client.py (1)

• create_client (660-1052)

apps/frontend/src/main/terminal/session-handler.ts (2)

apps/frontend/src/main/terminal/types.ts (1)

• TerminalProcess (8-24)

apps/frontend/src/main/terminal-session-store.ts (2)

• getTerminalSessionStore (447-452)
• TerminalSession (9-21)

apps/backend/core/dependency_validator.py (1)

apps/backend/core/platform/__init__.py (2)

• is_linux (76-78)
• is_windows (66-68)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

• GitHub Check: Seer Code Review
• GitHub Check: test-frontend (windows-latest)
• GitHub Check: CodeQL (javascript-typescript)
• GitHub Check: CodeQL (python)

🔇 Additional comments (13)

apps/backend/spec/pipeline/agent_runner.py (1)

122-124: LGTM! Lazy import correctly breaks circular dependency.

The lazy import of create_client inside run_agent() is a valid pattern for breaking the circular import chain. Python caches module imports, so subsequent calls to run_agent() will reuse the cached module without re-importing. The import source (core.client) complies with coding guidelines.

tests/test_dependency_validator.py (1)

261-313: LGTM! Comprehensive tests for _exit_with_secretstorage_warning.

The tests thoroughly verify:

• Warning message contains helpful installation instructions
• Fallback behavior (.env) is documented in the warning
• Virtual environment path is included
• Warning does NOT call sys.exit() (non-blocking behavior)

This aligns well with the PR objective of emitting a warning rather than blocking error for missing secretstorage.

apps/frontend/src/main/terminal/terminal-manager.ts (2)

42-44: Verify synchronous persistence won't block the main Electron process.

The periodic save timer now calls persistAllSessions synchronously every 30 seconds. If this involves significant I/O (writing to disk), it could cause brief UI freezes in the Electron main process. Ensure the underlying implementation in session-handler.ts uses efficient synchronous operations (e.g., fs.writeFileSync with small payloads) or consider keeping async persistence for the timer path to avoid blocking the event loop.

---

371-373: LGTM! Synchronous persistence is appropriate here.

Using persistSession synchronously when worktree config changes is reasonable since this is triggered by explicit user action and immediate persistence ensures data consistency.

apps/frontend/src/main/terminal/terminal-lifecycle.ts (4)

100-102: LGTM! Synchronous persistence at creation is acceptable.

Persisting session data synchronously immediately after terminal creation ensures the session is saved before any subsequent operations. The payload is typically small (single session object), so blocking impact should be minimal.

---

185-187: LGTM! Correct re-persistence after restoring session metadata.

Persisting after restoring title and worktreeConfig ensures the updated state is saved, which is necessary since createTerminal persisted before these values were set.

---

219-222: LGTM! Persistence of Claude mode state is correctly placed.

Persisting the Claude mode and pending resume state ensures these flags survive app restarts, enabling proper session restoration behavior.

---

279-279: LGTM! Synchronous persistence before shutdown is appropriate.

Using synchronous persistAllSessions before destroying all terminals during app shutdown ensures all session state is written to disk before the process terminates. This prevents data loss.

apps/backend/core/dependency_validator.py (2)

11-11: LGTM! Correctly uses platform abstraction module.

Importing is_linux and is_windows from core.platform complies with the coding guidelines that prohibit direct sys.platform checks. This ensures consistent platform detection across the codebase.

---

29-36: LGTM! Non-blocking validation for optional dependency.

The secretstorage check correctly emits a warning instead of exiting, allowing the app to fall back to .env storage. This mirrors the pattern mentioned in the PR objectives for Windows pywin32 (ACS-306) but appropriately differs in severity since secretstorage is optional.

apps/frontend/src/main/terminal/session-handler.ts (1)

173-182: LGTM — centralized persistence path.

Delegating to persistSession() keeps the guardrails consistent and simplifies the multi-terminal flow.

apps/frontend/src/main/terminal/claude-integration-handler.ts (1)

905-934: LGTM — sync persistence aligned with new API.

The resume flow now uses the unified persistSession entry point.

apps/frontend/src/main/terminal-session-store.ts (1)

1-3: LGTM — import aligns with sync persistence.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Preserve createdAt when re-saving existing sessions.

persistSession() sets createdAt to new Date() on every save, which overwrites the original creation timestamp. If createdAt is meant to represent the initial session creation, this silently changes semantics and can skew ordering/history.

Consider reusing the stored createdAt when the session already exists:

🛠️ Proposed fix

 export function persistSession(terminal: TerminalProcess): void {
   if (!terminal.projectPath) {
     return;
   }
 
   const store = getTerminalSessionStore();
+  const now = new Date().toISOString();
+  const existing = store.getSession(terminal.projectPath, terminal.id);
   const session: TerminalSession = {
     id: terminal.id,
     title: terminal.title,
     cwd: terminal.cwd,
     projectPath: terminal.projectPath,
     isClaudeMode: terminal.isClaudeMode,
     claudeSessionId: terminal.claudeSessionId,
     outputBuffer: terminal.outputBuffer,
-    createdAt: new Date().toISOString(),
-    lastActiveAt: new Date().toISOString(),
+    createdAt: existing?.createdAt ?? now,
+    lastActiveAt: now,
     worktreeConfig: terminal.worktreeConfig,
   };
   store.saveSession(session);
 }

🤖 Prompt for AI Agents

In `@apps/frontend/src/main/terminal/session-handler.ts` around lines 148 - 170,
persistSession currently overwrites createdAt every time it saves; modify
persistSession(terminal: TerminalProcess) to check the existing session in
getTerminalSessionStore() (e.g., via store.getSession or equivalent lookup by
terminal.id) and if a session exists reuse its createdAt value when building the
TerminalSession object, otherwise set createdAt to new Date().toISOString();
then call store.saveSession(session) as before so createdAt is preserved for
existing sessions.

[sentry]

Bug: The new validation logic no longer checks for __init__.py in directory-based packages, which could allow a corrupted package to be considered valid.
_{Severity: CRITICAL}

Suggested Fix

The validation logic should differentiate between directory-based packages and single-file modules. For directory packages, it should verify the existence of both the directory and its __init__.py file. For single-file modules, it should check for the .py file. A possible implementation is !fs.existsSync(moduleFile) && !(fs.existsSync(pkgPath) && fs.existsSync(initFile)), which ensures one of the two valid package structures exists.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: apps/frontend/scripts/download-python.cjs#L725

Potential issue: The updated package validation logic incorrectly approves
directory-based packages without verifying the presence of an `__init__.py` file. The
new condition `!fs.existsSync(pkgPath) && !fs.existsSync(moduleFile)` checks if both a
package directory and a single-file module are missing. For a directory-based package
like `secretstorage`, if the directory exists but its `__init__.py` is missing due to a
corrupted installation, the check will incorrectly pass. This will cause the application
to fail at runtime with an `ImportError` when it attempts to import the corrupted
package, defeating the purpose of the validation.

_{Did we get this right? 👍 / 👎 to inform future reviews.}

[sentry]

Bug: The call to secretstorage.get_default_collection(None) is incorrect. It requires a valid DBusConnection object, not None, which will cause a runtime error on Linux.
_{Severity: CRITICAL}

Suggested Fix

Initialize a DBus connection before getting the collection. First, call connection = secretstorage.dbus_init(). Then, pass the created connection object to the collection function like so: collection = secretstorage.get_default_collection(connection).

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: apps/backend/core/auth.py#L167

Potential issue: The function `get_default_collection` from the `secretstorage` library
is called with `None` as its first argument. According to the library's documentation,
this argument is mandatory and must be a `DBusConnection` object, which is obtained by
calling `secretstorage.dbus_init()`. This incorrect usage will lead to a runtime error,
such as a `TypeError` or `AttributeError`, when the code is executed on a Linux system.
This will cause the retrieval of OAuth tokens from the Linux keyring to fail. The unit
tests do not catch this issue because they mock the
`secretstorage.get_default_collection` call, bypassing the actual library's API
validation.

_{Did we get this right? 👍 / 👎 to inform future reviews.}

[sentry]

Bug: The function process_sdk_stream will crash with a NameError because it uses the variables last_progress_log, PROGRESS_LOG_INTERVAL, and model_info without initializing them.
_{Severity: CRITICAL}

Suggested Fix

Initialize last_progress_log = 0 and define a constant PROGRESS_LOG_INTERVAL at the start of the function. For model_info, use the provided model parameter and the _short_model_name helper, for example: model_info = f" ({_short_model_name(model)})" if model else "". Also, remove the duplicate safe_print statement.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: apps/backend/runners/github/services/sdk_utils.py#L182-L183

Potential issue: The function `process_sdk_stream` references three variables that are
never initialized: `last_progress_log`, `PROGRESS_LOG_INTERVAL`, and `model_info`. The
first use of `last_progress_log` in a conditional check on line 182 will immediately
raise a `NameError` at runtime, as it has no starting value. Similarly, `model_info` is
used in a log message on line 339 without being defined. Since this function is called
during PR review operations, any such operation will crash, preventing the feature from
working.

_{Did we get this right? 👍 / 👎 to inform future reviews.}

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

apps/frontend/src/__tests__/integration/terminal-copy-paste.test.ts (1)

259-432: Add macOS platform test for clipboard paste integration.

The clipboard read tests cover Windows (line 279) and Linux (line 358), but macOS is not tested. Based on learnings, platform-specific code should test all three platforms.

♻️ Suggested test case for macOS

Add a test for macOS platform alongside the existing Windows and Linux tests:

it('should integrate clipboard.readText() with xterm.paste() on macOS', async () => {
  const { useXterm } = await import('../../renderer/components/terminal/useXterm');

  // Mock macOS platform
  Object.defineProperty(navigator, 'platform', {
    value: 'MacIntel',
    writable: true
  });

  let keyEventHandler: ((event: KeyboardEvent) => boolean) | null = null;
  const mockPaste = vi.fn();

  // ... rest of test setup similar to Windows test ...
  
  // Test with metaKey instead of ctrlKey for macOS
  await act(async () => {
    const event = new KeyboardEvent('keydown', {
      key: 'v',
      metaKey: true  // Cmd+V on macOS
    });

    if (keyEventHandler) {
      keyEventHandler(event);
      await new Promise(resolve => setTimeout(resolve, 0));
    }
  });

  expect(mockClipboard.readText).toHaveBeenCalled();
  expect(mockPaste).toHaveBeenCalledWith('pasted text');
});

[AndyMik90]

✅ Auto Claude Review - APPROVED

Status: Ready to Merge

Summary: ### Merge Verdict: ✅ READY TO MERGE

✅ Ready to merge - All checks passing, no blocking issues found.

No blocking issues. 2 non-blocking suggestion(s) to consider

Risk Assessment

Factor	Level	Notes
Complexity	High	Based on lines changed
Security Impact	None	Based on security findings
Scope Coherence	Good	Based on structural review

Findings Summary

• Low: 2 issue(s)

---

Generated by Auto Claude PR Review

---

💡 Suggestions (2)

These are non-blocking suggestions for consideration:

🔵 [2772b017d793] [LOW] platformCriticalPackages definition duplicated across 3 locations

📁 apps/frontend/scripts/download-python.cjs:714

The platform-critical packages validation logic is duplicated in: (1) download-python.cjs lines 714-727, (2) download-python.cjs lines 826-839, (3) python-env-manager.ts lines 132-151. Comments acknowledge this with 'Note: Same list exists in python-env-manager.ts - keep them in sync'. While this works correctly, manual synchronization comments are a code smell that increases maintenance burden. The code is documented and functional, so this is a suggestion for future improvement rather than a blocking issue.

Suggested fix:

Consider extracting platformCriticalPackages to a shared config file that both download-python.cjs and python-env-manager.ts can import. Alternatively, extract the validation logic within download-python.cjs into a reusable function to eliminate internal duplication.

🔵 [17c74a3b2398] [LOW] AI comment about createdAt preservation is a false positive

📁 apps/frontend/src/main/terminal/session-handler.ts:165

CodeRabbit flagged that persistSession() overwrites createdAt on every save. However, examining terminal-session-store.ts reveals that saveSession() (lines 189-203) already preserves createdAt for existing sessions - only new sessions get a fresh timestamp. The AI misread the code flow between session-handler.ts and terminal-session-store.ts.

Suggested fix:

No fix needed - this is a false positive. The createdAt is already preserved for existing sessions.

---

This automated review found no blocking issues. The PR can be safely merged.

Generated by Auto Claude

[sentry]

Bug: The Claude invocation functions call undefined functions like escapeShellCommand and buildPathPrefix, and use an undefined variable escapedTempFile, which will cause a runtime crash.
_{Severity: CRITICAL}

Suggested Fix

Ensure that the helper functions escapeShellCommand and buildPathPrefix are properly defined or imported. Implement the missing logic to create the temporary file and define the escapedTempFile variable before it is used. Also, correct the property name passed to buildClaudeShellCommand from escapedTempFile to tempFile to match the ClaudeCommandConfig type definition.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: apps/frontend/src/main/terminal/claude-integration-handler.ts#L474-L475

Potential issue: The functions `invokeClaude`, `invokeClaudeAsync`, `resumeClaude`, and
`resumeClaudeAsync` will fail at runtime due to multiple `ReferenceError`s. The code
calls the functions `escapeShellCommand` and `buildPathPrefix`, which are not defined or
imported anywhere. Additionally, the variable `escapedTempFile` is used when building
the shell command, but it is never defined because the logic for creating the temporary
token file is missing. These errors appear to be the result of an incomplete refactoring
and will cause any attempt to invoke Claude to crash, making the feature non-functional.

_{Did we get this right? 👍 / 👎 to inform future reviews.}

[sentry]

Bug: The invokeClaude function contains calls to undefined functions and has a broken control flow with a dangling else if statement, which will cause runtime crashes.
_{Severity: CRITICAL}

Suggested Fix

Complete the refactoring of the invokeClaude and invokeClaudeAsync functions. This involves either restoring the deleted helper functions (escapeShellCommand, buildPathPrefix) or replacing their call sites with the correct new logic. The control flow structure must also be fixed by restoring the missing if condition or removing the orphaned else if block to resolve the syntax error.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: apps/frontend/src/main/terminal/claude-integration-handler.ts#L478-L482

Potential issue: An incomplete refactoring in the `invokeClaude` and `invokeClaudeAsync`
functions has resulted in code that is not executable. The functions attempt to call
`escapeShellCommand` and `buildPathPrefix`, which are no longer defined or imported
anywhere in the codebase. This will cause a `ReferenceError` at runtime. Additionally,
the control flow is broken due to an `else if` block that is missing its corresponding
`if` statement, which is a syntax error that will prevent the module from being parsed
and executed. These issues will cause any attempt to use the Claude integration to fail.

_{Did we get this right? 👍 / 👎 to inform future reviews.}

[coderabbitai]

Actionable comments posted: 13

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (10)

apps/backend/runners/github/services/sdk_utils.py (1)

299-306: Guard result_preview in DEBUG tool-result path.

result_preview is referenced without assignment in the non-subagent DEBUG_MODE branch.

🛠️ Suggested fix

@@
                     elif DEBUG_MODE:
                         status = "ERROR" if is_error else "OK"
                         safe_print(
                             f"[DEBUG {context_name}] Tool result: {tool_id} [{status}]"
                         )
-                        if result_preview:
-                            safe_print(
-                                f"[{context_name}] Tool result [{status}]: {result_preview}{'...' if len(str(result_content)) > 100 else ''}"
-                            )
+                        if result_content:
+                            result_preview = (
+                                str(result_content)[:100].replace("\n", " ").strip()
+                            )
+                            if result_preview:
+                                safe_print(
+                                    f"[{context_name}] Tool result [{status}]: {result_preview}{'...' if len(str(result_content)) > 100 else ''}"
+                                )

apps/frontend/src/renderer/components/terminal/WorktreeSelector.tsx (1)

177-183: Critical: JSX syntax error causes build failure.

The pipeline is failing with a parsing error. The JSX structure around the loading state conditional is malformed. Line 181 has a stray </> closing tag, and the ternary operator structure is broken.

Looking at the code, the loading conditional should be:

{isLoading ? (
  <div>...loading...</div>
) : (
  <>...content...</>
)}

But the current code has mismatched fragment tags causing the parse error.

🐛 Proposed fix

          {isLoading ? (
            <div className="flex items-center justify-center py-2">
              <Loader2 className="h-4 w-4 animate-spin text-muted-foreground" />
            </div>
-          </>
-        ) : (
-          <>
+          ) : (
+            <>
            {/* Terminal Worktrees Section */}

And ensure the fragment is properly closed before the end of the scrollable div at line 257:

+            </>
+          )}
+        </div>

apps/backend/core/client.py (1)

27-33: Remove unused import causing pipeline failure.

The import get_claude_detection_paths_structured is not used anywhere in this file, causing the lint error: F401 imported but unused.

🐛 Proposed fix

 from core.platform import (
-    get_claude_detection_paths_structured,
     get_comspec_path,
     is_macos,
     is_windows,
     validate_cli_path,
 )

apps/frontend/src/main/terminal/__tests__/claude-integration-handler.test.ts (3)

214-244: Critical: Unclosed try block without catch/finally.

The test at lines 214-244 has a try block that starts at line 218 but is never closed with a catch or finally clause. The closing brace on line 244 appears to end the it() block prematurely.

🐛 Proposed fix

   it('converts Windows PATH separators to colons for bash invocations', async () => {
     const originalPlatform = Object.getOwnPropertyDescriptor(process, 'platform');
     Object.defineProperty(process, 'platform', { value: 'win32' });

     try {
       // ... test code ...
       expect(profileManager.markProfileUsed).toHaveBeenCalledWith('default');
+    } finally {
+      if (originalPlatform) {
+        Object.defineProperty(process, 'platform', originalPlatform);
+      }
+    }
   });

---

77-83: Missing import: isWindows is used but not imported.

The mockPlatform helper function uses isWindows at line 81, but this function is not imported at the top of the file. This will cause a ReferenceError at runtime.

🐛 Proposed fix

Add the missing import at the top of the file:

 import { describe, expect, it, vi, beforeEach } from 'vitest';
 import type * as pty from '@lydell/node-pty';
 import type { TerminalProcess } from '../types';
 import { buildCdCommand } from '../../../shared/utils/shell-escape';
+import { isWindows } from '../../platform';

---

100-109: Missing import: escapeShellArg is used but not imported.

The getQuotedCommand helper function uses escapeShellArg at line 108, but this function is not imported. This will cause a ReferenceError at runtime.

🐛 Proposed fix

Add the missing import:

-import { buildCdCommand } from '../../../shared/utils/shell-escape';
+import { buildCdCommand, escapeShellArg } from '../../../shared/utils/shell-escape';

apps/frontend/src/main/terminal/claude-integration-handler.ts (4)

472-476: Critical: Undefined functions escapeShellCommand and buildPathPrefix will cause runtime crash.

The code calls escapeShellCommand(claudeCmd) and buildPathPrefix(claudeEnv.PATH || '') but these functions are not defined or imported anywhere in the file. This will cause a ReferenceError at runtime.

Additionally, line 478 uses escapedTempFile which is never defined - the logic to create the temporary token file and assign this variable is missing.

🐛 Required actions

1. Define or import escapeShellCommand - likely should be escapeShellArg from shell-escape.ts
2. Define or import buildPathPrefix - this helper needs to be created to generate the PATH prefix for shell commands
3. Add the missing temp file creation logic that should define escapedTempFile

Example fix for the escape function:

-const escapedClaudeCmd = escapeShellCommand(claudeCmd);
+const escapedClaudeCmd = escapeShellArg(claudeCmd);

The buildPathPrefix function needs to be defined. Based on the test expectations, it should generate platform-specific PATH prefix strings:

function buildPathPrefix(pathValue: string): string {
  if (!pathValue) return '';
  if (isWindows()) {
    return `set "PATH=${pathValue}" && `;
  }
  return `PATH='${normalizePathForBash(pathValue)}' `;
}

---

567-569: Critical: escapeShellCommand is not defined in resumeClaude.

Similar to invokeClaude, the resumeClaude function uses escapeShellCommand which is not defined or imported.

---

783-784: Critical: escapeShellCommand and buildPathPrefix not defined in resumeClaudeAsync.

The async resume function also uses the undefined escapeShellCommand and buildPathPrefix functions.

---

85-103: Windows support missing in buildClaudeShellCommand.

The buildClaudeShellCommand function only handles Unix/bash syntax for the temp-file and config-dir methods. According to the test file's helper functions and the AI summary, Windows support should use different syntax (e.g., call for batch files, cls instead of clear, etc.).

The function needs platform-conditional logic to support Windows.

🔧 Suggested approach

export function buildClaudeShellCommand(
  cwdCommand: string,
  pathPrefix: string,
  escapedClaudeCmd: string,
  config: ClaudeCommandConfig,
  extraFlags?: string
): string {
  const fullCmd = extraFlags ? `${escapedClaudeCmd}${extraFlags}` : escapedClaudeCmd;
  
  if (isWindows()) {
    switch (config.method) {
      case 'temp-file':
        // Windows: call batch file, then delete, then run command
        return `cls && ${cwdCommand}call "${config.escapedTempFile}" && del "${config.escapedTempFile}" && ${fullCmd}\r`;
      case 'config-dir':
        return `cls && ${cwdCommand}set "CLAUDE_CONFIG_DIR=${config.escapedConfigDir}" && ${fullCmd}\r`;
      default:
        return `${cwdCommand}${pathPrefix}${fullCmd}\r`;
    }
  }
  
  // Unix/macOS
  switch (config.method) {
    case 'temp-file':
      return `clear && ${cwdCommand}HISTFILE= HISTCONTROL=ignorespace ${pathPrefix}bash -c "source ${config.escapedTempFile} && rm -f ${config.escapedTempFile} && exec ${fullCmd}"\r`;
    case 'config-dir':
      return `clear && ${cwdCommand}HISTFILE= HISTCONTROL=ignorespace CLAUDE_CONFIG_DIR=${config.escapedConfigDir} ${pathPrefix}bash -c "exec ${fullCmd}"\r`;
    default:
      return `${cwdCommand}${pathPrefix}${fullCmd}\r`;
  }
}

As per coding guidelines, use platform abstraction functions like isWindows() from the platform module instead of hardcoding platform checks.

🤖 Fix all issues with AI agents

In `@apps/backend/core/client.py`:
- Line 163: Replace local platform checks that assign is_windows =
platform.system() == "Windows" with calls to the platform abstraction
is_windows() function imported from core.platform; specifically update the
occurrences in the module-level code, the _validate_claude_cli() function, and
the find_claude_cli() function to use is_windows() directly (and remove the
unused platform import if there are no other references). Ensure you do not
shadow the imported is_windows symbol with a local variable and keep existing
logic branches intact while switching to is_windows().
- Line 346: Replace the direct platform check platform.system() == "Darwin" with
the imported helper is_macos() from core.platform; locate the conditional in
client.py (the if using platform.system()) and change it to call is_macos() so
the code uses the centralized platform detection logic (ensure is_macos is
imported where used).

In `@apps/backend/security/shell_validators.py`:
- Around line 129-130: Replace the manual split with os.path.basename to
correctly handle Windows backslashes: compute base_cmd =
os.path.basename(first_cmd) (preserving existing behavior when first_cmd has no
separators), then continue using base_cmd for the nested command validation
against SHELL_INTERPRETERS; ensure this change touches the assignment of
base_cmd that currently uses first_cmd.rsplit("/", 1) so Windows paths like
"C:\\Windows\\..." are normalized correctly.

In `@apps/frontend/scripts/download-python.cjs`:
- Around line 825-839: Extract the duplicated package validation into two
helpers: create getCriticalPackages(nodePlatform) that returns the base array
['claude_agent_sdk','dotenv','pydantic_core'] merged with
platformCriticalPackages (use the existing platformCriticalPackages mapping),
and create findMissingPackages(packages, sitePackagesDir) that implements the
filter logic used to compute postInstallMissing (checking pkgPath + __init__.py
and pkg + '.py'). Replace the two duplicated blocks that build criticalPackages
and postInstallMissing with calls to getCriticalPackages(info.nodePlatform) and
findMissingPackages(criticalPackages, sitePackagesDir), keeping the original
variable names (criticalPackages, postInstallMissing) for compatibility.

In `@apps/frontend/src/__tests__/integration/terminal-copy-paste.test.ts`:
- Around line 80-85: The test overrides global.requestAnimationFrame with a
vi.fn which can leak into other tests; capture the original (e.g. const
originalRaf = global.requestAnimationFrame) before assigning the mock in the
test file and restore it in an afterEach block (using
global.requestAnimationFrame = originalRaf) so the global is reset after each
test; apply this change for both places where requestAnimationFrame is mocked
(the blocks that use vi.fn and callback.call) and ensure the afterEach runs even
if tests fail.

In
`@apps/frontend/src/main/terminal/__tests__/claude-integration-handler.test.ts`:
- Around line 541-756: There are duplicated, hardcoded Unix-style tests that
replicate cases already covered by the parameterized
describe.each(['win32','darwin','linux']) block; remove the duplicate it(...)
blocks whose titles include "uses the temp token flow when the active profile
has an oauth token", "prefers the temp token flow when profile has both oauth
token and config dir", "handles missing profiles by falling back to the default
command", "uses the config dir flow when the active profile has a config dir",
"uses profile switching when a non-default profile is requested", and "uses
--continue regardless of sessionId (sessionId is deprecated)"; delete these
duplicated tests (the blocks that call invokeClaude and resumeClaude and assert
terminal writes) so only the parameterized describe.each tests remain and avoid
name conflicts and platform assumptions.
- Around line 199-212: Test code invoking invokeClaude and asserting
side-effects is placed outside any it()/test() block and references an undefined
profileManager; wrap the async invocation and assertions in an it(...) async
test block (e.g., it('invokes Claude and persists session', async () => { ...
})) so the top-level await runs inside an async function, and ensure
profileManager is available in the test scope (import or mock profileManager
used in the assertions, or reference the existing mocked instance used elsewhere
in this file). Locate the invokeClaude call, createMockTerminal usage,
written/expect assertions, and the
mockReleaseSessionId/mockPersistSession/profileManager.getActiveProfile/profileManager.markProfileUsed
references to move into the new it() block and adjust imports/mocks accordingly.

In `@apps/frontend/src/main/terminal/claude-integration-handler.ts`:
- Around line 630-643: The block that sets up Claude mode and YOLO flags is
duplicated in invokeClaudeAsync (the pre-try block and again inside the try),
mirroring invokeClaude; remove the redundant pre-try initialization so the code
only sets extraFlags = dangerouslySkipPermissions ? YOLO_MODE_FLAG : undefined
and updates terminal.isClaudeMode, terminal.dangerouslySkipPermissions, calls
SessionHandler.releaseSessionId(terminal.id), and clears
terminal.claudeSessionId once — keep that single initialization where the try
block currently performs it (or move the unique one outside the try and delete
the duplicate inside) to avoid double-execution and ensure only one source of
truth for the YOLO_MODE_FLAG and terminal state.
- Around line 684-702: In invokeClaudeAsync there’s a mismatched/ orphaned "else
if" and duplicated unreachable post-catch block; fix by correcting the
surrounding try/catch/if brace structure so the "else if
(activeProfile.configDir)" branches attach to the original if chain, remove the
duplicated terminal invocation/finalize block after the catch, and consolidate
the terminal.pty.write, profileManager.markProfileUsed, and finalizeClaudeInvoke
calls into the single correct branch path (functions/idents to check:
invokeClaudeAsync, buildClaudeShellCommand, terminal.pty.write,
profileManager.markProfileUsed, finalizeClaudeInvoke) so control flow
returns/continues correctly without parse errors or dead code.
- Around line 478-496: Restore the proper if/else-if/else conditional chain
around the temp-file/config-dir/default branches so the orphaned "else if
(activeProfile.configDir)" is attached to the initial token check: wrap the
temp-file branch in if (activeProfile?.oauthToken) { ... } then use else if
(activeProfile?.configDir) { buildClaudeShellCommand(..., { method:
'config-dir', escapedConfigDir }, ...); terminal.pty.write(...);
profileManager.markProfileUsed(activeProfile.id); finalizeClaudeInvoke(...); }
else { debugLog('[ClaudeIntegration:invokeClaude] WARNING: No token or configDir
available for non-default profile'); } — ensure you reference the same symbols
(activeProfile, buildClaudeShellCommand, terminal.pty.write,
profileManager.markProfileUsed, finalizeClaudeInvoke) and keep the existing
debugLog/return behavior inside each branch so the syntax error is resolved.
- Around line 95-98: The ClaudeCommandConfig union is missing the escaped
property names used in command construction: update the type for the 'temp-file'
and 'config-dir' variants in ClaudeCommandConfig to include escapedTempFile:
string and escapedConfigDir: string respectively (keeping tempFile/configDir if
still needed), so the usages of config.escapedTempFile and
config.escapedConfigDir in the command-building code (where fullCmd is
assembled) match the type definition.
- Around line 527-541: Remove the unreachable duplicate block that appears after
the `throw error` in the catch path: delete the repeated calls to
buildClaudeShellCommand(..., { method: 'default' }, ...),
terminal.pty.write(command), profileManager.markProfileUsed(...),
finalizeClaudeInvoke(terminal, activeProfile, projectPath, startTime, getWindow,
onSessionCapture), and the trailing debugLog('[ClaudeIntegration:invokeClaude]
========== INVOKE CLAUDE COMPLETE (default) =========='), leaving only the
original working invocation logic earlier in the function (the code paths around
buildClaudeShellCommand, terminal.pty.write, profileManager.markProfileUsed, and
finalizeClaudeInvoke) so there is no duplicate/unreachable code after the `throw
error`.

In `@apps/frontend/src/renderer/components/terminal/WorktreeSelector.tsx`:
- Around line 210-218: The delete button in WorktreeSelector (the button that
calls setDeleteWorktree(wt) inside the dropdown) lacks an explicit type and will
default to submit; update the button element to include type="button" to prevent
accidental form submissions—locate the button with the onClick handler (calling
setDeleteWorktree) and add the type attribute while keeping existing props and
handlers intact.

♻️ Duplicate comments (4)

apps/backend/runners/github/services/sdk_utils.py (2)

185-201: Restore progress logging state initialization to avoid NameError.

last_progress_log and PROGRESS_LOG_INTERVAL are referenced without initialization, which will crash on the first message.

🛠️ Suggested fix

@@ async def process_sdk_stream(
-    result_text = ""
+    result_text = ""
     structured_output = None
     agents_invoked = []
     msg_count = 0
     stream_error = None
+    # Progress logging state (restore prior defaults)
+    PROGRESS_LOG_INTERVAL = 25
+    last_progress_log = 0

---

327-337: Define model_info and remove duplicate “Invoking agent” log.

model_info is undefined and will raise NameError; the log is also duplicated.

🛠️ Suggested fix

@@ async def process_sdk_stream(
-    result_text = ""
+    result_text = ""
     structured_output = None
     agents_invoked = []
     msg_count = 0
     stream_error = None
+    model_info = f" ({_short_model_name(model)})" if model else ""
@@
-                                    safe_print(
-                                        f"[{context_name}] Invoking agent: {agent_name}"
-                                    )
-                                    safe_print(
-                                        f"[{context_name}] Invoking agent: {agent_name}{model_info}"
-                                    )
+                                    safe_print(
+                                        f"[{context_name}] Invoking agent: {agent_name}{model_info}"
+                                    )

tests/test_dependency_validator.py (1)

236-256: Missing explicit is_windows mock for deterministic behavior.

This test mocks is_linux to False but doesn't explicitly mock is_windows. While the test happens to work because mock_import returns a MagicMock for pywintypes, it's inconsistent with the other tests and could behave unexpectedly in edge cases.

For clarity and consistency with test_macos_skips_secretstorage_validation (lines 270-271), explicitly mock is_windows to True.

♻️ Proposed fix

     def test_windows_skips_secretstorage_validation(self):
         """Windows should skip secretstorage validation."""
         import builtins

         original_import = builtins.__import__

         def mock_import(name, *args, **kwargs):
             # Allow pywintypes to succeed (Windows validation passes)
             if name == "pywintypes":
                 return MagicMock()
             # secretstorage import fails (but should be skipped on Windows)
             if name == "secretstorage":
                 raise ImportError("No module named 'secretstorage'")
             return original_import(name, *args, **kwargs)

-        with patch("core.dependency_validator.is_linux", return_value=False), \
+        with patch("core.dependency_validator.is_windows", return_value=True), \
+             patch("core.dependency_validator.is_linux", return_value=False), \
              patch("core.dependency_validator._warn_missing_secretstorage") as mock_warning, \
              patch("builtins.__import__", side_effect=mock_import):
             # Should not call warning function
             validate_platform_dependencies()
             mock_warning.assert_not_called()

Based on learnings, tests for platform-specific code should mock platform detection functions to test all three platforms deterministically.

apps/frontend/src/main/terminal/claude-integration-handler.ts (1)

429-441: Critical: Duplicate initialization code outside try block.

Lines 429-441 contain initialization code that is duplicated inside the try block (lines 448-460). This appears to be an incomplete refactoring. The code at lines 429-441 should be removed as it's duplicated and creates variable scoping issues.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Inconsistent platform detection pattern - local variables shadow imported functions.

Multiple places create local is_windows variables using platform.system() == "Windows", which shadows the imported is_windows function from core.platform. This violates the coding guideline to use platform abstraction functions and creates confusion.

♻️ Proposed fix

Use the imported is_windows() function directly instead of creating local variables:

 def _get_claude_detection_paths() -> dict[str, list[str]]:
     ...
     home_dir = Path.home()
-    is_windows = platform.system() == "Windows"
-
-    ...
-
-    if is_windows:
+    if is_windows():
         platform_paths = [

Apply similar changes to _validate_claude_cli() (line 248) and find_claude_cli() (line 320).

This also allows removing import platform from line 19 if no other usages remain.

As per coding guidelines: "Do not check process.platform directly in code - always import platform detection functions from the platform abstraction module."

Also applies to: 248-248, 320-320

🤖 Prompt for AI Agents

In `@apps/backend/core/client.py` at line 163, Replace local platform checks that
assign is_windows = platform.system() == "Windows" with calls to the platform
abstraction is_windows() function imported from core.platform; specifically
update the occurrences in the module-level code, the _validate_claude_cli()
function, and the find_claude_cli() function to use is_windows() directly (and
remove the unused platform import if there are no other references). Ensure you
do not shadow the imported is_windows symbol with a local variable and keep
existing logic branches intact while switching to is_windows().

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Use is_macos() instead of direct platform check.

This line uses platform.system() == "Darwin" directly instead of the imported is_macos() function from core.platform.

♻️ Proposed fix

     # 3. Homebrew paths (macOS)
-    if platform.system() == "Darwin":
+    if is_macos():
         for hb_path in paths["homebrew"]:

📝 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

	if platform.system() == "Darwin":
	# 3. Homebrew paths (macOS)
	if is_macos():
	for hb_path in paths["homebrew"]:

🤖 Prompt for AI Agents

In `@apps/backend/core/client.py` at line 346, Replace the direct platform check
platform.system() == "Darwin" with the imported helper is_macos() from
core.platform; locate the conditional in client.py (the if using
platform.system()) and change it to call is_macos() so the code uses the
centralized platform detection logic (ensure is_macos is imported where used).

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Cross-platform path handling broken — security bypass on Windows.

The simple / split does not handle Windows backslash paths. A command like C:\Windows\System32\bash.exe -c "malicious" would yield base_cmd = "C:\\Windows\\System32\\bash.exe" which won't match SHELL_INTERPRETERS, bypassing nested command validation entirely.

The removed _cross_platform_basename presumably handled both separators. Per coding guidelines, use the platform abstraction module for path handling.

Proposed fix using os.path.basename

-            # Handle paths like /bin/bash
-            base_cmd = first_cmd.rsplit("/", 1)[-1] if "/" in first_cmd else first_cmd
+            # Handle paths like /bin/bash or C:\Windows\System32\bash.exe
+            base_cmd = os.path.basename(first_cmd)

os.path.basename handles both Unix and Windows path separators correctly and is already available since os is imported at line 13.

🤖 Prompt for AI Agents

In `@apps/backend/security/shell_validators.py` around lines 129 - 130, Replace
the manual split with os.path.basename to correctly handle Windows backslashes:
compute base_cmd = os.path.basename(first_cmd) (preserving existing behavior
when first_cmd has no separators), then continue using base_cmd for the nested
command validation against SHELL_INTERPRETERS; ensure this change touches the
assignment of base_cmd that currently uses first_cmd.rsplit("/", 1) so Windows
paths like "C:\\Windows\\..." are normalized correctly.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Consider extracting duplicated validation logic into a helper function.

This block (Lines 825-839) is identical to the earlier block at Lines 713-727. Both define the same platformCriticalPackages, construct criticalPackages identically, and use the same validation filter. This duplication creates maintenance risk — if one is updated, the other may be forgotten.

♻️ Suggested refactor

Extract the shared logic to a helper function at the top of the file:

/**
 * Get platform-critical packages for validation.
 * `@param` {string} nodePlatform - Target platform (darwin, win32, linux)
 * `@returns` {string[]} Array of critical package names
 */
function getCriticalPackages(nodePlatform) {
  const platformCriticalPackages = {
    'win32': ['pywintypes'],
    'linux': ['secretstorage']
  };
  return ['claude_agent_sdk', 'dotenv', 'pydantic_core']
    .concat(platformCriticalPackages[nodePlatform] || []);
}

/**
 * Check for missing critical packages in site-packages directory.
 * `@param` {string[]} packages - Package names to check
 * `@param` {string} sitePackagesDir - Path to site-packages
 * `@returns` {string[]} Array of missing package names
 */
function findMissingPackages(packages, sitePackagesDir) {
  return packages.filter(pkg => {
    const pkgPath = path.join(sitePackagesDir, pkg);
    const initPath = path.join(pkgPath, '__init__.py');
    const moduleFile = path.join(sitePackagesDir, pkg + '.py');
    return !(fs.existsSync(pkgPath) && fs.existsSync(initPath)) && !fs.existsSync(moduleFile);
  });
}

Then use at both locations:

const criticalPackages = getCriticalPackages(info.nodePlatform);
const missingPackages = findMissingPackages(criticalPackages, sitePackagesDir);

🤖 Prompt for AI Agents

In `@apps/frontend/scripts/download-python.cjs` around lines 825 - 839, Extract
the duplicated package validation into two helpers: create
getCriticalPackages(nodePlatform) that returns the base array
['claude_agent_sdk','dotenv','pydantic_core'] merged with
platformCriticalPackages (use the existing platformCriticalPackages mapping),
and create findMissingPackages(packages, sitePackagesDir) that implements the
filter logic used to compute postInstallMissing (checking pkgPath + __init__.py
and pkg + '.py'). Replace the two duplicated blocks that build criticalPackages
and postInstallMissing with calls to getCriticalPackages(info.nodePlatform) and
findMissingPackages(criticalPackages, sitePackagesDir), keeping the original
variable names (criticalPackages, postInstallMissing) for compatibility.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Restore requestAnimationFrame after each test to avoid global pollution.

Overriding global.requestAnimationFrame without restoring it can leak state into other test files. Capture the original and restore it in afterEach.

♻️ Suggested fix

@@
 describe('Terminal copy/paste integration', () => {
+  let originalRequestAnimationFrame: typeof global.requestAnimationFrame | undefined;
@@
   beforeEach(() => {
     vi.clearAllMocks();
@@
-    global.requestAnimationFrame = vi.fn((callback: FrameRequestCallback) => {
+    originalRequestAnimationFrame = global.requestAnimationFrame;
+    global.requestAnimationFrame = vi.fn((callback: FrameRequestCallback) => {
       // Synchronously execute the callback to avoid timing issues in tests
       callback.call(window, 0);
       return 0;
     }) as unknown as Mock;
   });
@@
   afterEach(() => {
+    if (originalRequestAnimationFrame) {
+      global.requestAnimationFrame = originalRequestAnimationFrame;
+    }
     vi.restoreAllMocks();
   });

Also applies to: 104-106

🤖 Prompt for AI Agents

In `@apps/frontend/src/__tests__/integration/terminal-copy-paste.test.ts` around
lines 80 - 85, The test overrides global.requestAnimationFrame with a vi.fn
which can leak into other tests; capture the original (e.g. const originalRaf =
global.requestAnimationFrame) before assigning the mock in the test file and
restore it in an afterEach block (using global.requestAnimationFrame =
originalRaf) so the global is reset after each test; apply this change for both
places where requestAnimationFrame is mocked (the blocks that use vi.fn and
callback.call) and ensure the afterEach runs even if tests fail.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Critical: Parse error - orphaned else if clause without matching if/try.

Lines 485-496 contain an else if (activeProfile.configDir) block that appears without a preceding if statement or proper try-catch structure. This is a syntax error that prevents compilation.

The pipeline failure confirms: Parsing error: 'catch' or 'finally' expected.

Looking at the intended logic, this should be part of a conditional chain checking for OAuth token first, then config directory. The structure is broken.

🐛 Analysis of the structural issue

The code structure should be:

if (activeProfile?.oauthToken) {
  // temp-file method
} else if (activeProfile?.configDir) {
  // config-dir method  
} else {
  // default method
}

But the current code has:

1. A broken if condition that was likely removed
2. An orphaned else if at line 485
3. Code that falls through incorrectly

This entire section (lines 478-511) needs to be rewritten with proper control flow.

🧰 Tools

🪛 Biome (2.1.2)

[error] 485-485: Expected a catch clause but instead found 'else'.

Expected a catch clause here.

(parse)

🪛 GitHub Actions: Lint

[error] 485-485: Parsing error: 'catch' or 'finally' expected

🤖 Prompt for AI Agents

In `@apps/frontend/src/main/terminal/claude-integration-handler.ts` around lines
478 - 496, Restore the proper if/else-if/else conditional chain around the
temp-file/config-dir/default branches so the orphaned "else if
(activeProfile.configDir)" is attached to the initial token check: wrap the
temp-file branch in if (activeProfile?.oauthToken) { ... } then use else if
(activeProfile?.configDir) { buildClaudeShellCommand(..., { method:
'config-dir', escapedConfigDir }, ...); terminal.pty.write(...);
profileManager.markProfileUsed(activeProfile.id); finalizeClaudeInvoke(...); }
else { debugLog('[ClaudeIntegration:invokeClaude] WARNING: No token or configDir
available for non-default profile'); } — ensure you reference the same symbols
(activeProfile, buildClaudeShellCommand, terminal.pty.write,
profileManager.markProfileUsed, finalizeClaudeInvoke) and keep the existing
debugLog/return behavior inside each branch so the syntax error is resolved.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Critical: Unreachable duplicate code after throw statement.

Lines 527-541 contain code that duplicates lines 498-511, placed after the throw error statement in the catch block. This code is unreachable and appears to be a merge conflict artifact or incomplete refactoring.

🗑️ Proposed fix

Remove the duplicate unreachable code:

     throw error; // Re-throw to allow caller to handle
   }
-
-  if (activeProfile && !activeProfile.isDefault) {
-    debugLog('[ClaudeIntegration:invokeClaude] Using terminal environment for non-default profile:', activeProfile.name);
-  }
-
-  const command = buildClaudeShellCommand(cwdCommand, pathPrefix, escapedClaudeCmd, { method: 'default' }, extraFlags);
-  debugLog('[ClaudeIntegration:invokeClaude] Executing command (default method):', command);
-  terminal.pty.write(command);
-
-  if (activeProfile) {
-    profileManager.markProfileUsed(activeProfile.id);
-  }
-
-  finalizeClaudeInvoke(terminal, activeProfile, projectPath, startTime, getWindow, onSessionCapture);
-  debugLog('[ClaudeIntegration:invokeClaude] ========== INVOKE CLAUDE COMPLETE (default) ==========');
 }

📝 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

	if (activeProfile && !activeProfile.isDefault) {
	debugLog('[ClaudeIntegration:invokeClaude] Using terminal environment for non-default profile:', activeProfile.name);
	}
	const command = buildClaudeShellCommand(cwdCommand, pathPrefix, escapedClaudeCmd, { method: 'default' }, extraFlags);
	debugLog('[ClaudeIntegration:invokeClaude] Executing command (default method):', command);
	terminal.pty.write(command);
	if (activeProfile) {
	profileManager.markProfileUsed(activeProfile.id);
	}
	finalizeClaudeInvoke(terminal, activeProfile, projectPath, startTime, getWindow, onSessionCapture);
	debugLog('[ClaudeIntegration:invokeClaude] ========== INVOKE CLAUDE COMPLETE (default) ==========');

🧰 Tools

🪛 Biome (2.1.2)

[error] 533-533: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)

---

[error] 534-534: This variable is used before its declaration.

The variable is declared here:

(lint/correctness/noInvalidUseBeforeDeclaration)

🤖 Prompt for AI Agents

In `@apps/frontend/src/main/terminal/claude-integration-handler.ts` around lines
527 - 541, Remove the unreachable duplicate block that appears after the `throw
error` in the catch path: delete the repeated calls to
buildClaudeShellCommand(..., { method: 'default' }, ...),
terminal.pty.write(command), profileManager.markProfileUsed(...),
finalizeClaudeInvoke(terminal, activeProfile, projectPath, startTime, getWindow,
onSessionCapture), and the trailing debugLog('[ClaudeIntegration:invokeClaude]
========== INVOKE CLAUDE COMPLETE (default) =========='), leaving only the
original working invocation logic earlier in the function (the code paths around
buildClaudeShellCommand, terminal.pty.write, profileManager.markProfileUsed, and
finalizeClaudeInvoke) so there is no duplicate/unreachable code after the `throw
error`.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Critical: Duplicate initialization in invokeClaudeAsync before try block.

Lines 630-643 contain initialization code that is duplicated inside the try block (lines 648-661). This mirrors the same issue in invokeClaude.

🤖 Prompt for AI Agents

In `@apps/frontend/src/main/terminal/claude-integration-handler.ts` around lines
630 - 643, The block that sets up Claude mode and YOLO flags is duplicated in
invokeClaudeAsync (the pre-try block and again inside the try), mirroring
invokeClaude; remove the redundant pre-try initialization so the code only sets
extraFlags = dangerouslySkipPermissions ? YOLO_MODE_FLAG : undefined and updates
terminal.isClaudeMode, terminal.dangerouslySkipPermissions, calls
SessionHandler.releaseSessionId(terminal.id), and clears
terminal.claudeSessionId once — keep that single initialization where the try
block currently performs it (or move the unique one outside the try and delete
the duplicate inside) to avoid double-execution and ensure only one source of
truth for the YOLO_MODE_FLAG and terminal state.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Critical: Same structural issues in invokeClaudeAsync - orphaned else and duplicate code.

The async version has the same critical issues:

• Line 691: Orphaned else if clause (parse error)
• Lines 735-749: Unreachable duplicate code after catch block

Also applies to: 735-749

🧰 Tools

🪛 Biome (2.1.2)

[error] 691-691: Expected a catch clause but instead found 'else'.

Expected a catch clause here.

(parse)

🤖 Prompt for AI Agents

In `@apps/frontend/src/main/terminal/claude-integration-handler.ts` around lines
684 - 702, In invokeClaudeAsync there’s a mismatched/ orphaned "else if" and
duplicated unreachable post-catch block; fix by correcting the surrounding
try/catch/if brace structure so the "else if (activeProfile.configDir)" branches
attach to the original if chain, remove the duplicated terminal
invocation/finalize block after the catch, and consolidate the
terminal.pty.write, profileManager.markProfileUsed, and finalizeClaudeInvoke
calls into the single correct branch path (functions/idents to check:
invokeClaudeAsync, buildClaudeShellCommand, terminal.pty.write,
profileManager.markProfileUsed, finalizeClaudeInvoke) so control flow
returns/continues correctly without parse errors or dead code.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Add explicit type="button" to prevent unintended form submission.

The delete button inside the dropdown menu item is missing an explicit type attribute. The default type is submit, which could cause unintended form submissions if this component is ever placed inside a form element.

🔧 Proposed fix

                    <button
+                     type="button"
                      onClick={(e) => {
                        e.stopPropagation();
                        e.preventDefault();
                        setDeleteWorktree(wt);
                      }}

📝 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

	<button
	onClick={(e) => {
	e.stopPropagation();
	setIsOpen(false);
	onSelectWorktree(wt);
	e.preventDefault();
	setDeleteWorktree(wt);
	}}
	className="text-xs group"
	className="ml-2 p-1 rounded hover:bg-destructive/10 text-muted-foreground hover:text-destructive opacity-0 group-hover:opacity-100 transition-opacity shrink-0"
	title={t('common:delete')}
	>
	<button
	type="button"
	onClick={(e) => {
	e.stopPropagation();
	e.preventDefault();
	setDeleteWorktree(wt);
	}}
	className="ml-2 p-1 rounded hover:bg-destructive/10 text-muted-foreground hover:text-destructive opacity-0 group-hover:opacity-100 transition-opacity shrink-0"
	title={t('common:delete')}
	>

🧰 Tools

🪛 Biome (2.1.2)

[error] 210-218: Provide an explicit type prop for the button element.

The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset

(lint/a11y/useButtonType)

🤖 Prompt for AI Agents

In `@apps/frontend/src/renderer/components/terminal/WorktreeSelector.tsx` around
lines 210 - 218, The delete button in WorktreeSelector (the button that calls
setDeleteWorktree(wt) inside the dropdown) lacks an explicit type and will
default to submit; update the button element to include type="button" to prevent
accidental form submissions—locate the button with the onClick handler (calling
setDeleteWorktree) and add the type attribute while keeping existing props and
handlers intact.

[StillKnotKnown]

pr broken