Skip to content

Conversation

@hluisi
Copy link

@hluisi hluisi commented Jan 2, 2026

Summary

Fixes two critical bugs that were incorrectly closed as "working-as-intended":

  1. Security hook uses wrong project directory (fix(security): bash security hook uses wrong project directory #555)
  2. Tools like dotnet not found due to PATH issues (fix(backend): SDK_ENV_VARS missing PATH causes tools like dotnet to not be found #556)

Changes

1. Fix security hook cwd extraction (apps/backend/security/hooks.py)

Problem: Hook checked context.cwd but Claude Agent SDK passes cwd in input_data dict.

Fix:

# Before (incorrect)
cwd = os.getcwd()
if context and hasattr(context, "cwd"):
    cwd = context.cwd

# After (correct)
cwd = input_data.get("cwd", os.getcwd())

Evidence: Claude Agent SDK Python docs show PreToolUse hooks receive cwd in input_data, not context.

2. Fix PATH for tools in agent processes (apps/frontend/src/main/agent/agent-process.ts)

Problem: When Electron launches from Finder/Dock, process.env.PATH is minimal.

Fix: Use existing getAugmentedEnv() infrastructure:

// Before
return {
  ...process.env,  // Minimal PATH
  ...extraEnv,
  ...profileEnv,
}

// After
const augmentedEnv = getAugmentedEnv();  // Adds /opt/homebrew/bin, etc.
return {
  ...augmentedEnv,
  ...extraEnv,
  ...profileEnv,
}

Also added /usr/local/share/dotnet and ~/.dotnet/tools to COMMON_BIN_PATHS.

3. Pin electron version (apps/frontend/package.json)

Changed ^39.2.739.2.7 so electron-builder can compute version in monorepo.

Testing

Verified on macOS with C# project:

  • dotnet commands now allowed by security hook
  • dotnet executable found immediately (no more "command not found")
  • ✅ Security profile correctly analyzes target project, not runner directory

Related Issues

Closes #555
Closes #556

Summary by CodeRabbit

Release Notes

  • Improvements
    • macOS: .NET SDK installation directory and global tools directory now automatically available in the application environment
    • Process environment enhanced with common tool paths (including .NET, Homebrew, and other development tools) for improved tool accessibility in spawned processes

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

hluisi added 3 commits January 2, 2026 11:12
The bash_security_hook was checking context.cwd, but the Claude Agent
SDK passes cwd in input_data dict, not context object. This caused the
hook to always fall back to os.getcwd() which returns the runner
directory (apps/backend/) instead of the project directory.

According to Claude Agent SDK docs, PreToolUse hooks receive cwd in
input_data, not context. The context parameter is reserved for future
use in the Python SDK.

Fixes AndyMik90#555

Signed-off-by: Hunter Luisi <[email protected]>
When Electron launches from Finder/Dock on macOS, process.env.PATH is
minimal and doesn't include user shell paths. This caused tools like
dotnet, cargo, etc. to fail with 'command not found'.

Solution:
1. Use getAugmentedEnv() in agent-process.ts instead of raw process.env
2. Add /usr/local/share/dotnet and ~/.dotnet/tools to COMMON_BIN_PATHS

getAugmentedEnv() already exists and is used throughout the frontend
for Git/GitHub/GitLab operations. It adds common tool directories to
PATH based on platform.

Fixes AndyMik90#556

Signed-off-by: Hunter Luisi <[email protected]>
Pinning electron version (removing caret) so electron-builder can
compute the version from installed modules in monorepo setup.

Signed-off-by: Hunter Luisi <[email protected]>
@CLAassistant
Copy link

CLAassistant commented Jan 2, 2026

CLA assistant check
All committers have signed the CLA.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 2, 2026

📝 Walkthrough

Walkthrough

The changes address tool discovery and security hook issues across the backend and frontend. The security hook now reads the project directory from input data instead of context, while the frontend environment is augmented with common tool paths for .NET and homebrew, and Electron is locked to a specific version.

Changes

Cohort / File(s) Summary
Security Hook Directory Resolution
apps/backend/security/hooks.py
Changed project directory detection from context.cwd fallback to reading from input_data["cwd"], affecting how the security profile is loaded and command validation behavior.
Frontend Environment Augmentation
apps/frontend/src/main/agent/agent-process.ts
Replaced direct process.env usage with getAugmentedEnv() to enrich spawned process environment with common tool paths and improve discoverability.
Frontend Tool Path Configuration
apps/frontend/src/main/env-utils.ts
Added two macOS-specific paths to COMMON_BIN_PATHS: /usr/local/share/dotnet and ~/.dotnet/tools for .NET SDK discovery.
Dependency Version Lock
apps/frontend/package.json
Locked Electron dependency from ^39.2.7 to exact version 39.2.7, preventing range-based version resolution.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested labels

bug, area/backend

Suggested reviewers

  • AndyMik90

Poem

🐰 A hook that reads from data true,
Tool paths discovered, shiny and new,
The dotnet paths now in the light,
Electron pinned, all locked up tight!
Tools are found where they reside,
Security profiles won't hide! 🔐

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR partially addresses linked issues but lacks critical backend changes. Issue #555 requires setting AUTO_CLAUDE_PROJECT_DIR env var in apps/backend/core/client.py, and issue #556 requires adding PATH to SDK_ENV_VARS in apps/backend/core/auth.py, neither of which appear in the changeset. Implement the proposed backend fixes: (1) Add AUTO_CLAUDE_PROJECT_DIR environment variable setting in apps/backend/core/client.py, and (2) Add PATH to SDK_ENV_VARS list in apps/backend/core/auth.py to complete the fix.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes: fixing the security hook cwd extraction and PATH issues with specific issue references.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the security hook cwd extraction and PATH issues described in the linked issues; no out-of-scope changes detected.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ 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

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

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @hluisi, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses two critical bugs related to environment configuration and security hook behavior. It ensures that the security hook correctly identifies the project's working directory and that agent processes have a complete PATH environment variable, allowing tools like dotnet to be found. Additionally, it pins the Electron version for build stability. These changes collectively improve the reliability and functionality of the application's agent and security features.

Highlights

  • Security Hook CWD Extraction: Corrected the security hook's method for determining the current working directory (cwd) by retrieving it from input_data as passed by the Claude Agent SDK, rather than relying on context.cwd or os.getcwd().
  • Agent Process PATH Augmentation: Enhanced the environment variables for agent processes by utilizing getAugmentedEnv() to ensure common binary paths (like Homebrew and .NET tools) are available, especially when the application is launched from non-standard environments like Finder/Dock.
  • Electron Version Pinning: Pinned the Electron dependency version in package.json from ^39.2.7 to 39.2.7 to resolve issues with electron-builder correctly computing the version within a monorepo setup.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

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.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉 Thanks for your first PR!

A maintainer will review it soon. Please make sure:

  • Your branch is synced with develop
  • CI checks pass
  • You've followed our contribution guide

Welcome to the Auto Claude community!

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request effectively addresses two critical bugs related to security hook execution context and environment path resolution for agent processes. The fix for cwd extraction in the security hook is correct and aligns with the Claude Agent SDK's behavior. The use of getAugmentedEnv to resolve PATH issues on macOS is a solid improvement. I've made one suggestion to improve the path ordering for darwin to follow standard conventions and avoid potential tool version conflicts. The version pinning for electron is also a good practice for build stability.

Comment on lines 64 to 72
darwin: [
'/opt/homebrew/bin', // Apple Silicon Homebrew
'/usr/local/bin', // Intel Homebrew / system
'/usr/local/share/dotnet', // .NET SDK
'/opt/homebrew/sbin', // Apple Silicon Homebrew sbin
'/usr/local/sbin', // Intel Homebrew sbin
'~/.local/bin', // User-local binaries (Claude CLI)
'~/.dotnet/tools', // .NET global tools
],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The current order of paths gives system-wide directories (like /opt/homebrew/bin and /usr/local/bin) higher priority than user-specific directories (~/.local/bin, ~/.dotnet/tools). This is unconventional and can lead to issues where a user's local installation of a tool is ignored in favor of an older system version.

It's standard practice to prepend user-specific binary paths to the PATH to allow users to override system-wide tools. I suggest reordering the darwin paths to prioritize user directories.

Suggested change
darwin: [
'/opt/homebrew/bin', // Apple Silicon Homebrew
'/usr/local/bin', // Intel Homebrew / system
'/usr/local/share/dotnet', // .NET SDK
'/opt/homebrew/sbin', // Apple Silicon Homebrew sbin
'/usr/local/sbin', // Intel Homebrew sbin
'~/.local/bin', // User-local binaries (Claude CLI)
'~/.dotnet/tools', // .NET global tools
],
darwin: [
'~/.local/bin', // User-local binaries (Claude CLI)
'~/.dotnet/tools', // .NET global tools
'/opt/homebrew/bin', // Apple Silicon Homebrew
'/usr/local/bin', // Intel Homebrew / system
'/usr/local/share/dotnet', // .NET SDK
'/opt/homebrew/sbin', // Apple Silicon Homebrew sbin
'/usr/local/sbin', // Intel Homebrew sbin
],

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 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 4a83304 and 2ce1107.

📒 Files selected for processing (4)
  • apps/backend/security/hooks.py
  • apps/frontend/package.json
  • apps/frontend/src/main/agent/agent-process.ts
  • apps/frontend/src/main/env-utils.ts
🧰 Additional context used
📓 Path-based instructions (4)
apps/frontend/src/**/*.{ts,tsx,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings

Files:

  • apps/frontend/src/main/agent/agent-process.ts
  • apps/frontend/src/main/env-utils.ts
apps/frontend/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use useTranslation() hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components

Files:

  • apps/frontend/src/main/agent/agent-process.ts
  • apps/frontend/src/main/env-utils.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/agent/agent-process.ts
  • apps/frontend/src/main/env-utils.ts
apps/backend/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

apps/backend/**/*.py: Always use the Claude Agent SDK (claude-agent-sdk package) for all AI interactions, never use the Anthropic API directly
Use the create_client() function from apps/backend/core/client.py to instantiate Claude SDK clients, not direct ClaudeSDKClient initialization

Files:

  • apps/backend/security/hooks.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/security/hooks.py
🧬 Code graph analysis (1)
apps/frontend/src/main/agent/agent-process.ts (1)
apps/frontend/src/main/env-utils.ts (1)
  • getAugmentedEnv (97-146)
🔇 Additional comments (3)
apps/frontend/package.json (1)

117-117: LGTM: Electron version pinned for build stability.

Removing the caret to pin Electron to an exact version is appropriate for resolving electron-builder version computation issues in the monorepo. This ensures consistent builds across environments.

Note: This prevents automatic security updates, so remember to manually update Electron when security patches are released.

apps/frontend/src/main/agent/agent-process.ts (1)

19-19: LGTM: Environment augmentation correctly implemented.

The integration of getAugmentedEnv() properly ensures that common tool paths (dotnet, Homebrew, etc.) are available to spawned processes, even when the app is launched from Finder/Dock. The merge order is correct:

  1. augmentedEnv provides the base with augmented PATH
  2. extraEnv and profileEnv can override as needed
  3. Python-specific variables are set last

This change directly addresses the PR objective of ensuring tools in non-standard locations are discoverable.

Also applies to: 59-63

apps/backend/security/hooks.py (1)

68-69: Verify that the Claude Agent SDK passes cwd in input_data to PreToolUse hooks, not just as an agent option.

The code assumes the SDK populates input_data with cwd from the agent options, but the hook's docstring documents input_data as containing only tool_name and tool_input. If the SDK doesn't pass cwd through input_data, the fallback to os.getcwd() will analyze the runner's directory instead of the target project. Confirm with the SDK documentation or the development team that cwd is passed in input_data, or modify the implementation to extract cwd through context or another mechanism.

darwin: [
'/opt/homebrew/bin', // Apple Silicon Homebrew
'/usr/local/bin', // Intel Homebrew / system
'/usr/local/share/dotnet', // .NET SDK
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider adding .NET paths for Linux.

The .NET SDK and global tools paths are correctly added for macOS. However, .NET is also commonly used on Linux, where it's typically installed at:

  • /usr/share/dotnet (SDK)
  • ~/.dotnet/tools (global tools)
🔎 Proposed change to add Linux .NET support
  linux: [
    '/usr/local/bin',
    '/usr/bin',               // System binaries (Python, etc.)
+   '/usr/share/dotnet',      // .NET SDK
    '/snap/bin',              // Snap packages
    '~/.local/bin',           // User-local binaries
+   '~/.dotnet/tools',        // .NET global tools
    '/usr/sbin',              // System admin binaries
  ],

Also applies to: 71-71

🤖 Prompt for AI Agents
In apps/frontend/src/main/env-utils.ts around lines 67 and 71, the PATH list
includes macOS .NET locations but is missing common Linux .NET install
locations; add '/usr/share/dotnet' for the SDK and the current user's tools
folder (e.g. expand to `${HOME}/.dotnet/tools` or use os.homedir()/path.join) to
the array; if using os.homedir() or path.join, add the necessary imports and
ensure the entry only gets added when process.platform === 'linux' (or add
unconditionally if safe).

@hluisi
Copy link
Author

hluisi commented Jan 2, 2026

I have read the CLA Document and I hereby sign the CLA

@hluisi
Copy link
Author

hluisi commented Jan 2, 2026

@coderabbitai - Confirmed with official Claude Agent SDK documentation.

According to the Python SDK Reference, PreToolUse hooks receive cwd in the input_data dictionary:

Hook signature:

async def hook(input_data: dict, tool_use_id: str | None, context: HookContext)

input_data fields (from SDK docs):

  • hook_event_name: Hook type (e.g., "PreToolUse")
  • session_id: Current session identifier
  • cwd: Current working directory ← Passed here
  • tool_name: Name of tool being called
  • tool_input: Tool arguments

context parameter:

  • Reserved for future use in Python SDK
  • Does NOT contain cwd

This is how the SDK passes the cwd option from ClaudeAgentOptions(cwd=...) to hooks - via input_data, not context.

Tested on macOS with C# project - security hook now correctly analyzes the target project directory instead of falling back to os.getcwd() (which returns apps/backend/).

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 2, 2026

Tip

For best results, initiate chat on the files or code changes.

@hluisi Thank you for the thorough documentation and confirmation! This clarifies the SDK's hook interface perfectly.

The official SDK reference you provided confirms that cwd is indeed passed through input_data rather than context for PreToolUse hooks, which validates the approach taken in this PR. It's great to hear that your testing on macOS with the C# project confirms the security hook now correctly analyzes the target project directory instead of falling back to the runner's directory.

This is exactly the behavior needed to fix issue #555, and your documentation reference provides solid evidence for the implementation choice.


✏️ Learnings added
Learnt from: hluisi
URL: https://github.com/AndyMik90/Auto-Claude/pull/587

Timestamp: 2026-01-02T19:30:59.109Z
Learning: In the Claude Agent SDK (Python), PreToolUse hooks receive `cwd` (current working directory) in the `input_data` dictionary parameter, not in the `context` parameter. The hook signature is `async def hook(input_data: dict, tool_use_id: str | None, context: HookContext)` where `input_data["cwd"]` contains the working directory passed from `ClaudeAgentOptions(cwd=...)`. The `context` parameter is reserved for future use and does NOT contain `cwd`.
<!-- <review_comment_addressed>

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/backend/core/client.py : Implement agent-specific tool permissions in the Claude SDK client based on agent role (planner, coder, qa_reviewer, qa_fixer)

@AndyMik90 AndyMik90 self-assigned this Jan 2, 2026
@MikeeBuilds MikeeBuilds added bug Something isn't working area/backend This is backend only size/XS Extra small (< 10 lines) labels Jan 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/backend This is backend only bug Something isn't working size/XS Extra small (< 10 lines)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(backend): SDK_ENV_VARS missing PATH causes tools like dotnet to not be found fix(security): bash security hook uses wrong project directory

4 participants