Skip to content

Conversation

@StillKnotKnown
Copy link
Owner

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
  • 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

@github-actions github-actions bot added area/fullstack bug Something isn't working size/L labels Jan 16, 2026
@StillKnotKnown StillKnotKnown force-pushed the stillknotknown/acs-310-linux-binary-missing-secretstorage-dependency branch from 00a44a2 to 52aecc7 Compare January 16, 2026 19:55
StillKnotKnown added a commit that referenced this pull request Jan 16, 2026
…hardcoded fallbacks (ACS-294) (AndyMik90#1170)

* fix(runners): use resolve_model_id() for model resolution instead of hardcoded fallbacks (ACS-294)

This fix ensures that custom model configurations via environment variables
(ANTHROPIC_DEFAULT_SONNET_MODEL, ANTHROPIC_DEFAULT_OPUS_MODEL, etc.) are
properly respected in PR reviewer services, instead of falling back to
hardcoded Claude model IDs.

Changes:
- parallel_orchestrator_reviewer.py: Import and use resolve_model_id() from
  phase_config, with fallback to "sonnet" shorthand instead of hardcoded model
- parallel_followup_reviewer.py: Same pattern as above
- models.py: Update GitHubRunnerConfig default model from hardcoded
  "claude-sonnet-4-20250514" to "sonnet" shorthand (respects env overrides)
- batch_validator.py: Change DEFAULT_MODEL to "sonnet" shorthand and add
  _resolve_model() method to resolve via phase_config.resolve_model_id()
- batch_issues.py: Change validation_model default to "sonnet" shorthand
- Add comprehensive tests in test_model_resolution.py to verify model
  resolution behavior

This resolves the issue where logs would display hardcoded model IDs
(e.g., "claude-opus-4-5-20251101") instead of the actual configured model
(e.g., "glm-4.7"), which caused confusion about which model was being used.

Refs: ACS-294

* test: extract environment cleanup into pytest fixture to address PR review feedback

- Add clean_env pytest fixture to handle environment variable cleanup
- Remove duplicated environment backup/restore logic from 3 tests
- Fix import: use collections.abc.Generator instead of typing.Generator

This addresses review feedback from PR AndyMik90#1170:
- CodeRabbit: Extract duplicated environment cleanup into fixture
- ruff: Import Generator from collections.abc

The pytest fixture approach is the Pythonic way to handle setup/teardown
and reduces code duplication while maintaining test isolation.

* test: address CodeRabbit PR review feedback

- Fix absolute import issue in batch_validator.py: Use importlib.util.find_spec
  instead of "from phase_config import resolve_model_id" for more robust
  imports that don't rely on sys.path
- Improve test quality: Add behavioral tests for resolve_model_id() function
  (11 tests with full coverage of environment variable overrides)
- Add documentation explaining why source inspection is used for some tests
  (to avoid complex import dependencies while still verifying critical patterns)
- Add test for importlib.util pattern in batch_validator.py
- Add negative assertions to verify old hardcoded fallbacks are not present

Addresses CodeRabbit feedback from PR AndyMik90#1170:
- Comment #5: Absolute import style (fixed with importlib.util)
- Comments AndyMik90#7-11: Brittle source code tests (improved with behavioral tests
  and documentation explaining why source inspection is used where necessary)

All 21 tests pass, ruff checks pass.

Refs: ACS-294

* fix: add explanatory comment for empty except and broaden exception handling

- Add detailed comment explaining why the empty 'pass' is necessary
  (ensures BatchValidator remains functional even if phase_config has errors)
- Change from except (ImportError, AttributeError) to except Exception
  to catch broader exceptions for robustness
- Addresses GitHub Advanced Security alert about empty except clause
- Addresses CodeRabbit feedback about broader exception handling

All 21 tests pass, ruff checks pass.

Refs: ACS-294

* fix: add resolve_model_id to fallback imports in parallel reviewers

- Add resolve_model_id to fallback import in parallel_followup_reviewer.py:64
- Add resolve_model_id to fallback import in parallel_orchestrator_reviewer.py:60
- Fix hardcoded fallback in followup_reviewer.py:688 - use shorthand + resolve_model_id

This addresses 3 HIGH priority findings from Auto Claude PR Review:
- [e4d8064b75ce] Missing resolve_model_id import in fallback block (parallel_followup_reviewer.py)
- [f4beb99bb5d1] Missing resolve_model_id import in fallback block (parallel_orchestrator_reviewer.py)
- [c059fa0540e0] Missed hardcoded model fallback (followup_reviewer.py)

All 21 tests pass, ruff checks pass.

Refs: ACS-294

* fix(batch_issues): resolve model shorthand via resolve_model_id() in ClaudeBatchAnalyzer

This fixes the last hardcoded model fallback in ACS-294. The
ClaudeBatchAnalyzer.analyze_and_batch_issues() method was using a
hardcoded 'claude-sonnet-4-5-20250929' model ID instead of resolving
the 'sonnet' shorthand via resolve_model_id(), which prevented
environment variable overrides from being respected.

Changes:
- Import resolve_model_id from phase_config
- Replace hardcoded model with resolve_model_id('sonnet')
- Add tests to verify the fix

This completes the fix for all hardcoded model fallbacks in the
GitHub runner services.

* test: add UTF-8 encoding to read_text() calls for Windows compatibility

On Windows, Path.read_text() defaults to the system encoding (cp1252)
which can cause UnicodeDecodeError for files with UTF-8 content. This
fixes the CI test failure by explicitly specifying encoding='utf-8' for
all read_text() calls in the test file.

Fixes test failure:
- test_parallel_reviewers_use_sonnet_fallback

* test: document UTF-8 encoding requirement for Windows compatibility

Adds explanatory comment for encoding parameter in source inspection tests.
This clarifies why explicit UTF-8 is needed (platform-dependent defaults).

* fix: address PR review findings - import pattern consistency and test improvements

MEDIUM: Replace importlib.util pattern with established try/except import pattern
- batch_validator.py now uses relative imports with absolute fallback
- Matches the convention used across runners/github/services/
- Ensures proper module caching in sys.modules

LOW: Add debug logging to exception handler
- _resolve_model now logs failures at debug level for diagnosis
- Helps identify actual bugs vs expected fallbacks

LOW: Extract duplicate file paths to pytest fixtures
- Added GITHUB_RUNNER_DIR constant and fixtures for common file paths
- Reduces 11 duplicate path constructions to reusable fixtures
- Easier maintenance if directory structure changes

LOW: Add explanatory comment for implementation-detail test
- test_uses_try_except_import_pattern now documents why it tests implementation
- Explains the guard against circular dependency import patterns

All 23 tests pass.

* fix: resolve model shorthand in triage_engine and pr_review_engine (CRITICAL)

CRITICAL BUG FIX: Model shorthands (e.g., "sonnet") were being passed
directly to create_client() without resolving to full model IDs. The
Anthropic API does not recognize shorthands, causing runtime errors.

Fixed files:
- triage_engine.py: Added resolve_model_id() import and resolution
- pr_review_engine.py: Added resolve_model_id() import and resolution at 3 call sites
  - run_review_pass() (main review pass)
  - _run_structural_pass() (structural analysis)
  - _run_ai_triage_pass() (AI comment triage)

All changes follow the established pattern used in:
- parallel_orchestrator_reviewer.py
- parallel_followup_reviewer.py
- followup_reviewer.py

All 23 tests pass.

* fix: address PR review findings - correct import paths and hardcoded models

MEDIUM: Fix incorrect relative import paths for phase_config
- batch_validator.py: Changed .phase_config to ..phase_config (2 dots from runners/github/)
- triage_engine.py: Changed ..phase_config to ...phase_config (3 dots from services/)
- pr_review_engine.py: Changed ..phase_config to ...phase_config (3 dots from services/)
- Updated test to reflect correct import pattern for batch_validator.py

MEDIUM: Fix hardcoded model IDs in batch_processor.py
- Changed validation_model="claude-sonnet-4-5-20250929" to "sonnet" on lines 97 and 223
- Ensures consistency with model shorthand resolution pattern

LOW: Move inline import to module-level in batch_issues.py
- Moved resolve_model_id import to module-level try/except block
- Matches established codebase convention for consistency

All 23 tests pass.

* fix: correct import block ordering for ruff I001 compliance

Reordered imports in try/except blocks to comply with ruff's I001 rule:
- batch_issues.py: Parent directory imports before same-directory
- triage_engine.py: Parent directory imports before same-directory
- pr_review_engine.py: Parent directory imports before same-directory

All 23 tests pass and ruff checks pass.

* fix: improve exception handling robustness in BatchValidator._resolve_model

Wrap the fallback import in its own try/except block to ensure any
exception during the fallback is caught and logged before returning the
original model as a final fallback. This prevents unexpected exceptions
from propagating when the absolute import fails.

All 23 tests pass and ruff checks pass.

* style: add blank line after import for ruff format compliance

---------

Co-authored-by: StillKnotKnown <stillknotknown@users.noreply.github.com>
Co-authored-by: Andy <119136210+AndyMik90@users.noreply.github.com>
@StillKnotKnown StillKnotKnown force-pushed the stillknotknown/acs-310-linux-binary-missing-secretstorage-dependency branch 2 times, most recently from f6f66a1 to e8cf42b Compare January 16, 2026 20:02
StillKnotKnown added a commit that referenced this pull request Jan 16, 2026
…hardcoded fallbacks (ACS-294) (AndyMik90#1170)

* fix(runners): use resolve_model_id() for model resolution instead of hardcoded fallbacks (ACS-294)

This fix ensures that custom model configurations via environment variables
(ANTHROPIC_DEFAULT_SONNET_MODEL, ANTHROPIC_DEFAULT_OPUS_MODEL, etc.) are
properly respected in PR reviewer services, instead of falling back to
hardcoded Claude model IDs.

Changes:
- parallel_orchestrator_reviewer.py: Import and use resolve_model_id() from
  phase_config, with fallback to "sonnet" shorthand instead of hardcoded model
- parallel_followup_reviewer.py: Same pattern as above
- models.py: Update GitHubRunnerConfig default model from hardcoded
  "claude-sonnet-4-20250514" to "sonnet" shorthand (respects env overrides)
- batch_validator.py: Change DEFAULT_MODEL to "sonnet" shorthand and add
  _resolve_model() method to resolve via phase_config.resolve_model_id()
- batch_issues.py: Change validation_model default to "sonnet" shorthand
- Add comprehensive tests in test_model_resolution.py to verify model
  resolution behavior

This resolves the issue where logs would display hardcoded model IDs
(e.g., "claude-opus-4-5-20251101") instead of the actual configured model
(e.g., "glm-4.7"), which caused confusion about which model was being used.

Refs: ACS-294

* test: extract environment cleanup into pytest fixture to address PR review feedback

- Add clean_env pytest fixture to handle environment variable cleanup
- Remove duplicated environment backup/restore logic from 3 tests
- Fix import: use collections.abc.Generator instead of typing.Generator

This addresses review feedback from PR AndyMik90#1170:
- CodeRabbit: Extract duplicated environment cleanup into fixture
- ruff: Import Generator from collections.abc

The pytest fixture approach is the Pythonic way to handle setup/teardown
and reduces code duplication while maintaining test isolation.

* test: address CodeRabbit PR review feedback

- Fix absolute import issue in batch_validator.py: Use importlib.util.find_spec
  instead of "from phase_config import resolve_model_id" for more robust
  imports that don't rely on sys.path
- Improve test quality: Add behavioral tests for resolve_model_id() function
  (11 tests with full coverage of environment variable overrides)
- Add documentation explaining why source inspection is used for some tests
  (to avoid complex import dependencies while still verifying critical patterns)
- Add test for importlib.util pattern in batch_validator.py
- Add negative assertions to verify old hardcoded fallbacks are not present

Addresses CodeRabbit feedback from PR AndyMik90#1170:
- Comment #5: Absolute import style (fixed with importlib.util)
- Comments AndyMik90#7-11: Brittle source code tests (improved with behavioral tests
  and documentation explaining why source inspection is used where necessary)

All 21 tests pass, ruff checks pass.

Refs: ACS-294

* fix: add explanatory comment for empty except and broaden exception handling

- Add detailed comment explaining why the empty 'pass' is necessary
  (ensures BatchValidator remains functional even if phase_config has errors)
- Change from except (ImportError, AttributeError) to except Exception
  to catch broader exceptions for robustness
- Addresses GitHub Advanced Security alert about empty except clause
- Addresses CodeRabbit feedback about broader exception handling

All 21 tests pass, ruff checks pass.

Refs: ACS-294

* fix: add resolve_model_id to fallback imports in parallel reviewers

- Add resolve_model_id to fallback import in parallel_followup_reviewer.py:64
- Add resolve_model_id to fallback import in parallel_orchestrator_reviewer.py:60
- Fix hardcoded fallback in followup_reviewer.py:688 - use shorthand + resolve_model_id

This addresses 3 HIGH priority findings from Auto Claude PR Review:
- [e4d8064b75ce] Missing resolve_model_id import in fallback block (parallel_followup_reviewer.py)
- [f4beb99bb5d1] Missing resolve_model_id import in fallback block (parallel_orchestrator_reviewer.py)
- [c059fa0540e0] Missed hardcoded model fallback (followup_reviewer.py)

All 21 tests pass, ruff checks pass.

Refs: ACS-294

* fix(batch_issues): resolve model shorthand via resolve_model_id() in ClaudeBatchAnalyzer

This fixes the last hardcoded model fallback in ACS-294. The
ClaudeBatchAnalyzer.analyze_and_batch_issues() method was using a
hardcoded 'claude-sonnet-4-5-20250929' model ID instead of resolving
the 'sonnet' shorthand via resolve_model_id(), which prevented
environment variable overrides from being respected.

Changes:
- Import resolve_model_id from phase_config
- Replace hardcoded model with resolve_model_id('sonnet')
- Add tests to verify the fix

This completes the fix for all hardcoded model fallbacks in the
GitHub runner services.

* test: add UTF-8 encoding to read_text() calls for Windows compatibility

On Windows, Path.read_text() defaults to the system encoding (cp1252)
which can cause UnicodeDecodeError for files with UTF-8 content. This
fixes the CI test failure by explicitly specifying encoding='utf-8' for
all read_text() calls in the test file.

Fixes test failure:
- test_parallel_reviewers_use_sonnet_fallback

* test: document UTF-8 encoding requirement for Windows compatibility

Adds explanatory comment for encoding parameter in source inspection tests.
This clarifies why explicit UTF-8 is needed (platform-dependent defaults).

* fix: address PR review findings - import pattern consistency and test improvements

MEDIUM: Replace importlib.util pattern with established try/except import pattern
- batch_validator.py now uses relative imports with absolute fallback
- Matches the convention used across runners/github/services/
- Ensures proper module caching in sys.modules

LOW: Add debug logging to exception handler
- _resolve_model now logs failures at debug level for diagnosis
- Helps identify actual bugs vs expected fallbacks

LOW: Extract duplicate file paths to pytest fixtures
- Added GITHUB_RUNNER_DIR constant and fixtures for common file paths
- Reduces 11 duplicate path constructions to reusable fixtures
- Easier maintenance if directory structure changes

LOW: Add explanatory comment for implementation-detail test
- test_uses_try_except_import_pattern now documents why it tests implementation
- Explains the guard against circular dependency import patterns

All 23 tests pass.

* fix: resolve model shorthand in triage_engine and pr_review_engine (CRITICAL)

CRITICAL BUG FIX: Model shorthands (e.g., "sonnet") were being passed
directly to create_client() without resolving to full model IDs. The
Anthropic API does not recognize shorthands, causing runtime errors.

Fixed files:
- triage_engine.py: Added resolve_model_id() import and resolution
- pr_review_engine.py: Added resolve_model_id() import and resolution at 3 call sites
  - run_review_pass() (main review pass)
  - _run_structural_pass() (structural analysis)
  - _run_ai_triage_pass() (AI comment triage)

All changes follow the established pattern used in:
- parallel_orchestrator_reviewer.py
- parallel_followup_reviewer.py
- followup_reviewer.py

All 23 tests pass.

* fix: address PR review findings - correct import paths and hardcoded models

MEDIUM: Fix incorrect relative import paths for phase_config
- batch_validator.py: Changed .phase_config to ..phase_config (2 dots from runners/github/)
- triage_engine.py: Changed ..phase_config to ...phase_config (3 dots from services/)
- pr_review_engine.py: Changed ..phase_config to ...phase_config (3 dots from services/)
- Updated test to reflect correct import pattern for batch_validator.py

MEDIUM: Fix hardcoded model IDs in batch_processor.py
- Changed validation_model="claude-sonnet-4-5-20250929" to "sonnet" on lines 97 and 223
- Ensures consistency with model shorthand resolution pattern

LOW: Move inline import to module-level in batch_issues.py
- Moved resolve_model_id import to module-level try/except block
- Matches established codebase convention for consistency

All 23 tests pass.

* fix: correct import block ordering for ruff I001 compliance

Reordered imports in try/except blocks to comply with ruff's I001 rule:
- batch_issues.py: Parent directory imports before same-directory
- triage_engine.py: Parent directory imports before same-directory
- pr_review_engine.py: Parent directory imports before same-directory

All 23 tests pass and ruff checks pass.

* fix: improve exception handling robustness in BatchValidator._resolve_model

Wrap the fallback import in its own try/except block to ensure any
exception during the fallback is caught and logged before returning the
original model as a final fallback. This prevents unexpected exceptions
from propagating when the absolute import fails.

All 23 tests pass and ruff checks pass.

* style: add blank line after import for ruff format compliance

---------

Co-authored-by: StillKnotKnown <stillknotknown@users.noreply.github.com>
Co-authored-by: Andy <119136210+AndyMik90@users.noreply.github.com>
AndyMik90 and others added 5 commits January 16, 2026 22:05
* ci: add Azure auth test workflow

* fix(worktree): handle "already up to date" case correctly (ACS-226) (AndyMik90#961)

* fix(worktree): handle "already up to date" case correctly (ACS-226)

When git merge returns non-zero for "Already up to date", the merge
code incorrectly treated this as a conflict and aborted. Now checks
git output to distinguish between:
- "Already up to date" - treat as success (nothing to merge)
- Actual conflicts - abort as before
- Other errors - show actual error message

Also added comprehensive tests for edge cases:
- Already up to date with no_commit=True
- Already up to date with delete_after=True
- Actual merge conflict detection
- Merge conflict with no_commit=True

* test: strengthen merge conflict abort verification

Improve assertions in conflict detection tests to explicitly verify:
- MERGE_HEAD does not exist after merge abort
- git status returns clean (no staged/unstaged changes)

This is more robust than just checking for absence of "CONFLICT"
string, as git status --porcelain uses status codes, not literal words.

* test: add git command success assertions and branch deletion verification

- Add explicit returncode assertions for all subprocess.run git add/commit calls
- Add branch deletion verification in test_merge_worktree_already_up_to_date_with_delete_after
- Ensures tests fail early if git commands fail rather than continuing silently

---------

Co-authored-by: StillKnotKnown <stillknotknown@users.noreply.github.com>

* fix(terminal): add collision detection for terminal drag and drop reordering (AndyMik90#985)

* fix(terminal): add collision detection for terminal drag and drop reordering

Add closestCenter collision detection to DndContext to fix terminal
drag and drop swapping not detecting valid drop targets. The default
rectIntersection algorithm required too much overlap for grid layouts.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(terminal): handle file drops when closestCenter returns sortable ID

Address PR review feedback:
- Fix file drop handling to work when closestCenter collision detection
  returns the sortable ID instead of the droppable ID
- Add terminals to useCallback dependency array to prevent stale state

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

* fix(ACS-181): enable auto-switch on 401 auth errors & OAuth-only profiles (AndyMik90#900)

* fix(ACS-181): enable auto-switch for OAuth-only profiles

Add OAuth token check at the start of isProfileAuthenticated() so that
profiles with only an oauthToken (no configDir) are recognized as
authenticated. This allows the profile scorer to consider OAuth-only
profiles as valid alternatives for proactive auto-switching.

Previously, isProfileAuthenticated() immediately returned false if
configDir was missing, causing OAuth-only profiles to receive a -500
penalty in the scorer and never be selected for auto-switch.

Fixes: ACS-181

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: Black Circle Sentinel <mludlow000@icloud.com>

* fix(ACS-181): detect 'out of extra usage' rate limit messages

The previous patterns only matched "Limit reached · resets ..." but
Claude Code also shows "You're out of extra usage · resets ..." which
wasn't being detected. This prevented auto-switch from triggering.

Added new patterns to both output-parser.ts (terminal) and
rate-limit-detector.ts (agent processes) to detect:
- "out of extra usage · resets ..."
- "You're out of extra usage · resets ..."

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(ACS-181): add real-time rate limit detection and debug logging

- Add real-time rate limit detection in agent-process.ts processLog()
  so rate limits are detected immediately as output appears, not just
  when the process exits
- Add clear warning message when auto-switch is disabled in settings
- Add debug logging to profile-scorer.ts to trace profile evaluation
- Add debug logging to rate-limit-detector.ts to trace pattern matching

This enables immediate detection and auto-switch when rate limits occur
during task execution.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(frontend): enable auto-switch on 401 auth errors

- Propagate 401/403 errors from fetchUsageViaAPI to checkUsageAndSwap in UsageMonitor to trigger proactive profile swapping.
- Fix usage monitor race condition by ensuring it waits for ClaudeProfileManager initialization.
- Fix isProfileAuthenticated to correctly validate OAuth-only profiles.

* fix(ACS-181): address PR review feedback

- Revert unrelated files (rate-limit-detector, output-parser, agent-process) to upstream state
- Gate profile-scorer logging behind DEBUG flag
- Fix usage-monitor type safety and correct catch block syntax
- Fix misleading indentation in index.ts app updater block

* fix(frontend): enforce eslint compliance for logs in profile-scorer

- Replace all console.log with console.warn (per linter rules)
- Strictly gate all debug logs behind isDebug check to prevent production noise

* fix(ACS-181): add swap loop protection for auth failures

- Add authFailedProfiles Map to track profiles with recent auth failures
- Implement 5-minute cooldown before retrying failed profiles
- Exclude failed profiles from swap candidates to prevent infinite loops
- Gate TRACE logs behind DEBUG flag to reduce production noise
- Change console.log to console.warn for ESLint compliance

---------

Signed-off-by: Black Circle Sentinel <mludlow000@icloud.com>
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

* feat(frontend): add Claude Code version rollback feature (AndyMik90#983)

* feat(frontend): add Claude Code version rollback feature

Add ability for users to switch to any of the last 20 Claude Code CLI versions
directly from the Claude Code popup in the sidebar.

Changes:
- Add IPC channels for fetching available versions and installing specific version
- Add backend handlers to fetch versions from npm registry (with 1-hour cache)
- Add version selector dropdown in ClaudeCodeStatusBadge component
- Add warning dialog before switching versions (warns about closing sessions)
- Add i18n support for English and French translations

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix: address PR review feedback for Claude Code version rollback

- Add validation after semver filtering to handle empty version list
- Add error state and UI feedback for installation/version switch failures
- Extract magic number 5000ms to VERSION_RECHECK_DELAY_MS constant
- Bind Select value prop to selectedVersion state
- Normalize version comparison to handle 'v' prefix consistently
- Use normalized version comparison in SelectItem disabled check

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

* fix(security): inherit security profiles in worktrees and validate shell -c commands (AndyMik90#971)

* fix(security): inherit security profiles in worktrees and validate shell -c commands

- Add inherited_from field to SecurityProfile to mark profiles copied from parent projects
- Skip hash-based re-analysis for inherited profiles (fixes worktrees losing npm/npx etc.)
- Add shell_validators.py to validate commands inside bash/sh/zsh -c strings
- Register shell validators to close security bypass via bash -c "arbitrary_command"
- Add 13 new tests for inherited profiles and shell -c validation

Fixes worktree security config not being inherited, which caused agents to be
blocked from running npm/npx commands in isolated workspaces.

* docs: update README download links to v2.7.3 (AndyMik90#976)

- Update all stable download links from 2.7.2 to 2.7.3
- Add Flatpak download link (new in 2.7.3)

* fix(security): close shell -c bypass vectors and validate inherited profiles

- Fix combined shell flags bypass (-xc, -ec, -ic) in _extract_c_argument()
  Shell allows combining flags like `bash -xc 'cmd'` which bypassed -c detection
- Add recursive validation for nested shell invocations
  Prevents bypass via `bash -c "bash -c 'evil_cmd'"`
- Validate inherited_from path in should_reanalyze() with defense-in-depth
  - Must exist and be a directory
  - Must be an ancestor of current project
  - Must contain valid security profile
- Add comprehensive test coverage for all security fixes

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* style: fix import ordering in test_security.py

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* style: format shell_validators.py

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

* feat(frontend): add searchable branch combobox to worktree creation dialog (AndyMik90#979)

* feat(frontend): add searchable branch combobox to worktree creation dialog

- Replace limited Select dropdown with searchable Combobox for branch selection
- Add new Combobox UI component with search filtering and scroll support
- Remove 15-branch limit - now shows all branches with search
- Improve worktree name validation to allow dots and underscores
- Better sanitization: spaces become hyphens, preserve valid characters
- Add i18n keys for branch search UI in English and French

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(frontend): address PR review feedback for worktree dialog

- Extract sanitizeWorktreeName utility function to avoid duplication
- Replace invalid chars with hyphens instead of removing them (feat/new → feat-new)
- Trim trailing hyphens and dots from sanitized names
- Add validation to forbid '..' in names (invalid for Git branch names)
- Refactor branchOptions to use map/spread instead of forEach/push
- Add ARIA accessibility: listboxId, aria-controls, role="listbox"

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(frontend): align worktree name validation with backend regex

- Fix frontend validation to match backend WORKTREE_NAME_REGEX (no dots,
  must end with alphanumeric)
- Update sanitizeWorktreeName to exclude dots from allowed characters
- Update i18n messages (en/fr) to remove mention of dots
- Add displayName to Combobox component for React DevTools
- Export Combobox from UI component index.ts
- Add aria-label to Combobox listbox for accessibility

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(frontend): address PR review accessibility and cleanup issues

- Add forwardRef pattern to Combobox for consistency with other UI components
- Add keyboard navigation (ArrowUp/Down, Enter, Escape, Home, End)
- Add aria-activedescendant for screen reader focus tracking
- Add unique option IDs for ARIA compliance
- Add cleanup for async branch fetching to prevent state updates on unmounted component

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

* fix(frontend): sync worktree config to renderer on terminal restoration (AndyMik90#982)

* fix(frontend): sync worktree config to renderer on terminal restoration

When terminals are restored after app restart, the worktree config
was not being synced to the renderer, causing the worktree label
to not appear. This adds a new IPC channel to send worktree config
during restoration and a listener in useTerminalEvents to update
the terminal store.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(frontend): always sync worktreeConfig to handle deleted worktrees

Addresses PR review feedback: send worktreeConfig IPC message
unconditionally so the renderer can clear stale worktree labels
when a worktree is deleted while the app is closed.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

* fix(merge): include files with content changes even when semantic analysis is empty (AndyMik90#986)

* fix(merge): include files with content changes even when semantic analysis is empty

The merge system was discarding files that had real code changes but no
detected semantic changes. This happened because:

1. The semantic analyzer only detects imports and function additions/removals
2. Files with only function body modifications returned semantic_changes=[]
3. The filter used Python truthiness (empty list = False), excluding these files
4. This caused merges to fail with "0 files to merge" despite real changes

The fix uses content hash comparison as a fallback check. If the file content
actually changed (hash_before != hash_after), include it for merge regardless
of whether the semantic analyzer could parse the specific change types.

This fixes merging for:
- Files with function body modifications (most common case)
- Unsupported file types (Rust, Go, etc.) where semantic analysis returns empty
- Any file where the analyzer fails to detect the specific change pattern

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* refactor(merge): add TaskSnapshot.has_modifications property and handle DIRECT_COPY

Address PR review feedback:

1. DRY improvement: Add `has_modifications` property to TaskSnapshot
   - Centralizes the modification detection logic
   - Checks semantic_changes first, falls back to content hash comparison
   - Handles both complete tasks and in-progress tasks safely

2. Fix for files with empty semantic_changes (Cursor issue #2):
   - Add DIRECT_COPY MergeDecision for files that were modified but
     couldn't be semantically analyzed (body changes, unsupported languages)
   - MergePipeline returns DIRECT_COPY when has_modifications=True but
     semantic_changes=[] (single task case)
   - Orchestrator handles DIRECT_COPY by reading file directly from worktree
   - This prevents silent data loss where apply_single_task_changes would
     return baseline content unchanged

3. Update _update_stats to count DIRECT_COPY as auto-merged

The combination ensures:
- Files ARE detected for merge (has_modifications check)
- Files ARE properly merged (DIRECT_COPY reads from worktree)
- No silent data loss (worktree content used instead of baseline)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(merge): handle DIRECT_COPY in merge_tasks() and log missing files

- Add DIRECT_COPY handling to merge_tasks() for multi-task merges
  (was only handled in merge_task() for single-task merges)
- Add warning logging when worktree file doesn't exist during DIRECT_COPY
  in both merge_task() and merge_tasks()

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(merge): remove unnecessary f-string prefixes

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(merge): properly fail DIRECT_COPY when worktree file missing

- Extract _read_worktree_file_for_direct_copy() helper to DRY up logic
- Set decision to FAILED when worktree file not found (was silent success)
- Add warning when worktree_path is None in merge_tasks
- Use `is not None` check for merged_content to allow empty files
- Fix has_modifications for new files with empty hash_before
- Add debug_error() to merge_tasks exception handling for consistency

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* style(merge): fix ruff formatting for long line

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

* fix(terminal): detect Claude exit and reset label when user closes Claude (AndyMik90#990)

* fix(terminal): detect Claude exit and reset label when user closes Claude

Previously, the "Claude" label on terminals would persist even after the
user closed Claude (via /exit, Ctrl+D, etc.) because the system only
reset isClaudeMode when the entire terminal process exited.

This change adds robust Claude exit detection by:
- Adding shell prompt patterns to detect when Claude exits and returns
  to shell (output-parser.ts)
- Adding new IPC channel TERMINAL_CLAUDE_EXIT for exit notifications
- Adding handleClaudeExit() to reset terminal state in main process
- Adding onClaudeExit callback in terminal event handler
- Adding onTerminalClaudeExit listener in preload API
- Handling exit event in renderer to update terminal store

Now when a user closes Claude within a terminal, the label is removed
immediately while the terminal continues running.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(terminal): add line-start anchors to exit detection regex patterns

Address PR review findings:
- Add ^ anchors to CLAUDE_EXIT_PATTERNS to prevent false positive exit
  detection when Claude outputs paths, array access, or Unicode arrows
- Add comprehensive unit tests for detectClaudeExit and related functions
- Remove duplicate debugLog call in handleClaudeExit (keep console.warn)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(terminal): prevent false exit detection for emails and race condition

- Update user@host regex to require path indicator after colon,
  preventing emails like user@example.com: from triggering exit detection
- Add test cases for emails at line start to ensure they don't match
- Add guard in onTerminalClaudeExit to prevent setting status to 'running'
  if terminal has already exited (fixes potential race condition)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

* fix(app-update): persist downloaded update state for Install button visibility (AndyMik90#992)

* fix(app-update): persist downloaded update state for Install button visibility

When updates auto-download in background, users miss the update-downloaded
event if not on Settings page. This causes "Install and Restart" button
to never appear.

Changes:
- Add downloadedUpdateInfo state in app-updater.ts to persist downloaded info
- Add APP_UPDATE_GET_DOWNLOADED IPC handler to query downloaded state
- Add getDownloadedAppUpdate API method in preload
- Update AdvancedSettings to check for already-downloaded updates on mount

Now when user opens Settings after background download, the component
queries persisted state and shows "Install and Restart" correctly.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(app-update): resolve race condition and type safety issues

- Fix race condition where checkForAppUpdates() could overwrite downloaded
  update info with null, causing 'Unknown' version display
- Add proper type guard for releaseNotes (can be string | array | null)
  instead of unsafe type assertion

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(app-update): clear downloaded update state on channel change and add useEffect cleanup

- Clear downloadedUpdateInfo when update channel changes to prevent showing
  Install button for updates from a different channel (e.g., beta update
  showing after switching to stable channel)
- Add isCancelled flag to useEffect async operations in AdvancedSettings
  to prevent React state updates on unmounted components

Addresses CodeRabbit review findings.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

* fix(backend): add Sentry integration and fix broken pipe errors (AndyMik90#991)

* fix(backend): add Sentry integration and fix broken pipe errors

- Add sentry-sdk to Python backend for error tracking
- Create safe_print() utility to handle BrokenPipeError gracefully
- Initialize Sentry in CLI, GitHub runner, and spec runner entry points
- Use same SENTRY_DSN environment variable as Electron frontend
- Apply privacy path masking (usernames removed from stack traces)

Fixes "Review Failed: [Errno 32] Broken pipe" error in PR review

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(backend): address PR review findings for Sentry integration

- Fix ruff linting errors (unused imports, import sorting)
- Add path masking to set_context() and set_tag() for privacy
- Add defensive path masking to capture_exception() kwargs
- Add debug logging for bare except clauses in sentry.py
- Add top-level error handler in cli/main.py with Sentry capture
- Add error handling with Sentry capture in spec_runner.py
- Move safe_print to core/io_utils.py for broader reuse
- Migrate GitLab runner files to use safe_print()
- Add fallback import pattern in sdk_utils.py

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* style: apply ruff formatting

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(backend): address CodeRabbit review findings for Sentry and io_utils

- Add path masking to capture_message() kwargs for privacy consistency
- Add recursion depth limit (50) to _mask_object_paths() to prevent stack overflow
- Add WSL path masking support (/mnt/[a-z]/Users/...)
- Add consistent ImportError debug logging across Sentry wrapper functions
- Add ValueError handling in safe_print() for closed stdout scenarios
- Improve reset_pipe_state() documentation with usage warnings

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

* fix: improve Claude CLI detection and add installation selector (AndyMik90#1004)

* fix: improve Claude CLI detection and add installation selector

This PR addresses the "Claude Code not found" error when starting tasks by
improving CLI path detection across all platforms.

Backend changes:
- Add cross-platform `find_claude_cli()` function in `client.py` that checks:
  - CLAUDE_CLI_PATH environment variable for user override
  - System PATH via shutil.which()
  - Homebrew paths on macOS
  - NVM paths for Node.js version manager installations
  - Platform-specific standard locations (Windows: AppData, Program Files; Unix: .local/bin)
- Pass detected `cli_path` to ClaudeAgentOptions in both `create_client()` and `create_simple_client()`
- Improve Windows .cmd/.bat file execution using proper cmd.exe flags (/d, /s, /c)
  and correct quoting for paths with spaces

Frontend changes:
- Add IPC handlers for scanning all Claude CLI installations and switching active path
- Update ClaudeCodeStatusBadge to show current CLI path and allow selection when
  multiple installations are detected
- Add `writeSettingsFile()` to settings-utils for persisting CLI path selection
- Add translation keys for new UI elements (English and French)

Closes AndyMik90#1001

* fix: address PR review findings for Claude CLI detection

Addresses all 8 findings from Auto Claude PR Review:

Security improvements:
- Add path sanitization (_is_secure_path) to backend CLI validation
  to prevent command injection via malicious paths
- Add isSecurePath validation in frontend IPC handler before CLI execution
- Normalize paths with path.resolve() before execution

Architecture improvements:
- Refactor scanClaudeInstallations to use getClaudeDetectionPaths() from
  cli-tool-manager.ts as single source of truth (addresses code duplication)
- Add cross-reference comments between backend _get_claude_detection_paths()
  and frontend getClaudeDetectionPaths() to keep them in sync

Bug fixes:
- Fix path display truncation to use regex /[/\\]/ for cross-platform
  compatibility (Windows uses backslashes)
- Add null check for version in UI rendering (shows "version unknown"
  instead of "vnull")
- Use DEFAULT_APP_SETTINGS merge pattern for settings persistence

Debugging improvements:
- Add error logging in validateClaudeCliAsync catch block for better
  debugging of CLI detection issues

Translation additions:
- Add "versionUnknown" key to English and French navigation.json

* ci(release): move VirusTotal scan to separate post-release workflow (AndyMik90#980)

* ci(release): move VirusTotal scan to separate post-release workflow

VirusTotal scans were blocking release creation, taking 5+ minutes per
file. This change moves the scan to a separate workflow that triggers
after the release is published, allowing releases to be available
immediately.

- Create virustotal-scan.yml workflow triggered on release:published
- Remove blocking VirusTotal step from release.yml
- Scan results are appended to release notes after completion
- Add manual trigger option for rescanning old releases

* fix(ci): address PR review issues in VirusTotal scan workflow

- Add error checking on gh release view to prevent wiping release notes
- Replace || true with proper error handling to distinguish "no assets" from real errors
- Use file-based approach for release notes to avoid shell expansion issues
- Use env var pattern consistently for secret handling
- Remove placeholder text before appending VT results
- Document 32MB threshold with named constant
- Add HTTP status code validation on all curl requests

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(ci): add concurrency control and remove dead code in VirusTotal workflow

- Add concurrency group to prevent TOCTOU race condition when multiple
  workflow_dispatch runs target the same release tag
- Remove unused analysis_failed variable declaration

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(ci): improve error handling in VirusTotal workflow

- Fail workflow when download errors occur but scannable assets exist
- Add explicit timeout handling for analysis polling loop
- Use portable sed approach (works on both GNU and BSD sed)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

* fix(ui): display actual base branch name instead of hardcoded main (AndyMik90#969)

* fix(ui): display actual base branch name instead of hardcoded "main"

The merge conflict UI was showing "Main branch has X new commits"
regardless of the actual base branch. Now it correctly displays
the dynamic branch name (e.g., "develop branch has 40 new commits")
using the baseBranch value from gitConflicts.

* docs: update README download links to v2.7.3 (AndyMik90#976)

- Update all stable download links from 2.7.2 to 2.7.3
- Add Flatpak download link (new in 2.7.3)

* fix(i18n): add translation keys for branch divergence messages

- Add merge section to taskReview.json with pluralized translations
- Update WorkspaceStatus.tsx to use i18n for branch behind message
- Update MergePreviewSummary.tsx to use i18n for branch divergence text
- Add French translations for all new keys

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(i18n): add missing translation keys for branch behind details

- Add branchHasNewCommitsSinceBuild for build started message
- Add filesNeedAIMergeDueToRenames for path-mapped files
- Add fileRenamesDetected for rename detection message
- Add filesRenamedOrMoved for generic rename/move message
- Update WorkspaceStatus.tsx to use all new i18n keys

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(i18n): correct pluralization for rename count in AI merge message

The filesNeedAIMergeDueToRenames translation has two values that need
independent pluralization (fileCount and renameCount). Since i18next
only supports one count parameter, added separate translation keys
for singular/plural renames and select the correct key based on
renameCount value.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(i18n): use translation keys for merge button labels with dynamic branch

Replace hardcoded 'Stage to Main' and 'Merge to Main' button labels with
i18n translation keys that interpolate the actual target branch name.
Also adds translations for loading states (Resolving, Staging, Merging).

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

* fix(github-prs): prevent preloading of PRs currently under review (AndyMik90#1006)

- Updated logic to skip PRs that are currently being reviewed when determining which PRs need preloading.
- Enhanced condition to only fetch existing review data from disk if no review is in progress, ensuring that ongoing reviews are not overwritten by stale data.

* chore: bump version to 2.7.4

* hotfix/sentry-backend-build

* fix(github): resolve circular import issues in context_gatherer and services (AndyMik90#1026)

- Updated import statements in context_gatherer.py to import safe_print from core.io_utils to avoid circular dependencies with the services package.
- Introduced lazy imports in services/__init__.py to prevent circular import issues, detailing the import chain in comments for clarity.
- Added a lazy import handler to load classes on first access, improving module loading efficiency.

* feat(sentry): embed Sentry DSN at build time for packaged apps (AndyMik90#1025)

* feat(sentry): integrate Sentry configuration into Electron build

- Added build-time constants for Sentry DSN and sampling rates in electron.vite.config.ts.
- Enhanced environment variable handling in env-utils.ts to include Sentry settings for subprocesses.
- Implemented getSentryEnvForSubprocess function in sentry.ts to provide Sentry environment variables for Python backends.
- Updated Sentry-related functions to prioritize build-time constants over runtime environment variables for improved reliability.

This integration ensures that Sentry is properly configured for both local development and CI environments.

* fix(sentry): add typeof guards for build-time constants in tests

The __SENTRY_*__ constants are only defined when Vite's define plugin runs
during build. In test environments (vitest), these constants are undefined
and cause ReferenceError. Added typeof guards to safely handle both cases.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

* Fix Duplicate Kanban Task Creation on Rapid Button Clicks (AndyMik90#1021)

* auto-claude: subtask-1-1 - Add convertingIdeas state and guard logic to useIdeation hook

* auto-claude: subtask-1-2 - Update IdeaDetailPanel to accept isConverting prop

* auto-claude: subtask-2-1 - Add idempotency check for linked_task_id in task-c

* auto-claude: subtask-3-1 - Manual testing: Verify rapid clicking creates only one task

- Fixed missing convertingIdeas prop connection in Ideation.tsx
- Added convertingIdeas to destructured hook values
- Added isConverting prop to IdeaDetailPanel component
- Created detailed manual-test-report.md with code review and E2E testing instructions
- All code implementation verified via TypeScript checks (no errors)
- Multi-layer protection confirmed: UI disabled, guard check, backend idempotency
- Manual E2E testing required for final verification

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* fix: address PR review findings for duplicate task prevention

- Fix TOCTOU race condition by moving idempotency check inside lock
- Fix React state closure by using ref for synchronous tracking
- Add i18n translations for ideation UI (EN + FR)
- Add error handling with toast notifications for conversion failures

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>

* feat(terminal): add YOLO mode to invoke Claude with --dangerously-skip-permissions (AndyMik90#1016)

* feat(terminal): add YOLO mode to invoke Claude with --dangerously-skip-permissions

Add a toggle in Developer Tools settings that enables "YOLO Mode" which
starts Claude with the --dangerously-skip-permissions flag, bypassing
all safety prompts.

Changes:
- Add dangerouslySkipPermissions setting to AppSettings interface
- Add translation keys for YOLO mode (en/fr)
- Modify claude-integration-handler to accept and append extra flags
- Update terminal-manager and terminal-handlers to read and forward the setting
- Add Switch toggle with warning styling in DevToolsSettings UI

The toggle includes visual warnings (amber colors, AlertTriangle icon) to
clearly indicate this is a dangerous option that bypasses Claude's
permission system.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(terminal): address PR review issues for YOLO mode implementation

- Add async readSettingsFileAsync to avoid blocking main process during settings read
- Extract YOLO_MODE_FLAG constant to eliminate duplicate flag strings
- Store dangerouslySkipPermissions on terminal object to persist YOLO mode across profile switches
- Update switchClaudeProfile callback to pass stored YOLO mode setting

These fixes address:
- LOW: Synchronous file I/O in IPC handler
- LOW: Flag string duplicated in invokeClaude and invokeClaudeAsync
- MEDIUM: YOLO mode not persisting when switching Claude profiles

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

* Make worktree isolation prominent in UI (AndyMik90#1020)

* auto-claude: subtask-1-1 - Add i18n translation keys for worktree notice banner and merge tooltip

- Added wizard.worktreeNotice.title and wizard.worktreeNotice.description for task creation banner
- Added review.mergeTooltip for merge button explanation
- Translations added to both en/tasks.json and fr/tasks.json

* auto-claude: subtask-1-2 - Add visible info banner to TaskCreationWizard expl

* auto-claude: subtask-1-3 - Add tooltip to 'Merge with AI' button in WorkspaceStatus

- Import Tooltip components from ui/tooltip
- Wrap merge button with Tooltip, TooltipTrigger, TooltipContent
- Add contextual tooltip text explaining merge operation:
  * With AI: explains worktree merge, removal, and AI conflict resolution
  * Without AI: explains worktree merge and removal
- Follows Radix UI tooltip pattern from reference file

* fix: use i18n key for merge button tooltip in WorkspaceStatus

* fix: clarify merge tooltip - worktree removal is optional (qa-requested)

Fixes misleading tooltip text that implied worktree is automatically removed
during merge. In reality, after merge, users are shown a dialog where they can
choose to keep or remove the worktree. Updated tooltip to reflect this flow.

Changes:
- Updated en/tasks.json: Changed tooltip to clarify worktree removal is optional
- Updated fr/tasks.json: Updated French translation to match

QA Feedback: "Its currently saying on the tooltip that it will 'remove the worktree'
Please validate if this is the actual logic. As per my understanding, there will be
an extra button afterwards that will make sure that the user has access to the work
tree if they want to revert anything. The user has to manually accept to remove the
work tree."

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

* fix: use theme-aware colors for worktree info banner

Replace hardcoded blue colors with semantic theme classes to support
dark mode properly. Uses the same pattern as other info banners in
the codebase (bg-info/10, border-info/30, text-info).

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>

* fix(terminal): improve worktree name input UX (AndyMik90#1012)

* fix(terminal): improve worktree name input to not strip trailing characters while typing

- Allow trailing hyphens/underscores during input (only trim on submit)
- Add preview name that shows the final sanitized value for branch preview
- Remove invalid characters instead of replacing with hyphens
- Collapse consecutive underscores in addition to hyphens
- Final sanitization happens on submit to match backend WORKTREE_NAME_REGEX

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(terminal): address PR review findings for worktree name validation

- Fix submit button disabled check to use sanitized name instead of raw input
- Simplify trailing trim logic (apply once after all transformations)
- Apply lowercase in handleNameChange to reduce input/preview gap
- Internationalize 'name' fallback using existing translation key

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(terminal): improve header responsiveness for multiple terminals

- Hide text labels (Claude, Open in IDE) when ≥4 terminals, show icon only
- Add dynamic max-width to worktree name badge with truncation
- Add tooltips to all icon-only elements for accessibility
- Maintain full functionality while reducing header width requirements

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Co-authored-by: Test User <test@example.com>

* fix(terminal): enhance terminal recreation logic with retry mechanism (AndyMik90#1013)

* fix(terminal): enhance terminal recreation logic with retry mechanism

- Introduced a maximum retry limit and delay for terminal recreation when dimensions are not ready.
- Added cleanup for retry timers on component unmount to prevent memory leaks.
- Improved error handling to report failures after exceeding retry attempts, ensuring better user feedback during terminal setup.

* fix(terminal): address PR review feedback for retry mechanism

- Fix race condition: clear pending retry timer at START of effect
  to prevent multiple timers when dependencies change mid-retry
- Fix isCreatingRef: keep it true during retry window to prevent
  duplicate creation attempts from concurrent effect runs
- Extract duplicated retry logic into scheduleRetryOrFail helper
  (consolidated 5 duplicate instances into 1 reusable function)
- Add handleSuccess/handleError helpers to reduce code duplication
- Reduce file from 295 to 237 lines (~20% reduction)

Addresses review feedback from CodeRabbit, Gemini, and Auto Claude.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Test User <test@example.com>
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

* feat(terminal): add task worktrees section and remove terminal limit (AndyMik90#1033)

* feat(terminal): add task worktrees section and remove terminal limit

- Remove 12 terminal worktree limit (now unlimited)
- Add "Task Worktrees" section in worktree dropdown below terminal worktrees
- Task worktrees (created by kanban) now accessible for manual work
- Update translations for new section labels (EN + FR)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(terminal): address PR review feedback

- Clear taskWorktrees state when project is null or changes
- Parallelize API calls with Promise.all for better performance
- Use consistent path-based filtering for both worktree types
- Add clarifying comment for createdAt timestamp

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Co-authored-by: Test User <test@example.com>

* Add file/screenshot upload to QA feedback interface (AndyMik90#1018)

* auto-claude: subtask-1-1 - Add feedbackImages state and handlers to useTaskDetail

- Add feedbackImages state as ImageAttachment[] for storing feedback images
- Add setFeedbackImages setter for direct state updates
- Add addFeedbackImage handler for adding a single image
- Add addFeedbackImages handler for adding multiple images at once
- Add removeFeedbackImage handler for removing an image by ID
- Add clearFeedbackImages handler for clearing all images
- Import ImageAttachment type from shared/types

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* auto-claude: subtask-1-2 - Update IPC interface to support images in submitReview

- Add ImageAttachment import from ./task types
- Update submitReview signature to include optional images parameter

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* auto-claude: subtask-1-3 - Update submitReview function in task-store to accept and pass images

* auto-claude: subtask-2-1 - Add paste/drop handlers and image thumbnail displa

- Add paste event handler for screenshot/image clipboard support
- Add drag-over and drag-leave handlers for visual feedback during drag
- Add drop handler for image file drops
- Add image thumbnail display (64x64) with remove button on hover
- Import image utilities from ImageUpload.tsx (generateImageId, blobToBase64, etc.)
- Add i18n support for all new UI text
- Make new props optional for backward compatibility during incremental rollout
- Allow submission with either text feedback or images (not both required)
- Add visual drag feedback with border/background color change

* auto-claude: subtask-2-2 - Update TaskReview to pass image props to QAFeedbackSection

* auto-claude: subtask-2-3 - Update TaskDetailModal to manage image state and pass to TaskReview

- Pass feedbackImages and setFeedbackImages from useTaskDetail hook to TaskReview
- Update handleReject to include images in submitReview call
- Allow submission with images only (no text required)
- Clear images after successful submission

* auto-claude: subtask-3-1 - Add English translations for feedback image UI

* auto-claude: subtask-3-2 - Add French translations for feedback image UI

* fix(security): sanitize image filename to prevent path traversal

- Use path.basename() to strip directory components from filenames
- Validate sanitized filename is not empty, '.', or '..'
- Add defense-in-depth check verifying resolved path stays within target directory
- Fix base64 data URL regex to handle complex MIME types (e.g., svg+xml)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix: add MIME type validation and fix SVG file extension

- Add server-side MIME type validation for image uploads (defense in depth)
- Fix SVG file extension: map 'image/svg+xml' to '.svg' instead of '.svg+xml'
- Add MIME-to-extension mapping for all allowed image types

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix: require mimeType and apply SVG extension fix to drop handler

- Change MIME validation to reject missing mimeType (prevents bypass)
- Add 'image/jpg' to server-side allowlist for consistency
- Apply mimeToExtension mapping to drop handler (was only in paste handler)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Co-authored-by: Test User <test@example.com>

* fix(auth): await profile manager initialization before auth check (AndyMik90#1010)

* fix(auth): await profile manager initialization before auth check

Fixes race condition where hasValidAuth() was called before the
ClaudeProfileManager finished async initialization from disk.

The getClaudeProfileManager() returns a singleton immediately with
default profile data (no OAuth token). When hasValidAuth() runs
before initialization completes, it returns false even when valid
credentials exist.

Changed all pre-flight auth checks to use
await initializeClaudeProfileManager() which ensures initialization
completes via promise caching.

Signed-off-by: StillKnotKnown <stillknotknown@users.noreply.github.com>

* fix(auth): add error handling for profile manager initialization

Prevents unhandled promise rejections when initializeClaudeProfileManager()
throws due to filesystem errors (permissions, disk full, corrupt JSON).

The ipcMain.on handler for TASK_START doesn't await promises, so
unhandled rejections could crash the main process. Wrapped all
await initializeClaudeProfileManager() calls in try-catch blocks.

Found via automated code review.

Signed-off-by: StillKnotKnown <stillknotknown@users.noreply.github.com>

* test: mock initializeClaudeProfileManager in subprocess tests

The test mock was only mocking getClaudeProfileManager, but now we
also use initializeClaudeProfileManager which wasn't mocked, causing
test failures.

Signed-off-by: StillKnotKnown <stillknotknown@users.noreply.github.com>

* fix(auth): add try-catch for initializeClaudeProfileManager in remaining handlers

Addresses PR review feedback - TASK_UPDATE_STATUS and TASK_RECOVER_STUCK
handlers were missing try-catch blocks for initializeClaudeProfileManager(),
inconsistent with TASK_START handler.

If initialization fails, users now get specific file permissions guidance
instead of generic error messages.

Signed-off-by: StillKnotKnown <stillknotknown@users.noreply.github.com>

* refactor(auth): extract profile manager initialization into helper

Extract the repeated initializeClaudeProfileManager() + try/catch pattern
into a helper function ensureProfileManagerInitialized() that returns
a discriminated union for type-safe error handling.

This reduces code duplication across TASK_START, TASK_UPDATE_STATUS,
and TASK_RECOVER_STUCK handlers while preserving context-specific
error handling behavior.

The helper returns:
- { success: true, profileManager } on success
- { success: false, error } on failure

Signed-off-by: StillKnotKnown <stillknotknown@users.noreply.github.com>

* fix(auth): improve error details and allow retry after transient failures

Two improvements to profile manager initialization:

1. Include actual error details in failure response for better debugging.
   Previously, only a generic message was returned to users, making it
   hard to diagnose the root cause. Now the error message is appended.

2. Reset cached promise on failure to allow retries after transient errors.
   Previously, if initialize() failed (e.g., EACCES, ENOSPC), the rejected
   promise was cached forever, requiring app restart to recover. Now the
   cached promise is reset on failure, allowing subsequent calls to retry.

Signed-off-by: StillKnotKnown <stillknotknown@users.noreply.github.com>

---------

Signed-off-by: StillKnotKnown <stillknotknown@users.noreply.github.com>
Co-authored-by: StillKnotKnown <stillknotknown@users.noreply.github.com>
Co-authored-by: Andy <119136210+AndyMik90@users.noreply.github.com>

* fix(frontend): validate Windows claude.cmd reliably in GUI (AndyMik90#1023)

* fix: use absolute cmd.exe for Claude CLI validation

* fix: make cmd.exe validation type-safe for tests

* fix: satisfy frontend typecheck for cli tool tests

Signed-off-by: Umaru <caleb.1331@outlook.com>

* test: mock windows-paths exports for isSecurePath

Signed-off-by: Umaru <caleb.1331@outlook.com>

* test: make cli env tests platform-aware

Signed-off-by: Umaru <caleb.1331@outlook.com>

* test: cover isSecurePath guard in claude detection

Signed-off-by: Umaru <caleb.1331@outlook.com>

* test: align env-utils mocks with shouldUseShell

Signed-off-by: Umaru <caleb.1331@outlook.com>

* test: assert isSecurePath for cmd path

* fix(frontend): handle quoted claude.cmd paths in validation

---------

Signed-off-by: Umaru <caleb.1331@outlook.com>
Co-authored-by: Andy <119136210+AndyMik90@users.noreply.github.com>

* 2.7.4 release

* changelog 2.7.4

---------

Signed-off-by: Black Circle Sentinel <mludlow000@icloud.com>
Signed-off-by: StillKnotKnown <stillknotknown@users.noreply.github.com>
Signed-off-by: Umaru <caleb.1331@outlook.com>
Co-authored-by: StillKnotKnown <192589389+StillKnotKnown@users.noreply.github.com>
Co-authored-by: StillKnotKnown <stillknotknown@users.noreply.github.com>
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Co-authored-by: Michael Ludlow <mludlow000@icloud.com>
Co-authored-by: Test User <test@example.com>
Co-authored-by: Umaru <caleb.1331@outlook.com>
Linux binary installations were missing the `secretstorage` package,
causing OAuth token storage via Freedesktop.org Secret Service to fail
silently. The build script cache was created before secretstorage was
added to requirements.txt (Jan 16, 2026), so the package was not being
validated during bundling.

Changes:
- Add platform-aware critical packages validation in download-python.cjs
- Add platform-aware critical packages validation in python-env-manager.ts
- Add Linux secretstorage warning in dependency_validator.py
- Add comprehensive tests for Linux secretstorage validation

The 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

The Linux validation emits a warning (not blocking error) since the app
gracefully falls back to .env file storage when secretstorage is unavailable.

Refs: ACS-310
The tests test_windows_skips_secretstorage_validation and
test_macos_skips_secretstorage_validation were failing on Windows CI
because they didn't mock the pywintypes import. When running on
actual Windows in CI, the pywin32 validation runs first and exits
because pywin32 isn't installed in the test environment.

The fix mocks pywintypes to succeed so we can properly test that
secretstorage validation is skipped on non-Linux platforms.
Changes made based on CodeRabbit AI review:

Backend (dependency_validator.py):
- Rename _exit_with_secretstorage_warning to _warn_missing_secretstorage
  to accurately reflect that it doesn't exit (it only emits a warning)
- Add sys.stderr.flush() to ensure warning is immediately visible

Frontend (download-python.cjs):
- Fix critical __init__.py validation logic - packages are now only considered
  valid if directory+__init__.py exists OR single-file module exists

Frontend (python-env-manager.ts):
- Use platform abstraction (isWindows(), isLinux()) instead of process.platform
- Fix critical packages validation to match download-python.cjs logic

Tests (test_dependency_validator.py):
- Update function name to _warn_missing_secretstorage
- Add is_windows patches to Linux tests to prevent pywin32 validation
  from running on Windows CI

Refs: ACS-310
@StillKnotKnown StillKnotKnown force-pushed the stillknotknown/acs-310-linux-binary-missing-secretstorage-dependency branch from e8cf42b to 6a709a8 Compare January 16, 2026 20:05
@github-actions github-actions bot added size/XL and removed size/L labels Jan 16, 2026
StillKnotKnown pushed a commit that referenced this pull request Jan 18, 2026
…ik90#1259)

* fix(windows): prevent zombie process accumulation on app close

- Use taskkill /f /t on Windows to properly kill process trees
  (SIGTERM/SIGKILL are ignored on Windows)
- Make killAllProcesses() wait for process exit events with timeout
- Kill PTY daemon process on shutdown
- Clear periodic update check interval on app quit

Fixes process accumulation in Task Manager after closing Auto-Claude.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(windows): extract killProcessGracefully utility with timer cleanup

- Extract shared killProcessGracefully() to platform module
- Fix Issue #1: Move taskkill outside try-catch scope
- Fix Issue #2: Track exit state to skip unnecessary taskkill
- Fix Issue #3: Add GRACEFUL_KILL_TIMEOUT_MS constant
- Fix Issue #4: Use consistent 5000ms timeout everywhere
- Fix Issue #5: Add debug logging for catch blocks
- Fix Issue AndyMik90#6: Log warning when process.once unavailable
- Fix Issue AndyMik90#7: Eliminate code duplication across 3 files
- Fix timer leak: Clear timeout on process exit/error, unref timer

Add comprehensive tests (19 test cases) covering:
- Windows taskkill fallback behavior
- Unix SIGTERM/SIGKILL sequence
- Timer cleanup and memory leak prevention
- Edge cases and error handling

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Co-authored-by: Andy <119136210+AndyMik90@users.noreply.github.com>
@StillKnotKnown StillKnotKnown force-pushed the develop branch 2 times, most recently from 67a743f to e83e445 Compare January 21, 2026 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/fullstack bug Something isn't working size/XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants