-
Notifications
You must be signed in to change notification settings - Fork 3
feat: add codex claude sdk #5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 4 commits
6858fb0
a30d304
3ad0d0f
5305846
8ecfa54
09d985c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,303 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from __future__ import annotations | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from pathlib import Path | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from typing import Any, Dict, List, Optional | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from pydantic import Field, model_validator, PrivateAttr | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from base.agent.base_agent import BaseAgent | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from claude_agent_sdk import ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| query, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ClaudeAgentOptions, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ClaudeSDKError, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| CLINotFoundError, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ProcessError, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| CLIJSONDecodeError, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| CLAUDE_AGENT_AVAILABLE = True | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| except ImportError: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| CLAUDE_AGENT_AVAILABLE = False | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Fallback types | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| AssistantMessage = UserMessage = ResultMessage = Any | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| TextBlock = ToolUseBlock = ToolResultBlock = Any | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ClaudeSDKError = CLINotFoundError = ProcessError = CLIJSONDecodeError = Exception | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| class ClaudeCodeAgent(BaseAgent): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Claude Code Agent for code generation using Claude Agent SDK.""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Claude Code Agent for code generation using Claude Agent SDK.""" | |
| """Claude Code Agent for code generation using Claude Agent SDK. | |
| This agent integrates with the external `claude_agent_sdk` package to provide | |
| code-centric automation such as writing, editing, and executing code in a | |
| local project directory. It is designed to be used as a `BaseAgent` | |
| subclass within the autoenv framework. | |
| Installation | |
| ------------ | |
| The `claude_agent_sdk` package must be installed for this agent to be fully | |
| functional. If it is missing, `CLAUDE_AGENT_AVAILABLE` will be set to | |
| ``False`` and calls into the SDK will fail. | |
| Install the SDK via pip: | |
| pip install claude-agent-sdk | |
| or add it to your project's dependencies. | |
| Requirements | |
| ----------- | |
| - Python environment with access to the Claude Code CLI used by | |
| `claude_agent_sdk`. | |
| - Valid configuration/credentials for the underlying Claude tooling, as | |
| required by the SDK and CLI. | |
| - (Optional) A working directory ``cwd`` that points to the root of the | |
| project the agent should operate on. | |
| Basic usage example | |
| ------------------- | |
| Instantiate the agent and run it on a coding task: | |
| .. code-block:: python | |
| from pathlib import Path | |
| from autoenv.claude_code_agent import ClaudeCodeAgent | |
| agent = ClaudeCodeAgent( | |
| cwd=Path("/path/to/your/project"), | |
| allowed_tools=["Read", "Write", "Bash"], | |
| max_turns=8, | |
| ) | |
| # Example high-level call (actual API depends on BaseAgent interface): | |
| result = agent.run("Add unit tests for the user authentication module.") | |
| print(result) | |
| Within larger systems, `ClaudeCodeAgent` can be composed with other agents | |
| or orchestrators that expect a `BaseAgent`-compatible interface. | |
| """ |
Outdated
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same issue as CodexAgent - the permission_mode field lists four modes but doesn't explain what each mode does. This is especially important given the security implications of "bypassPermissions". Add detailed documentation for each permission mode.
| description="Permission mode: default|acceptEdits|bypassPermissions|plan" | |
| description=( | |
| "Controls how the agent handles permission prompts for edits, tool use, and commands. " | |
| "Supported values:\n" | |
| "- 'default': Use the Claude Code CLI / SDK's default interactive behavior for requesting\n" | |
| " permission before making changes or running tools.\n" | |
| "- 'acceptEdits': Automatically accept and apply code edits suggested by the agent while\n" | |
| " still requiring confirmation for other sensitive actions (e.g., shell commands).\n" | |
| "- 'bypassPermissions': Run tools, apply edits, and execute commands without asking for\n" | |
| " interactive confirmation. This effectively disables permission prompts and should only\n" | |
| " be used in fully trusted, sandboxed environments due to its security implications.\n" | |
| "- 'plan': Focus on generating and updating a plan of actions without automatically\n" | |
| " executing potentially destructive steps; execution typically requires separate approval." | |
| ) |
chenmancm169 marked this conversation as resolved.
Show resolved
Hide resolved
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The _handle_sdk_error method has inconsistent error handling logic. It checks for specific exception types using isinstance, but if CLAUDE_AGENT_AVAILABLE is False, all the exception types are set to the base Exception class (line 26). This means the isinstance checks will not work as intended, and all exceptions would be caught by the final else clause and re-raised. The method should handle the case where SDK types are unavailable.
chenmancm169 marked this conversation as resolved.
Show resolved
Hide resolved
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error handling in step() (line 201-202) catches all exceptions and passes them to _handle_sdk_error, which re-raises unexpected exceptions. However, this means that validation errors like FileNotFoundError and NotADirectoryError from _validate_cwd would be re-raised instead of being converted to error strings. This is inconsistent with the documented behavior and differs from how CodexAgent handles the same exceptions.
| return await self._process_query_stream(self._current_prompt, options) | |
| return await self._process_query_stream(self._current_prompt, options) | |
| except (FileNotFoundError, NotADirectoryError) as e: | |
| # Convert validation errors (e.g., invalid cwd) to user-facing error strings, | |
| # instead of letting them be re-raised as unexpected SDK errors. | |
| return f"Error: {e}" |
chenmancm169 marked this conversation as resolved.
Show resolved
Hide resolved
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same issue as in CodexAgent - the error message for unknown attributes is misleading. It says "Unknown attribute" when the attribute exists but is not in the modifiable_attrs whitelist. The message should clarify that the attribute cannot be modified via kwargs.
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to CodexAgent, the cwd parameter can be set via kwargs without proper sanitization. A user could set cwd to sensitive directories and potentially access or modify system files. The security implications are especially concerning when combined with permissive permission_mode settings.
| setattr(self, key, value) | |
| # Validate critical attributes after modification | |
| if key == 'cwd': | |
| if isinstance(self.cwd, str): | |
| self.cwd = Path(self.cwd) | |
| self._validate_cwd() | |
| elif key == 'permission_mode': | |
| valid_modes = ["default", "acceptEdits", "bypassPermissions", "plan"] | |
| if value not in valid_modes: | |
| raise ValueError(f"Invalid permission_mode: {value}. Must be one of: {valid_modes}") | |
| # Special handling and validation for cwd to avoid unsafe directories | |
| if key == 'cwd': | |
| # Normalize incoming cwd value to a Path | |
| if isinstance(value, str): | |
| new_cwd = Path(value) | |
| elif isinstance(value, Path): | |
| new_cwd = value | |
| else: | |
| raise ValueError("cwd must be a string or pathlib.Path") | |
| # Disallow absolute paths to prevent pointing cwd to arbitrary sensitive locations | |
| if new_cwd.is_absolute(): | |
| raise ValueError("cwd must be a relative path, absolute paths are not allowed") | |
| # Prevent directory traversal via parent directory components | |
| if ".." in new_cwd.parts: | |
| raise ValueError("cwd cannot contain parent directory references ('..')") | |
| # Resolve the new cwd relative to the current cwd | |
| base_cwd = self.cwd | |
| if isinstance(base_cwd, str): | |
| base_cwd = Path(base_cwd) | |
| resolved_cwd = (base_cwd / new_cwd).resolve() | |
| self.cwd = resolved_cwd | |
| self._validate_cwd() | |
| else: | |
| setattr(self, key, value) | |
| # Validate critical attributes after modification | |
| if key == 'permission_mode': | |
| valid_modes = ["default", "acceptEdits", "bypassPermissions", "plan"] | |
| if value not in valid_modes: | |
| raise ValueError(f"Invalid permission_mode: {value}. Must be one of: {valid_modes}") |
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error handling in run() method's kwargs restoration (lines 255-262) has the same issue as CodexAgent - it silently swallows all exceptions during restoration attempts. If setattr fails for any attribute, the agent could be left in an inconsistent state with some attributes modified and others not.
chenmancm169 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Outdated
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to CodexAgent, the run method modifies instance attributes without synchronization. If multiple coroutines call run() concurrently on the same ClaudeCodeAgent instance, they could interfere with each other's attribute modifications and restoration. Document that the agent is not safe for concurrent use or implement proper locking.
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to CodexAgent, the finally block silently ignores all exceptions during attribute restoration (lines 269-275). This means if restoration fails after successful execution, the agent will remain in a modified state, violating the contract that kwargs provide temporary overrides. This is the same issue as in CodexAgent.
| try: | |
| options = self._create_options() | |
| return await self._process_query_stream(request, options) | |
| except Exception as e: | |
| return self._handle_sdk_error(e) | |
| finally: | |
| # Restore original values (best effort) | |
| for key, value in original_values.items(): | |
| try: | |
| setattr(self, key, value) | |
| except Exception: | |
| pass # Ignore errors during restoration | |
| primary_exc: Optional[BaseException] = None | |
| result: str | |
| restoration_errors = [] | |
| try: | |
| options = self._create_options() | |
| result = await self._process_query_stream(request, options) | |
| except Exception as e: | |
| primary_exc = e | |
| result = self._handle_sdk_error(e) | |
| finally: | |
| # Restore original values (best effort) | |
| for key, value in original_values.items(): | |
| try: | |
| setattr(self, key, value) | |
| except Exception as restore_exc: | |
| # Record restoration errors; they may be surfaced if there was no primary error | |
| restoration_errors.append((key, restore_exc)) | |
| # If the main execution succeeded but restoration failed, report the restoration error | |
| if primary_exc is None and restoration_errors: | |
| failed_key, failed_exc = restoration_errors[0] | |
| return f"Error: Failed to restore attribute '{failed_key}': {failed_exc}" | |
| return result |
Copilot
AI
Dec 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same issue as CodexAgent - the call method accepts three different parameter names (request, task, prompt) which can lead to ambiguity. If multiple are provided, the precedence is undocumented. Consider standardizing on a single parameter name or clearly documenting the precedence order.
| """Execute the agent with given parameters.""" | |
| request = kwargs.pop('request', None) or kwargs.pop('task', None) or kwargs.pop('prompt', None) | |
| """Execute the agent with given parameters. | |
| Accepts a single text argument under one of the aliases: 'request', 'task', or 'prompt'. | |
| If more than one of these is provided, a ValueError is raised to avoid ambiguity. | |
| """ | |
| request = kwargs.pop("request", None) | |
| task = kwargs.pop("task", None) | |
| prompt = kwargs.pop("prompt", None) | |
| # Ensure we don't silently ignore conflicting inputs | |
| provided_count = sum(v is not None for v in (request, task, prompt)) | |
| if provided_count > 1: | |
| raise ValueError( | |
| "ClaudeCodeAgent.__call__ accepts only one of 'request', 'task', or 'prompt'. " | |
| f"Received: request={request!r}, task={task!r}, prompt={prompt!r}" | |
| ) | |
| # Preserve existing precedence: request > task > prompt | |
| if request is None: | |
| request = task if task is not None else prompt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fallback type assignments on lines 24-26 create type aliases that map to Any or Exception, which defeats the purpose of type hints. When CLAUDE_AGENT_AVAILABLE is False, these fallback types will allow any value without type checking, potentially hiding bugs. Consider using proper type stubs or Protocol definitions for better type safety when the SDK is not installed.