Repository navigation
[codex] Add local execution environment - #89
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds an Environment protocol and FileTools type, implements LocalEnvironment, and threads Environment objects through pipeline creation, binding resolution, CoreToolsPlugin, shell-session init, and session/agent wiring; tests added for protocol conformance, tool behavior, and environment propagation. ChangesEnvironment abstraction and local runtime wiring
Sequence Diagram(s)sequenceDiagram
participant SessionMgr as Session Manager
participant Binding as Binding Resolver
participant App as App Factory
participant Env as LocalEnvironment
participant Core as CoreToolsPlugin
participant Tools as Tool Factories
SessionMgr->>Binding: resolve_environment(binding)
Binding-->>SessionMgr: Environment (LocalEnvironment)
SessionMgr->>App: create_agent(environment=env)
App->>Env: tool_config()
Env-->>App: {"workspace_root": path}
App->>Core: init CoreToolsPlugin(environment=env)
Core->>Env: build_file_tools()
Env->>Tools: build_file_tools(workspace_root)
Tools-->>Core: file tool callables
Core->>Env: build_file_patch_tool()
Env->>Tools: build_file_patch_tool(workspace_root)
Tools-->>Core: patch callable
Core->>Env: build_shell_tool()
Env->>Tools: shell.bash_run
Tools-->>Core: shell callable
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 9/10 reviews remaining, refill in 6 minutes. Comment |
delete-cloud
marked this pull request as ready for review
May 2, 2026 06:21
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
coding-agent/src/agentkit/environment/protocols.py (1)
1-28:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRun the required AgentKit test suite for this change before merge.
This introduces new behavior under
src/agentkit/, but the listed verification only runs targetedtests/coding_agent/+tests/ui/files. Please add relevanttests/agentkit/coverage to the verification run.As per coding guidelines,
coding-agent/src/agentkit/**/*.py: “When changing agentkit code, run relevant tests/agentkit/ tests plus impacted tests/coding_agent/ tests”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@coding-agent/src/agentkit/environment/protocols.py` around lines 1 - 28, The change adds new behavior in agentkit (see Environment protocol and FileTools tuple in protocols.py) but CI only runs tests/coding_agent/ and tests/ui/; add and run relevant tests under tests/agentkit/ that cover Environment.kind, tool_config, build_file_tools, build_file_patch_tool, and build_shell_tool behavior (including return types of FileTools callables) and update the test run to include tests/agentkit/ alongside existing targeted suites before merging.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@coding-agent/src/coding_agent/plugins/core_tools.py`:
- Around line 127-135: The session context may have cwd==None so
_prepare_arguments (the method containing the shown snippet in CoreToolsPlugin)
should fall back to the injected workspace root; when self._shell_session is
present get the session via self._shell_session.get_session_context() and set
args["cwd"] to session.get("cwd") or self._workspace_root (i.e., use the session
cwd if present otherwise default to self._workspace_root), leaving env behavior
unchanged; alternatively you can instead update ShellSessionPlugin.do_mount() to
read ctx.config["workspace_root"] and initialize cwd with that value, but pick
one of these two fixes and apply it consistently.
---
Outside diff comments:
In `@coding-agent/src/agentkit/environment/protocols.py`:
- Around line 1-28: The change adds new behavior in agentkit (see Environment
protocol and FileTools tuple in protocols.py) but CI only runs
tests/coding_agent/ and tests/ui/; add and run relevant tests under
tests/agentkit/ that cover Environment.kind, tool_config, build_file_tools,
build_file_patch_tool, and build_shell_tool behavior (including return types of
FileTools callables) and update the test run to include tests/agentkit/
alongside existing targeted suites before merging.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 61b882b1-71d9-40cc-9702-060604c1b27e
📒 Files selected for processing (13)
coding-agent/src/agentkit/environment/__init__.pycoding-agent/src/agentkit/environment/protocols.pycoding-agent/src/coding_agent/app.pycoding-agent/src/coding_agent/environment/__init__.pycoding-agent/src/coding_agent/environment/local.pycoding-agent/src/coding_agent/plugins/core_tools.pycoding-agent/src/coding_agent/ui/binding_resolver.pycoding-agent/src/coding_agent/ui/session_manager.pycoding-agent/tests/coding_agent/environment/test_local_environment.pycoding-agent/tests/coding_agent/plugins/test_core_tools.pycoding-agent/tests/coding_agent/test_bootstrap.pycoding-agent/tests/ui/test_execution_binding.pycoding-agent/tests/ui/test_session_manager_runtime.py
delete-cloud
force-pushed
the
codex-environment-local-pr1
branch
from
May 2, 2026 06:37
3bb62f5 to
0571bfa
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
coding-agent/src/agentkit/environment/protocols.py (1)
6-12: 💤 Low valueConsider documenting the
FileToolstuple positions.The tuple type correctly captures the callable signatures, but the mapping to specific tools (file_read, file_write, file_replace, glob_files, grep_search) is implicit. A docstring or inline comment would improve maintainability.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@coding-agent/src/agentkit/environment/protocols.py` around lines 6 - 12, Add a short docstring or inline comment for the FileTools type that explicitly maps each tuple position to the tool name and its signature (e.g., position 0 = file_read: Callable[[str], str | dict[str, Any]], position 1 = file_write: Callable[[str, str], str], position 2 = file_replace: Callable[[str, str, str], str], position 3 = glob_files: Callable[[str, str], str], position 4 = grep_search: Callable[[str, str, str], str | dict[str, Any]]), so readers can immediately see which callable corresponds to file_read, file_write, file_replace, glob_files and grep_search when they encounter the FileTools alias.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@coding-agent/src/agentkit/environment/protocols.py`:
- Around line 6-12: Add a short docstring or inline comment for the FileTools
type that explicitly maps each tuple position to the tool name and its signature
(e.g., position 0 = file_read: Callable[[str], str | dict[str, Any]], position 1
= file_write: Callable[[str, str], str], position 2 = file_replace:
Callable[[str, str, str], str], position 3 = glob_files: Callable[[str, str],
str], position 4 = grep_search: Callable[[str, str, str], str | dict[str,
Any]]), so readers can immediately see which callable corresponds to file_read,
file_write, file_replace, glob_files and grep_search when they encounter the
FileTools alias.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 30f72bb1-02d8-46a6-8386-5bcc9605439a
📒 Files selected for processing (16)
coding-agent/src/agentkit/environment/__init__.pycoding-agent/src/agentkit/environment/protocols.pycoding-agent/src/coding_agent/app.pycoding-agent/src/coding_agent/environment/__init__.pycoding-agent/src/coding_agent/environment/local.pycoding-agent/src/coding_agent/plugins/core_tools.pycoding-agent/src/coding_agent/plugins/shell_session.pycoding-agent/src/coding_agent/ui/binding_resolver.pycoding-agent/src/coding_agent/ui/session_manager.pycoding-agent/tests/agentkit/environment/test_protocols.pycoding-agent/tests/coding_agent/environment/test_local_environment.pycoding-agent/tests/coding_agent/plugins/test_core_tools.pycoding-agent/tests/coding_agent/plugins/test_shell_session.pycoding-agent/tests/coding_agent/test_bootstrap.pycoding-agent/tests/ui/test_execution_binding.pycoding-agent/tests/ui/test_session_manager_runtime.py
✅ Files skipped from review due to trivial changes (4)
- coding-agent/src/agentkit/environment/init.py
- coding-agent/src/coding_agent/ui/binding_resolver.py
- coding-agent/tests/ui/test_execution_binding.py
- coding-agent/src/coding_agent/environment/init.py
🚧 Files skipped from review as they are similar to previous changes (4)
- coding-agent/src/coding_agent/environment/local.py
- coding-agent/tests/coding_agent/plugins/test_core_tools.py
- coding-agent/tests/coding_agent/test_bootstrap.py
- coding-agent/tests/coding_agent/environment/test_local_environment.py
delete-cloud
force-pushed
the
codex-environment-local-pr1
branch
from
May 2, 2026 06:56
0571bfa to
cd2de83
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
coding-agent/src/coding_agent/ui/session_manager.py (1)
1299-1302: ⚡ Quick winPrefer
environmentas the single source of workspace context here.At Line 1299-1302 (and similarly in restore/build runtime paths), passing both
workspace_rootandenvironmentduplicates resolution and keeps an unnecessary local-path dependency. Sincecreate_agent/create_child_pipelinealready derive workspace fromenvironment, you can pass onlyenvironmentin these paths.Proposed simplification
pipeline, ctx = self._create_agent_for_session( - workspace_root=self._resolve_workspace_root(session), environment=self._resolve_environment(session), model_override=session.model_name, provider_override=session.provider_name, base_url_override=session.base_url,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@coding-agent/src/coding_agent/ui/session_manager.py` around lines 1299 - 1302, The code is passing both workspace_root and environment into _create_agent_for_session (and similar restore/build runtime call sites) which duplicates workspace resolution; update those calls to stop passing workspace_root and only pass environment (use _resolve_environment(session) as the single source of workspace context), and remove any dependence on _resolve_workspace_root(session) in these call sites; ensure _create_agent_for_session and downstream create_agent/create_child_pipeline still derive workspace from the provided environment and adjust any related parameter lists or docs if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@coding-agent/src/coding_agent/ui/binding_resolver.py`:
- Around line 47-48: The LocalExecutionBinding branch constructs
LocalEnvironment using resolve_workspace_root() which calls Path(...).resolve()
but doesn't expand ~, so paths like "~/repo" can be misresolved; update
resolve_workspace_root (or the call site) to expand user home first (e.g., use
Path(root).expanduser().resolve()) before passing the path into
LocalEnvironment, or alternatively pass the raw binding.root after expanding the
user component so LocalEnvironment always receives an expanded absolute path;
refer to LocalExecutionBinding, LocalEnvironment, and resolve_workspace_root
when making the change.
---
Nitpick comments:
In `@coding-agent/src/coding_agent/ui/session_manager.py`:
- Around line 1299-1302: The code is passing both workspace_root and environment
into _create_agent_for_session (and similar restore/build runtime call sites)
which duplicates workspace resolution; update those calls to stop passing
workspace_root and only pass environment (use _resolve_environment(session) as
the single source of workspace context), and remove any dependence on
_resolve_workspace_root(session) in these call sites; ensure
_create_agent_for_session and downstream create_agent/create_child_pipeline
still derive workspace from the provided environment and adjust any related
parameter lists or docs if needed.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d09dcf59-f53b-4315-9b51-47482bbf4926
📒 Files selected for processing (16)
coding-agent/src/agentkit/environment/__init__.pycoding-agent/src/agentkit/environment/protocols.pycoding-agent/src/coding_agent/app.pycoding-agent/src/coding_agent/environment/__init__.pycoding-agent/src/coding_agent/environment/local.pycoding-agent/src/coding_agent/plugins/core_tools.pycoding-agent/src/coding_agent/plugins/shell_session.pycoding-agent/src/coding_agent/ui/binding_resolver.pycoding-agent/src/coding_agent/ui/session_manager.pycoding-agent/tests/agentkit/environment/test_protocols.pycoding-agent/tests/coding_agent/environment/test_local_environment.pycoding-agent/tests/coding_agent/plugins/test_core_tools.pycoding-agent/tests/coding_agent/plugins/test_shell_session.pycoding-agent/tests/coding_agent/test_bootstrap.pycoding-agent/tests/ui/test_execution_binding.pycoding-agent/tests/ui/test_session_manager_runtime.py
✅ Files skipped from review due to trivial changes (7)
- coding-agent/src/agentkit/environment/protocols.py
- coding-agent/src/agentkit/environment/init.py
- coding-agent/src/coding_agent/environment/init.py
- coding-agent/tests/coding_agent/plugins/test_core_tools.py
- coding-agent/tests/ui/test_session_manager_runtime.py
- coding-agent/src/coding_agent/plugins/core_tools.py
- coding-agent/tests/coding_agent/test_bootstrap.py
🚧 Files skipped from review as they are similar to previous changes (3)
- coding-agent/src/coding_agent/environment/local.py
- coding-agent/tests/coding_agent/plugins/test_shell_session.py
- coding-agent/tests/agentkit/environment/test_protocols.py
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
This was referenced May 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Environmentprotocol and app-levelLocalEnvironmentimplementation.create_agent,SessionManager, andCoreToolsPluginso local file, patch, and shell tools use the environment boundary.workspace_rootconstruction for existing callers.Verification
uv run pytest tests/agentkit tests/coding_agent/environment/test_local_environment.py tests/ui/test_execution_binding.py tests/coding_agent/plugins/test_core_tools.py tests/coding_agent/plugins/test_core_tools_parity.py tests/coding_agent/plugins/test_shell_session.py tests/coding_agent/test_bootstrap.py tests/ui/test_session_manager_runtime.py -quv run basedpyright src/agentkit/environment src/coding_agent/environment src/coding_agent/app.py src/coding_agent/plugins/core_tools.py src/coding_agent/plugins/shell_session.py src/coding_agent/ui/binding_resolver.py src/coding_agent/ui/session_manager.py tests/agentkit/environment/test_protocols.py tests/coding_agent/environment/test_local_environment.py tests/ui/test_execution_binding.py tests/coding_agent/plugins/test_core_tools.py tests/coding_agent/plugins/test_shell_session.py tests/coding_agent/test_bootstrap.py tests/ui/test_session_manager_runtime.py --level erroruv run python -m compileall -q src testsgit diff --checkSummary by CodeRabbit
New Features
Tests