diff --git a/Cargo.lock b/Cargo.lock index 18c4e2273b..109f4243d4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6951,9 +6951,9 @@ dependencies = [ [[package]] name = "pctx_code_mode" -version = "0.2.2" +version = "0.2.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5788a3f1aaa7bc0fdcbae6d9ff24a080a6cc0d202d8affa8f1c29097e40a504a" +checksum = "1006bf745381b3726592032cbee129f27873296810be199e04dfe759820e96f8" dependencies = [ "futures", "pctx_code_execution_runtime", @@ -6971,9 +6971,9 @@ dependencies = [ [[package]] name = "pctx_codegen" -version = "0.1.1" +version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "aebab01af80ced5df67e44d85c6caa9ccc5d0c6e07e0881e7d573447f1ee0f13" +checksum = "fe8bc4029f5177e4c7a3fd03f702832fe56ef77c34bb165990875fba25980f95" dependencies = [ "biome_formatter", "biome_js_formatter", @@ -7029,9 +7029,9 @@ dependencies = [ [[package]] name = "pctx_executor" -version = "0.1.2" +version = "0.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ba5377dedd1b2f8dd5eed9baed09ae8efe46d131ed4eb1669a579e2588e91386" +checksum = "1d0d78d239a4840d4452bafa4fde7c5ad1f128af85f14c2e3d4c72360551183f" dependencies = [ "deno_core", "deno_resolver", @@ -7053,9 +7053,9 @@ dependencies = [ [[package]] name = "pctx_type_check_runtime" -version = "0.1.1" +version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "71b3e2fa9831b26e71f86d714c08a4b2cb052b594bfae8528efbc6ccd3b0b1ce" +checksum = "46d99b3ee5c8b345ba4038552ed4f6776a7a702e9739d35b99ffcc4eb7647e9b" dependencies = [ "deno_ast", "deno_core", diff --git a/crates/goose-acp/tests/common_tests/mod.rs b/crates/goose-acp/tests/common_tests/mod.rs index 73e1122695..10c7cc55c4 100644 --- a/crates/goose-acp/tests/common_tests/mod.rs +++ b/crates/goose-acp/tests/common_tests/mod.rs @@ -30,7 +30,7 @@ pub async fn run_basic_completion() { let output = session .prompt("what is 1+1", PermissionDecision::Cancel) .await; - assert!(output.text.contains("2")); + assert_eq!(output.text, "2"); expected_session_id.assert_matches(&session.id().0); } @@ -65,7 +65,7 @@ pub async fn run_mcp_http_server() { PermissionDecision::Cancel, ) .await; - assert!(output.text.contains(FAKE_CODE)); + assert_eq!(output.text, FAKE_CODE); expected_session_id.assert_matches(&session.id().0); } @@ -85,7 +85,7 @@ pub async fn run_builtin_and_mcp() { include_str!("../test_data/openai_builtin_execute.txt"), ), ( - r#""writeResult": "Successfully wrote to /tmp/result.txt"#.into(), + r#"\"writeResult\": \"Successfully wrote to /tmp/result.txt"#.into(), include_str!("../test_data/openai_builtin_final.txt"), ), ], @@ -104,10 +104,13 @@ pub async fn run_builtin_and_mcp() { let mut session = S::new(config, openai).await; expected_session_id.set(session.id()); - let _ = session.prompt(prompt, PermissionDecision::Cancel).await; + let output = session.prompt(prompt, PermissionDecision::Cancel).await; + if matches!(output.tool_status, Some(ToolCallStatus::Failed)) || output.text.contains("error") { + panic!("{}", output.text); + } let result = fs::read_to_string("/tmp/result.txt").unwrap_or_default(); - assert!(result.contains(FAKE_CODE)); + assert_eq!(result, format!("{FAKE_CODE}\n")); expected_session_id.assert_matches(&session.id().0); } @@ -215,6 +218,6 @@ pub async fn run_configured_extension() { expected_session_id.set(session.id()); let output = session.prompt(prompt, PermissionDecision::Cancel).await; - assert!(output.text.contains(FAKE_CODE)); + assert_eq!(output.text, FAKE_CODE); expected_session_id.assert_matches(&session.id().0); } diff --git a/crates/goose-acp/tests/fixtures/mod.rs b/crates/goose-acp/tests/fixtures/mod.rs index 271206b1cc..56a834bc53 100644 --- a/crates/goose-acp/tests/fixtures/mod.rs +++ b/crates/goose-acp/tests/fixtures/mod.rs @@ -1,7 +1,6 @@ #![recursion_limit = "256"] #![allow(unused_attributes)] -use assert_json_diff::{assert_json_matches_no_panic, CompareMode, Config}; use async_trait::async_trait; use fs_err as fs; use goose::builtin_extension::register_builtin_extensions; @@ -153,8 +152,9 @@ impl OpenAiFixture { let queue = queue.clone(); let expected_session_id = expected_session_id.clone(); move |req: &wiremock::Request| { - let body = String::from_utf8_lossy(&req.body); + let body = std::str::from_utf8(&req.body).unwrap_or(""); + // Validate session ID header let actual = req .headers .get(SESSION_ID_HEADER) @@ -174,28 +174,30 @@ impl OpenAiFixture { )); } - let (expected_body, response) = { - let mut q = queue.lock().unwrap(); - q.pop_front().unwrap_or_default() - }; - - if body.contains(&expected_body) && !expected_body.is_empty() { + // See if the actual request matches the expected pattern + let mut q = queue.lock().unwrap(); + let (expected_body, response) = q.front().cloned().unwrap_or_default(); + if !expected_body.is_empty() && body.contains(&expected_body) { + q.pop_front(); return ResponseTemplate::new(200) .insert_header("content-type", "text/event-stream") .set_body_string(response); } + drop(q); - // Coerce non-json to allow a uniform JSON diff error response. - let exp = serde_json::from_str(&expected_body) - .unwrap_or(serde_json::Value::String(expected_body.clone())); - let act = serde_json::from_str(&body) - .unwrap_or(serde_json::Value::String(body.to_string())); - let diff = - assert_json_matches_no_panic(&exp, &act, Config::new(CompareMode::Strict)) - .unwrap_err(); + // If there was no body, the request was unexpected. Otherwise, it is a mismatch. + let message = if expected_body.is_empty() { + format!("Unexpected request:\n {}", body) + } else { + format!( + "Expected body to contain:\n {}\n\nActual body:\n {}", + expected_body, body + ) + }; + // Use OpenAI's error response schema so the provider will pass the error through. ResponseTemplate::new(417) .insert_header("content-type", "application/json") - .set_body_json(serde_json::json!({"error": {"message": diff}})) + .set_body_json(serde_json::json!({"error": {"message": message}})) } }) .mount(&mock_server) @@ -464,7 +466,10 @@ where runtime.block_on(fut); }) .unwrap(); - handle.join().unwrap(); + if let Err(err) = handle.join() { + // Re-raise the original panic so the test shows the real failure message. + std::panic::resume_unwind(err); + } } pub mod server; diff --git a/crates/goose/Cargo.toml b/crates/goose/Cargo.toml index 584a7a70ad..3da0200a57 100644 --- a/crates/goose/Cargo.toml +++ b/crates/goose/Cargo.toml @@ -131,7 +131,7 @@ shellexpand = "3.1.1" indexmap = "2.12.0" ignore = { workspace = true } which = { workspace = true } -pctx_code_mode = "^0.2.2" +pctx_code_mode = "^0.2.3" unbinder = "0.1.7" [target.'cfg(target_os = "windows")'.dependencies] diff --git a/crates/goose/src/providers/gcpvertexai.rs b/crates/goose/src/providers/gcpvertexai.rs index 195d185f37..db7b74df43 100644 --- a/crates/goose/src/providers/gcpvertexai.rs +++ b/crates/goose/src/providers/gcpvertexai.rs @@ -26,6 +26,7 @@ use crate::providers::formats::gcpvertexai::{ ModelProvider, RequestContext, DEFAULT_MODEL, KNOWN_MODELS, }; use crate::providers::gcpauth::GcpAuth; +use crate::providers::openai_compatible::map_http_error_to_provider_error; use crate::providers::retry::RetryConfig; use crate::providers::utils::RequestLog; use crate::session_context::SESSION_ID_HEADER; @@ -351,9 +352,8 @@ impl GcpVertexAIProvider { ))); } else { let response_text = response.text().await.unwrap_or_default(); - return Err(ProviderError::RequestFailed(format!( - "Request failed with status {status}: {response_text}" - ))); + let payload = serde_json::from_str::(&response_text).ok(); + return Err(map_http_error_to_provider_error(status, payload)); } } } diff --git a/crates/goose/src/recipe/template_recipe.rs b/crates/goose/src/recipe/template_recipe.rs index c46842bede..f127c4c660 100644 --- a/crates/goose/src/recipe/template_recipe.rs +++ b/crates/goose/src/recipe/template_recipe.rs @@ -41,6 +41,12 @@ fn filter_unparseable_variables(template_variables: &[String]) -> Result ## RPI Workflow In goose, we use a structured RPI workflow using recipes to systematically tackle complex codebase changes. The workflow consists of slash commands that guide goose through disciplined phases of work: -1. `research` – Document what exists today. No opinions. -2. `plan` - Design the change with clear phases and success criteria. -3. `implement` - Execute the plan step by step with verification. -4. `iterate` – (optional) Adjust the plan if necessary. +1. `research_codebase` – Document what exists today. No opinions. +2. `create_plan` - Design the change with clear phases and success criteria. +3. `implement_plan` - Execute the plan step by step with verification. +4. `iterate_plan` – (optional) Adjust the plan if necessary. ```md ┌─────────────────────────────────────────────────────────────────────────────-┐ │ RPI WORKFLOW │ ├─────────────────────────────────────────────────────────────────────────────-┤ │ │ -│ /research "topic" │ +│ /research_codebase "topic" │ │ │ │ │ ├──► Spawns parallel sub-agents: │ │ │ • find_files (rpi-codebase-locator) │ @@ -71,7 +71,7 @@ In goose, we use a structured RPI workflow using recipes to systematically tackl │ │ │ │ └──► Output: thoughts/research/YYYY-MM-DD-HHmm-topic.md │ │ │ -│ /plan "feature/task" │ +│ /create_plan "feature/task" │ │ │ │ │ ├──► Reads research docs │ │ ├──► Asks clarifying questions │ @@ -79,7 +79,7 @@ In goose, we use a structured RPI workflow using recipes to systematically tackl │ │ │ │ └──► Output: thoughts/plans/YYYY-MM-DD-HHmm-description.md │ │ │ -│ /implement "plan path" │ +│ /implement_plan "plan path" │ │ │ │ │ ├──► Executes phase by phase │ │ ├──► Runs verification after each phase │ @@ -87,7 +87,7 @@ In goose, we use a structured RPI workflow using recipes to systematically tackl │ │ │ │ └──► Working code │ │ │ -│ /iterate "plan path" + feedback │ +│ /iterate_plan "plan path" + feedback │ │ │ │ │ ├──► Researches only what changed │ │ ├──► Updates the plan surgically │ @@ -127,10 +127,10 @@ So instead of jumping to implementation, we'll use RPI. The concept of planning before implementation has become a widely accepted practice. However, planning without research can lead to assumptions that come back to bite you. So, in RPI, we begin with research. -I start the prompt with the `/research` command followed by a topic written in natural language +I start the prompt with the `/research_codebase` command followed by a topic written in natural language ``` -/research "look through the cloned goose repo and research how the LLM Tool Discovery is implemented" +/research_codebase "look through the cloned goose repo and research how the LLM Tool Discovery is implemented" ``` @@ -155,7 +155,7 @@ After goose began researching, I noticed that it was researching "tool discovery That wasn't a failure. In fact, this is exactly why research exists. Had I told goose to "remove the LLM Tool Discovery feature", it may have removed our other tool discovery methods as well. Fortunately, catching these types of mistakes early is cheap and easy to recover from. ::: -The output from the `/research` session was a detailed research document: +The output from the `/research_codebase` session was a detailed research document:
./thoughts/research/2025-12-22-llm-tool-selection-strategy.md @@ -186,7 +186,7 @@ It's important to do each phase in a new session to keep the LLM laser focused o ::: ``` -/plan a removal of the Tool Selection Strategy feature +/create_plan a removal of the Tool Selection Strategy feature ``` The **[RPI Create Plan](https://raw.githubusercontent.com/block/goose/refs/heads/main/documentation/src/pages/recipes/data/recipes/rpi-plan.yaml)** recipe starts by reading the research document goose created. @@ -231,14 +231,14 @@ At this point, the plan became the source of truth. The key shift here is that w The plan is explicit enough that someone else could execute it. That's not an accident. Remember that the implementation will be in a fresh new session, so the plan must have enough context to actually execute it. -Again, you as the human need to step in here to review the plan and make sure it's solid. If there's anything amiss, instead of starting over you can run the **[RPI Iterate Plan](https://raw.githubusercontent.com/block/goose/refs/heads/main/documentation/src/pages/recipes/data/recipes/rpi-iterate.yaml)** plan (`/iterate`) with details on what's wrong. goose will then read the existing plan, research only what needs rethinking, propose targeted updates, and edit the plan accordingly. +Again, you as the human need to step in here to review the plan and make sure it's solid. If there's anything amiss, instead of starting over you can run the **[RPI Iterate Plan](https://raw.githubusercontent.com/block/goose/refs/heads/main/documentation/src/pages/recipes/data/recipes/rpi-iterate.yaml)** plan (`/iterate_plan`) with details on what's wrong. goose will then read the existing plan, research only what needs rethinking, propose targeted updates, and edit the plan accordingly. ## Session 3: Implement Only after research and planning are complete should you move to implementation. Pass in the plan document. ``` -/implement thoughts/plans/2025-12-23-remove-tool-selection-strategy.md +/implement_plan thoughts/plans/2025-12-23-remove-tool-selection-strategy.md ``` The **[RPI Implement Plan](https://raw.githubusercontent.com/block/goose/refs/heads/main/documentation/src/pages/recipes/data/recipes/rpi-implement.yaml)** recipe is intentionally boring. In fact, I fell asleep while goose was running it. Implementation should feel mechanical. If it feels creative, something upstream is missing. But knowing that you have a rock solid plan, I advise you to go do something else with your time while goose works (unless there are manual steps in the plan). diff --git a/ui/desktop/src/components/settings/models/subcomponents/SwitchModelModal.tsx b/ui/desktop/src/components/settings/models/subcomponents/SwitchModelModal.tsx index e486f96927..d005534f46 100644 --- a/ui/desktop/src/components/settings/models/subcomponents/SwitchModelModal.tsx +++ b/ui/desktop/src/components/settings/models/subcomponents/SwitchModelModal.tsx @@ -83,7 +83,11 @@ export const SwitchModelModal = ({ const [provider, setProvider] = useState( initialProvider || currentProvider || null ); - const [model, setModel] = useState(currentModel || ''); + // Only use currentModel if we're not switching to a different provider + // Otherwise, let the auto-select logic pick an appropriate model for the new provider + const [model, setModel] = useState( + initialProvider && initialProvider !== currentProvider ? '' : currentModel || '' + ); const [isCustomModel, setIsCustomModel] = useState(false); const [validationErrors, setValidationErrors] = useState({ provider: '', diff --git a/ui/desktop/src/components/settings/providers/ProviderGrid.tsx b/ui/desktop/src/components/settings/providers/ProviderGrid.tsx index 11fade92d6..26397e4718 100644 --- a/ui/desktop/src/components/settings/providers/ProviderGrid.tsx +++ b/ui/desktop/src/components/settings/providers/ProviderGrid.tsx @@ -204,6 +204,7 @@ function ProviderCards({ api_key: '', models: editingProvider.config.models.map((m) => m.name), supports_streaming: editingProvider.config.supports_streaming ?? true, + requires_auth: editingProvider.config.requires_auth ?? true, }; const editable = editingProvider ? editingProvider.isEditable : true; diff --git a/ui/desktop/src/components/settings/providers/modal/subcomponents/forms/CustomProviderForm.tsx b/ui/desktop/src/components/settings/providers/modal/subcomponents/forms/CustomProviderForm.tsx index ddf4e59eff..5c60a365a4 100644 --- a/ui/desktop/src/components/settings/providers/modal/subcomponents/forms/CustomProviderForm.tsx +++ b/ui/desktop/src/components/settings/providers/modal/subcomponents/forms/CustomProviderForm.tsx @@ -3,7 +3,6 @@ import { Input } from '../../../../../ui/input'; import { Select } from '../../../../../ui/Select'; import { Button } from '../../../../../ui/button'; import { SecureStorageNotice } from '../SecureStorageNotice'; -import { Checkbox } from '@radix-ui/themes'; import { UpdateCustomProviderRequest } from '../../../../../../api'; interface CustomProviderFormProps { @@ -24,7 +23,7 @@ export default function CustomProviderForm({ const [apiUrl, setApiUrl] = useState(''); const [apiKey, setApiKey] = useState(''); const [models, setModels] = useState(''); - const [noAuthRequired, setNoAuthRequired] = useState(false); + const [requiresApiKey, setRequiresApiKey] = useState(false); const [supportsStreaming, setSupportsStreaming] = useState(true); const [validationErrors, setValidationErrors] = useState>({}); @@ -40,13 +39,13 @@ export default function CustomProviderForm({ setApiUrl(initialData.api_url); setModels(initialData.models.join(', ')); setSupportsStreaming(initialData.supports_streaming ?? true); - setNoAuthRequired(!(initialData.requires_auth ?? true)); + setRequiresApiKey(initialData.requires_auth ?? true); } }, [initialData]); - const handleNoAuthChange = (checked: boolean) => { - setNoAuthRequired(!!checked); - if (checked) { + const handleRequiresApiKeyChange = (checked: boolean) => { + setRequiresApiKey(checked); + if (!checked) { setApiKey(''); } }; @@ -58,7 +57,7 @@ export default function CustomProviderForm({ if (!displayName) errors.displayName = 'Display name is required'; if (!apiUrl) errors.apiUrl = 'API URL is required'; const existingHadAuth = initialData && (initialData.requires_auth ?? true); - if (!noAuthRequired && !apiKey && !existingHadAuth) errors.apiKey = 'API key is required'; + if (requiresApiKey && !apiKey && !existingHadAuth) errors.apiKey = 'API key is required'; if (!models) errors.models = 'At least one model is required'; if (Object.keys(errors).length > 0) { @@ -78,7 +77,7 @@ export default function CustomProviderForm({ api_key: apiKey, models: modelList, supports_streaming: supportsStreaming, - requires_auth: !noAuthRequired, + requires_auth: requiresApiKey, }); }; @@ -174,22 +173,25 @@ export default function CustomProviderForm({ )}
-
- Authentication +

+ Local LLMs like Ollama typically don't require an API key. +

+
+ handleRequiresApiKeyChange(e.target.checked)} + className="rounded border-borderStandard" /> -
- {!noAuthRequired && ( - <> + {requiresApiKey && ( +
)}
{isEditable && ( @@ -241,15 +243,14 @@ export default function CustomProviderForm({ )}
- setSupportsStreaming(checked as boolean)} + onChange={(e) => setSupportsStreaming(e.target.checked)} + className="rounded border-borderStandard" /> -
diff --git a/ui/desktop/src/hooks/useChatStream.ts b/ui/desktop/src/hooks/useChatStream.ts index efbccdbdd2..415152c903 100644 --- a/ui/desktop/src/hooks/useChatStream.ts +++ b/ui/desktop/src/hooks/useChatStream.ts @@ -705,6 +705,8 @@ export function useChatStream({ async (messageId: string, newContent: string, editType: 'fork' | 'edit' = 'fork') => { const currentState = stateRef.current; + dispatch({ type: 'SET_CHAT_STATE', payload: ChatState.Thinking }); + try { const { forkSession } = await import('../api'); const message = currentState.messages.find((m) => m.id === messageId); @@ -731,6 +733,7 @@ export function useChatStream({ } if (editType === 'fork') { + dispatch({ type: 'SET_CHAT_STATE', payload: ChatState.Idle }); const event = new CustomEvent(AppEvents.SESSION_FORKED, { detail: { newSessionId: targetSessionId, @@ -748,11 +751,61 @@ export function useChatStream({ }); if (sessionResponse.data?.conversation) { + const updatedMessages = [...sessionResponse.data.conversation]; + + if (updatedMessages.length > 0) { + const lastMessage = updatedMessages[updatedMessages.length - 1]; + if (lastMessage.role === 'user') { + lastMessage.content = [{ type: 'text', text: newContent }]; + + for (const content of message.content) { + if (content.type === 'image') { + lastMessage.content.push(content); + } + } + + dispatch({ type: 'SET_MESSAGES', payload: updatedMessages }); + dispatch({ type: 'START_STREAMING' }); + abortControllerRef.current = new AbortController(); + + try { + const placeholderMessage = createUserMessage(newContent); + + const { stream } = await reply({ + body: { + session_id: targetSessionId, + user_message: placeholderMessage, + }, + throwOnError: true, + signal: abortControllerRef.current.signal, + }); + + await streamFromResponse( + stream, + updatedMessages, + dispatch, + onFinish, + targetSessionId + ); + } catch (error) { + if (error instanceof Error && error.name === 'AbortError') { + dispatch({ type: 'SET_CHAT_STATE', payload: ChatState.Idle }); + } else { + throw error; + } + } + return; + } + } + dispatch({ type: 'SET_MESSAGES', payload: sessionResponse.data.conversation }); + await handleSubmit({ msg: newContent, images: [] }); + } else { + await handleSubmit({ msg: newContent, images: [] }); } - await handleSubmit({ msg: newContent, images: [] }); } } catch (error) { + dispatch({ type: 'SET_CHAT_STATE', payload: ChatState.Idle }); const errorMsg = errorMessage(error); console.error('Failed to edit message:', error); const { toastError } = await import('../toasts'); @@ -762,7 +815,7 @@ export function useChatStream({ }); } }, - [sessionId, handleSubmit] + [sessionId, handleSubmit, onFinish] ); const setChatState = useCallback((newState: ChatState) => {