diff --git a/.changeset/legacy-acp-optionid-compat.md b/.changeset/legacy-acp-optionid-compat.md new file mode 100644 index 000000000..2da8e7b52 --- /dev/null +++ b/.changeset/legacy-acp-optionid-compat.md @@ -0,0 +1,6 @@ +--- +"@moonshot-ai/acp-adapter": patch +"@moonshot-ai/kimi-code": patch +--- + +Fix ACP custom client tools being auto-rejected when using legacy Python ACP SDK optionId values (`approve`, `approve_for_session`) diff --git a/packages/acp-adapter/src/approval.ts b/packages/acp-adapter/src/approval.ts index 8e183fdae..874116002 100644 --- a/packages/acp-adapter/src/approval.ts +++ b/packages/acp-adapter/src/approval.ts @@ -22,6 +22,17 @@ export const APPROVE_ONCE_OPTION_ID = 'approve_once'; export const APPROVE_ALWAYS_OPTION_ID = 'approve_always'; export const REJECT_OPTION_ID = 'reject'; +/** + * Legacy option ids sent by older ACP clients (e.g. Python ACP SDK 0.8.x). + * Mapped to the canonical ids so that clients built against the older SDK + * continue to work without modification. + */ +const LEGACY_OPTION_ID_MAP: Record = { + approve: APPROVE_ONCE_OPTION_ID, + approve_for_session: APPROVE_ALWAYS_OPTION_ID, + reject: REJECT_OPTION_ID, +}; + /** * Phase 13.2 plan_review optionId namespace. Picked deliberately so the * `plan_*` prefix never collides with the canonical `approve_*` / @@ -151,10 +162,11 @@ export function permissionResponseToApprovalResponse( if (response.outcome.outcome === 'cancelled') { return { decision: 'cancelled' }; } - const optionId = response.outcome.optionId; + const rawOptionId = response.outcome.optionId; if (req?.display.kind === 'plan_review') { - return mapPlanReviewOptionId(req.display, optionId); + return mapPlanReviewOptionId(req.display, rawOptionId); } + const optionId = LEGACY_OPTION_ID_MAP[rawOptionId] ?? rawOptionId; switch (optionId) { case APPROVE_ONCE_OPTION_ID: return { decision: 'approved' }; @@ -310,7 +322,14 @@ export function attachSelectedLabel( ) { return approval; } - const matched = options.find((o) => o.optionId === outcome.optionId); + // Normalize legacy ids (Python ACP SDK 0.8.x sends `approve` / + // `approve_for_session` / `reject`) to their canonical equivalents + // before the lookup. Without this, legacy approvals are correctly + // routed by `permissionResponseToApprovalResponse` but drop the + // human-readable `selectedLabel` (e.g. "Approve for this session"), + // so PermissionResult hooks and approval records lose context. + const normalizedOptionId = LEGACY_OPTION_ID_MAP[outcome.optionId] ?? outcome.optionId; + const matched = options.find((o) => o.optionId === normalizedOptionId); if (!matched) return approval; return { ...approval, selectedLabel: matched.name }; } diff --git a/packages/acp-adapter/test/approval-display.test.ts b/packages/acp-adapter/test/approval-display.test.ts index bbedb07e5..cec2ce548 100644 --- a/packages/acp-adapter/test/approval-display.test.ts +++ b/packages/acp-adapter/test/approval-display.test.ts @@ -259,6 +259,30 @@ describe('attachSelectedLabel', () => { expect(result).toEqual({ decision: 'rejected', selectedLabel: 'Reject' }); }); + it('attaches "Approve once" when the legacy "approve" optionId is selected', () => { + const approval: ApprovalResponse = { decision: 'approved' }; + const result = attachSelectedLabel( + { outcome: { outcome: 'selected', optionId: 'approve' } }, + approval, + options, + ); + expect(result).toEqual({ decision: 'approved', selectedLabel: 'Approve once' }); + }); + + it('attaches "Approve for this session" when the legacy "approve_for_session" optionId is selected', () => { + const approval: ApprovalResponse = { decision: 'approved', scope: 'session' }; + const result = attachSelectedLabel( + { outcome: { outcome: 'selected', optionId: 'approve_for_session' } }, + approval, + options, + ); + expect(result).toEqual({ + decision: 'approved', + scope: 'session', + selectedLabel: 'Approve for this session', + }); + }); + it('returns the input unchanged when the optionId is unknown', () => { const approval: ApprovalResponse = { decision: 'rejected' }; const result = attachSelectedLabel( diff --git a/packages/acp-adapter/test/approval.test.ts b/packages/acp-adapter/test/approval.test.ts index 309466697..3a8001d07 100644 --- a/packages/acp-adapter/test/approval.test.ts +++ b/packages/acp-adapter/test/approval.test.ts @@ -175,6 +175,21 @@ describe('permissionResponseToApprovalResponse', () => { expect(result).toEqual({ decision: 'rejected' }); }); + it('maps legacy "approve" → { decision: approved } (Python ACP SDK compat)', () => { + const result = permissionResponseToApprovalResponse(undefined, { + outcome: { outcome: 'selected', optionId: 'approve' }, + }); + expect(result).toEqual({ decision: 'approved' }); + expect(result.scope).toBeUndefined(); + }); + + it('maps legacy "approve_for_session" → { decision: approved, scope: session } (Python ACP SDK compat)', () => { + const result = permissionResponseToApprovalResponse(undefined, { + outcome: { outcome: 'selected', optionId: 'approve_for_session' }, + }); + expect(result).toEqual({ decision: 'approved', scope: 'session' }); + }); + it('defensively maps an unknown optionId to { decision: rejected }', () => { const result = permissionResponseToApprovalResponse(undefined, { outcome: { outcome: 'selected', optionId: 'unknown_option_id' },