fix(agent-sdk): stop pausing file_editor for workspace access#1020
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 Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThe agent now automatically allows ChangesWorkspace Access Auto-Allow Behavior
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
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. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/agent-sdk/src/sdk/runtime/Agent.ts (1)
823-835:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDefer
allowPath()until after confirmation is approved.
workspace.allowPath()runs before the confirmation gate. With a confirming policy (e.g.,always), a user can reject the tool call but the external path remains allowlisted, which widens access without approval.Suggested fix
const workspaceAccess = this.getRequiredWorkspaceAccess(toolCall.function.name, actionArgs); - if (workspaceAccess) { - // File-editor paths outside the current workspace are still real SDK use cases - // (for example, SmolPaws skills under ~/.openhands). Auto-allow the exact path - // instead of forcing a separate workspace-access confirmation stop here. - for (const p of workspaceAccess.paths) { - this.workspace.allowPath(p); - } - } - if (this.requiresConfirmation(recordedAction)) { - this.pauseForConfirmation({ toolCall, actionEvent: recordedAction, args: actionArgs }); + this.pauseForConfirmation( + { toolCall, actionEvent: recordedAction, args: actionArgs }, + workspaceAccess, + ); return 'paused'; } + if (workspaceAccess) { + // Auto-allow when no confirmation pause is required. + for (const p of workspaceAccess.paths) { + this.workspace.allowPath(p); + } + }🤖 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 `@packages/agent-sdk/src/sdk/runtime/Agent.ts` around lines 823 - 835, The code currently calls workspace.allowPath(...) inside the block after getRequiredWorkspaceAccess(...) before checking requiresConfirmation(...), which allowlists external paths even when the user later rejects the confirmation; move the allowPath calls so they only run after confirmation is granted: remove or skip the for-loop that calls this.workspace.allowPath(p) prior to the requiresConfirmation check and instead invoke those same allowPath calls in the confirmation-success path (e.g., after pauseForConfirmation returns approval or in the continuation that handles confirmed tool execution), preserving use of the same symbols (getRequiredWorkspaceAccess, workspace.allowPath, requiresConfirmation, pauseForConfirmation, recordedAction, toolCall, actionArgs) so no allowlisting happens while paused.
🤖 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.
Outside diff comments:
In `@packages/agent-sdk/src/sdk/runtime/Agent.ts`:
- Around line 823-835: The code currently calls workspace.allowPath(...) inside
the block after getRequiredWorkspaceAccess(...) before checking
requiresConfirmation(...), which allowlists external paths even when the user
later rejects the confirmation; move the allowPath calls so they only run after
confirmation is granted: remove or skip the for-loop that calls
this.workspace.allowPath(p) prior to the requiresConfirmation check and instead
invoke those same allowPath calls in the confirmation-success path (e.g., after
pauseForConfirmation returns approval or in the continuation that handles
confirmed tool execution), preserving use of the same symbols
(getRequiredWorkspaceAccess, workspace.allowPath, requiresConfirmation,
pauseForConfirmation, recordedAction, toolCall, actionArgs) so no allowlisting
happens while paused.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3e2e4981-744f-46c6-a856-10966f798fd6
📒 Files selected for processing (3)
packages/agent-sdk/src/sdk/__tests__/agent.loop.test.tspackages/agent-sdk/src/sdk/__tests__/persistence.test.tspackages/agent-sdk/src/sdk/runtime/Agent.ts
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
🔧 VSCode Extension Built Successfully• File: openhands-tab-0.9.4.vsix (548 KB) To install:
Built with Node 22. Commit bfcdf82. |
2 participants
Summary
file_editoreven when the user choseNeverConfirmDesired Behavior
There should be one policy for whether the agent pauses for approval, and it is chosen by the user via Confirmation Policy.
NeverConfirm: never ask for confirmationThe SDK should not have an overlapping workspace-access confirmation policy trying to answer the same question differently.
Testing
npm test -- --run src/sdk/__tests__/agent.loop.test.ts src/sdk/__tests__/persistence.test.ts src/workspace/__tests__/local.workspace.test.ts src/workspace/__tests__/local.workspace.vscodeRemoteRoots.test.ts src/workspace/__tests__/remote.workspace.test.tsnpm run build