Repository navigation
feat: add remote runtime profiles - #135
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR implements a server-configured Docker runtime profile selector threaded from CLI ( ChangesRuntime Profile Selection Feature
Sequence DiagramsequenceDiagram
participant User as CLI User
participant Client as Remote Client
participant Server as HTTP Server
participant Provider as Docker Provider
participant Docker as Docker Engine
User->>Client: remote run --runtime universal
Client->>Server: POST /sessions (workspace_source.runtime_profile=universal)
Server->>Server: Load runtime_profiles config
Server->>Provider: Provision with runtime_profile=universal
Provider->>Provider: Lookup profile & validate provider=="docker"
Provider->>Provider: Merge profile overrides into effective config
Provider->>Docker: docker run --image [profile.image] ...[overrides]
Docker->>Docker: Create & run workspace container
Provider->>Server: Return CloudWorkspaceBinding (runtime_profile)
Server->>Client: Session response with binding
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 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 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/environment/docker_workspace_provider.py`:
- Around line 687-690: The loop that copies keys from profile_config into
effective currently allows unsafe overrides (network, exec_user, pids_limit) via
the tuple ("image","network","cpus","memory","pids_limit","exec_user"); update
this so production sandbox constraints cannot be weakened by a profile: either
(A) restrict the allowed override keys to a safe whitelist (e.g., remove
"network", "exec_user", "pids_limit") when running in production/sandbox mode,
or (B) perform explicit validation when copying those keys by comparing
profile_config["network"/"exec_user"/"pids_limit"] against the validated
cloud_workspace baseline and reject or normalize any weaker values; implement
this logic around the existing effective/profile_config copy in
docker_workspace_provider.py so the effective dict cannot be weakened by
profiles.
🪄 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: 6946070a-0956-4f1e-952d-43c81a08b5a4
📒 Files selected for processing (13)
coding-agent/docker/runtime/universal.Dockerfilecoding-agent/docs/adr/0022-runtime-profiles-and-sandbox-policy.mdcoding-agent/docs/remote-sandbox-production.mdcoding-agent/src/coding_agent/__main__.pycoding-agent/src/coding_agent/environment/docker_workspace_provider.pycoding-agent/src/coding_agent/remote/client.pycoding-agent/src/coding_agent/ui/execution_binding.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/coding_agent/environment/test_docker_workspace_provider.pycoding-agent/tests/ui/test_execution_binding.pycoding-agent/tests/ui/test_http_server.py
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
coding-agent/tests/coding_agent/environment/test_docker_workspace_provider.py (1)
910-938: ⚡ Quick winClarify test intent by removing redundant configuration.
The test sets both
default_runtime_profile="missing"(line 932) and explicitly passesruntime_profile="missing"(line 936). This makes the test intent ambiguous—is it validating the explicitruntime_profileparameter or thedefault_runtime_profileconfiguration?Consider changing line 932 to reference a valid profile name (e.g.,
"python-basic") so the test clearly validates that an unknown explicitly-requested profile is rejected.♻️ Proposed fix to clarify test intent
_docker_config( { "workspace_root": str(workspace_root), - "default_runtime_profile": "missing", + "default_runtime_profile": "python-basic", "runtime_profiles": {}, } ),🤖 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/tests/coding_agent/environment/test_docker_workspace_provider.py` around lines 910 - 938, The test test_docker_workspace_provider_rejects_unknown_runtime_profile is ambiguous because it sets default_runtime_profile to "missing" while also passing runtime_profile="missing"; update the fixture created by _docker_config so default_runtime_profile is a valid profile (e.g., "python-basic") and keep the explicit runtime_profile="missing" in the provision_cloud_binding_from_config call so the test clearly asserts that an explicitly-requested unknown runtime_profile is rejected by provision_cloud_binding_from_config.
🤖 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/environment/docker_workspace_provider.py`:
- Around line 678-680: When runtime_profile is None the code currently returns
the base config (if runtime_profile is None: return config) which causes
downstream fallback to _DEFAULT_DOCKER_IMAGE and image_allowlist validation
failures; instead resolve and apply the configured default by reading
cloud_workspace.default_runtime_profile and using it to select/merge the
appropriate entry from config.get("runtime_profiles") before returning. Update
the logic in docker_workspace_provider.py (the block that defines
runtime_profiles and handles runtime_profile) to: if runtime_profile is None,
read cloud_workspace.default_runtime_profile, look up that key in
runtime_profiles, and return the merged profile/config (or raise a clear error
if the default key is missing) so subsequent checks (e.g., image_allowlist
validation around _DEFAULT_DOCKER_IMAGE) see the correct resolved image. Ensure
the same fix is applied to the other similar places mentioned (around lines
591-594 and 639-642).
In
`@coding-agent/tests/coding_agent/environment/test_docker_workspace_provider.py`:
- Around line 761-834: The test
test_docker_workspace_provider_applies_requested_runtime_profile currently
passes because _effective_docker_workspace_config intentionally only merges
image, cpus, and memory from a runtime profile; update the test to include a
short comment above the provision_cloud_binding_from_config call (or near the
runtime_profiles dict) stating that runtime profiles only override image, cpus,
and memory and do not affect network, pids_limit, or exec_user to make this
explicit for future readers; alternatively, add a clarifying code comment inside
_effective_docker_workspace_config describing this deliberate limitation and why
those fields are excluded.
---
Nitpick comments:
In
`@coding-agent/tests/coding_agent/environment/test_docker_workspace_provider.py`:
- Around line 910-938: The test
test_docker_workspace_provider_rejects_unknown_runtime_profile is ambiguous
because it sets default_runtime_profile to "missing" while also passing
runtime_profile="missing"; update the fixture created by _docker_config so
default_runtime_profile is a valid profile (e.g., "python-basic") and keep the
explicit runtime_profile="missing" in the provision_cloud_binding_from_config
call so the test clearly asserts that an explicitly-requested unknown
runtime_profile is rejected by provision_cloud_binding_from_config.
🪄 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: 2258c688-7101-4737-97b1-a68d07416be7
📒 Files selected for processing (7)
coding-agent/docs/adr/0022-runtime-profiles-and-sandbox-policy.mdcoding-agent/docs/remote-sandbox-production.mdcoding-agent/src/coding_agent/environment/docker_workspace_provider.pycoding-agent/src/coding_agent/ui/http_server.pycoding-agent/tests/coding_agent/environment/test_docker_workspace_provider.pycoding-agent/tests/coding_agent/environment/test_docker_workspace_provider_transfer.pycoding-agent/tests/ui/test_http_server.py
✅ Files skipped from review due to trivial changes (1)
- coding-agent/docs/remote-sandbox-production.md
Summary
Validation
Summary by CodeRabbit
New Features
Documentation