Skip to content

removed unnecessary call setRecipeParams for acp enabled path #10003

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lifeizhou-ap merged 2 commits into from
Jun 25, 2026
Merged

removed unnecessary call setRecipeParams for acp enabled path #10003

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cec7e7f
removed unnecessary call setRecipeParams for acp enabled path
lifeizhou-ap Jun 25, 2026
397559e
used acp get SessionInfo
lifeizhou-ap Jun 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 37 additions & 1 deletion ui/desktop/src/acp/__tests__/sessions.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { SessionInfo } from '@agentclientprotocol/sdk';
import { beforeEach, describe, expect, it, vi } from 'vitest';
import { getAcpClient } from '../acpConnection';
import { acpLoadSession, sessionInfoToSession } from '../sessions';
import { acpGetSessionListItem, acpLoadSession, sessionInfoToSession } from '../sessions';

vi.mock('../acpConnection', () => ({
getAcpClient: vi.fn(),
Expand Down Expand Up @@ -69,4 +69,40 @@ describe('ACP sessions', () => {
'claude-sonnet-4-5'
);
});

it('returns a list item from ACP session info', async () => {
const client = {
goose: {
sessionInfo_unstable: vi.fn().mockResolvedValue({
session: sessionInfo({
title: 'Subagent session',
_meta: {
createdAt: '2026-01-01T00:00:00Z',
lastMessageAt: '2026-01-01T00:01:00Z',
messageCount: 3,
sessionType: 'sub_agent',
providerId: 'anthropic',
modelId: 'claude-sonnet-4-5',
},
}),
}),
},
};
vi.mocked(getAcpClient).mockResolvedValue(
client as unknown as Awaited<ReturnType<typeof getAcpClient>>
);

const item = await acpGetSessionListItem('session-1');

expect(client.goose.sessionInfo_unstable).toHaveBeenCalledWith({ sessionId: 'session-1' });
expect(item).toMatchObject({
id: 'session-1',
name: 'Subagent session',
workingDir: '/tmp',
messageCount: 3,
lastMessageAt: '2026-01-01T00:01:00Z',
providerId: 'anthropic',
modelId: 'claude-sonnet-4-5',
});
});
});
39 changes: 1 addition & 38 deletions ui/desktop/src/acp/chatSessionController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { v7 as uuidv7 } from 'uuid';
import { updateSessionUserRecipeValues, type Message, type Session } from '../api';
import type { Message, Session } from '../api';
import type { GooseExtension } from '@aaif/goose-sdk';
import { AppEvents } from '../constants/events';
import { ChatState } from '../types/chatState';
Expand Down Expand Up @@ -57,11 +57,6 @@ export interface AcpChatSessionController {
editType: 'fork' | 'edit' | undefined,
options: AcpSubmitMessageOptions
): Promise<void>;
setRecipeUserParams(
sessionId: string,
userRecipeValues: Record<string, string>,
options: AcpSnapshotOptions
): Promise<void>;
}

function createAcpCreditsExhaustedMessage(error: AcpCreditsExhaustedError): Message {
Expand Down Expand Up @@ -271,42 +266,10 @@ async function updateMessage(
}
}

async function setRecipeUserParams(
sessionId: string,
userRecipeValues: Record<string, string>,
options: AcpSnapshotOptions
): Promise<void> {
const currentSession =
options.getCurrentSnapshot()?.session ?? acpChatSessionStore.getSnapshot(sessionId)?.session;

if (currentSession) {
await updateSessionUserRecipeValues({
path: {
session_id: sessionId,
},
body: {
userRecipeValues,
},
throwOnError: true,
});
const updatedSession = {
...currentSession,
user_recipe_values: userRecipeValues,
};
acpChatSessionActions.setSessionMetadata(sessionId, updatedSession);
} else {
acpChatSessionActions.setSessionLoadError(
sessionId,
"can't call setRecipeParams without a session"
);
}
}

export const acpChatSessionController: AcpChatSessionController = {
createSession,
loadSession,
submitMessage,
stop,
updateMessage,
setRecipeUserParams,
};
6 changes: 6 additions & 0 deletions ui/desktop/src/acp/sessions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,12 @@ export async function acpListRecentSessions(maxSessions: number): Promise<Sessio
return response.sessions.slice(0, maxSessions).map(sessionInfoToListItem);
}

export async function acpGetSessionListItem(sessionId: string): Promise<SessionListItem> {
const client = await getAcpClient();
const response = await client.goose.sessionInfo_unstable({ sessionId });
return sessionInfoToListItem(response.session);
}

export async function acpLoadSession(sessionId: string): Promise<AcpLoadSessionResult> {
const pendingLoad = inFlightSessionLoads.get(sessionId);
if (pendingLoad) {
Expand Down
3 changes: 2 additions & 1 deletion ui/desktop/src/components/BaseChat.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,8 @@ export default function BaseChat({
/>
)}

{recipe?.parameters &&
{!USE_ACP_CHAT &&
recipe?.parameters &&
recipe.parameters.length > 0 &&
!session?.user_recipe_values &&
session?.session_type !== 'scheduled' && (
Comment on lines +557 to 561

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep a parameter prompt for loaded ACP recipes

When USE_ACP_CHAT is enabled, this guard removes the only fallback prompt for sessions that already have recipe.parameters but no persisted user_recipe_values (for example legacy/imported recipe sessions or sessions where params were never saved). ACP requests parameters during newSession, but loadSession only returns stored metadata, and useAutoSubmit.hasUnfilledParameters still refuses to submit these sessions, so the user is left with an unstartable recipe chat and no way to fill the missing values.

Useful? React with 👍 / 👎.

@lifeizhou-ap lifeizhou-ap Jun 25, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For ACP, new recipe sessions already request params during newSession. Also, zero-message sessions do not appear in history, so a missing-param zero-message session should not be selectable later.

Expand Down
11 changes: 3 additions & 8 deletions ui/desktop/src/hooks/useAcpChatSession.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,14 +263,9 @@ export function useAcpChatSession({
[getCurrentSnapshot, sessionId]
);

const setRecipeUserParams = useCallback(
async (user_recipe_values: Record<string, string>) => {
await acpChatSessionController.setRecipeUserParams(sessionId, user_recipe_values, {
getCurrentSnapshot,
});
},
[getCurrentSnapshot, sessionId]
);
const setRecipeUserParams = useCallback((_userRecipeValues: Record<string, string>) => {
return Promise.reject(new Error('ACP recipe parameters are handled during session creation'));
}, []);

useEffect(() => {
if (session) {
Expand Down
24 changes: 16 additions & 8 deletions ui/desktop/src/hooks/useNavigationSessions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,21 @@
import { useState, useEffect, useRef, useCallback } from 'react';
import { useNavigate, useLocation, useSearchParams } from 'react-router-dom';
import { getSession } from '../api';
import { useChatContext } from '../contexts/ChatContext';
import { getSessionDisplayName } from '../sessions';
import { AppEvents } from '../constants/events';
import type { Session } from '../api';
import { acpListRecentSessions, type SessionListItem } from '../acp/sessions';
import {
acpGetSessionListItem,
acpListRecentSessions,
type SessionListItem,
} from '../acp/sessions';

const MAX_RECENT_SESSIONS = 25;

export function prependUnique(prev: SessionListItem[], session: SessionListItem): SessionListItem[] {
export function prependUnique(
prev: SessionListItem[],
session: SessionListItem
): SessionListItem[] {
if (prev.some((s) => s.id === session.id)) return prev;
return [session, ...prev].slice(0, MAX_RECENT_SESSIONS);
}
Expand Down Expand Up @@ -74,11 +80,13 @@ export function useNavigationSessions() {
if (!activeSessionId) return;
if (recentSessions.some((s) => s.id === activeSessionId)) return;

getSession({ path: { session_id: activeSessionId }, throwOnError: false }).then((response) => {
if (!response.data) return;
const item = sessionToListItem(response.data as Session);
setRecentSessions((prev) => prependUnique(prev, item));
});
acpGetSessionListItem(activeSessionId)
.then((item) => {
setRecentSessions((prev) => prependUnique(prev, item));
})
.catch((error) => {
console.error('Failed to fetch active session:', error);
});
}, [activeSessionId, recentSessions]);

useEffect(() => {
Expand Down
Loading