fix: consume canonical approvalToolKey from sidecar

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
David Kaya
2026-04-17 14:21:53 +02:00
co-authored by Copilot
parent fce0430091
commit 26847cf00d
5 changed files with 70 additions and 5 deletions
+4 -3
View File
@@ -7,9 +7,9 @@ import type {
import { import {
dequeuePendingApprovalState, dequeuePendingApprovalState,
enqueuePendingApprovalState, enqueuePendingApprovalState,
getPendingApprovalToolKey,
listPendingApprovals, listPendingApprovals,
resolvePendingApproval, resolvePendingApproval,
resolveApprovalToolKey,
type ApprovalDecision, type ApprovalDecision,
type PendingApprovalRecord, type PendingApprovalRecord,
} from '@shared/domain/approval'; } from '@shared/domain/approval';
@@ -109,7 +109,7 @@ export class ApprovalCoordinator {
this.setSessionPendingApprovalState(session, dequeuePendingApprovalState(session, approvalId)); this.setSessionPendingApprovalState(session, dequeuePendingApprovalState(session, approvalId));
session.updatedAt = resolvedAt; session.updatedAt = resolvedAt;
const approvalKey = resolveApprovalToolKey(approval.toolName, approval.permissionKind); const approvalKey = getPendingApprovalToolKey(approval);
if (decision === 'approved' && alwaysApprove && approvalKey) { if (decision === 'approved' && alwaysApprove && approvalKey) {
const existing = session.approvalSettings?.autoApprovedToolNames ?? []; const existing = session.approvalSettings?.autoApprovedToolNames ?? [];
if (!existing.includes(approvalKey)) { if (!existing.includes(approvalKey)) {
@@ -127,7 +127,7 @@ export class ApprovalCoordinator {
continue; continue;
} }
const queuedKey = resolveApprovalToolKey(queued.toolName, queued.permissionKind); const queuedKey = getPendingApprovalToolKey(queued);
if (queuedKey !== approvalKey) { if (queuedKey !== approvalKey) {
continue; continue;
} }
@@ -361,6 +361,7 @@ export class ApprovalCoordinator {
agentName: event.agentName, agentName: event.agentName,
toolName: event.toolName, toolName: event.toolName,
permissionKind: event.permissionKind, permissionKind: event.permissionKind,
approvalToolKey: event.approvalToolKey,
title: event.title, title: event.title,
detail: event.detail, detail: event.detail,
permissionDetail: event.permissionDetail, permissionDetail: event.permissionDetail,
@@ -3,7 +3,7 @@ import { Bot, Check, ChevronDown, Loader2, ShieldAlert, ShieldBan, ShieldCheck,
import { MarkdownContent } from '@renderer/components/MarkdownContent'; import { MarkdownContent } from '@renderer/components/MarkdownContent';
import { permissionDetailSummary, PermissionDetailView } from '@renderer/components/chat/PermissionDetailView'; import { permissionDetailSummary, PermissionDetailView } from '@renderer/components/chat/PermissionDetailView';
import { resolveApprovalToolKey } from '@shared/domain/approval'; import { getPendingApprovalToolKey } from '@shared/domain/approval';
import type { ApprovalDecision, PendingApprovalRecord } from '@shared/domain/approval'; import type { ApprovalDecision, PendingApprovalRecord } from '@shared/domain/approval';
import { resolveToolLabel } from '@shared/domain/tooling'; import { resolveToolLabel } from '@shared/domain/tooling';
@@ -25,7 +25,7 @@ export function ApprovalBanner({
const kindLabel = approval.kind === 'final-response' ? 'Final response review' : 'Tool call approval'; const kindLabel = approval.kind === 'final-response' ? 'Final response review' : 'Tool call approval';
const hasMessages = approval.messages && approval.messages.length > 0; const hasMessages = approval.messages && approval.messages.length > 0;
const showPosition = position !== undefined && total !== undefined && total > 1; const showPosition = position !== undefined && total !== undefined && total > 1;
const approvalToolKey = resolveApprovalToolKey(approval.toolName, approval.permissionKind); const approvalToolKey = getPendingApprovalToolKey(approval);
const canAlwaysApprove = approval.kind === 'tool-call' && !!approvalToolKey; const canAlwaysApprove = approval.kind === 'tool-call' && !!approvalToolKey;
const approvalToolLabel = approvalToolKey ? resolveToolLabel(approvalToolKey) : undefined; const approvalToolLabel = approvalToolKey ? resolveToolLabel(approvalToolKey) : undefined;
+1
View File
@@ -518,6 +518,7 @@ export interface ApprovalRequestedEvent {
agentName?: string; agentName?: string;
toolName?: string; toolName?: string;
permissionKind?: string; permissionKind?: string;
approvalToolKey?: string;
title: string; title: string;
detail?: string; detail?: string;
permissionDetail?: PermissionDetail; permissionDetail?: PermissionDetail;
+18
View File
@@ -34,6 +34,7 @@ export interface PendingApprovalRecord {
agentName?: string; agentName?: string;
toolName?: string; toolName?: string;
permissionKind?: string; permissionKind?: string;
approvalToolKey?: string;
title: string; title: string;
detail?: string; detail?: string;
messages?: PendingApprovalMessageRecord[]; messages?: PendingApprovalMessageRecord[];
@@ -215,6 +216,22 @@ export function resolveApprovalToolKey(
return toolName; return toolName;
} }
/**
* Returns the canonical approval key for a pending approval.
*
* Prefers the explicit `approvalToolKey` supplied by the sidecar. Falls back to
* re-deriving the key from `toolName` and `permissionKind` to support legacy
* records persisted before the sidecar contract change.
*/
export function getPendingApprovalToolKey(
record: Pick<PendingApprovalRecord, 'approvalToolKey' | 'toolName' | 'permissionKind'>,
): string | undefined {
return (
normalizeOptionalString(record.approvalToolKey)
?? resolveApprovalToolKey(record.toolName, record.permissionKind)
);
}
export function approvalPolicyAutoApprovesTool( export function approvalPolicyAutoApprovesTool(
policy: ApprovalPolicy | undefined, policy: ApprovalPolicy | undefined,
toolName?: string, toolName?: string,
@@ -312,6 +329,7 @@ export function normalizePendingApproval(
agentName: normalizeOptionalString(approval?.agentName), agentName: normalizeOptionalString(approval?.agentName),
toolName: normalizeOptionalString(approval?.toolName), toolName: normalizeOptionalString(approval?.toolName),
permissionKind: normalizeOptionalString(approval?.permissionKind), permissionKind: normalizeOptionalString(approval?.permissionKind),
approvalToolKey: normalizeOptionalString(approval?.approvalToolKey),
title, title,
detail: normalizeOptionalString(approval?.detail), detail: normalizeOptionalString(approval?.detail),
messages: normalizePendingApprovalMessages(approval?.messages), messages: normalizePendingApprovalMessages(approval?.messages),
+45
View File
@@ -10,6 +10,7 @@ import {
normalizeSessionApprovalSettings, normalizeSessionApprovalSettings,
pruneSessionApprovalSettings, pruneSessionApprovalSettings,
resolveApprovalToolKey, resolveApprovalToolKey,
getPendingApprovalToolKey,
dequeuePendingApprovalState, dequeuePendingApprovalState,
enqueuePendingApprovalState, enqueuePendingApprovalState,
listPendingApprovals, listPendingApprovals,
@@ -261,4 +262,48 @@ describe('approval helpers', () => {
expect(resolveApprovalToolKey(undefined, undefined)).toBeUndefined(); expect(resolveApprovalToolKey(undefined, undefined)).toBeUndefined();
expect(resolveApprovalToolKey(undefined, 'mcp')).toBeUndefined(); expect(resolveApprovalToolKey(undefined, 'mcp')).toBeUndefined();
}); });
test('getPendingApprovalToolKey prefers the explicit approval tool key from the sidecar', () => {
expect(
getPendingApprovalToolKey({
approvalToolKey: 'write',
toolName: 'apply_patch',
permissionKind: 'hook',
}),
).toBe('write');
expect(
getPendingApprovalToolKey({
approvalToolKey: 'web_fetch',
toolName: 'web_search',
permissionKind: 'hook',
}),
).toBe('web_fetch');
expect(
getPendingApprovalToolKey({
approvalToolKey: 'mcp_server:icm-mcp',
toolName: 'icm-mcp-get_schedule',
permissionKind: 'mcp',
}),
).toBe('mcp_server:icm-mcp');
});
test('getPendingApprovalToolKey falls back to derivation for legacy approvals without the explicit key', () => {
expect(
getPendingApprovalToolKey({
toolName: 'view',
permissionKind: 'read',
}),
).toBe('read');
expect(
getPendingApprovalToolKey({
toolName: 'git.status',
permissionKind: 'mcp',
}),
).toBe('git.status');
expect(getPendingApprovalToolKey({})).toBeUndefined();
});
}); });