feat(settings): add UI for sandbox execution backend configuration#3265
Conversation
…inyhumansai#3263) Add a Settings panel under Agents for configuring sandbox execution backends (Docker image, memory/CPU limits, backend selection) and viewing sandbox status (Docker availability, detected OS backend). Rust: config_get_sandbox_settings / config_update_sandbox_settings RPCs in the config controller, reading from config.sandbox + config.runtime.docker. Frontend: SandboxSettingsPanel with auto-persist on change, backend dropdown, Docker settings inputs, env passthrough display, status badges. 12 Vitest tests covering load, render, persist, validation, and error states. i18n keys added to en.ts and all 13 locale files.
|
Warning Review limit reached
More reviews will be available in 33 minutes and 30 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (17)
📝 WalkthroughWalkthroughThis PR implements a complete sandbox settings configuration UI, adding a backend RPC API for managing sandbox execution (Docker image, memory/CPU limits, env passthrough, backend selection), TypeScript types and wrappers, a React settings panel with load/persist flow, navigation integration into Settings, and translations across 14 locales. ChangesSandbox Settings Configuration Feature
Sequence Diagram(s)sequenceDiagram
participant React Panel
participant Tauri Bridge
participant Rust RPC Handler
participant Config Ops
participant Disk
participant Docker CLI
rect rgba(200, 220, 255, 0.5)
note over React Panel,Docker CLI: Load sandbox settings on mount
React Panel->>Tauri Bridge: openhumanGetSandboxSettings()
Tauri Bridge->>Rust RPC Handler: call openhuman.config_get_sandbox_settings
Rust RPC Handler->>Config Ops: get_sandbox_settings()
Config Ops->>Disk: load config file
Config Ops->>Docker CLI: docker info (availability check)
Config Ops->>Config Ops: detect_os_sandbox_backend (Linux/macOS/Windows)
Config Ops-->>Rust RPC Handler: merged sandbox+docker JSON
Rust RPC Handler-->>Tauri Bridge: CommandResponse<SandboxSettings>
Tauri Bridge-->>React Panel: settings state, isLoading=false
end
rect rgba(200, 255, 220, 0.5)
note over React Panel,Docker CLI: User updates backend selection
React Panel->>React Panel: onChange backend value
React Panel->>Tauri Bridge: openhumanUpdateSandboxSettings({backend: "docker"})
Tauri Bridge->>Rust RPC Handler: call openhuman.config_update_sandbox_settings
Rust RPC Handler->>Config Ops: load_and_apply_sandbox_settings(patch)
Config Ops->>Config Ops: apply_sandbox_settings: map "docker" to SandboxBackend enum
Config Ops->>Disk: save updated config
Config Ops-->>Rust RPC Handler: post-save config snapshot JSON
Rust RPC Handler-->>Tauri Bridge: CommandResponse<ConfigSnapshot>
Tauri Bridge-->>React Panel: update resolved, isSaving=false, show savedNote
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Add config_get_sandbox_settings and config_update_sandbox_settings to the expected controller schema list in worker_a_controller_schemas_are_fully_exposed.
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/openhuman/sandbox/ops.rs (1)
28-44:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftThe new sandbox settings are never consulted during policy resolution.
apply_sandbox_settings()writesconfig.sandbox.backendandconfig.sandbox.enabledinsrc/openhuman/config/ops.rsLines 2031-2049, but this resolver still derives the backend solely fromruntime_config.kindandis_remote_session. That makes the new Settings UI effectively a no-op for actual execution, especially forlandlock,firejail,bubblewrap, andnone, and it also ignores the enable/disable toggle.🤖 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 `@src/openhuman/sandbox/ops.rs` around lines 28 - 44, resolve_sandbox_policy currently ignores the new UI-written sandbox settings and derives backend only from runtime_config.kind and is_remote_session; update resolve_sandbox_policy to read and honor runtime_config.sandbox.enabled and runtime_config.sandbox.backend (as set by apply_sandbox_settings) and use those values to determine SandboxBackendKind (mapping values like "landlock", "firejail", "bubblewrap", "none" to the corresponding SandboxBackendKind) and to disable sandboxing when enabled==false, falling back to the existing runtime_config.kind/is_remote_session logic only when sandbox settings are absent or unspecified; locate and change the logic in resolve_sandbox_policy so it queries config.sandbox.backend and config.sandbox.enabled (or runtime_config.sandbox.*) before deciding the backend.tests/config_auth_app_state_connectivity_e2e.rs (1)
2799-2818:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd real JSON-RPC coverage for the sandbox methods.
This only proves the methods are registered in
/schema. It still won't catch param/response drift for the new sandbox settings flow, and the UI depends on the get/update payload shape over the live/rpcsurface.A small follow-up test that calls both methods and asserts the
enabled/backend/docker_*round-trip would close that gap.Based on learnings: "Add/update E2E coverage for user-visible flows and cross-process integration behavior" and "Add or extend JSON-RPC integration-style tests that call the real HTTP JSON-RPC surface ... so methods, params, and outcomes match what the UI will call."
🤖 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 `@tests/config_auth_app_state_connectivity_e2e.rs` around lines 2799 - 2818, The sandbox methods are only being verified as present in /schema, so the live JSON-RPC payload shape is still untested. Add an E2E-style JSON-RPC call path in tests/config_auth_app_state_connectivity_e2e.rs that exercises the real /rpc surface through the sandbox get/update methods, using the existing openhuman.config_get_sandbox_settings and openhuman.config_update_sandbox_settings symbols. Assert a full round-trip for the user-visible fields, especially enabled, backend, and docker_* values, so param/response drift is caught.
🤖 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 `@app/src/components/settings/panels/SandboxSettingsPanel.tsx`:
- Around line 56-57: The component initializes memory/cpu inputs by mapping null
to '' (via setMemoryLimitMb/setCpuLimit) but the blur handlers only send
positive numbers, so clearing the input in the UI doesn’t persist (backend stays
unchanged); update the blur/update handlers in SandboxSettingsPanel (and the
analogous handlers around the 106-118 block) to treat an empty string as null
and send null to the save/update API instead of omitting the field or sending 0,
ensuring the persisted value is cleared; add regression tests that set a limit,
clear the input (simulate blur), and assert the backend receives null for both
docker_memory_limit_mb and docker_cpu_limit.
- Line 267: In SandboxSettingsPanel, replace the hard-coded visible literals
with translations via useT(): import and call useT() in the component and
replace the Docker image placeholder string "alpine:3.20" (placeholder prop on
the image input) and the memory unit label "MB" (rendered next to the memory
input) with calls to t('...') so both the placeholder and unit come from the
i18n keys; ensure keys are descriptive (e.g., sandbox.imagePlaceholder,
sandbox.memoryUnit) and used where the placeholder prop and the MB label are
rendered.
- Around line 26-27: The effect is calling setIsLoading synchronously; instead
initialize isLoading from isTauri() and remove the synchronous set inside
useEffect: change the useState line to const [isLoading, setIsLoading] =
useState<boolean>(isTauri()); then in the useEffect for SandboxSettingsPanel
remove the branch that does if (!isTauri()) { setIsLoading(false); return; } so
the effect only contains asynchronous work and early-returns without setting
state synchronously.
In `@app/src/lib/i18n/de.ts`:
- Around line 3787-3795: Update the two German translation strings: change the
value of 'settings.sandbox.backend.none' from "Keines (kein Sandbox)" to "Keine
(ohne Sandbox)" and change the value of 'settings.sandbox.envPassthroughDesc'
from "Umgebungsvariablen, die in den Sandbox weitergeleitet werden." to
"Umgebungsvariablen, die in die Sandbox weitergeleitet werden."; ensure you edit
those exact keys in app/src/lib/i18n/de.ts so the wording is idiomatic and
grammatically correct.
In `@app/src/lib/i18n/pt.ts`:
- Around line 3732-3764: The new sandbox i18n keys under settings.sandbox mix
pt-PT phrasing into a pt-BR file; update the values for keys like
'settings.sandbox.desktopOnly', 'settings.sandbox.loading',
'settings.sandbox.saving', 'settings.sandbox.saved',
'settings.sandbox.available', 'settings.sandbox.unavailable',
'settings.sandbox.detectedBackend', 'settings.sandbox.enableDesc',
'settings.sandbox.backendDesc', 'settings.sandbox.envPassthroughDesc',
'settings.sandbox.noEnvVars' and related backend labels to use consistent pt-BR
wording (e.g., "aplicativo para desktop" or "aplicativo desktop" instead of
"aplicação de ambiente de trabalho", "Carregando…" instead of "A carregar…",
"Salvando…" instead of "A guardar…", "Salvar — aplica-se a novas sessões do
agente." instead of "Guardado — aplica-se a novas sessões do agente.", and other
phrasing adjustments) so all sandbox strings match the existing pt-BR variant.
In `@app/src/utils/tauriCommands/config.ts`:
- Around line 477-485: Update the partial update type to allow clearing limits
by making docker_memory_limit_mb and docker_cpu_limit nullable (change their
types from number to number | null) in the SandboxSettingsUpdate interface; this
lets callers (including the patch type derived by openhumanUpdateSandboxSettings
/ SandboxSettingsPanel) send null to remove saved limits. Ensure both
docker_memory_limit_mb and docker_cpu_limit fields are updated to accept null
values so the backend receives explicit clears.
In `@src/openhuman/config/ops.rs`:
- Around line 2092-2104: The is_docker_available() probe can block; wrap the
await of the docker Command::status() in a tokio::time::timeout (e.g., a few
seconds) and treat a timeout as "false" (unavailable). Specifically, in
is_docker_available() use tokio::time::timeout(Duration::from_secs(...),
child.status().await) and return false if the timeout or any Err occurs,
otherwise return status.success(); this ensures get_sandbox_settings() won't
hang waiting on the daemon.
- Around line 2050-2052: The current assignment writes empty strings into
config.runtime.docker.image when update.docker_image is Some(""), causing
failures; change the logic around update.docker_image so you trim the string and
only persist it if not empty—i.e. replace the unconditional clone with something
like: if let Some(ref image) = update.docker_image { let image = image.trim();
if !image.is_empty() { config.runtime.docker.image = image.to_string(); } } (or,
alternatively, if your codebase provides a canonical default image getter, set
config.runtime.docker.image = get_default_docker_image() when the trimmed value
is empty). Ensure you reference update.docker_image and
config.runtime.docker.image when making the change.
---
Outside diff comments:
In `@src/openhuman/sandbox/ops.rs`:
- Around line 28-44: resolve_sandbox_policy currently ignores the new UI-written
sandbox settings and derives backend only from runtime_config.kind and
is_remote_session; update resolve_sandbox_policy to read and honor
runtime_config.sandbox.enabled and runtime_config.sandbox.backend (as set by
apply_sandbox_settings) and use those values to determine SandboxBackendKind
(mapping values like "landlock", "firejail", "bubblewrap", "none" to the
corresponding SandboxBackendKind) and to disable sandboxing when enabled==false,
falling back to the existing runtime_config.kind/is_remote_session logic only
when sandbox settings are absent or unspecified; locate and change the logic in
resolve_sandbox_policy so it queries config.sandbox.backend and
config.sandbox.enabled (or runtime_config.sandbox.*) before deciding the
backend.
In `@tests/config_auth_app_state_connectivity_e2e.rs`:
- Around line 2799-2818: The sandbox methods are only being verified as present
in /schema, so the live JSON-RPC payload shape is still untested. Add an
E2E-style JSON-RPC call path in tests/config_auth_app_state_connectivity_e2e.rs
that exercises the real /rpc surface through the sandbox get/update methods,
using the existing openhuman.config_get_sandbox_settings and
openhuman.config_update_sandbox_settings symbols. Assert a full round-trip for
the user-visible fields, especially enabled, backend, and docker_* values, so
param/response drift is caught.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 76736e49-692b-47ec-9248-a4fead1d406f
📒 Files selected for processing (25)
app/src/components/settings/hooks/useSettingsNavigation.tsapp/src/components/settings/panels/SandboxSettingsPanel.tsxapp/src/components/settings/panels/__tests__/SandboxSettingsPanel.test.tsxapp/src/lib/i18n/ar.tsapp/src/lib/i18n/bn.tsapp/src/lib/i18n/de.tsapp/src/lib/i18n/en.tsapp/src/lib/i18n/es.tsapp/src/lib/i18n/fr.tsapp/src/lib/i18n/hi.tsapp/src/lib/i18n/id.tsapp/src/lib/i18n/it.tsapp/src/lib/i18n/ko.tsapp/src/lib/i18n/pl.tsapp/src/lib/i18n/pt.tsapp/src/lib/i18n/ru.tsapp/src/lib/i18n/zh-CN.tsapp/src/pages/Settings.tsxapp/src/services/rpcMethods.tsapp/src/utils/tauriCommands/config.tssrc/openhuman/config/ops.rssrc/openhuman/config/schema/types.rssrc/openhuman/config/schemas.rssrc/openhuman/sandbox/ops.rstests/config_auth_app_state_connectivity_e2e.rs
- Reject blank Docker images in update_sandbox_settings (Rust) - Add 5s timeout to Docker availability probe - Allow null for docker_memory_limit_mb/docker_cpu_limit to clear limits - Handle empty field clearing in memory/CPU blur handlers - Move hard-coded placeholder/MB to i18n (dockerImagePlaceholder, memoryUnit) - Fix synchronous setState in useEffect (init isLoading from isTauri()) - Fix German: "Keines (kein Sandbox)" → "Keine (ohne Sandbox)", article fix - Fix Portuguese: pt-PT → pt-BR consistency (carregar→carregando, etc.)
Summary
Problem
The sandbox backend configuration (Docker image, memory/CPU limits, backend selection) introduced in #3249 / PR #3261 is currently only configurable via TOML config files. There is no UI surface for operators or users to view or change sandbox settings.
Solution
Rust (core):
config_get_sandbox_settingsandconfig_update_sandbox_settingsRPCs in the config controllersandboxfield to the top-levelConfigstruct using the existingSandboxConfigtypeFrontend (app):
SandboxSettingsPanelcomponent with auto-persist pattern (no Save button, matches AgentAccessPanel)tauriCommands/config.ts/settings/sandbox-settings, navigation item under Agents sectionen.tsand all 13 locale files with real translationsSubmission Checklist
Closes #3263Impact
isTauri(), shows "desktop only" message otherwisedocker infoasync on settings load (no perf impact on main thread)/sys/kernel/security/landlockon Linux, hardcoded on macOS/Windows)sandboxconfig field defaults cleanly viaSandboxConfig::default()Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
issue/3263-add-ui-for-sandbox-execution-backend-conValidation Run
pnpm typecheck— passesSandboxSettingsPanel.test.tsx)cargo fmt && cargo check)Validation Blocked
Behavior Changes
Parity Contract
Duplicate / Superseded PR Handling
Summary by CodeRabbit
Release Notes