From 752c1fbe331d65ff39787e65ed6a8c440a26a629 Mon Sep 17 00:00:00 2001 From: Jasper Date: Wed, 12 Aug 2026 12:21:15 +0200 Subject: [PATCH] fix: bind deeplink recipe parameters to startup session (#11123) --- .../src/custom_requests/recipe.rs | 2 + crates/goose/acp-schema.json | 6 + crates/goose/src/acp/server.rs | 15 +- crates/goose/src/acp/server/new_session.rs | 8 +- crates/goose/src/acp/server/recipe/mod.rs | 8 +- ui/desktop/src/App.test.tsx | 32 ++- ui/desktop/src/App.tsx | 18 +- .../src/__tests__/createSession.test.ts | 88 ++++++ .../src/acp/__tests__/capabilities.test.ts | 25 +- .../acp/__tests__/recipeParamRequests.test.ts | 267 ++++++++++++------ ui/desktop/src/acp/__tests__/sessions.test.ts | 35 ++- ui/desktop/src/acp/capabilities.ts | 23 ++ ui/desktop/src/acp/errors.ts | 16 ++ ui/desktop/src/acp/recipeParamRequests.ts | 100 ++++++- ui/desktop/src/acp/sessions.ts | 4 + ui/desktop/src/sessions.ts | 39 ++- ui/sdk/src/generated/types.gen.ts | 1 + ui/sdk/src/generated/zod.gen.ts | 6 +- 18 files changed, 565 insertions(+), 128 deletions(-) diff --git a/crates/goose-sdk-types/src/custom_requests/recipe.rs b/crates/goose-sdk-types/src/custom_requests/recipe.rs index 101fa230d..7c0003fc3 100644 --- a/crates/goose-sdk-types/src/custom_requests/recipe.rs +++ b/crates/goose-sdk-types/src/custom_requests/recipe.rs @@ -261,6 +261,8 @@ pub struct RecipeListEntryDto { pub struct RequestRecipeParams { pub session_id: String, pub parameters: Vec, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub parameter_scope_id: Option, } #[derive(Debug, Clone, Copy, Default, Serialize, Deserialize, JsonSchema)] diff --git a/crates/goose/acp-schema.json b/crates/goose/acp-schema.json index a99397e5d..7f93204b2 100644 --- a/crates/goose/acp-schema.json +++ b/crates/goose/acp-schema.json @@ -6847,6 +6847,12 @@ "items": { "$ref": "#/$defs/RecipeParameterDto" } + }, + "parameterScopeId": { + "type": [ + "string", + "null" + ] } }, "required": [ diff --git a/crates/goose/src/acp/server.rs b/crates/goose/src/acp/server.rs index 9ef6b077d..6bb3f2597 100644 --- a/crates/goose/src/acp/server.rs +++ b/crates/goose/src/acp/server.rs @@ -243,14 +243,11 @@ fn meta_string( fn agent_capabilities_meta() -> Option { let mut goose = serde_json::Map::new(); + goose.insert("recipeParameterScopes".to_string(), serde_json::json!({})); if cfg!(feature = "local-inference") { goose.insert("localInference".to_string(), serde_json::json!({})); } - if goose.is_empty() { - return None; - } - let mut meta = serde_json::Map::new(); meta.insert("goose".to_string(), serde_json::Value::Object(goose)); Some(meta) @@ -2755,6 +2752,16 @@ print(\"hello, world\") )); } + #[test] + fn test_agent_capabilities_advertise_recipe_parameter_scopes() { + assert_eq!( + agent_capabilities_meta() + .and_then(|meta| meta.get("goose").cloned()) + .and_then(|goose| goose.get("recipeParameterScopes").cloned()), + Some(serde_json::json!({})) + ); + } + #[test] fn test_goose_custom_notifications_capability_reads_client_meta() { let mut goose_meta = serde_json::Map::new(); diff --git a/crates/goose/src/acp/server/new_session.rs b/crates/goose/src/acp/server/new_session.rs index db05ce73b..768dfb17a 100644 --- a/crates/goose/src/acp/server/new_session.rs +++ b/crates/goose/src/acp/server/new_session.rs @@ -122,8 +122,14 @@ impl GooseAcpAgent { recipe: Option<(Recipe, PathBuf)>, meta: NewSessionMetaFields, ) -> Result, agent_client_protocol::Error> { + let recipe_parameter_scope_id = meta_string(args.meta.as_ref(), "recipeParameterScopeId")?; let (rendered, user_recipe_values) = self - .render_recipe_for_session(cx, &session.id, recipe.as_ref()) + .render_recipe_for_session( + cx, + &session.id, + recipe.as_ref(), + recipe_parameter_scope_id.as_deref(), + ) .await?; let recipe_settings = rendered.as_ref().and_then(|r| r.settings.as_ref()); diff --git a/crates/goose/src/acp/server/recipe/mod.rs b/crates/goose/src/acp/server/recipe/mod.rs index 78a99a916..75304a50b 100644 --- a/crates/goose/src/acp/server/recipe/mod.rs +++ b/crates/goose/src/acp/server/recipe/mod.rs @@ -355,13 +355,14 @@ impl GooseAcpAgent { cx: &ConnectionTo, session_id: &str, recipe: Option<&(Recipe, PathBuf)>, + parameter_scope_id: Option<&str>, ) -> Result<(Option, Option>), agent_client_protocol::Error> { let Some((recipe, recipe_dir)) = recipe else { return Ok((None, None)); }; let (rendered, values) = self - .render_recipe_with_params(cx, session_id, recipe, recipe_dir) + .render_recipe_with_params(cx, session_id, recipe, recipe_dir, parameter_scope_id) .await?; Ok((Some(rendered), values)) } @@ -372,6 +373,7 @@ impl GooseAcpAgent { session_id: &str, recipe: &Recipe, recipe_dir: &Path, + parameter_scope_id: Option<&str>, ) -> Result<(Recipe, Option>), agent_client_protocol::Error> { let parameters = recipe.parameters.clone().unwrap_or_default(); @@ -385,7 +387,7 @@ impl GooseAcpAgent { } let response = self - .request_recipe_params(cx, session_id, parameters) + .request_recipe_params(cx, session_id, parameters, parameter_scope_id) .await?; if matches!(response.action, RecipeParamsAction::Cancel) { return Err(recipe_params_cancelled_error()); @@ -403,6 +405,7 @@ impl GooseAcpAgent { cx: &ConnectionTo, session_id: &str, parameters: Vec, + parameter_scope_id: Option<&str>, ) -> Result { let request = RequestRecipeParams { session_id: session_id.to_string(), @@ -410,6 +413,7 @@ impl GooseAcpAgent { .into_iter() .map(RecipeParameterDto::from) .collect(), + parameter_scope_id: parameter_scope_id.map(str::to_string), }; let (tx, rx) = oneshot::channel(); cx.send_request(RequestRecipeParamsMessage(request)) diff --git a/ui/desktop/src/App.test.tsx b/ui/desktop/src/App.test.tsx index 5fbd0a969..162304818 100644 --- a/ui/desktop/src/App.test.tsx +++ b/ui/desktop/src/App.test.tsx @@ -6,10 +6,14 @@ import React from 'react'; import { screen, render, waitFor } from '@testing-library/react'; import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest'; -import { AppInner, resolveSessionInitialMessage } from './App'; +import { AppInner, PairRouteWrapper, resolveSessionInitialMessage } from './App'; import { IntlTestWrapper } from './i18n/test-utils'; import { FeaturesProvider } from './contexts/FeaturesContext'; import { reconnectAcpAfterSystemResume } from './acp/acpConnection'; +import { createSession } from './sessions'; +import { RecipeParameterScopesUnsupportedError } from './acp/errors'; + +const mockToastError = vi.hoisted(() => vi.fn()); // Set up globals for jsdom Object.defineProperty(window, 'location', { @@ -40,7 +44,8 @@ vi.mock('./acp/sessions', () => ({ acpDeleteSession: vi.fn().mockResolvedValue(undefined), })); -vi.mock('./sessions', () => ({ +vi.mock('./sessions', async (importOriginal) => ({ + ...(await importOriginal()), fetchSessionDetails: vi .fn() .mockResolvedValue({ sessionId: 'test', messages: [], metadata: { description: '' } }), @@ -122,6 +127,9 @@ vi.mock('./components/ui/ConfirmationModal', () => ({ vi.mock('react-toastify', () => ({ ToastContainer: () => null, + toast: { + error: mockToastError, + }, })); vi.mock('./components/GoosehintsModal', () => ({ @@ -284,6 +292,26 @@ describe('App Component - Brand New State', () => { expect(mockNavigate).not.toHaveBeenCalled(); }); + it('shows the scoped-parameter incompatibility before returning home', async () => { + mockAppConfig.get.mockImplementation((key: string): string | null => { + if (key === 'GOOSE_WORKING_DIR') return '/test/dir'; + if (key === 'recipeDeeplink') return 'goose://recipe?url=example'; + return null; + }); + vi.mocked(createSession).mockRejectedValueOnce(new RecipeParameterScopesUnsupportedError()); + + render(, { + wrapper: AppInnerTestWrapper, + }); + + await waitFor(() => { + expect(mockToastError).toHaveBeenCalledWith( + 'The connected Goose server does not support securely scoped deeplink recipe parameters. Update the server and try again.' + ); + }); + expect(mockNavigate).toHaveBeenCalledWith('/'); + }); + it('should navigate home when the main process emits new-chat', async () => { mockElectron.getConfig.mockReturnValue({ GOOSE_DEFAULT_PROVIDER: 'openai', diff --git a/ui/desktop/src/App.tsx b/ui/desktop/src/App.tsx index c731e13d0..763af9f36 100644 --- a/ui/desktop/src/App.tsx +++ b/ui/desktop/src/App.tsx @@ -1,18 +1,11 @@ import { useEffect, useState, useRef } from 'react'; import { IpcRendererEvent } from 'electron'; -import { - HashRouter, - Routes, - Route, - useNavigate, - useLocation, - useSearchParams, -} from 'react-router'; +import { HashRouter, Routes, Route, useNavigate, useLocation, useSearchParams } from 'react-router'; import { importNostrSessionFromDeepLink } from './sessionLinks'; import { ErrorUI } from './components/ErrorBoundary'; import { ExtensionInstallModal } from './components/ExtensionInstallModal'; import RecipeParamsModalContainer from './components/RecipeParamsModalContainer'; -import { isRecipeParamsCancelled } from './acp/errors'; +import { isRecipeParamsCancelled, isRecipeParameterScopesUnsupported } from './acp/errors'; import { toast, ToastContainer } from 'react-toastify'; import AnnouncementModal from './components/AnnouncementModal'; import TelemetryConsentPrompt from './components/TelemetryConsentPrompt'; @@ -81,7 +74,7 @@ export function resolveSessionInitialMessage( ); } -const PairRouteWrapper = ({ +export const PairRouteWrapper = ({ activeSessions, }: { activeSessions: Array<{ @@ -144,6 +137,11 @@ const PairRouteWrapper = ({ navigate('/'); return; } + if (isRecipeParameterScopesUnsupported(error)) { + toast.error(error.message); + navigate('/'); + return; + } console.error('Failed to create session:', error); trackErrorWithContext(error, { component: 'PairRouteWrapper', diff --git a/ui/desktop/src/__tests__/createSession.test.ts b/ui/desktop/src/__tests__/createSession.test.ts index 70899e4bb..8f8918ea9 100644 --- a/ui/desktop/src/__tests__/createSession.test.ts +++ b/ui/desktop/src/__tests__/createSession.test.ts @@ -6,6 +6,8 @@ import type { FixedExtensionEntry } from '../components/ConfigContext'; import type { GooseExtension, GooseExtensionEntry } from '@aaif/goose-sdk'; import { getConfiguredGooseExtensions } from '../acp/extensions'; import { acpChatSessionController } from '../acp/chatSessionController'; +import { beginConfiguredRecipeParameterScope } from '../acp/recipeParamRequests'; +import { getAcpFeatureCapabilities } from '../acp/capabilities'; vi.mock('../acp/extensions', async (importOriginal) => { const actual = await importOriginal(); @@ -21,6 +23,14 @@ vi.mock('../acp/chatSessionController', () => ({ }, })); +vi.mock('../acp/recipeParamRequests', () => ({ + beginConfiguredRecipeParameterScope: vi.fn(), +})); + +vi.mock('../acp/capabilities', () => ({ + getAcpFeatureCapabilities: vi.fn(), +})); + const testSession: Session = { id: 'session-1', name: 'untitled', @@ -55,6 +65,9 @@ const gooseExtensionEntry = (name: string): GooseExtensionEntry => ({ const mockedGetConfiguredGooseExtensions = vi.mocked(getConfiguredGooseExtensions); const mockedCreateAcpSession = vi.mocked(acpChatSessionController.createSession); +const mockedBeginConfiguredRecipeParameterScope = vi.mocked(beginConfiguredRecipeParameterScope); +const mockedGetAcpFeatureCapabilities = vi.mocked(getAcpFeatureCapabilities); +const finishConfiguredRecipeParameterScope = vi.fn(); describe('createSession ACP session extensions', () => { beforeEach(() => { @@ -65,6 +78,17 @@ describe('createSession ACP session extensions', () => { ]); mockedCreateAcpSession.mockReset(); mockedCreateAcpSession.mockResolvedValue(testSession); + finishConfiguredRecipeParameterScope.mockReset(); + mockedBeginConfiguredRecipeParameterScope.mockReset(); + mockedBeginConfiguredRecipeParameterScope.mockReturnValue({ + id: 'scope-1', + finish: finishConfiguredRecipeParameterScope, + }); + mockedGetAcpFeatureCapabilities.mockReset(); + mockedGetAcpFeatureCapabilities.mockResolvedValue({ + localInference: false, + recipeParameterScopes: true, + }); }); it('sends non-empty extension configs as ACP session extensions', async () => { @@ -76,6 +100,7 @@ describe('createSession ACP session extensions', () => { expect(mockedCreateAcpSession).toHaveBeenCalledWith('/tmp', [gooseExtension('developer')], { recipeDeeplink: undefined, recipeId: undefined, + recipeParameterScopeId: undefined, }); }); @@ -89,6 +114,7 @@ describe('createSession ACP session extensions', () => { expect(mockedCreateAcpSession).toHaveBeenCalledWith('/tmp', [gooseExtension('developer')], { recipeDeeplink: undefined, recipeId: undefined, + recipeParameterScopeId: undefined, }); }); @@ -101,6 +127,68 @@ describe('createSession ACP session extensions', () => { expect(mockedCreateAcpSession).toHaveBeenCalledWith('/tmp', [], { recipeDeeplink: undefined, recipeId: undefined, + recipeParameterScopeId: undefined, }); }); + + it('scopes startup parameters to recipe deeplink session creation', async () => { + await createSession('/tmp', { recipeDeeplink: 'goose://recipe?url=example' }); + + expect(mockedBeginConfiguredRecipeParameterScope).toHaveBeenCalledOnce(); + expect(mockedCreateAcpSession).toHaveBeenCalledWith('/tmp', [], { + recipeDeeplink: 'goose://recipe?url=example', + recipeId: undefined, + recipeParameterScopeId: 'scope-1', + }); + expect(finishConfiguredRecipeParameterScope).toHaveBeenCalledOnce(); + }); + + it('finishes the deeplink parameter scope when session creation fails', async () => { + mockedCreateAcpSession.mockRejectedValueOnce(new Error('session creation failed')); + + await expect( + createSession('/tmp', { recipeDeeplink: 'goose://recipe?url=example' }) + ).rejects.toThrow('session creation failed'); + + expect(mockedBeginConfiguredRecipeParameterScope).toHaveBeenCalledOnce(); + expect(finishConfiguredRecipeParameterScope).toHaveBeenCalledOnce(); + }); + + it('finishes the deeplink parameter scope when extension lookup fails', async () => { + mockedGetConfiguredGooseExtensions.mockRejectedValueOnce(new Error('extension lookup failed')); + + await expect( + createSession('/tmp', { + recipeDeeplink: 'goose://recipe?url=example', + extensionConfigs: [extensionConfig('developer')], + }) + ).rejects.toThrow('extension lookup failed'); + + expect(mockedBeginConfiguredRecipeParameterScope).toHaveBeenCalledOnce(); + expect(mockedCreateAcpSession).not.toHaveBeenCalled(); + expect(finishConfiguredRecipeParameterScope).toHaveBeenCalledOnce(); + }); + + it('reports incompatible Goose servers before sending scoped parameters', async () => { + mockedGetAcpFeatureCapabilities.mockResolvedValueOnce({ + localInference: false, + recipeParameterScopes: false, + }); + + await expect( + createSession('/tmp', { recipeDeeplink: 'goose://recipe?url=example' }) + ).rejects.toThrow( + 'The connected Goose server does not support securely scoped deeplink recipe parameters. Update the server and try again.' + ); + + expect(mockedCreateAcpSession).not.toHaveBeenCalled(); + expect(finishConfiguredRecipeParameterScope).toHaveBeenCalledOnce(); + }); + + it('does not activate startup parameters for ordinary or recipe-id sessions', async () => { + await createSession('/tmp'); + await createSession('/tmp', { recipeId: 'recipe-1' }); + + expect(mockedBeginConfiguredRecipeParameterScope).not.toHaveBeenCalled(); + }); }); diff --git a/ui/desktop/src/acp/__tests__/capabilities.test.ts b/ui/desktop/src/acp/__tests__/capabilities.test.ts index 5a3f2249a..007439cae 100644 --- a/ui/desktop/src/acp/__tests__/capabilities.test.ts +++ b/ui/desktop/src/acp/__tests__/capabilities.test.ts @@ -1,6 +1,6 @@ import type { InitializeResponse } from '@agentclientprotocol/sdk'; import { describe, expect, it } from 'vitest'; -import { hasLocalInferenceCapability } from '../capabilities'; +import { hasLocalInferenceCapability, hasRecipeParameterScopesCapability } from '../capabilities'; function initializeResponseWithMeta(meta?: unknown): Pick { return { @@ -23,6 +23,29 @@ describe('ACP capabilities', () => { ).toBe(true); }); + it('detects scoped recipe-parameter support from Goose metadata', () => { + expect( + hasRecipeParameterScopesCapability( + initializeResponseWithMeta({ + goose: { + recipeParameterScopes: {}, + }, + }) + ) + ).toBe(true); + }); + + it('treats missing or malformed scoped recipe-parameter metadata as unsupported', () => { + expect(hasRecipeParameterScopesCapability(initializeResponseWithMeta())).toBe(false); + expect(hasRecipeParameterScopesCapability(initializeResponseWithMeta({}))).toBe(false); + expect(hasRecipeParameterScopesCapability(initializeResponseWithMeta({ goose: {} }))).toBe( + false + ); + expect(hasRecipeParameterScopesCapability(initializeResponseWithMeta({ goose: true }))).toBe( + false + ); + }); + it('treats missing local inference metadata as unsupported', () => { expect(hasLocalInferenceCapability(initializeResponseWithMeta())).toBe(false); expect(hasLocalInferenceCapability(initializeResponseWithMeta({}))).toBe(false); diff --git a/ui/desktop/src/acp/__tests__/recipeParamRequests.test.ts b/ui/desktop/src/acp/__tests__/recipeParamRequests.test.ts index b2f9511ea..f3e28a92d 100644 --- a/ui/desktop/src/acp/__tests__/recipeParamRequests.test.ts +++ b/ui/desktop/src/acp/__tests__/recipeParamRequests.test.ts @@ -1,15 +1,15 @@ import type { RequestRecipeParams_unstable } from '@aaif/goose-sdk'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { - cancelAcpRecipeParamRequest, - getAcpRecipeParamRequestsSnapshot, - requestAcpRecipeParams, - resolveAcpRecipeParamRequest, -} from '../recipeParamRequests'; -function recipeParamRequest(): RequestRecipeParams_unstable { +type RecipeParamRequestsModule = typeof import('../recipeParamRequests'); + +function recipeParamRequest( + sessionId = 'session-1', + parameterScopeId?: string +): RequestRecipeParams_unstable { return { - sessionId: 'session-1', + sessionId, + parameterScopeId, parameters: [ { key: 'topic', @@ -21,113 +21,198 @@ function recipeParamRequest(): RequestRecipeParams_unstable { }; } -function optionalRecipeParamRequest(): RequestRecipeParams_unstable { - return { - sessionId: 'session-1', - parameters: [ - { - key: 'tone', - description: 'Tone', - input_type: 'string', - requirement: 'optional', - default: 'concise', - }, - ], - }; -} - -function setRecipeParameters(values: Record): void { +function setRecipeParameters(values: Record) { + const get = vi.fn((key: string) => (key === 'recipeParameters' ? values : undefined)); Object.defineProperty(window, 'appConfig', { configurable: true, - value: { - get: vi.fn((key: string) => (key === 'recipeParameters' ? values : undefined)), - }, + value: { get }, }); -} - -function cancelPendingRecipeParamRequests(): void { - for (const request of getAcpRecipeParamRequestsSnapshot()) { - cancelAcpRecipeParamRequest(request.id); - } + return get; } describe('ACP recipe param requests', () => { - beforeEach(() => { - cancelPendingRecipeParamRequests(); + let requests: RecipeParamRequestsModule; + + beforeEach(async () => { + vi.resetModules(); + requests = await import('../recipeParamRequests'); }); afterEach(() => { - cancelPendingRecipeParamRequests(); + for (const request of requests.getAcpRecipeParamRequestsSnapshot()) { + requests.cancelAcpRecipeParamRequest(request.id); + } Reflect.deleteProperty(window, 'appConfig'); }); - it('keeps missing user_prompt parameters pending for user input', async () => { - setRecipeParameters({}); - - const response = requestAcpRecipeParams(recipeParamRequest()); - const [pendingRequest] = getAcpRecipeParamRequestsSnapshot(); - - expect(pendingRequest).toMatchObject({ - sessionId: 'session-1', - parameters: [ - { - key: 'topic', - requirement: 'user_prompt', - }, - ], - initialValues: {}, - }); - - cancelAcpRecipeParamRequest(pendingRequest.id); - await expect(response).resolves.toEqual({ action: 'cancel' }); - }); - - it('keeps user_prompt parameters pending when configured values are available', async () => { + it('keeps ordinary requests pending without startup values', async () => { setRecipeParameters({ topic: 'release notes' }); - const response = requestAcpRecipeParams(recipeParamRequest()); - const [pendingRequest] = getAcpRecipeParamRequestsSnapshot(); + const response = requests.requestAcpRecipeParams(recipeParamRequest()); + const [pendingRequest] = requests.getAcpRecipeParamRequestsSnapshot(); - expect(pendingRequest).toMatchObject({ - sessionId: 'session-1', - parameters: [ - { - key: 'topic', - requirement: 'user_prompt', - }, - ], - initialValues: { topic: 'release notes' }, + expect(pendingRequest.initialValues).toEqual({}); + requests.resolveAcpRecipeParamRequest(pendingRequest.id, { topic: 'manual value' }); + await expect(response).resolves.toEqual({ + action: 'submit', + values: { topic: 'manual value' }, + }); + }); + + it('offers startup values only to the callback carrying the deeplink scope id', async () => { + setRecipeParameters({ topic: 'release notes' }); + const scope = requests.beginConfiguredRecipeParameterScope()!; + + const otherResponse = requests.requestAcpRecipeParams(recipeParamRequest('session-2')); + const ownerResponse = requests.requestAcpRecipeParams( + recipeParamRequest('session-1', scope.id) + ); + const pending = requests.getAcpRecipeParamRequestsSnapshot(); + + expect(pending.find((request) => request.sessionId === 'session-2')?.initialValues).toEqual({}); + expect(pending.find((request) => request.sessionId === 'session-1')?.initialValues).toEqual({ + topic: 'release notes', }); - resolveAcpRecipeParamRequest(pendingRequest.id, { topic: 'release notes' }); - await expect(response).resolves.toEqual({ + for (const request of pending) { + requests.cancelAcpRecipeParamRequest(request.id); + } + await expect(otherResponse).resolves.toEqual({ action: 'cancel' }); + await expect(ownerResponse).resolves.toEqual({ action: 'cancel' }); + scope.finish(); + }); + + it('allows same-session retries until a terminal response consumes the values', async () => { + setRecipeParameters({ topic: 'release notes' }); + const scope = requests.beginConfiguredRecipeParameterScope()!; + + const firstResponse = requests.requestAcpRecipeParams( + recipeParamRequest('session-1', scope.id) + ); + const retryResponse = requests.requestAcpRecipeParams( + recipeParamRequest('session-1', scope.id) + ); + const [firstRequest, retryRequest] = requests.getAcpRecipeParamRequestsSnapshot(); + + expect(firstRequest.initialValues).toEqual({ topic: 'release notes' }); + expect(retryRequest.initialValues).toEqual({ topic: 'release notes' }); + + requests.resolveAcpRecipeParamRequest(firstRequest.id, { topic: 'release notes' }); + await expect(firstResponse).resolves.toEqual({ action: 'submit', values: { topic: 'release notes' }, }); + expect(requests.getAcpRecipeParamRequestsSnapshot()[0].initialValues).toEqual({}); + + requests.cancelAcpRecipeParamRequest(retryRequest.id); + await expect(retryResponse).resolves.toEqual({ action: 'cancel' }); + scope.finish(); }); - it('keeps optional-only parameters pending for user confirmation', async () => { - setRecipeParameters({}); + it('does not reuse startup values after submission', async () => { + setRecipeParameters({ topic: 'release notes' }); + const scope = requests.beginConfiguredRecipeParameterScope()!; - const response = requestAcpRecipeParams(optionalRecipeParamRequest()); - const [pendingRequest] = getAcpRecipeParamRequestsSnapshot(); + const firstResponse = requests.requestAcpRecipeParams( + recipeParamRequest('session-1', scope.id) + ); + const [firstRequest] = requests.getAcpRecipeParamRequestsSnapshot(); + requests.resolveAcpRecipeParamRequest(firstRequest.id, { topic: 'release notes' }); + await firstResponse; - expect(pendingRequest).toMatchObject({ - sessionId: 'session-1', - parameters: [ - { - key: 'tone', - requirement: 'optional', - default: 'concise', - }, - ], - initialValues: {}, - }); + const laterResponse = requests.requestAcpRecipeParams( + recipeParamRequest('session-1', scope.id) + ); + const [laterRequest] = requests.getAcpRecipeParamRequestsSnapshot(); + expect(laterRequest.initialValues).toEqual({}); - resolveAcpRecipeParamRequest(pendingRequest.id, { tone: 'detailed' }); - await expect(response).resolves.toEqual({ - action: 'submit', - values: { tone: 'detailed' }, - }); + requests.cancelAcpRecipeParamRequest(laterRequest.id); + await laterResponse; + scope.finish(); + }); + + it('does not reuse startup values after cancellation', async () => { + setRecipeParameters({ topic: 'release notes' }); + const scope = requests.beginConfiguredRecipeParameterScope()!; + + const firstResponse = requests.requestAcpRecipeParams( + recipeParamRequest('session-1', scope.id) + ); + const [firstRequest] = requests.getAcpRecipeParamRequestsSnapshot(); + requests.cancelAcpRecipeParamRequest(firstRequest.id); + await firstResponse; + + const laterResponse = requests.requestAcpRecipeParams( + recipeParamRequest('session-1', scope.id) + ); + const [laterRequest] = requests.getAcpRecipeParamRequestsSnapshot(); + expect(laterRequest.initialValues).toEqual({}); + + requests.cancelAcpRecipeParamRequest(laterRequest.id); + await laterResponse; + scope.finish(); + }); + + it('does not let an unrelated cancellation consume the owner values', async () => { + setRecipeParameters({ topic: 'release notes' }); + const scope = requests.beginConfiguredRecipeParameterScope()!; + + const ownerResponse = requests.requestAcpRecipeParams( + recipeParamRequest('session-1', scope.id) + ); + const otherResponse = requests.requestAcpRecipeParams(recipeParamRequest('session-2')); + const otherRequest = requests + .getAcpRecipeParamRequestsSnapshot() + .find((request) => request.sessionId === 'session-2')!; + requests.cancelAcpRecipeParamRequest(otherRequest.id); + await otherResponse; + + const retryResponse = requests.requestAcpRecipeParams( + recipeParamRequest('session-1', scope.id) + ); + const ownerRequests = requests + .getAcpRecipeParamRequestsSnapshot() + .filter((request) => request.sessionId === 'session-1'); + expect(ownerRequests).toHaveLength(2); + expect(ownerRequests[1].initialValues).toEqual({ topic: 'release notes' }); + + for (const request of ownerRequests) { + requests.cancelAcpRecipeParamRequest(request.id); + } + await ownerResponse; + await retryResponse; + scope.finish(); + }); + + it('consumes startup values when a scope finishes without a callback', async () => { + setRecipeParameters({ topic: 'release notes' }); + const scope = requests.beginConfiguredRecipeParameterScope()!; + scope.finish(); + + const laterResponse = requests.requestAcpRecipeParams( + recipeParamRequest('session-1', scope.id) + ); + const [laterRequest] = requests.getAcpRecipeParamRequestsSnapshot(); + expect(laterRequest.initialValues).toEqual({}); + + requests.cancelAcpRecipeParamRequest(laterRequest.id); + await laterResponse; + }); + + it('reads app configuration only once after consumption', async () => { + const get = setRecipeParameters({ topic: 'release notes' }); + const scope = requests.beginConfiguredRecipeParameterScope()!; + scope.finish(); + + const secondScope = requests.beginConfiguredRecipeParameterScope(); + const laterResponse = requests.requestAcpRecipeParams(recipeParamRequest()); + const [laterRequest] = requests.getAcpRecipeParamRequestsSnapshot(); + + expect(get).toHaveBeenCalledTimes(1); + expect(secondScope).toBeUndefined(); + expect(laterRequest.initialValues).toEqual({}); + + requests.cancelAcpRecipeParamRequest(laterRequest.id); + await laterResponse; }); }); diff --git a/ui/desktop/src/acp/__tests__/sessions.test.ts b/ui/desktop/src/acp/__tests__/sessions.test.ts index c3437c945..a88d17677 100644 --- a/ui/desktop/src/acp/__tests__/sessions.test.ts +++ b/ui/desktop/src/acp/__tests__/sessions.test.ts @@ -1,7 +1,12 @@ import type { SessionInfo } from '@agentclientprotocol/sdk'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { getAcpClient } from '../acpConnection'; -import { acpGetSessionListItem, acpLoadSession, sessionInfoToSession } from '../sessions'; +import { + acpGetSessionListItem, + acpLoadSession, + acpNewSession, + sessionInfoToSession, +} from '../sessions'; vi.mock('../acpConnection', () => ({ getAcpClient: vi.fn(), @@ -76,6 +81,34 @@ describe('ACP sessions', () => { ); }); + it('carries the recipe parameter scope id in new-session metadata', async () => { + const createdSessionInfo = sessionInfo(); + const client = { + goose: { + sessionInfo_unstable: vi.fn().mockResolvedValue({ session: createdSessionInfo }), + }, + newSession: vi.fn().mockResolvedValue({ sessionId: 'session-1' }), + }; + vi.mocked(getAcpClient).mockResolvedValue( + client as unknown as Awaited> + ); + + await acpNewSession('/tmp', [], { + recipeDeeplink: 'goose://recipe?url=example', + recipeParameterScopeId: 'scope-1', + }); + + expect(client.newSession).toHaveBeenCalledWith({ + cwd: '/tmp', + mcpServers: [], + _meta: { + client: 'goose-desktop', + recipeDeeplink: 'goose://recipe?url=example', + recipeParameterScopeId: 'scope-1', + }, + }); + }); + it('returns a list item from ACP session info', async () => { const client = { goose: { diff --git a/ui/desktop/src/acp/capabilities.ts b/ui/desktop/src/acp/capabilities.ts index bc5c6b9ed..f91c1d5ab 100644 --- a/ui/desktop/src/acp/capabilities.ts +++ b/ui/desktop/src/acp/capabilities.ts @@ -3,6 +3,7 @@ import { getAcpInitializeResponse } from './acpConnection'; export interface AcpFeatureCapabilities { localInference: boolean; + recipeParameterScopes: boolean; } export async function getAcpFeatureCapabilities(): Promise { @@ -10,6 +11,7 @@ export async function getAcpFeatureCapabilities(): Promise +): boolean { + const agentCapabilities = initializeResponse.agentCapabilities; + if (!agentCapabilities) { + return false; + } + + const meta = agentCapabilities._meta; + if (!isRecord(meta)) { + return false; + } + + const goose = meta.goose; + if (!isRecord(goose)) { + return false; + } + + return 'recipeParameterScopes' in goose; +} + function isRecord(value: unknown): value is Record { return typeof value === 'object' && value !== null; } diff --git a/ui/desktop/src/acp/errors.ts b/ui/desktop/src/acp/errors.ts index f94719e83..16dd95f3a 100644 --- a/ui/desktop/src/acp/errors.ts +++ b/ui/desktop/src/acp/errors.ts @@ -8,6 +8,22 @@ const CREDITS_EXHAUSTED_REASON = 'credits_exhausted'; // Kept in sync with RECIPE_PARAMS_CANCELLED_REASON in crates/goose/src/acp/server/recipe.rs. const RECIPE_PARAMS_CANCELLED_REASON = 'recipe_params_cancelled'; +export const RECIPE_PARAMETER_SCOPES_UNSUPPORTED_MESSAGE = + 'The connected Goose server does not support securely scoped deeplink recipe parameters. Update the server and try again.'; + +export class RecipeParameterScopesUnsupportedError extends Error { + constructor() { + super(RECIPE_PARAMETER_SCOPES_UNSUPPORTED_MESSAGE); + this.name = 'RecipeParameterScopesUnsupportedError'; + } +} + +export function isRecipeParameterScopesUnsupported( + error: unknown +): error is RecipeParameterScopesUnsupportedError { + return error instanceof RecipeParameterScopesUnsupportedError; +} + export function isRecipeParamsCancelled(error: unknown): boolean { return asAcpJsonRpcError(error)?.data?.reason === RECIPE_PARAMS_CANCELLED_REASON; } diff --git a/ui/desktop/src/acp/recipeParamRequests.ts b/ui/desktop/src/acp/recipeParamRequests.ts index 1fcbcbafa..0b9a465f2 100644 --- a/ui/desktop/src/acp/recipeParamRequests.ts +++ b/ui/desktop/src/acp/recipeParamRequests.ts @@ -15,11 +15,28 @@ export interface AcpRecipeParamRequest { interface PendingRecipeParamRequest { request: AcpRecipeParamRequest; resolve: (response: RecipeParamsResponse_unstable) => void; + usesConfiguredParameters: boolean; +} + +type ConfiguredParameterState = + | { status: 'uninitialized' } + | { + status: 'active'; + scopeId: string; + values: Record; + sessionId?: string; + } + | { status: 'consumed' }; + +export interface ConfiguredRecipeParameterScope { + id: string; + finish(): void; } const pendingRequests = new Map(); const listeners = new Set<() => void>(); let snapshot: AcpRecipeParamRequest[] = []; +let configuredParameterState: ConfiguredParameterState = { status: 'uninitialized' }; function emit(): void { snapshot = Array.from(pendingRequests.values(), (pending) => pending.request); @@ -39,17 +56,82 @@ export function getAcpRecipeParamRequestsSnapshot(): AcpRecipeParamRequest[] { return snapshot; } -function configuredParameterValues(): Record { +function consumeConfiguredParameters(): boolean { + if (configuredParameterState.status !== 'active') { + return false; + } + + const sessionId = configuredParameterState.sessionId; + configuredParameterState = { status: 'consumed' }; + let scrubbedPendingRequest = false; + if (sessionId) { + for (const pending of pendingRequests.values()) { + if (pending.usesConfiguredParameters && pending.request.sessionId === sessionId) { + pending.request.initialValues = {}; + scrubbedPendingRequest = true; + } + } + } + return scrubbedPendingRequest; +} + +export function beginConfiguredRecipeParameterScope(): ConfiguredRecipeParameterScope | undefined { + if (configuredParameterState.status !== 'uninitialized') { + return undefined; + } + const configured = window.appConfig?.get('recipeParameters') as | Record | undefined; - return configured ?? {}; + if (!configured || Object.keys(configured).length === 0) { + configuredParameterState = { status: 'consumed' }; + return undefined; + } + + const scopeId = `configured_recipe_parameters_${uuidv7()}`; + configuredParameterState = { + status: 'active', + scopeId, + values: { ...configured }, + }; + return { + id: scopeId, + finish: () => { + if ( + configuredParameterState.status === 'active' && + configuredParameterState.scopeId === scopeId && + consumeConfiguredParameters() + ) { + emit(); + } + }, + }; +} + +function configuredParameterValues(request: RequestRecipeParams_unstable): { + values: Record; + usesConfiguredParameters: boolean; +} { + if ( + configuredParameterState.status !== 'active' || + request.parameterScopeId !== configuredParameterState.scopeId + ) { + return { values: {}, usesConfiguredParameters: false }; + } + configuredParameterState.sessionId ??= request.sessionId; + if (configuredParameterState.sessionId !== request.sessionId) { + return { values: {}, usesConfiguredParameters: false }; + } + return { + values: { ...configuredParameterState.values }, + usesConfiguredParameters: true, + }; } export async function requestAcpRecipeParams( request: RequestRecipeParams_unstable ): Promise { - const initialValues = configuredParameterValues(); + const { values: initialValues, usesConfiguredParameters } = configuredParameterValues(request); const paramRequest: AcpRecipeParamRequest = { id: `acp_recipe_params_${uuidv7()}`, sessionId: request.sessionId, @@ -58,7 +140,11 @@ export async function requestAcpRecipeParams( }; return new Promise((resolve) => { - pendingRequests.set(paramRequest.id, { request: paramRequest, resolve }); + pendingRequests.set(paramRequest.id, { + request: paramRequest, + resolve, + usesConfiguredParameters, + }); emit(); }); } @@ -69,6 +155,9 @@ export function resolveAcpRecipeParamRequest(id: string, values: Record { - const selectedNames = new Set(selectedExtensionConfigs(options).map((config) => config.name)); - const gooseExtensions = - selectedNames.size > 0 - ? (await getConfiguredGooseExtensions()) - .filter((entry) => selectedNames.has(gooseExtensionName(entry.extension))) - .map((entry) => entry.extension) - : []; - return acpChatSessionController.createSession(workingDir, gooseExtensions, { - recipeId: options?.recipeId, - recipeDeeplink: options?.recipeDeeplink, - }); + const configuredParameterScope = options?.recipeDeeplink + ? beginConfiguredRecipeParameterScope() + : undefined; + try { + if (configuredParameterScope) { + const capabilities = await getAcpFeatureCapabilities(); + if (!capabilities.recipeParameterScopes) { + throw new RecipeParameterScopesUnsupportedError(); + } + } + const selectedNames = new Set(selectedExtensionConfigs(options).map((config) => config.name)); + const gooseExtensions = + selectedNames.size > 0 + ? (await getConfiguredGooseExtensions()) + .filter((entry) => selectedNames.has(gooseExtensionName(entry.extension))) + .map((entry) => entry.extension) + : []; + return await acpChatSessionController.createSession(workingDir, gooseExtensions, { + recipeId: options?.recipeId, + recipeDeeplink: options?.recipeDeeplink, + recipeParameterScopeId: configuredParameterScope?.id, + }); + } finally { + configuredParameterScope?.finish(); + } } export async function createSession( diff --git a/ui/sdk/src/generated/types.gen.ts b/ui/sdk/src/generated/types.gen.ts index 73b83b4e8..c460fad5d 100644 --- a/ui/sdk/src/generated/types.gen.ts +++ b/ui/sdk/src/generated/types.gen.ts @@ -2622,6 +2622,7 @@ export type MessageUsageUpdate = { export type RequestRecipeParams_unstable = { sessionId: string; parameters: Array; + parameterScopeId?: string | null; }; export type RecipeParamsResponse_unstable = { diff --git a/ui/sdk/src/generated/zod.gen.ts b/ui/sdk/src/generated/zod.gen.ts index 88769d46d..8d4a73993 100644 --- a/ui/sdk/src/generated/zod.gen.ts +++ b/ui/sdk/src/generated/zod.gen.ts @@ -2811,7 +2811,11 @@ export const zGooseSessionNotification_unstable = z.object({ export const zRequestRecipeParams_unstable = z.object({ sessionId: z.string(), - parameters: z.array(zRecipeParameterDto) + parameters: z.array(zRecipeParameterDto), + parameterScopeId: z.union([ + z.string(), + z.null() + ]).optional() }); export const zRecipeParamsAction = z.enum(['submit', 'cancel']);