Repository navigation
feat: add remote phase policy wiring - #137
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 (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds ADR-0023 remote phases: request schema ChangesRemote Phases Setup/Agent Policy Implementation
Sequence DiagramsequenceDiagram
participant Client
participant HTTPServer
participant ConfigLoader as _load_cloud_workspace_config
participant Validator as _validate_production_remote_phases
participant Provisioner as _provisioned_execution_binding_from_request
Client->>HTTPServer: POST /sessions with workspace_source (may include setup_commands)
HTTPServer->>ConfigLoader: load cloud_workspace_config (including remote_phases)
ConfigLoader->>Validator: provide remote_phases for production validation
HTTPServer->>Validator: validate workspace_source.setup_commands policy
HTTPServer->>Provisioner: proceed or reject provisioning based on policy/validation
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)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
coding-agent/docs/remote-sandbox-production.md (1)
68-74: ⚡ Quick winConsider clarifying the empty setup commands configuration.
The example configuration shows
commands = [](line 72) combined withallow_request_commands = false(line 74), which means no setup commands will run—neither server-configured nor client-provided.However:
- Lines 101-102 describe the setup phase as "configured for dependency bootstrap with explicit commands"
- Lines 232-233 recommend "server-configured setup commands for production deployments that need repeatable bootstrap behavior"
This creates ambiguity about whether:
- The empty
commands = []is intentional to show a minimal no-setup baseline that users should customize, or- It's a placeholder that should contain example commands
Consider adding a brief inline comment or note in the configuration example to clarify that
commands = []should be populated with project-specific setup commands when dependency installation is required.📝 Proposed clarification
[remote_phases.setup] enabled = true network = "bridge" timeout_seconds = 600 -commands = [] +commands = [] # Populate with setup commands if dependency installation is needed secret_env_allowlist = ["PIP_INDEX_URL", "GITHUB_TOKEN"] allow_request_commands = falseAlso applies to: 101-107, 223-233
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@coding-agent/docs/remote-sandbox-production.md` around lines 68 - 74, Update the remote_sandbox production example to clarify that the empty array in remote_phases.setup (commands = []) is intentional as a minimal/no-setup baseline and should be replaced with project-specific bootstrap commands when dependencies must be installed; add a short inline note adjacent to remote_phases.setup or next to the commands entry (and mirror the same clarification in the other examples mentioned around remote_phases.setup and the sections at lines ~101-107 and ~223-233) so readers know whether to populate commands or rely on allow_request_commands toggles.coding-agent/src/coding_agent/ui/http_server.py (1)
266-270: ⚡ Quick winAdd comment explaining the timeout_seconds validation asymmetry between setup and agent phases.
The setup phase validates
timeout_secondsas required whenenabled=True(lines 266-270), while the agent phase treats it as optional with an explicit None check (lines 295-301). Although this asymmetry is intentional—setup is a bootstrap operation requiring a timeout, while agent timeout is optional—the design decision is not documented in the code. Add a comment explaining why setup demands timeout_seconds while agent makes it optional, referencing ADR-0023 for context.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@coding-agent/src/coding_agent/ui/http_server.py` around lines 266 - 270, Add an inline comment near the call to _require_positive_int_field for setup_phase explaining that setup_phase.timeout_seconds is required when enabled because the bootstrap/setup operation must complete within a bounded time (it cannot rely on optional timeouts), whereas the agent phase intentionally treats timeout_seconds as optional (explicit None allowed) because runtime agents may operate with no enforced timeout; cite ADR-0023 to document this deliberate design asymmetry between setup and agent phases (reference setup_phase, remote_phases.setup, timeout_seconds, and the agent phase handling).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@coding-agent/docs/remote-sandbox-production.md`:
- Around line 68-74: Update the remote_sandbox production example to clarify
that the empty array in remote_phases.setup (commands = []) is intentional as a
minimal/no-setup baseline and should be replaced with project-specific bootstrap
commands when dependencies must be installed; add a short inline note adjacent
to remote_phases.setup or next to the commands entry (and mirror the same
clarification in the other examples mentioned around remote_phases.setup and the
sections at lines ~101-107 and ~223-233) so readers know whether to populate
commands or rely on allow_request_commands toggles.
In `@coding-agent/src/coding_agent/ui/http_server.py`:
- Around line 266-270: Add an inline comment near the call to
_require_positive_int_field for setup_phase explaining that
setup_phase.timeout_seconds is required when enabled because the bootstrap/setup
operation must complete within a bounded time (it cannot rely on optional
timeouts), whereas the agent phase intentionally treats timeout_seconds as
optional (explicit None allowed) because runtime agents may operate with no
enforced timeout; cite ADR-0023 to document this deliberate design asymmetry
between setup and agent phases (reference setup_phase, remote_phases.setup,
timeout_seconds, and the agent phase handling).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a5a3c27c-33f4-4e06-9449-e579c94de31c
📒 Files selected for processing (7)
coding-agent/docs/remote-sandbox-production.mdcoding-agent/src/coding_agent/__main__.pycoding-agent/src/coding_agent/remote/client.pycoding-agent/src/coding_agent/ui/http_server.pycoding-agent/src/coding_agent/ui/schemas.pycoding-agent/tests/cli/test_remote_client.pycoding-agent/tests/ui/test_http_server.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
coding-agent/src/coding_agent/ui/http_server.py (1)
788-795: ⚡ Quick winUse
remote_phases.setup.allow_request_commandsin request policy checks.Line 790 passes
cloud_workspace_config, but the function ignores it and always rejectssetup_commands. Add an explicit policy gate first, then keep the current “not implemented yet” rejection.Proposed fix
def _validate_workspace_source_phase_policy( workspace_source: dict[str, object], cloud_workspace_config: dict[str, Any], ) -> None: setup_commands = workspace_source.get("setup_commands") if setup_commands is None: return + remote_phases = cloud_workspace_config.get("remote_phases") + setup_phase = ( + remote_phases.get("setup") + if isinstance(remote_phases, dict) + else None + ) + allow_request_commands = ( + isinstance(setup_phase, dict) + and setup_phase.get("allow_request_commands") is True + ) + if not allow_request_commands: + raise ValueError( + "workspace_source.setup_commands requires remote_phases.setup.allow_request_commands=true" + ) raise ValueError("setup phase execution is not implemented yet")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@coding-agent/src/coding_agent/ui/http_server.py` around lines 788 - 795, The function _validate_workspace_source_phase_policy currently ignores cloud_workspace_config and always rejects workspace_source["setup_commands"]; update it to first read the request policy flag remote_phases.setup.allow_request_commands from cloud_workspace_config (or its nested policy structure) and only proceed to reject when that flag is False or missing, otherwise allow the existence of setup_commands (but still keep the existing ValueError("setup phase execution is not implemented yet") as the fallback rejection for now); use the function name _validate_workspace_source_phase_policy and the policy key remote_phases.setup.allow_request_commands to locate where to add this policy gate.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@coding-agent/src/coding_agent/ui/http_server.py`:
- Around line 248-311: _validate_production_remote_phases currently allows
setup.enabled=True but with no effective commands (empty/missing "commands" and
"allow_request_commands" false), creating a no-op; update the logic inside the
setup_phase.enabled branch to require a usable command source by ensuring either
"commands" is present and non-empty or "allow_request_commands" is True, e.g.
after calling _require_string_list_field for "commands" check that
setup_phase.get("commands") is a non-empty list (and if not, raise
ValueError("remote_phases.setup must provide non-empty 'commands' or set
'allow_request_commands' to true")), and ensure the existing boolean validation
for allow_request_commands is preserved.
---
Nitpick comments:
In `@coding-agent/src/coding_agent/ui/http_server.py`:
- Around line 788-795: The function _validate_workspace_source_phase_policy
currently ignores cloud_workspace_config and always rejects
workspace_source["setup_commands"]; update it to first read the request policy
flag remote_phases.setup.allow_request_commands from cloud_workspace_config (or
its nested policy structure) and only proceed to reject when that flag is False
or missing, otherwise allow the existence of setup_commands (but still keep the
existing ValueError("setup phase execution is not implemented yet") as the
fallback rejection for now); use the function name
_validate_workspace_source_phase_policy and the policy key
remote_phases.setup.allow_request_commands to locate where to add this policy
gate.
🪄 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: 7f9daf0f-664c-4076-ab67-dc635847524e
📒 Files selected for processing (3)
coding-agent/docs/remote-sandbox-production.mdcoding-agent/src/coding_agent/ui/http_server.pycoding-agent/tests/ui/test_http_server.py
✅ Files skipped from review due to trivial changes (1)
- coding-agent/docs/remote-sandbox-production.md
This was referenced May 12, 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
Validation
Summary by CodeRabbit
Documentation
New Features
Bug Fixes / Validation
Tests