Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
66ae443
added back recipe parameter support
zanesq Jul 19, 2025
8e145d3
fix chat submitting text when unrelated button elements are clicked
zanesq Jul 19, 2025
97732f3
fix navigating away from active recipe resets recipe in progress
zanesq Jul 19, 2025
135ad72
revert button changes
zanesq Jul 20, 2025
e7f8b58
actually use select, boolean and number fields
zanesq Jul 20, 2025
7bba69d
update recipe preview to show all possible recipe values
zanesq Jul 20, 2025
17db8ee
added recipe deeplink to preview
zanesq Jul 20, 2025
4e1988c
change display to one line for deeplink
zanesq Jul 20, 2025
520f448
merge in main and add missing recipe types
zanesq Jul 21, 2025
4644484
change deeplink in recipe preview to use new server version
zanesq Jul 21, 2025
4863743
move previousTitle outside of conditional
zanesq Jul 21, 2025
4e8363c
refactor hasRecipeChanged logic to be more readable
zanesq Jul 21, 2025
353baf3
remove redundant check for previousTitle
zanesq Jul 21, 2025
66dc3a5
simplify complex validation logic
zanesq Jul 21, 2025
67ebebc
remove comment
zanesq Jul 21, 2025
91291e2
preserve existing params and add new ones
zanesq Jul 21, 2025
73017ae
change lookups to be more efficient with maps and set
zanesq Jul 21, 2025
1dbb5fd
remove comment
zanesq Jul 21, 2025
893c2a0
change initialPrompt to return the actual recipe prompt when available
zanesq Jul 21, 2025
0d603a0
Merge branch 'main' of github.com:block/goose into zane/recipe-parame…
zanesq Jul 21, 2025
1a0cd8c
Merge branch 'main' of github.com:block/goose into zane/recipe-parame…
zanesq Jul 22, 2025
8b4fd1d
refactor to shared ChatType
zanesq Jul 22, 2025
c8dc5f6
Merge branch 'zane/recipe-parameters' of github.com:block/goose into …
zanesq Jul 22, 2025
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
5 changes: 5 additions & 0 deletions ui/desktop/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -1827,6 +1827,11 @@
"format": "double",
"description": "Cost per token for output (optional)",
"nullable": true
},
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

upstream changes can ignore

"supports_cache_control": {
"type": "boolean",
"description": "Whether this model supports cache control",
"nullable": true
}
}
},
Expand Down
45 changes: 34 additions & 11 deletions ui/desktop/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,12 @@ const PairRouteWrapper = ({

// Check if we have a resumed session or recipe config from navigation state
useEffect(() => {
// Only process if we actually have navigation state
if (!location.state) {
console.log('No navigation state, preserving existing chat state');
return;
}

const resumedSession = location.state?.resumedSession as SessionDetails | undefined;
const recipeConfig = location.state?.recipeConfig as Recipe | undefined;
const resetChat = location.state?.resetChat as boolean | undefined;
Expand All @@ -205,30 +211,47 @@ const PairRouteWrapper = ({

// Clear the navigation state to prevent reloading on navigation
window.history.replaceState({}, document.title);
} else if (recipeConfig) {
console.log('Loading recipe config in pair view:', recipeConfig.title);
} else if (recipeConfig && resetChat) {
console.log('Loading new recipe config in pair view:', recipeConfig.title);

// Load recipe config and optionally reset chat
// Use the ref to get the current chat state without adding it as a dependency
const currentChat = chatRef.current;
const updatedChat: ChatType = {
...currentChat,
recipeConfig: recipeConfig,
id: chatRef.current.id, // Keep the same ID
title: recipeConfig.title || 'Recipe Chat',
messages: [], // Clear messages to start fresh
messageHistoryIndex: 0,
recipeConfig: recipeConfig,
recipeParameters: null, // Clear parameters for new recipe
};

if (resetChat) {
updatedChat.messages = [];
updatedChat.messageHistoryIndex = 0;
}
// Update both the local chat state and the app-level pairChat state
setChat(updatedChat);
setPairChat(updatedChat);

// Clear the navigation state to prevent reloading on navigation
window.history.replaceState({}, document.title);
} else if (recipeConfig && !chatRef.current.recipeConfig) {
// Only set recipe config if we don't already have one (e.g., from deeplinks)

const updatedChat: ChatType = {
...chatRef.current,
recipeConfig: recipeConfig,
title: recipeConfig.title || chatRef.current.title,
};

// Update both the local chat state and the app-level pairChat state
setChat(updatedChat);
setPairChat(updatedChat);

// Clear the navigation state to prevent reloading on navigation
window.history.replaceState({}, document.title);
} else if (location.state) {
// We have navigation state but it doesn't match our conditions
// Clear it to prevent future processing, but don't modify chat state
console.log('Clearing unprocessed navigation state');
window.history.replaceState({}, document.title);
}
// If we have a recipe config but resetChat is false and we already have a recipe,
// do nothing - just continue with the existing chat state
}, [location.state, setChat, setPairChat]);

return (
Expand Down
4 changes: 4 additions & 0 deletions ui/desktop/src/api/types.gen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,10 @@ export type ModelInfo = {
* Cost per token for output (optional)
*/
output_token_cost?: number | null;
/**
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

upstream changes can ignore

Copy link
Collaborator

Choose a reason for hiding this comment

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

should we maybe have something in CI that checks whether regenerating the openapi.json is a no-op?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not sure if ci can help this other than maybe flagging when someone didn't manually generate it? It should be manually generated whenever someone makes a related change. I run the full npm run start locally which generates the types so occasionally I run into this issue where someone didn't generate the types upstream.

Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah, I meant we could run the typegen in ci and compare if it changes anything and if it does, tell the user they forgot to run it but did make a change

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah gotcha, good idea! Will follow up with that 👍

* Whether this model supports cache control
*/
supports_cache_control?: boolean | null;
};

export type PermissionConfirmationRequest = {
Expand Down
41 changes: 14 additions & 27 deletions ui/desktop/src/components/BaseChat.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ export interface ChatType {
messageHistoryIndex: number;
messages: Message[];
recipeConfig?: Recipe | null; // Add recipe configuration to chat state
recipeParameters?: Record<string, string> | null; // Add recipe parameters to chat state
}

interface BaseChatProps {
Expand Down Expand Up @@ -200,16 +201,25 @@ function BaseChatContent({
// Reset recipe usage tracking when recipe changes
useEffect(() => {
if (recipeConfig?.title !== currentRecipeTitle) {
const previousTitle = currentRecipeTitle;
Copy link
Collaborator

Choose a reason for hiding this comment

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

suggest to move this outside the if, so the comparisons are easier to follow

setCurrentRecipeTitle(recipeConfig?.title || null);
setHasStartedUsingRecipe(false);

// Clear existing messages when a new recipe is loaded
if (recipeConfig?.title && recipeConfig.title !== currentRecipeTitle) {
// Only reset usage if we're switching between different recipes
// Don't reset if we're going from no recipe to a recipe (initial load)
// or if we already have messages (ongoing conversation)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we have this comment because the code is hard to follow; let's see if we can fix that and drop the comment instead

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good call refactored and simplified

if (previousTitle && recipeConfig?.title && previousTitle !== recipeConfig.title) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

here and below, we already checked previousTitle != recipeConfig.title so that bit is always going to be true. So for this to not be true, either previousTitle or recipeConfig?title needs to be falsey. if that's the case we should check on that

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch, fixed

console.log('Switching from recipe:', previousTitle, 'to:', recipeConfig.title);
setHasStartedUsingRecipe(false);
setMessages([]);
setAncestorMessages([]);
} else if (!previousTitle && recipeConfig?.title && messages.length === 0) {
setHasStartedUsingRecipe(false);
// Only clear messages if we don't have any yet
} else if (recipeConfig?.title && messages.length > 0) {
setHasStartedUsingRecipe(true); // Mark as started since we have messages
}
}
}, [recipeConfig?.title, currentRecipeTitle, setMessages, setAncestorMessages]);
}, [recipeConfig?.title, currentRecipeTitle, messages.length, setMessages, setAncestorMessages]);

// Handle recipe auto-execution
useEffect(() => {
Expand Down Expand Up @@ -421,29 +431,6 @@ function BaseChatContent({
{error.message || 'Honk! Goose experienced an error while responding'}
</div>

{/* Expandable Error Details */}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed this expandable error details area as it wasn't providing any other useful context than what is displayed on the screen.

<details className="w-full max-w-2xl mb-2">
<summary className="text-xs text-textSubtle cursor-pointer hover:text-textStandard transition-colors">
Error details
</summary>
<div className="mt-2 p-3 bg-bgSubtle border border-borderSubtle rounded-lg text-xs font-mono text-textStandard">
<div className="mb-2">
<strong>Error Type:</strong> {error.name || 'Unknown'}
</div>
<div className="mb-2">
<strong>Message:</strong> {error.message || 'No message'}
</div>
{error.stack && (
<div>
<strong>Stack Trace:</strong>
<pre className="mt-1 whitespace-pre-wrap text-xs overflow-x-auto">
{error.stack}
</pre>
</div>
)}
</div>
</details>

{/* Regular retry button for non-token-limit errors */}
<div
className="px-3 py-2 mt-2 text-center whitespace-nowrap cursor-pointer text-textStandard border border-borderSubtle hover:bg-bgSubtle rounded-full inline-block transition-all duration-150"
Expand Down
Loading
Loading