[Improve] Streamline DCG command approval - #1062
Merged
Merged
Conversation
navedmerchant
requested review from
JamesRobert20,
edelauna,
hannesrudolph and
taltas
as code owners
July 30, 2026 04:25
Contributor
📝 WalkthroughWalkthroughThe change integrates destructive-command-guard evaluation with command auto-approval, persists approval decisions on messages, adds localized denial details, and updates chat rendering for denied commands. ChangesDestructive command approval and persistence
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ExecuteCommandTool
participant DCG
participant AutoApproval
participant Task
participant ChatRow
ExecuteCommandTool->>DCG: Install and evaluate command
DCG-->>ExecuteCommandTool: Allow or denial details
ExecuteCommandTool->>AutoApproval: Request approval
AutoApproval-->>ExecuteCommandTool: Approve or deny
ExecuteCommandTool->>Task: Persist autoApprovalDecision
Task-->>ChatRow: Render command message state
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/types/src/__tests__/message.test.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. packages/types/src/message.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. src/core/auto-approval/__tests__/dcg.spec.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.
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 |
9 tasks
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
github-actions
Bot
added
the
awaiting-review
navedmerchant
force-pushed
the
feat/dcg-setting
branch
from
July 30, 2026 04:50
e655ee2 to
28fb788
Compare
navedmerchant
force-pushed
the
feat/dcg-command-integration
branch
from
July 30, 2026 04:50
3e1b109 to
5f6e0a8
Compare
github-actions
Bot
added
awaiting-review
edelauna
requested changes
Jul 31, 2026
github-actions
Bot
added
awaiting-author
navedmerchant
force-pushed
the
feat/dcg-setting
branch
from
August 1, 2026 17:21
28fb788 to
f7ae3cb
Compare
navedmerchant
force-pushed
the
feat/dcg-command-integration
branch
from
August 1, 2026 17:23
5f6e0a8 to
dd00d03
Compare
github-actions
Bot
removed
the
awaiting-author
zoomote
Bot
changed the title
This was referenced Aug 2, 2026
Contributor
|
Addressed the implementation and coverage feedback in 900282f. DCG reads the persisted flag cheaply before approval, preserves deferred full-state loading, and covers allow and pending chat-row paths. |
edelauna
previously approved these changes
Aug 2, 2026
edelauna
left a comment
edelauna
left a comment
Contributor
There was a problem hiding this comment.
Thanks for addressing the feedback, approving - but still unclear on how you want to handle the design for a DCG blocked command: #1062 (comment)
github-actions
Bot
added
the
awaiting-review
github-actions
Bot
added
awaiting-author
github-actions
Bot
removed
the
awaiting-author
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/core/tools/__tests__/executeCommandTool.spec.ts (1)
243-247: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse typed test doubles instead of repeated double assertions.
Define typed Task and callback test doubles once in the fixture. Remove the repeated
as unknown ascasts. If a cast is unavoidable, document why near the fixture.
src/core/tools/__tests__/executeCommandTool.spec.ts#L243-L247: pass typed Task and callback doubles.src/core/tools/__tests__/executeCommandTool.spec.ts#L266-L270: reuse the typed doubles.src/core/tools/__tests__/executeCommandTool.spec.ts#L284-L288: reuse the typed doubles.src/core/tools/__tests__/executeCommandTool.spec.ts#L304-L308: reuse the typed doubles.src/core/tools/__tests__/executeCommandTool.spec.ts#L329-L333: reuse the typed doubles.As per coding guidelines: “Avoid
as any; use typed APIs, bracket notation for private members, or precise test doubles and type guards. Use double assertions only as a last resort and explain them with a nearby comment.”🤖 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/core/tools/__tests__/executeCommandTool.spec.ts` around lines 243 - 247, Replace the repeated double assertions in the executeCommandTool.handle test calls with typed Task and callback test doubles defined once in the fixture. Reuse those doubles at src/core/tools/__tests__/executeCommandTool.spec.ts lines 243-247, 266-270, 284-288, 304-308, and 329-333; if any cast remains necessary, document its reason beside the fixture.Source: Coding guidelines
webview-ui/src/components/chat/__tests__/CommandExecution.spec.tsx (1)
114-140: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winType the extension-state fixtures instead of using
as any.
mockExtensionState,ExtensionStateWrapper, and the local state passed toExtensionStateContext.Providerare all cast toany, which removes compile-time checks for required context fields likedestructiveCommandGuardEnabled,setAllowedCommands, andsetDeniedCommands. Use the typed context value type or a typed wrapper inCommandExecution.spec.tsx.🤖 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 `@webview-ui/src/components/chat/__tests__/CommandExecution.spec.tsx` around lines 114 - 140, Replace the any casts in the CommandExecution tests with the actual ExtensionStateContext value type, including the local state and ExtensionStateWrapper fixtures. Ensure mockExtensionState and the provider value satisfy required fields such as destructiveCommandGuardEnabled, setAllowedCommands, and setDeniedCommands without weakening type checking.Source: Coding guidelines
🤖 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/core/tools/__tests__/executeCommandTool.spec.ts`:
- Around line 243-247: Replace the repeated double assertions in the
executeCommandTool.handle test calls with typed Task and callback test doubles
defined once in the fixture. Reuse those doubles at
src/core/tools/__tests__/executeCommandTool.spec.ts lines 243-247, 266-270,
284-288, 304-308, and 329-333; if any cast remains necessary, document its
reason beside the fixture.
In `@webview-ui/src/components/chat/__tests__/CommandExecution.spec.tsx`:
- Around line 114-140: Replace the any casts in the CommandExecution tests with
the actual ExtensionStateContext value type, including the local state and
ExtensionStateWrapper fixtures. Ensure mockExtensionState and the provider value
satisfy required fields such as destructiveCommandGuardEnabled,
setAllowedCommands, and setDeniedCommands without weakening type checking.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 84b524a3-87e4-4028-9574-e9338ea3e230
📒 Files selected for processing (48)
AGENTS.mdpackages/types/src/__tests__/message.test.tspackages/types/src/message.tssrc/core/auto-approval/__tests__/dcg.spec.tssrc/core/auto-approval/index.tssrc/core/task/Task.tssrc/core/tools/ExecuteCommandTool.tssrc/core/tools/__tests__/executeCommandTool.spec.tssrc/i18n/locales/ca/tools.jsonsrc/i18n/locales/de/tools.jsonsrc/i18n/locales/en/tools.jsonsrc/i18n/locales/es/tools.jsonsrc/i18n/locales/fr/tools.jsonsrc/i18n/locales/hi/tools.jsonsrc/i18n/locales/id/tools.jsonsrc/i18n/locales/it/tools.jsonsrc/i18n/locales/ja/tools.jsonsrc/i18n/locales/ko/tools.jsonsrc/i18n/locales/nl/tools.jsonsrc/i18n/locales/pl/tools.jsonsrc/i18n/locales/pt-BR/tools.jsonsrc/i18n/locales/ru/tools.jsonsrc/i18n/locales/tr/tools.jsonsrc/i18n/locales/vi/tools.jsonsrc/i18n/locales/zh-CN/tools.jsonsrc/i18n/locales/zh-TW/tools.jsonwebview-ui/src/components/chat/ChatRow.tsxwebview-ui/src/components/chat/CommandExecution.tsxwebview-ui/src/components/chat/__tests__/ChatRow.command-denied.spec.tsxwebview-ui/src/components/chat/__tests__/CommandExecution.spec.tsxwebview-ui/src/i18n/locales/ca/chat.jsonwebview-ui/src/i18n/locales/de/chat.jsonwebview-ui/src/i18n/locales/en/chat.jsonwebview-ui/src/i18n/locales/es/chat.jsonwebview-ui/src/i18n/locales/fr/chat.jsonwebview-ui/src/i18n/locales/hi/chat.jsonwebview-ui/src/i18n/locales/id/chat.jsonwebview-ui/src/i18n/locales/it/chat.jsonwebview-ui/src/i18n/locales/ja/chat.jsonwebview-ui/src/i18n/locales/ko/chat.jsonwebview-ui/src/i18n/locales/nl/chat.jsonwebview-ui/src/i18n/locales/pl/chat.jsonwebview-ui/src/i18n/locales/pt-BR/chat.jsonwebview-ui/src/i18n/locales/ru/chat.jsonwebview-ui/src/i18n/locales/tr/chat.jsonwebview-ui/src/i18n/locales/vi/chat.jsonwebview-ui/src/i18n/locales/zh-CN/chat.jsonwebview-ui/src/i18n/locales/zh-TW/chat.json
edelauna
approved these changes
Aug 5, 2026
Merged
myk1yt
pushed a commit
to myk1yt/Zoo-Code
that referenced
this pull request
Aug 6, 2026
* refactor: extract managed binary installation infrastructure Refs Zoo-Code-Org#1055 * fix: address managed binary review feedback * feat: add destructive command guard binary service Refs Zoo-Code-Org#1056 * test: strengthen DCG binary service coverage * feat: add persisted destructive command guard setting Refs Zoo-Code-Org#1057 * fix: clarify DCG enablement errors * test: cover DCG global setting schema * feat: integrate destructive command guard with auto-approval Closes Zoo-Code-Org#1058 Completes Zoo-Code-Org#1049 * test: document DCG auto-approval precedence * test: raise DCG setting patch coverage * fix: address DCG service review feedback * fix: address managed binary review feedback * test: cover managed binary cleanup boundaries * fix: address DCG binary service feedback * fix: address DCG integration feedback * fix: finalize managed binary download handling * test: mirror download stream close events * test: mirror download stream close events --------- Co-authored-by: Naved Merchant <14171946+navedmerchant@users.noreply.github.com> Co-authored-by: edelauna <54631123+edelauna@users.noreply.github.com>
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
2 participants
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
Reads the persisted DCG setting without eagerly building full provider state, restores full state loading until after approval, and adds coverage for an allowed command continuing through execution and for command rows awaiting a decision.
Why this change was made
The DCG integration should avoid unnecessary state work on ordinary commands while preserving explicit approval for guarded commands. Closes #1058 and completes #1049.
Impact
Command approval remains responsive when DCG is disabled, while guarded-command behavior and denied-state rendering are covered more completely.
Related PRs
Summary by CodeRabbit
New Features
Bug Fixes
Tests