From 0da1a2026a9ca8661c56dec2953462d1236c4304 Mon Sep 17 00:00:00 2001 From: Manav Sharma <123449950+manavmax@users.noreply.github.com> Date: Mon, 4 May 2026 23:23:03 +0530 Subject: [PATCH] fix(cli)#21297: clear skills consent dialog before reload (#26431) Co-authored-by: Tommaso Sciortino --- .../cli/src/config/extensions/consent.test.ts | 29 +++++++++++++++++ packages/cli/src/config/extensions/consent.ts | 6 +++- .../cli/src/ui/commands/skillsCommand.test.ts | 31 +++++++++++++++++++ packages/cli/src/ui/commands/skillsCommand.ts | 1 + packages/cli/src/ui/commands/types.ts | 2 +- 5 files changed, 67 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/config/extensions/consent.test.ts b/packages/cli/src/config/extensions/consent.test.ts index 8de884cdd5..9bde2705bf 100644 --- a/packages/cli/src/config/extensions/consent.test.ts +++ b/packages/cli/src/config/extensions/consent.test.ts @@ -149,6 +149,35 @@ describe('consent', () => { expect(consent).toBe(expected); }, ); + + it('should clear the active confirmation request before resolving', async () => { + const clearConfirmationRequest = vi.fn(); + const steps: string[] = []; + const addExtensionUpdateConfirmationRequest = vi + .fn() + .mockImplementation((request: ConfirmationRequest) => { + steps.push('prompted'); + request.onConfirm(true); + steps.push('confirmed'); + }); + + const consentPromise = requestConsentInteractive( + 'Test consent', + addExtensionUpdateConfirmationRequest, + () => { + steps.push('cleared'); + clearConfirmationRequest(); + }, + ).then((consent) => { + steps.push('resolved'); + return consent; + }); + + expect(clearConfirmationRequest).toHaveBeenCalledTimes(1); + expect(steps).toEqual(['prompted', 'cleared', 'confirmed']); + await expect(consentPromise).resolves.toBe(true); + expect(steps).toEqual(['prompted', 'cleared', 'confirmed', 'resolved']); + }); }); describe('maybeRequestConsentOrFail', () => { diff --git a/packages/cli/src/config/extensions/consent.ts b/packages/cli/src/config/extensions/consent.ts index 5c35c0d899..b39609b961 100644 --- a/packages/cli/src/config/extensions/consent.ts +++ b/packages/cli/src/config/extensions/consent.ts @@ -78,10 +78,12 @@ export async function requestConsentNonInteractive( export async function requestConsentInteractive( consentDescription: string, addExtensionUpdateConfirmationRequest: (value: ConfirmationRequest) => void, + clearConfirmationRequest?: () => void, ): Promise { return promptForConsentInteractive( consentDescription + '\n\nDo you want to continue?', addExtensionUpdateConfirmationRequest, + clearConfirmationRequest, ); } @@ -129,12 +131,14 @@ export async function promptForConsentNonInteractive( async function promptForConsentInteractive( prompt: string, addExtensionUpdateConfirmationRequest: (value: ConfirmationRequest) => void, + clearConfirmationRequest?: () => void, ): Promise { return new Promise((resolve) => { addExtensionUpdateConfirmationRequest({ prompt, onConfirm: (resolvedConfirmed) => { - resolve(resolvedConfirmed); + clearConfirmationRequest?.(); + setImmediate(() => resolve(resolvedConfirmed)); }, }); }); diff --git a/packages/cli/src/ui/commands/skillsCommand.test.ts b/packages/cli/src/ui/commands/skillsCommand.test.ts index 438f09b182..7cc0629f2e 100644 --- a/packages/cli/src/ui/commands/skillsCommand.test.ts +++ b/packages/cli/src/ui/commands/skillsCommand.test.ts @@ -37,6 +37,7 @@ vi.mock('../../config/extensions/consent.js', async (importOriginal) => { }); import { linkSkill } from '../../utils/skillUtils.js'; +import { requestConsentInteractive } from '../../config/extensions/consent.js'; vi.mock('../../config/settings.js', async (importOriginal) => { const actual = @@ -253,6 +254,36 @@ describe('skillsCommand', () => { ); }); + it('should pass a cleanup callback for interactive workspace consent', async () => { + const linkCmd = skillsCommand.subCommands!.find( + (s) => s.name === 'link', + )!; + context.ui.setConfirmationRequest = vi.fn(); + vi.mocked(linkSkill).mockImplementation( + async (_sourcePath, _scope, _addItem, requestConsent) => { + expect(requestConsent).toBeDefined(); + await requestConsent!( + [{ name: 'test-skill', location: '/path' } as SkillDefinition], + '/workspace/.gemini/skills', + ); + return [{ name: 'test-skill', location: '/path' }]; + }, + ); + + await linkCmd.action!(context, '/some/path --scope workspace'); + + const requestConsentCall = vi + .mocked(requestConsentInteractive) + .mock.calls.at(-1); + expect(requestConsentCall?.[1]).toEqual(expect.any(Function)); + + const clearConfirmationRequest = requestConsentCall?.[2]; + expect(clearConfirmationRequest).toBeTypeOf('function'); + + clearConfirmationRequest?.(); + expect(context.ui.setConfirmationRequest).toHaveBeenCalledWith(null); + }); + it('should show error if link fails', async () => { const linkCmd = skillsCommand.subCommands!.find( (s) => s.name === 'link', diff --git a/packages/cli/src/ui/commands/skillsCommand.ts b/packages/cli/src/ui/commands/skillsCommand.ts index ea1888db40..291186e628 100644 --- a/packages/cli/src/ui/commands/skillsCommand.ts +++ b/packages/cli/src/ui/commands/skillsCommand.ts @@ -118,6 +118,7 @@ async function linkAction( return requestConsentInteractive( consentString, context.ui.setConfirmationRequest.bind(context.ui), + () => context.ui.setConfirmationRequest(null), ); }, ); diff --git a/packages/cli/src/ui/commands/types.ts b/packages/cli/src/ui/commands/types.ts index 328e8fc5e4..266a3bcf02 100644 --- a/packages/cli/src/ui/commands/types.ts +++ b/packages/cli/src/ui/commands/types.ts @@ -89,7 +89,7 @@ export interface CommandContext { * * @param value The confirmation request details. */ - setConfirmationRequest: (value: ConfirmationRequest) => void; + setConfirmationRequest: (value: ConfirmationRequest | null) => void; removeComponent: () => void; toggleBackgroundTasks: () => void; toggleShortcutsHelp: () => void;