Skip to content

revert: remove command sandbox preview - #287

Merged
alwaysmavs merged 1 commit into
mainfrom
codex/remove-command-sandbox
Jul 29, 2026
Merged

revert: remove command sandbox preview#287
alwaysmavs merged 1 commit into
mainfrom
codex/remove-command-sandbox

Conversation

@alwaysmavs

Copy link
Copy Markdown
Contributor

Summary

  • remove the macOS command-sandbox runtime, broker, network proxy, policy layer, package patch, build wiring, and related settings
  • restore direct execution for approved Bash commands so Registry Skills inherit the sidecar's managed Node runtime and OOMOL authentication environment
  • retain the Default Access permission prompts and require an explicit confirmation before enabling Full Access
  • add a regression test covering the combined OO_API_KEY and managed Node environment exposed to Skill commands

Why

The sandbox shell replaced the normal command execution environment and scrubbed or isolated values that external Registry Skills need. In particular, proxy-based Skills such as Public Social Registry execute local shell scripts after resolving credentials; those scripts need both the OOMOL session environment and Wanta's Electron-as-Node runtime. The sandbox boundary made that flow fail even though the desktop application itself was logged in.

This rollback deliberately removes the sandbox boundary instead of adding SDK-specific behavior to Wanta. Approved local commands now inherit the sidecar environment again, including managed OOMOL credentials. Default Access still keeps per-request approval checks, while Full Access remains an explicitly confirmed session-level mode.

Validation

  • pnpm install --frozen-lockfile
  • pnpm format
  • pnpm lint
  • pnpm ts-check
  • pnpm test — 281 files / 2107 tests passed before the final regression test
  • pnpm exec vitest run electron/agent/manager.test.ts — 33 tests passed, including the new credential/runtime regression
  • pnpm build:app
  • git diff --check
  • verified no command-sandbox runtime references remain

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

The command-sandbox implementation and its agent, chat, runtime, build, dependency, settings, and documentation integrations are removed. Permission handling no longer synchronizes sandbox policies, and trusted session copying excludes attachment paths. Full-access mode now requires an acknowledgement dialog before activation. Runtime capabilities and OpenCode configuration no longer expose command-sandbox fields. Localized messages, tests, packaging configuration, and verification scripts are updated accordingly.

Sequence Diagram(s)

sequenceDiagram
  participant ChatComposer
  participant ChatArea
  participant FullAccessConfirmDialog
  ChatComposer->>ChatArea: request full access
  ChatArea->>FullAccessConfirmDialog: open confirmation
  FullAccessConfirmDialog->>ChatArea: confirm acknowledgement
  ChatArea->>ChatComposer: activate full_access
Loading

Possibly related PRs

  • oomol-lab/wanta#281: Introduces the command-sandbox architecture removed by this PR.
  • oomol-lab/wanta#286: Reintroduces command-sandbox modules and related integration overlapping this removal.
🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title is related, but it does not follow the required type(scope): subject English format. Change it to an English title like revert(command-sandbox): remove command sandbox preview.
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The description covers the summary, rationale, and validation, with only the template's Safety and Compatibility checklist not filled explicitly.
✨ Finishing Touches
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch codex/remove-command-sandbox

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/routes/Chat/index.tsx (1)

317-339: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider test coverage for the full-access confirmation gate.

Both sites implement the same new security-relevant flow (confirmation required before enabling full access) with no accompanying test.

  • src/routes/Chat/index.tsx#L317-L339: add a test asserting onPermissionModeChange("full_access") is only called after confirmFullAccess runs (i.e., requestFullAccess alone must not switch the mode).
  • src/routes/Chat/FullAccessConfirmDialog.tsx#L1-L70: add a component test asserting the confirm button is disabled until the checkbox is checked, and that onConfirm fires only then.
🤖 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/routes/Chat/index.tsx` around lines 317 - 339, The full-access
confirmation gate lacks tests covering both the request flow and dialog
behavior. In src/routes/Chat/index.tsx lines 317-339, add a test around
requestFullAccess and confirmFullAccess verifying that requesting alone does not
call onPermissionModeChange and confirmation calls it with "full_access"; in
src/routes/Chat/FullAccessConfirmDialog.tsx lines 1-70, add a component test
verifying the confirm button remains disabled until the checkbox is checked and
onConfirm fires only after confirmation.
🤖 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 `@src/routes/Chat/index.tsx`:
- Around line 317-339: The full-access confirmation gate lacks tests covering
both the request flow and dialog behavior. In src/routes/Chat/index.tsx lines
317-339, add a test around requestFullAccess and confirmFullAccess verifying
that requesting alone does not call onPermissionModeChange and confirmation
calls it with "full_access"; in src/routes/Chat/FullAccessConfirmDialog.tsx
lines 1-70, add a component test verifying the confirm button remains disabled
until the checkbox is checked and onConfirm fires only after confirmation.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 79dffc80-6b5b-4357-b773-d3a9b830b352

📥 Commits

Reviewing files that changed from the base of the PR and between c95b509 and 6d5f43b.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (43)
  • AGENTS.md
  • docs/ai/README.md
  • docs/ai/command-sandbox-implementation.md
  • docs/architecture.md
  • docs/conventions.md
  • docs/key-decisions.md
  • electron-builder.ts
  • electron/agent/command-sandbox-shell.ts
  • electron/agent/command-sandbox/broker.test.ts
  • electron/agent/command-sandbox/broker.ts
  • electron/agent/command-sandbox/network-address.test.ts
  • electron/agent/command-sandbox/network-address.ts
  • electron/agent/command-sandbox/network-proxy.test.ts
  • electron/agent/command-sandbox/network-proxy.ts
  • electron/agent/command-sandbox/plugin.ts
  • electron/agent/command-sandbox/policy-reviewer.test.ts
  • electron/agent/command-sandbox/policy-reviewer.ts
  • electron/agent/command-sandbox/policy.test.ts
  • electron/agent/command-sandbox/policy.ts
  • electron/agent/command-sandbox/runtime.ts
  • electron/agent/command-sandbox/shell-bin.test.ts
  • electron/agent/command-sandbox/shell-bin.ts
  • electron/agent/config.ts
  • electron/agent/manager.test.ts
  • electron/agent/manager.ts
  • electron/chat/node.test.ts
  • electron/chat/node.ts
  • electron/chat/trusted-local-access.test.ts
  • electron/chat/trusted-local-access.ts
  • electron/main.ts
  • electron/runtime/common.test.ts
  • electron/runtime/common.ts
  • package.json
  • patches/@vscode__sandbox-runtime@0.0.1.patch
  • scripts/verify-command-sandbox.ts
  • src/i18n/app-messages.en.ts
  • src/i18n/app-messages.zh.ts
  • src/i18n/i18n.test.ts
  • src/routes/Chat/FullAccessConfirmDialog.tsx
  • src/routes/Chat/PermissionModePicker.tsx
  • src/routes/Chat/index.tsx
  • src/routes/Settings/index.tsx
  • vite.config.ts
💤 Files with no reviewable changes (28)
  • electron/agent/command-sandbox/shell-bin.test.ts
  • docs/ai/command-sandbox-implementation.md
  • electron/agent/command-sandbox-shell.ts
  • electron/agent/command-sandbox/plugin.ts
  • electron/agent/command-sandbox/broker.test.ts
  • electron/agent/command-sandbox/policy.test.ts
  • electron/agent/command-sandbox/network-address.test.ts
  • scripts/verify-command-sandbox.ts
  • electron/agent/command-sandbox/network-proxy.test.ts
  • electron/agent/command-sandbox/runtime.ts
  • docs/ai/README.md
  • electron/agent/command-sandbox/shell-bin.ts
  • electron/agent/command-sandbox/broker.ts
  • AGENTS.md
  • electron/agent/command-sandbox/policy-reviewer.test.ts
  • electron/agent/command-sandbox/policy.ts
  • vite.config.ts
  • electron/agent/command-sandbox/network-proxy.ts
  • electron/agent/command-sandbox/policy-reviewer.ts
  • electron/agent/command-sandbox/network-address.ts
  • electron/main.ts
  • electron/agent/config.ts
  • patches/@vscode__sandbox-runtime@0.0.1.patch
  • electron/runtime/common.test.ts
  • electron/chat/trusted-local-access.test.ts
  • src/routes/Settings/index.tsx
  • electron/runtime/common.ts
  • electron/chat/node.test.ts

@alwaysmavs
alwaysmavs marked this pull request as ready for review July 29, 2026 05:42
@alwaysmavs
alwaysmavs merged commit fc6d49a into main Jul 29, 2026
3 checks passed
@alwaysmavs
alwaysmavs deleted the codex/remove-command-sandbox branch July 29, 2026 05:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant