[fix] Say that the approval checkbox auto-approves - #6308
Open
ashrafchowdury wants to merge 3 commits into
Open
Conversation
… it stops asking "Don't ask again for this" described the prompt going away, leaving what happens instead unstated — a user could reasonably read it as the tool being skipped. It now says "Auto-approve this tool from now on", with a line naming both halves: the agent runs the tool without asking, on this run and every future one. The checkbox moved to its own row, since two lines of explicit copy would collide with the buttons at 375px. The config pane's confirmation banner follows the same wording. Two defects fixed alongside: the checkbox had no accessible name, so a screen reader announced a bare checkbox; and the wording carried a pointer cursor while only the 16px box was hittable, which is rough on touch. The wrapping label already forwards a click to Radix's button — a test pins that, so nobody adds a handler that double-toggles it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The two-line version pushed the checkbox onto its own row and made the card taller than it was before. "Always auto-approve" carries the same meaning in two words and sits inline with Deny/Approve again, as it did originally. The row now wraps instead of squeezing, so turning Redirect on cannot shove Approve off a 375px screen. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
dosubot
Bot
added
the
size:S
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe approval flow now supports granting and revoking multiple tools at once. The approval card derives eligible tools from the full batch, while the notice undoes all affected tools. Copy, accessibility labeling, responsive layout, and tests were updated. ChangesBatch auto-approve
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: 🔵 Low · up to The PR improves approval wording, accessibility, responsive layout, and batched permission handling. It is mergeable with owner awareness that seven new comments should be condensed to the repository’s one-line comment standard. Sequence Diagram(s)sequenceDiagram
participant ApprovalCard
participant useAlwaysAllowTool
participant foldPermissions
participant AlwaysAllowedNotice
ApprovalCard->>useAlwaysAllowTool: grantMany(grantableTools)
useAlwaysAllowTool->>foldPermissions: apply batch permissions
foldPermissions-->>useAlwaysAllowTool: applied tool names
useAlwaysAllowTool-->>AlwaysAllowedNotice: draft-change signal with toolNames
AlwaysAllowedNotice->>useAlwaysAllowTool: revokeMany(toolNames) on Undo
useAlwaysAllowTool->>foldPermissions: revoke batch permissions
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![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: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: c46267e1-665d-4ef1-a800-bfd4e3f37003
📒 Files selected for processing (3)
web/oss/src/components/Playground/Components/AlwaysAllowedNotice.tsxweb/packages/agenta-chat/src/components/ApprovalCard.tsxweb/packages/agenta-chat/tests/unit/ApprovalCard.test.tsx
Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review.
Contributor
Railway Preview Environment
|
Approve all answers every pending gate, but the grant only wrote the first gate's tool, so a batch spanning several tools left the rest to stop the next run. The card now collects every grantable tool in the batch and grants them together. They have to land as ONE write. Each withToolPermission/withHarnessToolAllow call returns a whole new parameters object, so calling the single-tool path per tool would build each one on the same stale base and keep only the last. foldPermissions threads the result of each flip into the next and returns a single config, and the draft-change signal now carries the full list so the config pane's Undo reverts all of them. Ineligible members are skipped, not blocked: a commit_revision mixed into the batch stays gated while the shell and file tools beside it are granted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
dosubot
Bot
added
size:L
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 61cfb37f-d15d-47f0-9091-0da0aa4e733e
📒 Files selected for processing (6)
web/oss/src/components/Playground/Components/AlwaysAllowedNotice.tsxweb/packages/agenta-chat/src/components/ApprovalCard.tsxweb/packages/agenta-chat/src/hooks/useAlwaysAllowTool.tsweb/packages/agenta-chat/tests/unit/ApprovalCard.test.tsxweb/packages/agenta-chat/tests/unit/hooks/foldPermissions.test.tsweb/packages/agenta-shared/src/state/draftConfigChangeSignal.ts
Included review availability: Your plan provides up to 8 included reviews per hour; 3 remain after this review.
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.
Context
The grant checkbox on the approval card read "Don't ask again for this". That describes the prompt going away, not what happens instead, so a cautious reader could take it as the tool being skipped rather than run.
Ticking the box and approving writes that tool's permission to
allowin the agent's draft config. The tool then runs with no prompt, on this run and on every future one. The label never said so.Changes
The label now reads "Always auto-approve", and it stays on the same line as Deny and Approve.
The confirmation banner in the config pane follows the same wording. "Always allowing bash / Saved to this draft, this tool won't ask again" becomes "Auto-approving bash / Saved to this draft, it runs without asking from now on".
Two fixes in the same row:
The checkbox had no accessible name. Radix renders it as a
<button role="checkbox">, and a wrapping<label>gives it no name, so a screen reader announced an unlabelled checkbox. It now points at the label text witharia-labelledby.The action row wraps instead of squeezing. With
NEXT_PUBLIC_AGENT_CHAT_STEERon, the extra Redirect button next to the longer label overflowed a 375px screen and pushed Approve out of view. The buttons now drop to a second line.Batched approvals granted only the first tool
"Approve all" answers every pending gate, but the grant wrote only
approvals[0].toolName. A batch spanning several tools left the rest to stop the next run, with nothing on screen to say so.The card now collects every grantable tool in the batch and grants them together. Ineligible members are skipped rather than blocking the rest, so a
commit_revisionmixed into a batch stays gated while the shell and file tools beside it are granted.They have to land as one write. Every
withToolPermissionandwithHarnessToolAllowcall returns a whole newparametersobject, so calling the single-tool path once per tool would build each flip on the same stale base and keep only the last one.foldPermissionsthreads each result into the next call and returns a single config. The draft-change signal carries the full list, so the config pane's Undo reverts all of them.Why some gates still never offer the checkbox
Worth stating, because "auto-approve" invites the question of why it is not offered everywhere.
gateRulePatternrefuses three classes, for two different reasons, and this PR does not change either.Platform ops (
commit_revision,test_run, schedules, subscriptions) are excluded by design.runner.permissions.defaultstays as authored specifically so these keep gating, and the code describescommit_revisionand destructive ops as gated by construction.commit_revisionpublishes a revision that triggers then run on their own, so it is the one action that escapes the session you are watching.MCP tools are excluded for a mechanical reason.
wire_author_permission_rulesdropsmcp__patterns from the runner plan, so a rule written for one would silently never take effect. MCP is governed per server instead.Client tools (
request_connection,request_input) are excluded because they exist to ask the user for something. There is nothing to auto-approve.Tests
vitestin@agenta/chat, 14 tests across the card and the new fold. The card tests cover granting a whole batch and granting nothing on a denial.foldPermissionshas its own tests for the stale-base trap, for skipping ineligible names, and for reversing a batch.<button>is a labelable element, so the label already forwards the click natively; the test guards against someone adding a manual handler that double toggles it.What to QA
<tool>" banner with Undo, and the same tool does not ask again later in the run.commit_revisiongate. The checkbox is absent, as it was before./mat phone width. The row still fits on one line and Approve is fully visible.