mirror of
https://github.com/NanmiCoder/claude-code-haha.git
synced 2026-10-10 11:53:10 +08:00
fix(plan-mode): approve plans with an explicit permission mode (#1254)
Approving a plan only sent the plan's addRules, so the CLI restored the mode from prePlanMode and fell back to default when a session had nothing to restore (it was launched in plan mode) — implementation then asked for permission on every tool call. The desktop plan dialog now offers "Approve & auto-accept edits" and "Approve & bypass permissions", mirroring the official terminal dialog, and pins the chosen mode through a setMode permission update. A mode delivered through a permission update also skipped the plan exit transition: ExitPlanMode's cleanup only ran while the mode was still plan, so the exit flags, the exit/re-entry reminders, the auto-mode teardown and the rules stripped on entry were left half-applied. That path now reuses the same transition the CLI runs for its own switches, and refuses bypassPermissions when the session cannot use it. Fixes #1254 Validation: new PermissionUpdate regressions plus the plan dialog tests; src/utils/permissions + src/services/tools + src/cli 103 passed; check:policy 330 passed; desktop suite 330 files passed; compiled claude-sidecar smoke passed; real-CLI A/B keeps the exit state identical with and without the pinned mode.
This commit is contained in:
@@ -8,9 +8,11 @@ import { Button } from '@/components/ui/Button'
|
||||
import { DiffViewer } from './DiffViewer'
|
||||
import {
|
||||
PlanPreviewCard,
|
||||
buildPlanApprovalPermissionUpdates,
|
||||
buildPromptPermissionUpdates,
|
||||
extractPlanPreview,
|
||||
isExitPlanModeTool,
|
||||
type PlanApprovalMode,
|
||||
} from './PlanModePreview'
|
||||
|
||||
type Props = {
|
||||
@@ -320,6 +322,17 @@ function ExitPlanModePermissionDialog({
|
||||
const permissionUpdates = buildPromptPermissionUpdates(preview.allowedPrompts)
|
||||
const trimmedFeedback = feedback.trim()
|
||||
|
||||
// Without an explicit mode the CLI resumes with whatever the session had
|
||||
// before planning, falling back to `default` when there was nothing to
|
||||
// restore (a session launched in plan mode) — which is why implementation
|
||||
// used to start by asking for every tool. Approving with a mode pins it.
|
||||
const approveWithMode = (mode: PlanApprovalMode) => {
|
||||
if (!sessionId) return
|
||||
respondToPermission(sessionId, requestId, true, {
|
||||
permissionUpdates: buildPlanApprovalPermissionUpdates(mode, preview.allowedPrompts),
|
||||
})
|
||||
}
|
||||
|
||||
return (
|
||||
<div className={`mb-4 overflow-hidden rounded-[var(--radius-lg)] border ${
|
||||
isPending
|
||||
@@ -380,7 +393,7 @@ function ExitPlanModePermissionDialog({
|
||||
</div>
|
||||
|
||||
{isPending ? (
|
||||
<div className="flex items-center gap-2 border-t border-[var(--color-border)] bg-[var(--color-surface-container-low)] px-4 py-3">
|
||||
<div className="flex flex-wrap items-center gap-2 border-t border-[var(--color-border)] bg-[var(--color-surface-container-low)] px-4 py-3">
|
||||
<Button
|
||||
variant="primary"
|
||||
size="sm"
|
||||
@@ -389,6 +402,22 @@ function ExitPlanModePermissionDialog({
|
||||
>
|
||||
{t('permission.planApprove')}
|
||||
</Button>
|
||||
<Button
|
||||
variant="secondary"
|
||||
size="sm"
|
||||
onClick={() => approveWithMode('acceptEdits')}
|
||||
icon={<span aria-hidden="true" className="material-symbols-outlined text-[14px]">bolt</span>}
|
||||
>
|
||||
{t('permission.planApproveAcceptEdits')}
|
||||
</Button>
|
||||
<Button
|
||||
variant="danger-outline"
|
||||
size="sm"
|
||||
onClick={() => approveWithMode('bypassPermissions')}
|
||||
icon={<span aria-hidden="true" className="material-symbols-outlined text-[14px]">gavel</span>}
|
||||
>
|
||||
{t('permission.planApproveBypass')}
|
||||
</Button>
|
||||
<div className="flex-1" />
|
||||
<Button
|
||||
variant="ghost"
|
||||
|
||||
@@ -167,6 +167,63 @@ describe('plan mode permission UI', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('approves with an explicit bypass permission update', () => {
|
||||
render(
|
||||
<PermissionDialog
|
||||
sessionId="session-1"
|
||||
requestId="perm-plan"
|
||||
toolName="ExitPlanMode"
|
||||
input={{ plan: PLAN, planFilePath: '/tmp/claude-plan.md' }}
|
||||
/>,
|
||||
)
|
||||
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Approve & bypass permissions' }))
|
||||
|
||||
// The CLI resumes implementation with the session mode pinned here; without
|
||||
// it a session that launched in plan mode falls back to `default` and
|
||||
// prompts for every tool call.
|
||||
expect(sendMock).toHaveBeenCalledWith('session-1', {
|
||||
type: 'permission_response',
|
||||
requestId: 'perm-plan',
|
||||
allowed: true,
|
||||
permissionUpdates: [
|
||||
{ type: 'setMode', mode: 'bypassPermissions', destination: 'session' },
|
||||
],
|
||||
})
|
||||
})
|
||||
|
||||
it('approves with auto-accept edits and keeps the requested prompt rules', () => {
|
||||
render(
|
||||
<PermissionDialog
|
||||
sessionId="session-1"
|
||||
requestId="perm-plan"
|
||||
toolName="ExitPlanMode"
|
||||
input={{
|
||||
plan: PLAN,
|
||||
allowedPrompts: [{ tool: 'Bash', prompt: 'run tests' }],
|
||||
}}
|
||||
/>,
|
||||
)
|
||||
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Approve & auto-accept edits' }))
|
||||
|
||||
// Order matters: the mode rides first, the prompt rules follow it.
|
||||
expect(sendMock).toHaveBeenCalledWith('session-1', {
|
||||
type: 'permission_response',
|
||||
requestId: 'perm-plan',
|
||||
allowed: true,
|
||||
permissionUpdates: [
|
||||
{ type: 'setMode', mode: 'acceptEdits', destination: 'session' },
|
||||
{
|
||||
type: 'addRules',
|
||||
rules: [{ toolName: 'Bash', ruleContent: 'prompt: run tests' }],
|
||||
behavior: 'allow',
|
||||
destination: 'session',
|
||||
},
|
||||
],
|
||||
})
|
||||
})
|
||||
|
||||
it('renders approved ExitPlanMode results as a markdown plan card', () => {
|
||||
const { container } = render(
|
||||
<ToolCallBlock
|
||||
|
||||
@@ -127,6 +127,25 @@ export function buildPromptPermissionUpdates(allowedPrompts: AllowedPrompt[]): P
|
||||
]
|
||||
}
|
||||
|
||||
/** Modes the approval dialog can hand the session before implementation starts. */
|
||||
export type PlanApprovalMode = 'acceptEdits' | 'bypassPermissions'
|
||||
|
||||
/**
|
||||
* Approval with an explicit mode, mirroring the official CLI: every "yes" in
|
||||
* its plan dialog carries a `setMode` update, so a session that entered plan
|
||||
* mode without a previous mode to restore (e.g. it launched in plan mode) does
|
||||
* not silently land on `default` and start prompting for every tool call.
|
||||
*/
|
||||
export function buildPlanApprovalPermissionUpdates(
|
||||
mode: PlanApprovalMode,
|
||||
allowedPrompts: AllowedPrompt[],
|
||||
): PermissionUpdate[] {
|
||||
return [
|
||||
{ type: 'setMode', mode, destination: 'session' },
|
||||
...buildPromptPermissionUpdates(allowedPrompts),
|
||||
]
|
||||
}
|
||||
|
||||
function asRecord(value: unknown): Record<string, unknown> {
|
||||
return value && typeof value === 'object' && !Array.isArray(value)
|
||||
? value as Record<string, unknown>
|
||||
|
||||
@@ -2141,6 +2141,8 @@ Row 9, all 8 cells: continuing from straight down, turning left through lower-le
|
||||
'permission.planPreviewTitle': "Claude's plan",
|
||||
'permission.planRequestedPermissions': 'Requested permissions',
|
||||
'permission.planApprove': 'Approve plan',
|
||||
'permission.planApproveAcceptEdits': 'Approve & auto-accept edits',
|
||||
'permission.planApproveBypass': 'Approve & bypass permissions',
|
||||
'permission.planKeepPlanning': 'Keep planning',
|
||||
'permission.planFeedbackPlaceholder': 'Tell Claude what to change',
|
||||
'permission.planEmpty': 'No plan content available.',
|
||||
|
||||
@@ -2143,6 +2143,8 @@ export const jp: Record<TranslationKey, string> = {
|
||||
'permission.planPreviewTitle': 'Claude の計画',
|
||||
'permission.planRequestedPermissions': '要求された権限',
|
||||
'permission.planApprove': '計画を承認',
|
||||
'permission.planApproveAcceptEdits': '承認して編集を自動承認',
|
||||
'permission.planApproveBypass': '承認して権限をバイパス',
|
||||
'permission.planKeepPlanning': '計画を続ける',
|
||||
'permission.planFeedbackPlaceholder': 'Claude に変更内容を伝える',
|
||||
'permission.planEmpty': '計画内容はありません。',
|
||||
|
||||
@@ -2143,6 +2143,8 @@ export const kr: Record<TranslationKey, string> = {
|
||||
'permission.planPreviewTitle': 'Claude의 계획',
|
||||
'permission.planRequestedPermissions': '요청된 권한',
|
||||
'permission.planApprove': '계획 승인',
|
||||
'permission.planApproveAcceptEdits': '승인 후 편집 자동 수락',
|
||||
'permission.planApproveBypass': '승인 후 권한 건너뛰기',
|
||||
'permission.planKeepPlanning': '계속 계획하기',
|
||||
'permission.planFeedbackPlaceholder': 'Claude에게 변경할 내용을 알려주세요',
|
||||
'permission.planEmpty': '계획 내용이 없습니다.',
|
||||
|
||||
@@ -2142,6 +2142,8 @@ export const zh: Record<TranslationKey, string> = {
|
||||
'permission.planPreviewTitle': 'Claude 的計劃',
|
||||
'permission.planRequestedPermissions': '請求的權限',
|
||||
'permission.planApprove': '批准計劃',
|
||||
'permission.planApproveAcceptEdits': '批准並自動接受編輯',
|
||||
'permission.planApproveBypass': '批准並跳過權限',
|
||||
'permission.planKeepPlanning': '繼續規劃',
|
||||
'permission.planFeedbackPlaceholder': '告訴 Claude 需要修改什麼',
|
||||
'permission.planEmpty': '暫無計劃內容。',
|
||||
|
||||
@@ -2142,6 +2142,8 @@ export const zh: Record<TranslationKey, string> = {
|
||||
'permission.planPreviewTitle': 'Claude 的计划',
|
||||
'permission.planRequestedPermissions': '请求的权限',
|
||||
'permission.planApprove': '批准计划',
|
||||
'permission.planApproveAcceptEdits': '批准并自动接受编辑',
|
||||
'permission.planApproveBypass': '批准并跳过权限',
|
||||
'permission.planKeepPlanning': '继续规划',
|
||||
'permission.planFeedbackPlaceholder': '告诉 Claude 需要修改什么',
|
||||
'permission.planEmpty': '暂无计划内容。',
|
||||
|
||||
@@ -0,0 +1,102 @@
|
||||
import { beforeEach, describe, expect, it } from 'bun:test'
|
||||
import {
|
||||
getEmptyToolPermissionContext,
|
||||
type ToolPermissionContext,
|
||||
} from '../../Tool.js'
|
||||
import {
|
||||
hasExitedPlanModeInSession,
|
||||
needsPlanModeExitAttachment,
|
||||
setHasExitedPlanMode,
|
||||
setNeedsPlanModeExitAttachment,
|
||||
} from '../../bootstrap/state.js'
|
||||
import {
|
||||
applyPermissionUpdate,
|
||||
applyPermissionUpdates,
|
||||
} from './PermissionUpdate.js'
|
||||
import type { PermissionUpdate } from './PermissionUpdateSchema.js'
|
||||
|
||||
function permissionContext(
|
||||
overrides: Partial<ToolPermissionContext>,
|
||||
): ToolPermissionContext {
|
||||
return {
|
||||
...getEmptyToolPermissionContext(),
|
||||
...overrides,
|
||||
}
|
||||
}
|
||||
|
||||
function setMode(mode: PermissionUpdate extends { mode: infer M } ? M : string): PermissionUpdate {
|
||||
return { type: 'setMode', mode, destination: 'session' } as PermissionUpdate
|
||||
}
|
||||
|
||||
describe('setMode permission updates', () => {
|
||||
beforeEach(() => {
|
||||
setHasExitedPlanMode(false)
|
||||
setNeedsPlanModeExitAttachment(false)
|
||||
})
|
||||
|
||||
it('runs the plan-exit transition when an approval leaves plan mode', () => {
|
||||
// The desktop's plan dialog approves ExitPlanMode with
|
||||
// `[{ setMode: bypassPermissions }]`. That write used to skip the
|
||||
// bookkeeping the CLI does on its own switches (handleSetPermissionMode),
|
||||
// so the session kept a half-applied plan exit.
|
||||
const next = applyPermissionUpdate(
|
||||
permissionContext({
|
||||
mode: 'plan',
|
||||
prePlanMode: 'bypassPermissions',
|
||||
isBypassPermissionsModeAvailable: true,
|
||||
}),
|
||||
setMode('bypassPermissions'),
|
||||
)
|
||||
|
||||
expect(next.mode).toBe('bypassPermissions')
|
||||
expect(next.prePlanMode).toBeUndefined()
|
||||
expect(hasExitedPlanModeInSession()).toBe(true)
|
||||
expect(needsPlanModeExitAttachment()).toBe(true)
|
||||
})
|
||||
|
||||
it('applies the same transition through the batch helper hosts call', () => {
|
||||
const next = applyPermissionUpdates(
|
||||
permissionContext({ mode: 'plan', prePlanMode: 'default' }),
|
||||
[
|
||||
setMode('acceptEdits'),
|
||||
{
|
||||
type: 'addRules',
|
||||
rules: [{ toolName: 'Bash', ruleContent: 'prompt: run tests' }],
|
||||
behavior: 'allow',
|
||||
destination: 'session',
|
||||
},
|
||||
],
|
||||
)
|
||||
|
||||
expect(next.mode).toBe('acceptEdits')
|
||||
expect(next.prePlanMode).toBeUndefined()
|
||||
expect(hasExitedPlanModeInSession()).toBe(true)
|
||||
})
|
||||
|
||||
it('refuses bypassPermissions when the session cannot use it', () => {
|
||||
// Same gate the CLI applies to its own switches: an org policy or a
|
||||
// session launched without the capability must not be overridden by a
|
||||
// permission update.
|
||||
const context = permissionContext({
|
||||
mode: 'plan',
|
||||
prePlanMode: 'default',
|
||||
isBypassPermissionsModeAvailable: false,
|
||||
})
|
||||
|
||||
const next = applyPermissionUpdate(context, setMode('bypassPermissions'))
|
||||
|
||||
expect(next.mode).toBe('plan')
|
||||
expect(next).toBe(context)
|
||||
})
|
||||
|
||||
it('leaves the plan-exit flags alone for mode changes outside plan mode', () => {
|
||||
const next = applyPermissionUpdate(
|
||||
permissionContext({ mode: 'default', prePlanMode: undefined }),
|
||||
setMode('acceptEdits'),
|
||||
)
|
||||
|
||||
expect(next.mode).toBe('acceptEdits')
|
||||
expect(hasExitedPlanModeInSession()).toBe(false)
|
||||
expect(needsPlanModeExitAttachment()).toBe(false)
|
||||
})
|
||||
})
|
||||
@@ -24,6 +24,12 @@ import {
|
||||
} from './permissionRuleParser.js'
|
||||
import { addPermissionRulesToSettings } from './permissionsLoader.js'
|
||||
|
||||
/* eslint-disable @typescript-eslint/no-require-imports */
|
||||
// permissionSetup imports this module back (`applyPermissionUpdate`), so a
|
||||
// static import would close a cycle. Resolve it at call time instead.
|
||||
const permissionSetupModule = require('./permissionSetup.js') as typeof import('./permissionSetup.js')
|
||||
/* eslint-enable @typescript-eslint/no-require-imports */
|
||||
|
||||
// Re-export for backwards compatibility
|
||||
export type { AdditionalWorkingDirectory, WorkingDirectorySource }
|
||||
|
||||
@@ -57,14 +63,39 @@ export function applyPermissionUpdate(
|
||||
update: PermissionUpdate,
|
||||
): ToolPermissionContext {
|
||||
switch (update.type) {
|
||||
case 'setMode':
|
||||
case 'setMode': {
|
||||
logForDebugging(
|
||||
`Applying permission update: Setting mode to '${update.mode}'`,
|
||||
)
|
||||
// Same gate the CLI applies to its own mode switches: a session that
|
||||
// cannot use bypassPermissions (launched without the capability, or the
|
||||
// org policy disables it) must not get it through a permission update
|
||||
// either. Drops the mode change; the rest of the batch still applies.
|
||||
if (
|
||||
update.mode === 'bypassPermissions' &&
|
||||
!context.isBypassPermissionsModeAvailable
|
||||
) {
|
||||
logForDebugging(
|
||||
'Ignoring permission update: bypassPermissions is not available in this session',
|
||||
{ level: 'warn' },
|
||||
)
|
||||
return context
|
||||
}
|
||||
// Changing modes is not just a field write: leaving plan mode sets the
|
||||
// exit attachments and undoes what entering it stashed (auto-mode state,
|
||||
// rules stripped on the way in). The CLI's own switches run that
|
||||
// transition in handleSetPermissionMode; a mode arriving through an
|
||||
// update — the plan-approval dialog, a host, an edit suggestion — used to
|
||||
// skip it and leave a half-applied plan exit behind.
|
||||
return {
|
||||
...context,
|
||||
...permissionSetupModule.transitionPermissionMode(
|
||||
context.mode,
|
||||
update.mode,
|
||||
context,
|
||||
),
|
||||
mode: update.mode,
|
||||
}
|
||||
}
|
||||
|
||||
case 'addRules': {
|
||||
const ruleStrings = update.rules.map(rule =>
|
||||
|
||||
Reference in New Issue
Block a user