feat(wework): add Codex permission modes - #2044
Open
qdaxb wants to merge 14 commits into
Open
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (20)
🚧 Files skipped from review as they are similar to previous changes (20)
📝 WalkthroughWalkthroughCodex permission modes now persist in preferences and runtime tasks, flow through Codex execution, and appear in desktop and compact chat composers. Codex approval requests are rendered as interactive cards whose responses return to the executor, with supporting settings, localization, tests, E2E scenarios, and documentation. ChangesCodex permissions and approvals
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ChatComposer
participant WorkbenchSession
participant RuntimeTask
participant Codex
participant ApprovalCard
User->>ChatComposer: Select permission mode
ChatComposer->>WorkbenchSession: Update permission mode
WorkbenchSession->>RuntimeTask: Create or update task permissions
RuntimeTask->>Codex: Start turn with permission parameters
Codex->>ApprovalCard: Request approval
ApprovalCard->>RuntimeTask: Submit approval response
RuntimeTask->>Codex: Return approval result
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
qdaxb
marked this pull request as ready for review
July 19, 2026 03:16
…permissions # Conflicts: # executor/src/agents/codex.rs # wework/src/api/local/localServices.ts
![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: 7
🧹 Nitpick comments (2)
wework/src/components/chat/composer/ComposerToolbar.module.css (1)
36-51: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace deprecated
clipproperty withclip-path.The
clipproperty is deprecated in CSS. Both CSS module files use it to accessibly hide text in.modeControl [data-mode-label], which triggers modern linting errors. Useclip-path: inset(50%);instead.
wework/src/components/chat/composer/ComposerToolbar.module.css#L36-L51: replaceclip: rect(0, 0, 0, 0);withclip-path: inset(50%);.wework/src/components/chat/composer/CompactChatComposer.module.css#L32-L43: replaceclip: rect(0, 0, 0, 0);withclip-path: inset(50%);.🤖 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 `@wework/src/components/chat/composer/ComposerToolbar.module.css` around lines 36 - 51, The visually hidden data-mode-label styles in wework/src/components/chat/composer/ComposerToolbar.module.css lines 36-51 and wework/src/components/chat/composer/CompactChatComposer.module.css lines 32-43 must replace the deprecated clip declaration with clip-path: inset(50%); in both .modeControl [data-mode-label] rules, preserving the existing accessible hiding styles.Source: Linters/SAST tools
wework/src/components/chat/ApprovalCard.test.tsx (1)
6-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider covering decline/session buttons and the permission-request payload branch.
Current tests only exercise the accept-once and rule-amendment paths. The decline and accept-for-session buttons, and the
permissionRequestbranch (payload.method === 'item/permissions/requestApproval', which changes the submitted shape to{ permissions, scope }), are untested.🤖 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 `@wework/src/components/chat/ApprovalCard.test.tsx` around lines 6 - 46, The ApprovalCard tests only cover one-time acceptance and execpolicy amendments. Extend the ApprovalCard test suite to cover the decline and accept-for-session buttons, asserting their expected onSubmit decisions, and add a permissionRequest payload case with method item/permissions/requestApproval that verifies submission uses the { permissions, scope } shape.
🤖 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 `@executor/src/agents/codex.rs`:
- Around line 4018-4061: The permission-mode mapping is private and duplicated
by handler.rs. Make CodexPermissionMode and its canonical conversion methods
available as pub(crate), add a stable as_str()/to_string() representation for
RuntimeTaskLink.permission_mode, and update handler.rs functions
request_permission_mode and update_task_permissions to reuse from_request,
permission_profile, approval_policy, approvals_reviewer, and the new string
conversion instead of maintaining separate parsing and mapping logic.
In `@executor/src/runtime_work/handler.rs`:
- Around line 4039-4051: Remove the duplicated parsing logic from
request_permission_mode and reuse CodexPermissionMode::from_request from
codex.rs as the single source of truth. Convert the returned enum to the
required String label using its existing representation, while preserving the
current full_access fallback and accepted mode values.
- Around line 1953-1988: The update_task_permissions flow should parse the
requested mode through CodexPermissionMode, normalize unknown or missing values
to the canonical "full_access" mode before persisting, and derive permissions,
approval policy, and reviewer via that type’s permission_profile(),
approval_policy(), and approvals_reviewer() methods. Expose CodexPermissionMode
at the needed visibility and remove the local mode match so stored and
Codex-applied values cannot diverge.
- Line 1871: Update the permission_mode resolution near request_permission_mode
and the corresponding flow around the additionally affected lines to fall back
to existing_link.permission_mode when the request omits both permission_mode
keys. Preserve explicit request values, and only use "full_access" when neither
the request nor the existing task link provides a setting, mirroring the
workspace_path fallback before update_task persists the value.
In `@wework/src/api/local/localServices.ts`:
- Line 1331: Update the send execution request construction around
permissionMode so omitted normalizedData.permissionMode falls back to the task’s
current permission mode, matching the create-task path. Apply this fallback
consistently to both send branches while preserving an explicitly provided
normalizedData.permissionMode.
In `@wework/src/components/chat/ApprovalCard.tsx`:
- Around line 13-18: Update approvalDescription in ApprovalCard.tsx so its
fallback returns an empty string instead of payload.reason, preventing the
reason from being duplicated in the preformatted description while preserving
command, grantRoot, and cwd precedence.
In `@wework/src/components/layout/useWorkbenchPaneSession.ts`:
- Around line 156-174: Update useWorkbenchPaneSession’s updatePermissionMode to
use a functional setPermissionMode rollback that restores previous only when the
current mode still matches the failed request’s mode, preserving newer
selections during overlapping requests. Initialize the local
useTranslation('common') hook in the hook and replace the catch block’s global
i18n.t call with the returned t function.
---
Nitpick comments:
In `@wework/src/components/chat/ApprovalCard.test.tsx`:
- Around line 6-46: The ApprovalCard tests only cover one-time acceptance and
execpolicy amendments. Extend the ApprovalCard test suite to cover the decline
and accept-for-session buttons, asserting their expected onSubmit decisions, and
add a permissionRequest payload case with method
item/permissions/requestApproval that verifies submission uses the {
permissions, scope } shape.
In `@wework/src/components/chat/composer/ComposerToolbar.module.css`:
- Around line 36-51: The visually hidden data-mode-label styles in
wework/src/components/chat/composer/ComposerToolbar.module.css lines 36-51 and
wework/src/components/chat/composer/CompactChatComposer.module.css lines 32-43
must replace the deprecated clip declaration with clip-path: inset(50%); in both
.modeControl [data-mode-label] rules, preserving the existing accessible hiding
styles.
🪄 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
Run ID: 971ab1c2-7677-4293-a831-8cf5b92da142
📒 Files selected for processing (39)
docs/en/user-guide/chat/codex-permissions.mddocs/zh/user-guide/chat/codex-permissions.mdexecutor/src/agents/codex.rsexecutor/src/runtime_work/events.rsexecutor/src/runtime_work/handler.rsexecutor/src/runtime_work/response.rsexecutor/src/runtime_work/worktrees.rswework/src-tauri/src/lib.rswework/src/api/local/localServices.tswework/src/components/chat/ApprovalCard.test.tsxwework/src/components/chat/ApprovalCard.tsxwework/src/components/chat/ChatInput.test.tsxwework/src/components/chat/ChatInput.tsxwework/src/components/chat/MessageList.tsxwework/src/components/chat/ScrollableMessageArea.tsxwework/src/components/chat/blocks/ToolBlocksDisplay.tsxwework/src/components/chat/composer/CompactChatComposer.module.csswework/src/components/chat/composer/CompactChatComposer.tsxwework/src/components/chat/composer/ComposerToolbar.module.csswework/src/components/chat/composer/ComposerToolbar.tsxwework/src/components/chat/composer/GoalDraftPill.tsxwework/src/components/chat/composer/PermissionModeSelector.test.tsxwework/src/components/chat/composer/PermissionModeSelector.tsxwework/src/components/chat/composer/ProjectChatComposer.tsxwework/src/components/chat/composer/QuickPhraseMenu.tsxwework/src/components/chat/runtimeApproval.tswework/src/components/layout/DesktopWorkbenchMain.tsxwework/src/components/layout/MobileWorkbenchLayout.tsxwework/src/components/layout/useWorkbenchPaneSession.tswework/src/components/settings/GeneralSettingsPage.tsxwework/src/features/workbench/useWorkbenchRuntimeMessaging.tswework/src/features/workbench/workbenchContextTypes.tswework/src/i18n/locales/en/chat.jsonwework/src/i18n/locales/en/common.jsonwework/src/i18n/locales/zh-CN/chat.jsonwework/src/i18n/locales/zh-CN/common.jsonwework/src/tauri/appPreferences.test.tswework/src/tauri/appPreferences.tswework/src/types/api.ts
added 6 commits
July 19, 2026 11:51
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
wework/src/features/workbench/WorkbenchProvider.test.tsx (1)
6368-6440: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest can't distinguish override vs. persisted metadata for
permissionMode.The mocked task's
listRuntimeWorkentry already setspermissionMode: 'request_approval'(Line 6395), and the "open approval runtime a" button also explicitly passespermissionMode: 'request_approval'toopenRuntimeTask(Line 1406). Since both agree, the test can't tell whether the per-session override is actually applied, or whetherpaneSession.permissionMode/thesendRuntimeMessagepayload is simply reading the task's own persisted metadata. Using two different values would make this a meaningful precedence test.🤖 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 `@wework/src/features/workbench/WorkbenchProvider.test.tsx` around lines 6368 - 6440, Make the permission precedence test use conflicting values: set the mocked task metadata in listRuntimeWork to a different permissionMode than the value passed by the “open approval runtime a” probe to openRuntimeTask. Keep the expected follow-up and sendRuntimeMessage assertion focused on the explicit per-session override, proving it takes precedence over persisted task metadata.wework/src/e2e/automation.ts (1)
400-406: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate select-value logic between
fillDesktopControlElementand the newselectValuecase.
fillDesktopControlElementalready knows how to set a<select>'s value via the native prototype setter and dispatchinput+change(Lines 403-406, 457-465). The newselectValuecase reimplements value-setting inline with a different technique (directelement.value =assignment, dispatching onlychange). Two divergent implementations for the same operation will drift over time.♻️ Proposed refactor to reuse `fillDesktopControlElement`
case 'selectValue': { const element = findDesktopControlElements(command.selector)[0] if (!(element instanceof HTMLSelectElement)) { throw new Error(`Selector "${command.selector}" is not a select element`) } - element.value = command.value ?? '' - element.dispatchEvent(new Event('change', { bubbles: true })) + fillDesktopControlElement(element, command.value ?? '') return element.value }Also applies to: 550-558
🤖 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 `@wework/src/e2e/automation.ts` around lines 400 - 406, Update the new selectValue handling to reuse fillDesktopControlElement instead of assigning element.value and dispatching events inline. Preserve the existing select-value behavior, including native setter usage and both input and change events, by routing the operation through fillDesktopControlElement.wework/e2e/desktop/task-flow.e2e.mjs (1)
386-428: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
selectEscalatedShellToolduplicatesselectShellToolalmost entirely.Both functions branch on
exec_commandvsshell_commandand build near-identical tool argument objects; onlysandbox_permissions/justificationand thecommandstring differ. Consider extracting a shared helper parameterized by the extra fields to avoid the two helpers drifting apart as the tool contract evolves.♻️ Proposed refactor
-function selectShellTool(request, workspacePath) { - const command = 'pwd' +function selectShellTool(request, workspacePath, extra = {}) { + const command = extra.command ?? 'pwd' const tools = Array.isArray(request.tools) ? request.tools : [] if (tools.some(tool => tool?.name === 'exec_command')) { return selectTool(request, 'exec_command', { cmd: command, workdir: workspacePath, yield_time_ms: 1000, + ...extra.execCommandFields, }) } if (tools.some(tool => tool?.name === 'shell_command')) { return selectTool(request, 'shell_command', { command, workdir: workspacePath, timeout_ms: 10_000, + ...extra.shellCommandFields, }) } throw new Error('Real Codex did not advertise a supported shell tool') } - -function selectEscalatedShellTool(request, workspacePath) { - ... -} + +function selectEscalatedShellTool(request, workspacePath) { + return selectShellTool(request, workspacePath, { + command: 'ps -Ao pid,command | head -n 3', + execCommandFields: { + sandbox_permissions: 'require_escalated', + justification: 'Allow the desktop E2E host process query?', + }, + shellCommandFields: { + sandbox_permissions: 'require_escalated', + justification: 'Allow the desktop E2E host process query?', + }, + }) +}🤖 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 `@wework/e2e/desktop/task-flow.e2e.mjs` around lines 386 - 428, Refactor selectShellTool and selectEscalatedShellTool to use a shared helper that selects between exec_command and shell_command and builds their common arguments. Parameterize the helper with the command, workspacePath, and optional escalation fields, then preserve the current non-escalated and escalated values and unsupported-tool error behavior.
🤖 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 `@wework/e2e/desktop/task-flow.e2e.mjs`:
- Around line 386-428: Refactor selectShellTool and selectEscalatedShellTool to
use a shared helper that selects between exec_command and shell_command and
builds their common arguments. Parameterize the helper with the command,
workspacePath, and optional escalation fields, then preserve the current
non-escalated and escalated values and unsupported-tool error behavior.
In `@wework/src/e2e/automation.ts`:
- Around line 400-406: Update the new selectValue handling to reuse
fillDesktopControlElement instead of assigning element.value and dispatching
events inline. Preserve the existing select-value behavior, including native
setter usage and both input and change events, by routing the operation through
fillDesktopControlElement.
In `@wework/src/features/workbench/WorkbenchProvider.test.tsx`:
- Around line 6368-6440: Make the permission precedence test use conflicting
values: set the mocked task metadata in listRuntimeWork to a different
permissionMode than the value passed by the “open approval runtime a” probe to
openRuntimeTask. Keep the expected follow-up and sendRuntimeMessage assertion
focused on the explicit per-session override, proving it takes precedence over
persisted task metadata.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 750b5ff0-d681-47c6-8022-b4eb7dc1bb15
📒 Files selected for processing (20)
executor/src/agents/codex.rsexecutor/src/agents/mod.rsexecutor/src/runtime_work/handler.rswework/e2e/desktop/task-flow.e2e.mjswework/src/api/local/localServices.test.tswework/src/api/local/localServices.tswework/src/components/chat/ApprovalCard.test.tsxwework/src/components/chat/ApprovalCard.tsxwework/src/components/chat/blocks/ToolBlocksDisplay.tsxwework/src/components/layout/runtimeTaskSidebarHelpers.tswework/src/components/layout/useWorkbenchPaneSession.tswework/src/e2e/automation.tswework/src/features/workbench/WorkbenchProvider.test.tsxwework/src/features/workbench/useWorkbenchRuntimeMessaging.tswework/src/features/workbench/workbenchReducer.tswework/src/features/workbench/workbenchRuntimeHelpers.tswework/src/i18n/locales/en/chat.jsonwework/src/i18n/locales/en/common.jsonwework/src/i18n/locales/zh-CN/chat.jsonwework/src/i18n/locales/zh-CN/common.json
🚧 Files skipped from review as they are similar to previous changes (11)
- wework/src/i18n/locales/zh-CN/chat.json
- wework/src/components/chat/ApprovalCard.test.tsx
- wework/src/i18n/locales/zh-CN/common.json
- wework/src/i18n/locales/en/chat.json
- wework/src/i18n/locales/en/common.json
- wework/src/api/local/localServices.ts
- wework/src/components/chat/blocks/ToolBlocksDisplay.tsx
- wework/src/components/chat/ApprovalCard.tsx
- executor/src/runtime_work/handler.rs
- executor/src/agents/codex.rs
- wework/src/components/layout/useWorkbenchPaneSession.ts
added 2 commits
July 20, 2026 00:54
…permissions # Conflicts: # executor/src/runtime_work/handler.rs # wework/src/components/chat/composer/ComposerToolbar.tsx # wework/src/components/chat/composer/ProjectChatComposer.tsx
This was referenced Jul 28, 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
1 participant
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.
What changed
Why
Wework previously started Codex sessions with unrestricted access and no approval choice. Users now have an explicit safety/automation tradeoff while preserving full access as the compatibility default.
Validation
Summary by CodeRabbit