feat(claude): allow enabling skip-permissions toggle on running tasks#1685
feat(claude): allow enabling skip-permissions toggle on running tasks#1685Fizza-Mukhtar wants to merge 2 commits intogeneralaction:mainfrom
Conversation
|
@Fizza-Mukhtar is attempting to deploy a commit to the General Action Team on Vercel. A member of the Team first needs to authorize it. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughReplaced static auto-approve indicator with an interactive toggle in ChatInterface that stores an Changes
Sequence DiagramsequenceDiagram
participant User
participant ChatInterface
participant Database as rpc.db
participant ElectronAPI as window.electronAPI
participant Terminal
User->>ChatInterface: click auto-approve toggle
ChatInterface->>Database: saveTask(updated task.metadata.autoApprove)
Database-->>ChatInterface: success / error
alt success and enabling
ChatInterface->>ElectronAPI: ptyKill(terminalId)
ElectronAPI->>Terminal: kill terminal
Terminal-->>ElectronAPI: terminated
ElectronAPI-->>ChatInterface: success
else error
ChatInterface-->>User: revert override, show destructive toast
end
ChatInterface->>User: update button state & tooltip
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/renderer/components/ChatInterface.tsx (2)
1206-1238: Consider addingaria-pressedfor accessibility.For toggle buttons,
aria-pressedcommunicates the current state to screen readers.Add aria-pressed attribute
<button type="button" + aria-pressed={autoApproveEnabled} onClick={() => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/ChatInterface.tsx` around lines 1206 - 1238, The toggle button rendering in the ChatInterface component doesn't expose its pressed state to assistive tech; update the button element that uses autoApproveEnabled (the onClick handler that calls rpc.db.saveTask and window.electronAPI.ptyKill(terminalId)) to include an aria-pressed attribute set to the boolean autoApproveEnabled so screen readers can announce the toggle state (e.g., aria-pressed={autoApproveEnabled}), ensuring the rest of the click behavior remains unchanged.
1217-1221: Comment may be misleading given the current implementation.The comment suggests the session manager will restart "with autoApprove: true," but per the code flow,
TerminalPanepassesautoApprove={autoApproveEnabled}which is derived from thetaskprop. The session manager doesn't re-read from the database — it uses the value passed by the renderer.Consider updating the comment to reflect the actual mechanism, or ensuring the parent refreshes the
taskprop before respawn occurs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/ChatInterface.tsx` around lines 1217 - 1221, The comment on the ptyKill call is misleading: it claims the session manager will restart "with autoApprove: true" but the restart uses the prop passed by the renderer, not a DB re-read. Update the comment in ChatInterface.tsx near window.electronAPI.ptyKill(terminalId) to state that the next spawn uses the current autoApprove prop passed from TerminalPane (autoApproveEnabled derived from task) or, alternatively, ensure the parent refreshes/recomputes the task prop (so TerminalPane receives the updated autoApproveEnabled) before calling ptyKill; reference TerminalPane, autoApproveEnabled, task prop, window.electronAPI.ptyKill, and terminalId when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/components/ChatInterface.tsx`:
- Around line 1208-1221: The click handler races killing the PTY before the
`rpc.db.saveTask` completes and lacks error handling or cache invalidation;
change the onClick to perform an optimistic UI update of the local
`autoApproveEnabled` state (so `TerminalPane` sees the new value immediately),
then await `rpc.db.saveTask({...task, metadata:{...task.metadata, autoApprove:
next}})`, handle errors (rollback optimistic state and surface/log the error)
and only call `window.electronAPI.ptyKill(terminalId)` after the save resolves
successfully; additionally, on success invalidate or refresh the parent task
cache or emit an update so the `task` prop used by `TerminalPane` (and any query
cache) is updated consistently—reference functions/values: rpc.db.saveTask,
window.electronAPI.ptyKill, autoApproveEnabled, task, terminalId, and
TerminalPane.
---
Nitpick comments:
In `@src/renderer/components/ChatInterface.tsx`:
- Around line 1206-1238: The toggle button rendering in the ChatInterface
component doesn't expose its pressed state to assistive tech; update the button
element that uses autoApproveEnabled (the onClick handler that calls
rpc.db.saveTask and window.electronAPI.ptyKill(terminalId)) to include an
aria-pressed attribute set to the boolean autoApproveEnabled so screen readers
can announce the toggle state (e.g., aria-pressed={autoApproveEnabled}),
ensuring the rest of the click behavior remains unchanged.
- Around line 1217-1221: The comment on the ptyKill call is misleading: it
claims the session manager will restart "with autoApprove: true" but the restart
uses the prop passed by the renderer, not a DB re-read. Update the comment in
ChatInterface.tsx near window.electronAPI.ptyKill(terminalId) to state that the
next spawn uses the current autoApprove prop passed from TerminalPane
(autoApproveEnabled derived from task) or, alternatively, ensure the parent
refreshes/recomputes the task prop (so TerminalPane receives the updated
autoApproveEnabled) before calling ptyKill; reference TerminalPane,
autoApproveEnabled, task prop, window.electronAPI.ptyKill, and terminalId when
making the change.
🪄 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: b85031d8-ad49-404f-a917-81f247153deb
📒 Files selected for processing (1)
src/renderer/components/ChatInterface.tsx
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/components/ChatInterface.tsx`:
- Around line 940-946: The autoApproveOverride state in ChatInterface persists
across task switches causing stale overrides; add a useEffect in the
ChatInterface component that watches the task (e.g., task.id or the task object)
and calls setAutoApproveOverride(null) whenever the task changes so the override
resets for the new task; reference autoApproveOverride, setAutoApproveOverride,
and autoApproveEnabled to ensure the new effect clears the override before
autoApproveEnabled is recomputed.
- Around line 1228-1230: The catch block currently swallows the error and only
calls setAutoApproveOverride(null); change it to capture the error (catch (err))
and provide user-facing feedback: reset the override as before, then surface the
error via the app's notification mechanism (for example call an existing
notifyError/toast/enqueueSnackbar or set an error state) including err.message
so the user sees why the toggle failed; if no notification helper exists in this
component, create a small error state (e.g., autoApproveError) and render a
brief inline message or toast after catching the error.
🪄 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: 0e66d97a-faf1-441c-ad50-87ed5a6af3cf
📒 Files selected for processing (1)
src/renderer/components/ChatInterface.tsx
Fizza-Mukhtar
force-pushed
the
feat/toggle-auto-approve-running-task
branch
from
817b529 to
125531a
Compare
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
1 participant
What
Converts the read-only "Auto-approve" badge in the task tab bar into a
clickable toggle. Users can now enable or disable skip-permissions
for file operations on an already-running task — no restart required.
Closes #1671
Why
Previously, if a user forgot to enable "Skip permissions for file
operations" at task creation time, they had no way to enable it
mid-session. The only option was to stop the task and lose context.
How
<button>that togglestask.metadata.autoApproverestarts it with
--dangerously-skip-permissionsand Claude's resumeflag, preserving session context
uninterrupted; the flag is removed on next spawn
autoApproveFlagdefined(Claude, Codex, Gemini, etc.)
Testing
click → PTY restarts with flag → button turns orange
autoApproveFlag): button not shownpnpm run type-checkandpnpm run lintpassType of change
Mandatory Tasks
Checklist
Summary by CodeRabbit
New Features
Style