fix(ui): bind ACP permissions to request generations (#11415)
Signed-off-by: Jasper Hugo <jasper@spiral.xyz>
This commit is contained in:
@@ -12,6 +12,7 @@ import {
|
||||
useAcpChatSessionSnapshot,
|
||||
} from '../chatSessionStore';
|
||||
import type { AcpElicitationRequest } from '../elicitationRequests';
|
||||
import type { AcpPermissionRequest } from '../permissionRequestTypes';
|
||||
|
||||
function message(id: string, text: string): Message {
|
||||
return {
|
||||
@@ -39,8 +40,8 @@ function session(id: string, conversation: Message[] = []): Session {
|
||||
} as Session;
|
||||
}
|
||||
|
||||
function permissionRequest(sessionId: string, toolCallId = 'tool-1'): RequestPermissionRequest {
|
||||
return {
|
||||
function permissionRequest(sessionId: string, toolCallId = 'tool-1'): AcpPermissionRequest {
|
||||
const request: RequestPermissionRequest = {
|
||||
sessionId,
|
||||
options: [{ optionId: 'allow-once', name: 'Allow once', kind: 'allow_once' }],
|
||||
toolCall: {
|
||||
@@ -62,6 +63,7 @@ function permissionRequest(sessionId: string, toolCallId = 'tool-1'): RequestPer
|
||||
},
|
||||
},
|
||||
};
|
||||
return { generation: `generation-${toolCallId}`, request };
|
||||
}
|
||||
|
||||
function elicitationRequest(sessionId: string): AcpElicitationRequest {
|
||||
@@ -559,6 +561,20 @@ describe('acpChatSessionStore', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('removes the current permission card when the request is cancelled', () => {
|
||||
const currentSessionId = sessionId('session-1');
|
||||
const request = permissionRequest(currentSessionId, 'tool-1');
|
||||
acpChatSessionActions.applyPermissionRequest(request);
|
||||
|
||||
const snapshot = acpChatSessionActions.cancelPermissionRequest(
|
||||
currentSessionId,
|
||||
'tool-1',
|
||||
request.generation
|
||||
);
|
||||
|
||||
expect(snapshot?.messages).toEqual([]);
|
||||
});
|
||||
|
||||
it('resumes streaming only after the final pending user input request resolves', () => {
|
||||
const currentSessionId = sessionId('session-1');
|
||||
|
||||
|
||||
@@ -11,6 +11,7 @@ vi.mock('../chatSessionStore', () => ({
|
||||
acpPermissionUserInputRequestId: (toolCallId: string) => `permission:${toolCallId}`,
|
||||
acpChatSessionActions: {
|
||||
applyPermissionRequest: vi.fn(),
|
||||
cancelPermissionRequest: vi.fn(),
|
||||
resolveUserInputRequest: vi.fn(),
|
||||
},
|
||||
}));
|
||||
@@ -54,6 +55,11 @@ async function expectStillPending(promise: Promise<RequestPermissionResponse>):
|
||||
expect(settled).toBe(false);
|
||||
}
|
||||
|
||||
function appliedGeneration(callIndex = 0): string {
|
||||
return vi.mocked(acpChatSessionActions.applyPermissionRequest).mock.calls[callIndex][0]
|
||||
.generation;
|
||||
}
|
||||
|
||||
describe('ACP permission requests', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
@@ -70,10 +76,11 @@ describe('ACP permission requests', () => {
|
||||
|
||||
it('keeps permission requests pending until explicit resolve', async () => {
|
||||
const response = requestAcpPermission(permissionRequest('session-1', 'tool-1'));
|
||||
const generation = appliedGeneration();
|
||||
|
||||
await expectStillPending(response);
|
||||
|
||||
expect(resolveAcpPermissionRequest('session-1', 'tool-1', 'allow_once')).toBe(true);
|
||||
expect(resolveAcpPermissionRequest('session-1', 'tool-1', generation, 'allow_once')).toBe(true);
|
||||
expect(acpChatSessionActions.resolveUserInputRequest).toHaveBeenCalledWith(
|
||||
'session-1',
|
||||
'permission:tool-1'
|
||||
@@ -89,6 +96,7 @@ describe('ACP permission requests', () => {
|
||||
it('cancels only pending requests for the requested session', async () => {
|
||||
const sessionOneResponse = requestAcpPermission(permissionRequest('session-1', 'tool-1'));
|
||||
const sessionTwoResponse = requestAcpPermission(permissionRequest('session-2', 'tool-2'));
|
||||
const sessionTwoGeneration = appliedGeneration(1);
|
||||
|
||||
cancelAcpPermissionRequestsForSession('session-1');
|
||||
|
||||
@@ -98,8 +106,15 @@ describe('ACP permission requests', () => {
|
||||
},
|
||||
});
|
||||
await expectStillPending(sessionTwoResponse);
|
||||
expect(acpChatSessionActions.cancelPermissionRequest).toHaveBeenCalledWith(
|
||||
'session-1',
|
||||
'tool-1',
|
||||
appliedGeneration()
|
||||
);
|
||||
|
||||
expect(resolveAcpPermissionRequest('session-2', 'tool-2', 'deny_once')).toBe(true);
|
||||
expect(
|
||||
resolveAcpPermissionRequest('session-2', 'tool-2', sessionTwoGeneration, 'deny_once')
|
||||
).toBe(true);
|
||||
await expect(sessionTwoResponse).resolves.toEqual({
|
||||
outcome: {
|
||||
outcome: 'selected',
|
||||
@@ -110,7 +125,9 @@ describe('ACP permission requests', () => {
|
||||
|
||||
it('cancels an older duplicate request for the same session and tool call', async () => {
|
||||
const firstResponse = requestAcpPermission(permissionRequest('session-1', 'tool-1'));
|
||||
const firstGeneration = appliedGeneration();
|
||||
const secondResponse = requestAcpPermission(permissionRequest('session-1', 'tool-1'));
|
||||
const secondGeneration = appliedGeneration(1);
|
||||
|
||||
await expect(firstResponse).resolves.toEqual({
|
||||
outcome: {
|
||||
@@ -118,8 +135,16 @@ describe('ACP permission requests', () => {
|
||||
},
|
||||
});
|
||||
await expectStillPending(secondResponse);
|
||||
expect(firstGeneration).not.toBe(secondGeneration);
|
||||
|
||||
expect(resolveAcpPermissionRequest('session-1', 'tool-1', 'allow_once')).toBe(true);
|
||||
expect(resolveAcpPermissionRequest('session-1', 'tool-1', firstGeneration, 'allow_once')).toBe(
|
||||
false
|
||||
);
|
||||
await expectStillPending(secondResponse);
|
||||
|
||||
expect(resolveAcpPermissionRequest('session-1', 'tool-1', secondGeneration, 'allow_once')).toBe(
|
||||
true
|
||||
);
|
||||
await expect(secondResponse).resolves.toEqual({
|
||||
outcome: {
|
||||
outcome: 'selected',
|
||||
@@ -127,4 +152,29 @@ describe('ACP permission requests', () => {
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it('fails closed when a legacy card has no permission generation', async () => {
|
||||
const response = requestAcpPermission(permissionRequest('session-1', 'tool-1'));
|
||||
|
||||
expect(resolveAcpPermissionRequest('session-1', 'tool-1', undefined, 'allow_once')).toBe(false);
|
||||
await expectStillPending(response);
|
||||
});
|
||||
|
||||
it('keeps distinct tool call IDs independently resolvable', async () => {
|
||||
const firstResponse = requestAcpPermission(permissionRequest('session-1', 'tool-1'));
|
||||
const firstGeneration = appliedGeneration();
|
||||
const secondResponse = requestAcpPermission(permissionRequest('session-1', 'tool-2'));
|
||||
const secondGeneration = appliedGeneration(1);
|
||||
|
||||
expect(resolveAcpPermissionRequest('session-1', 'tool-1', firstGeneration, 'allow_once')).toBe(
|
||||
true
|
||||
);
|
||||
await expect(firstResponse).resolves.toMatchObject({ outcome: { outcome: 'selected' } });
|
||||
await expectStillPending(secondResponse);
|
||||
|
||||
expect(resolveAcpPermissionRequest('session-1', 'tool-2', secondGeneration, 'deny_once')).toBe(
|
||||
true
|
||||
);
|
||||
await expect(secondResponse).resolves.toMatchObject({ outcome: { outcome: 'selected' } });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -114,6 +114,24 @@ function firstContent(message: Message): Message['content'][number] {
|
||||
return content;
|
||||
}
|
||||
|
||||
function permissionRequest(
|
||||
sessionId: string,
|
||||
toolCallId: string,
|
||||
title: string,
|
||||
path: string
|
||||
): RequestPermissionRequest {
|
||||
return {
|
||||
sessionId,
|
||||
options: [{ optionId: 'allow', name: 'Allow', kind: 'allow_once' }],
|
||||
toolCall: {
|
||||
toolCallId,
|
||||
title,
|
||||
rawInput: { path },
|
||||
content: [{ type: 'content', content: { type: 'text', text: `Allow ${title}?` } }],
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
describe('createAcpSessionNotificationAdapter', () => {
|
||||
describe('apply', () => {
|
||||
describe('message chunks', () => {
|
||||
@@ -925,7 +943,10 @@ describe('createAcpSessionNotificationAdapter', () => {
|
||||
},
|
||||
};
|
||||
|
||||
const permissionStateChanges = adapter.applyPermissionRequest(request);
|
||||
const permissionStateChanges = adapter.applyPermissionRequest({
|
||||
generation: 'permission-generation-1',
|
||||
request,
|
||||
});
|
||||
const messages = expectOnlyMessagesChange(permissionStateChanges);
|
||||
|
||||
expect(messages).toHaveLength(1);
|
||||
@@ -934,6 +955,7 @@ describe('createAcpSessionNotificationAdapter', () => {
|
||||
type: 'actionRequired',
|
||||
data: {
|
||||
actionType: 'toolConfirmation',
|
||||
generation: 'permission-generation-1',
|
||||
id: 'tool-1',
|
||||
toolName: 'edit_file',
|
||||
arguments: { path: 'README.md' },
|
||||
@@ -941,6 +963,43 @@ describe('createAcpSessionNotificationAdapter', () => {
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it('replaces reused tool call IDs with the current permission details', () => {
|
||||
const adapter = createAcpSessionNotificationAdapter();
|
||||
const first = permissionRequest(SESSION_ID, 'tool-1', 'Read file', 'README.md');
|
||||
const second = permissionRequest(SESSION_ID, 'tool-1', 'Run command', 'secrets.txt');
|
||||
|
||||
adapter.applyPermissionRequest({ generation: 'generation-a', request: first });
|
||||
const messages = expectOnlyMessagesChange(
|
||||
adapter.applyPermissionRequest({ generation: 'generation-b', request: second })
|
||||
);
|
||||
|
||||
expect(messages).toHaveLength(1);
|
||||
expect(firstContent(messages[0])).toMatchObject({
|
||||
type: 'actionRequired',
|
||||
data: {
|
||||
actionType: 'toolConfirmation',
|
||||
generation: 'generation-b',
|
||||
id: 'tool-1',
|
||||
toolName: 'Run command',
|
||||
arguments: { path: 'secrets.txt' },
|
||||
prompt: 'Allow Run command?',
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it('removes only the matching permission generation when cancelled', () => {
|
||||
const adapter = createAcpSessionNotificationAdapter();
|
||||
const request = permissionRequest(SESSION_ID, 'tool-1', 'Read file', 'README.md');
|
||||
adapter.applyPermissionRequest({ generation: 'generation-a', request });
|
||||
|
||||
expect(adapter.cancelPermissionRequest('tool-1', 'generation-stale')).toEqual([]);
|
||||
const messages = expectOnlyMessagesChange(
|
||||
adapter.cancelPermissionRequest('tool-1', 'generation-a')
|
||||
);
|
||||
|
||||
expect(messages).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('session_info_update with queuedSteer', () => {
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import type { RequestPermissionRequest } from '@agentclientprotocol/sdk';
|
||||
import type { AcpPermissionRequest } from '../permissionRequestTypes';
|
||||
import {
|
||||
type AcpChatStateChange,
|
||||
type AdapterState,
|
||||
@@ -10,20 +11,11 @@ import {
|
||||
|
||||
export function applyPermissionRequest(
|
||||
state: AdapterState,
|
||||
request: RequestPermissionRequest
|
||||
permissionRequest: AcpPermissionRequest
|
||||
): AcpChatStateChange[] {
|
||||
const { generation, request } = permissionRequest;
|
||||
const toolCallId = request.toolCall.toolCallId;
|
||||
const existing = state.messages.some((message) =>
|
||||
message.content.some(
|
||||
(content) =>
|
||||
content.type === 'actionRequired' &&
|
||||
content.data.actionType === 'toolConfirmation' &&
|
||||
content.data.id === toolCallId
|
||||
)
|
||||
);
|
||||
if (existing) {
|
||||
return messagesChange(state);
|
||||
}
|
||||
removePermissionRequestFromState(state, toolCallId);
|
||||
|
||||
const identity = toolIdentity(request.toolCall);
|
||||
const prompt = permissionPrompt(request);
|
||||
@@ -37,6 +29,7 @@ export function applyPermissionRequest(
|
||||
type: 'actionRequired',
|
||||
data: {
|
||||
actionType: 'toolConfirmation',
|
||||
generation,
|
||||
id: toolCallId,
|
||||
toolName: identity.toolName ?? request.toolCall.title ?? toolCallId,
|
||||
arguments: rawInputToArguments(request.toolCall.rawInput),
|
||||
@@ -50,6 +43,44 @@ export function applyPermissionRequest(
|
||||
return messagesChange(state);
|
||||
}
|
||||
|
||||
export function cancelPermissionRequest(
|
||||
state: AdapterState,
|
||||
toolCallId: string,
|
||||
generation: string
|
||||
): AcpChatStateChange[] {
|
||||
return removePermissionRequestFromState(state, toolCallId, generation)
|
||||
? messagesChange(state)
|
||||
: [];
|
||||
}
|
||||
|
||||
function removePermissionRequestFromState(
|
||||
state: AdapterState,
|
||||
toolCallId: string,
|
||||
generation?: string
|
||||
): boolean {
|
||||
let changed = false;
|
||||
|
||||
state.messages = state.messages.flatMap((message) => {
|
||||
const content = message.content.filter((content) => {
|
||||
const matches =
|
||||
content.type === 'actionRequired' &&
|
||||
content.data.actionType === 'toolConfirmation' &&
|
||||
content.data.id === toolCallId &&
|
||||
(generation === undefined || content.data.generation === generation);
|
||||
changed ||= matches;
|
||||
return !matches;
|
||||
});
|
||||
|
||||
if (content.length === message.content.length) {
|
||||
return [message];
|
||||
}
|
||||
|
||||
return content.length > 0 ? [{ ...message, content }] : [];
|
||||
});
|
||||
|
||||
return changed;
|
||||
}
|
||||
|
||||
function permissionPrompt(request: RequestPermissionRequest): string | undefined {
|
||||
for (const content of request.toolCall.content ?? []) {
|
||||
if (content.type === 'content' && content.content.type === 'text') {
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { useEffect, useState } from 'react';
|
||||
import type { GooseSessionNotification_unstable } from '@aaif/goose-sdk';
|
||||
import type { RequestPermissionRequest, SessionNotification } from '@agentclientprotocol/sdk';
|
||||
import type { SessionNotification } from '@agentclientprotocol/sdk';
|
||||
import type { TokenState } from '../types/chat';
|
||||
import { ChatState } from '../types/chatState';
|
||||
import type { Message, NotificationEvent } from '../types/message';
|
||||
@@ -13,6 +13,7 @@ import {
|
||||
import type { ElicitationStatus } from './adapter/elicitations';
|
||||
import { cloneMessage } from './adapter/shared';
|
||||
import type { AcpElicitationRequest } from './elicitationRequests';
|
||||
import type { AcpPermissionRequest } from './permissionRequestTypes';
|
||||
|
||||
export interface AcpChatSessionSnapshot {
|
||||
session: Session | undefined;
|
||||
@@ -65,7 +66,12 @@ export interface AcpChatSessionActions {
|
||||
applyAcpGooseSessionNotification(
|
||||
notification: GooseSessionNotification_unstable
|
||||
): AcpChatSessionSnapshot;
|
||||
applyPermissionRequest(request: RequestPermissionRequest): AcpChatSessionSnapshot;
|
||||
applyPermissionRequest(request: AcpPermissionRequest): AcpChatSessionSnapshot;
|
||||
cancelPermissionRequest(
|
||||
sessionId: string,
|
||||
toolCallId: string,
|
||||
generation: string
|
||||
): AcpChatSessionSnapshot | undefined;
|
||||
applyElicitationRequest(request: AcpElicitationRequest): AcpChatSessionSnapshot;
|
||||
setElicitationStatus(
|
||||
sessionId: string,
|
||||
@@ -483,14 +489,30 @@ function createAcpChatSessionStoreInternal(): AcpChatSessionStoreInternal {
|
||||
};
|
||||
|
||||
const applyPermissionRequest: AcpChatSessionActions['applyPermissionRequest'] = (request) => {
|
||||
const entry = getOrCreateEntry(request.sessionId);
|
||||
const entry = getOrCreateEntry(request.request.sessionId);
|
||||
const changes = entry.adapter.applyPermissionRequest(request);
|
||||
applyChatStateChanges(entry, changes);
|
||||
entry.pendingUserInputRequestIds.add(
|
||||
acpPermissionUserInputRequestId(request.toolCall.toolCallId)
|
||||
acpPermissionUserInputRequestId(request.request.toolCall.toolCallId)
|
||||
);
|
||||
entry.chatState = ChatState.WaitingForUserInput;
|
||||
return notify(request.sessionId, entry);
|
||||
return notify(request.request.sessionId, entry);
|
||||
};
|
||||
|
||||
const cancelPermissionRequest: AcpChatSessionActions['cancelPermissionRequest'] = (
|
||||
sessionId,
|
||||
toolCallId,
|
||||
generation
|
||||
) => {
|
||||
const entry = sessionsById.get(sessionId);
|
||||
if (!entry) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const changes = entry.adapter.cancelPermissionRequest(toolCallId, generation);
|
||||
applyChatStateChanges(entry, changes);
|
||||
entry.pendingUserInputRequestIds.delete(acpPermissionUserInputRequestId(toolCallId));
|
||||
return notify(sessionId, entry);
|
||||
};
|
||||
|
||||
const applyElicitationRequest: AcpChatSessionActions['applyElicitationRequest'] = (request) => {
|
||||
@@ -545,6 +567,7 @@ function createAcpChatSessionStoreInternal(): AcpChatSessionStoreInternal {
|
||||
applyAcpSessionNotification,
|
||||
applyAcpGooseSessionNotification,
|
||||
applyPermissionRequest,
|
||||
cancelPermissionRequest,
|
||||
applyElicitationRequest,
|
||||
setElicitationStatus,
|
||||
};
|
||||
@@ -601,6 +624,7 @@ function actionsFromStore(store: AcpChatSessionStoreInternal): AcpChatSessionAct
|
||||
applyAcpSessionNotification: store.applyAcpSessionNotification,
|
||||
applyAcpGooseSessionNotification: store.applyAcpGooseSessionNotification,
|
||||
applyPermissionRequest: store.applyPermissionRequest,
|
||||
cancelPermissionRequest: store.cancelPermissionRequest,
|
||||
applyElicitationRequest: store.applyElicitationRequest,
|
||||
setElicitationStatus: store.setElicitationStatus,
|
||||
setSessionMetadata: store.setSessionMetadata,
|
||||
|
||||
@@ -0,0 +1,6 @@
|
||||
import type { RequestPermissionRequest } from '@agentclientprotocol/sdk';
|
||||
|
||||
export interface AcpPermissionRequest {
|
||||
generation: string;
|
||||
request: RequestPermissionRequest;
|
||||
}
|
||||
@@ -1,9 +1,11 @@
|
||||
import type { RequestPermissionRequest, RequestPermissionResponse } from '@agentclientprotocol/sdk';
|
||||
import type { Permission } from '../types/permissions';
|
||||
import { acpChatSessionActions, acpPermissionUserInputRequestId } from './chatSessionStore';
|
||||
import type { AcpPermissionRequest } from './permissionRequestTypes';
|
||||
|
||||
interface PendingPermissionRequest {
|
||||
request: RequestPermissionRequest;
|
||||
generation: string;
|
||||
resolve: (response: RequestPermissionResponse) => void;
|
||||
}
|
||||
|
||||
@@ -19,19 +21,24 @@ export async function requestAcpPermission(
|
||||
}
|
||||
|
||||
return new Promise<RequestPermissionResponse>((resolve) => {
|
||||
pendingRequests.set(key, { request, resolve });
|
||||
acpChatSessionActions.applyPermissionRequest(request);
|
||||
const permissionRequest: AcpPermissionRequest = {
|
||||
generation: globalThis.crypto.randomUUID(),
|
||||
request,
|
||||
};
|
||||
pendingRequests.set(key, { ...permissionRequest, resolve });
|
||||
acpChatSessionActions.applyPermissionRequest(permissionRequest);
|
||||
});
|
||||
}
|
||||
|
||||
export function resolveAcpPermissionRequest(
|
||||
sessionId: string,
|
||||
toolCallId: string,
|
||||
generation: string | undefined,
|
||||
action: Permission
|
||||
): boolean {
|
||||
const key = permissionRequestKey(sessionId, toolCallId);
|
||||
const pending = pendingRequests.get(key);
|
||||
if (!pending) {
|
||||
if (!pending || !generation || pending.generation !== generation) {
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -48,6 +55,11 @@ export function cancelAcpPermissionRequestsForSession(sessionId: string): void {
|
||||
for (const [key, pending] of pendingRequests) {
|
||||
if (pending.request.sessionId === sessionId) {
|
||||
pendingRequests.delete(key);
|
||||
acpChatSessionActions.cancelPermissionRequest(
|
||||
sessionId,
|
||||
pending.request.toolCall.toolCallId,
|
||||
pending.generation
|
||||
);
|
||||
pending.resolve(cancelledPermissionResponse());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import type { GooseSessionNotification_unstable } from '@aaif/goose-sdk';
|
||||
import type { RequestPermissionRequest, SessionNotification } from '@agentclientprotocol/sdk';
|
||||
import type { SessionNotification } from '@agentclientprotocol/sdk';
|
||||
import type { Message } from '../types/message';
|
||||
import {
|
||||
applyElicitationRequest as applyElicitationRequestToState,
|
||||
@@ -8,7 +8,10 @@ import {
|
||||
} from './adapter/elicitations';
|
||||
import { applyGooseSessionNotification } from './adapter/gooseSessionNotifications';
|
||||
import { applyContentChunk, applyThoughtChunk } from './adapter/messages';
|
||||
import { applyPermissionRequest as applyPermissionRequestToState } from './adapter/permissions';
|
||||
import {
|
||||
applyPermissionRequest as applyPermissionRequestToState,
|
||||
cancelPermissionRequest as cancelPermissionRequestInState,
|
||||
} from './adapter/permissions';
|
||||
import {
|
||||
type AcpChatStateChange,
|
||||
type AdapterState,
|
||||
@@ -18,13 +21,15 @@ import {
|
||||
} from './adapter/shared';
|
||||
import { applyToolCall, applyToolCallUpdate } from './adapter/tools';
|
||||
import type { AcpElicitationRequest } from './elicitationRequests';
|
||||
import type { AcpPermissionRequest } from './permissionRequestTypes';
|
||||
|
||||
export type { AcpChatStateChange } from './adapter/shared';
|
||||
|
||||
export interface AcpSessionNotificationAdapter {
|
||||
apply(notification: SessionNotification): AcpChatStateChange[];
|
||||
applyGoose(notification: GooseSessionNotification_unstable): AcpChatStateChange[];
|
||||
applyPermissionRequest(request: RequestPermissionRequest): AcpChatStateChange[];
|
||||
applyPermissionRequest(request: AcpPermissionRequest): AcpChatStateChange[];
|
||||
cancelPermissionRequest(toolCallId: string, generation: string): AcpChatStateChange[];
|
||||
applyElicitationRequest(request: AcpElicitationRequest): AcpChatStateChange[];
|
||||
applyElicitationStatus(elicitationId: string, status: ElicitationStatus): AcpChatStateChange[];
|
||||
getMessages(): Message[];
|
||||
@@ -50,6 +55,9 @@ export function createAcpSessionNotificationAdapter(
|
||||
applyPermissionRequest(request) {
|
||||
return applyPermissionRequestToState(state, request);
|
||||
},
|
||||
cancelPermissionRequest(toolCallId, generation) {
|
||||
return cancelPermissionRequestInState(state, toolCallId, generation);
|
||||
},
|
||||
applyElicitationRequest(request) {
|
||||
return applyElicitationRequestToState(state, request);
|
||||
},
|
||||
|
||||
@@ -26,6 +26,7 @@ describe('ToolApprovalButtons', () => {
|
||||
<ToolApprovalButtons
|
||||
data={{
|
||||
id: 'tool-call-approved',
|
||||
generation: 'permission-generation-1',
|
||||
toolName: 'developer__shell',
|
||||
sessionId: 'session-1',
|
||||
}}
|
||||
@@ -37,6 +38,7 @@ describe('ToolApprovalButtons', () => {
|
||||
expect(resolveAcpPermissionRequestMock).toHaveBeenCalledWith(
|
||||
'session-1',
|
||||
'tool-call-approved',
|
||||
'permission-generation-1',
|
||||
'allow_once'
|
||||
);
|
||||
expect(screen.getByText('developer__shell - Allowed once')).toBeInTheDocument();
|
||||
@@ -60,9 +62,46 @@ describe('ToolApprovalButtons', () => {
|
||||
expect(resolveAcpPermissionRequestMock).toHaveBeenCalledWith(
|
||||
'session-1',
|
||||
'tool-call-rerun',
|
||||
undefined,
|
||||
'allow_once'
|
||||
);
|
||||
expect(screen.getByText('This approval request is no longer active.')).toBeInTheDocument();
|
||||
expect(screen.queryByText('developer__shell - Allowed once')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('resets the displayed decision for a new permission generation', async () => {
|
||||
resolveAcpPermissionRequestMock.mockReturnValue(true);
|
||||
const { rerender } = renderWithIntl(
|
||||
<ToolApprovalButtons
|
||||
data={{
|
||||
id: 'tool-call-reused',
|
||||
generation: 'permission-generation-a',
|
||||
toolName: 'developer__shell',
|
||||
sessionId: 'session-1',
|
||||
}}
|
||||
/>
|
||||
);
|
||||
|
||||
await userEvent.click(screen.getByRole('button', { name: 'Allow Once' }));
|
||||
expect(screen.getByText('developer__shell - Allowed once')).toBeInTheDocument();
|
||||
|
||||
rerender(
|
||||
<ToolApprovalButtons
|
||||
data={{
|
||||
id: 'tool-call-reused',
|
||||
generation: 'permission-generation-b',
|
||||
toolName: 'developer__shell',
|
||||
sessionId: 'session-1',
|
||||
}}
|
||||
/>
|
||||
);
|
||||
await userEvent.click(await screen.findByRole('button', { name: 'Allow Once' }));
|
||||
|
||||
expect(resolveAcpPermissionRequestMock).toHaveBeenLastCalledWith(
|
||||
'session-1',
|
||||
'tool-call-reused',
|
||||
'permission-generation-b',
|
||||
'allow_once'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -52,6 +52,7 @@ const globalApprovalState = new Map<
|
||||
>();
|
||||
|
||||
export interface ToolApprovalData {
|
||||
generation?: string;
|
||||
id: string;
|
||||
toolName: string;
|
||||
prompt?: string;
|
||||
@@ -61,9 +62,10 @@ export interface ToolApprovalData {
|
||||
|
||||
export default function ToolApprovalButtons({ data }: { data: ToolApprovalData }) {
|
||||
const intl = useIntl();
|
||||
const { id, toolName, prompt, sessionId, isClicked: initialIsClicked } = data;
|
||||
const { generation, id, toolName, prompt, sessionId, isClicked: initialIsClicked } = data;
|
||||
const approvalStateKey = generation ?? id;
|
||||
|
||||
const storedState = globalApprovalState.get(id);
|
||||
const storedState = globalApprovalState.get(approvalStateKey);
|
||||
const [decision, setDecision] = useState<Permission | null>(storedState?.decision ?? null);
|
||||
const [isClicked, setIsClicked] = useState(storedState?.isClicked ?? initialIsClicked ?? false);
|
||||
const [approvalError, setApprovalError] = useState<string | null>(null);
|
||||
@@ -75,21 +77,24 @@ export default function ToolApprovalButtons({ data }: { data: ToolApprovalData }
|
||||
};
|
||||
|
||||
useEffect(() => {
|
||||
const currentState = globalApprovalState.get(id);
|
||||
const currentState = globalApprovalState.get(approvalStateKey);
|
||||
if (currentState) {
|
||||
setDecision(currentState.decision);
|
||||
setIsClicked(currentState.isClicked);
|
||||
} else {
|
||||
setDecision(null);
|
||||
setIsClicked(initialIsClicked ?? false);
|
||||
}
|
||||
setApprovalError(null);
|
||||
}, [id]);
|
||||
}, [approvalStateKey, initialIsClicked]);
|
||||
|
||||
useEffect(() => {
|
||||
globalApprovalState.set(id, { decision, isClicked });
|
||||
}, [id, decision, isClicked]);
|
||||
globalApprovalState.set(approvalStateKey, { decision, isClicked });
|
||||
}, [approvalStateKey, decision, isClicked]);
|
||||
|
||||
const handleAction = async (action: Permission) => {
|
||||
try {
|
||||
if (resolveAcpPermissionRequest(sessionId, id, action)) {
|
||||
if (resolveAcpPermissionRequest(sessionId, id, generation, action)) {
|
||||
setResolvedDecision(action);
|
||||
} else {
|
||||
setApprovalError(intl.formatMessage(i18n.staleApprovalRequest));
|
||||
|
||||
@@ -5,7 +5,9 @@ import type { ActionRequired } from '../types/message';
|
||||
import ToolCallConfirmation from './ToolCallConfirmation';
|
||||
|
||||
vi.mock('./ToolApprovalButtons', () => ({
|
||||
default: () => <div data-testid="approval-buttons" />,
|
||||
default: ({ data }: { data: { generation?: string } }) => (
|
||||
<div data-testid="approval-buttons" data-generation={data.generation} />
|
||||
),
|
||||
}));
|
||||
|
||||
const securityPrompt = 'This command sends a local file to a remote service.';
|
||||
@@ -14,6 +16,7 @@ const actionRequiredContent = {
|
||||
type: 'actionRequired',
|
||||
data: {
|
||||
actionType: 'toolConfirmation',
|
||||
generation: 'permission-generation-1',
|
||||
id: 'request-1',
|
||||
toolName: 'developer__shell',
|
||||
arguments: {
|
||||
@@ -36,7 +39,10 @@ describe('ToolCallConfirmation', () => {
|
||||
|
||||
expect(screen.getByText('command')).toBeInTheDocument();
|
||||
expect(screen.getByText(/upload \/home\/alice\/private\.txt/)).toBeInTheDocument();
|
||||
expect(screen.getByTestId('approval-buttons')).toBeInTheDocument();
|
||||
expect(screen.getByTestId('approval-buttons')).toHaveAttribute(
|
||||
'data-generation',
|
||||
'permission-generation-1'
|
||||
);
|
||||
});
|
||||
|
||||
it('shows the security prompt before approval', () => {
|
||||
|
||||
@@ -36,7 +36,7 @@ export default function ToolConfirmation({
|
||||
}: ToolConfirmationProps) {
|
||||
const intl = useIntl();
|
||||
const data = actionRequiredContent.data as ToolConfirmationData;
|
||||
const { id, toolName, arguments: toolArguments, prompt } = data;
|
||||
const { generation, id, toolName, arguments: toolArguments, prompt } = data;
|
||||
const displayName = formatToolName(toolName);
|
||||
|
||||
return (
|
||||
@@ -50,7 +50,7 @@ export default function ToolConfirmation({
|
||||
{prompt && <div className="py-2 text-sm text-amber-600 dark:text-amber-400">{prompt}</div>}
|
||||
<ToolCallArguments args={toolArguments as Record<string, ToolCallArgumentValue>} />
|
||||
<ToolApprovalButtons
|
||||
data={{ id, toolName, prompt: prompt ?? undefined, sessionId, isClicked }}
|
||||
data={{ generation, id, toolName, prompt: prompt ?? undefined, sessionId, isClicked }}
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
@@ -1,13 +1,21 @@
|
||||
import { render, screen } from '@testing-library/react';
|
||||
import userEvent from '@testing-library/user-event';
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import { IntlTestWrapper } from '../i18n/test-utils';
|
||||
import type {
|
||||
Message,
|
||||
NotificationEvent,
|
||||
ToolRequestMessageContent,
|
||||
ToolResponseMessageContent,
|
||||
} from '../types/message';
|
||||
import { getAnyToolConfirmationData } from '../types/message';
|
||||
import { resolveAcpPermissionRequest } from '../acp/permissionRequests';
|
||||
import ToolCallWithResponse from './ToolCallWithResponse';
|
||||
|
||||
vi.mock('../acp/permissionRequests', () => ({
|
||||
resolveAcpPermissionRequest: vi.fn(),
|
||||
}));
|
||||
|
||||
const toolRequest: ToolRequestMessageContent = {
|
||||
type: 'toolRequest',
|
||||
id: 'tool-1',
|
||||
@@ -100,4 +108,44 @@ describe('ToolCallWithResponse live output', () => {
|
||||
expect(screen.queryByText(/starting/)).not.toBeInTheDocument();
|
||||
expect(await screen.findByText('final result')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('passes the current ACP permission generation through inline approval', async () => {
|
||||
const permissionMessage: Message = {
|
||||
content: [
|
||||
{
|
||||
type: 'actionRequired',
|
||||
data: {
|
||||
actionType: 'toolConfirmation',
|
||||
arguments: { command: 'build' },
|
||||
generation: 'permission-generation-1',
|
||||
id: 'tool-1',
|
||||
toolName: 'developer__shell',
|
||||
},
|
||||
},
|
||||
],
|
||||
created: 0,
|
||||
metadata: { agentVisible: true, userVisible: true },
|
||||
role: 'assistant',
|
||||
};
|
||||
|
||||
render(
|
||||
<ToolCallWithResponse
|
||||
sessionId="session-1"
|
||||
isCancelledMessage={false}
|
||||
toolRequest={toolRequest}
|
||||
isPendingApproval
|
||||
confirmationContent={getAnyToolConfirmationData(permissionMessage)}
|
||||
/>,
|
||||
{ wrapper: IntlTestWrapper }
|
||||
);
|
||||
|
||||
await userEvent.click(screen.getByRole('button', { name: 'Allow Once' }));
|
||||
|
||||
expect(resolveAcpPermissionRequest).toHaveBeenCalledWith(
|
||||
'session-1',
|
||||
'tool-1',
|
||||
'permission-generation-1',
|
||||
'allow_once'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -284,6 +284,7 @@ export default function ToolCallWithResponse({
|
||||
<div className="px-4 pb-2">
|
||||
<ToolApprovalButtons
|
||||
data={{
|
||||
generation: confirmationContent.generation,
|
||||
id: confirmationContent.id,
|
||||
toolName: confirmationContent.toolName,
|
||||
prompt: confirmationContent.prompt ?? undefined,
|
||||
|
||||
@@ -113,6 +113,7 @@ export type ActionRequiredData =
|
||||
| {
|
||||
actionType: 'toolConfirmation';
|
||||
arguments: JsonObject;
|
||||
generation?: string;
|
||||
id: string;
|
||||
prompt?: string | null;
|
||||
toolName: string;
|
||||
@@ -405,6 +406,7 @@ export function getToolConfirmationRequestContent(
|
||||
}
|
||||
|
||||
export interface ToolConfirmationData {
|
||||
generation?: string;
|
||||
id: string;
|
||||
toolName: string;
|
||||
arguments: Record<string, unknown>;
|
||||
@@ -425,6 +427,7 @@ export function getAnyToolConfirmationData(message: Message): ToolConfirmationDa
|
||||
const actionRequired = getToolConfirmationContent(message);
|
||||
if (actionRequired && actionRequired.data.actionType === 'toolConfirmation') {
|
||||
return {
|
||||
generation: actionRequired.data.generation,
|
||||
id: actionRequired.data.id,
|
||||
toolName: actionRequired.data.toolName,
|
||||
arguments: actionRequired.data.arguments,
|
||||
|
||||
Reference in New Issue
Block a user