Merge remote-tracking branch 'origin/main' into summon_extension

* origin/main:
  fix: detect context length errors in GCP Vertex AI provider (#6976)
  Added the ability to escape Jinja variables in recipes to include them in prompts (#6975)
  Bug Fix: bump pctx (#6967)
  fix(acp): fixtures now raise content mismatch errors (#6912)
  custom provider form minor improvements (#6966)
  Fix 'Edit In Place' feature appending instead of editing messages (#6955)
  docs: change RPI slash commands (#6963)
This commit is contained in:
Tyler Longwell
2026-02-05 08:56:45 -05:00
11 changed files with 165 additions and 84 deletions
Generated
+8 -8
View File
@@ -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",
+9 -6
View File
@@ -30,7 +30,7 @@ pub async fn run_basic_completion<S: Session>() {
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<S: Session>() {
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<S: Session>() {
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<S: Session>() {
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<S: Session>() {
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);
}
+23 -18
View File
@@ -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;
+1 -1
View File
@@ -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]
+3 -3
View File
@@ -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::<Value>(&response_text).ok();
return Err(map_http_error_to_provider_error(status, payload));
}
}
}
@@ -41,6 +41,12 @@ fn filter_unparseable_variables(template_variables: &[String]) -> Result<Vec<Str
let mut vars_to_convert = Vec::new();
for var in template_variables {
let trimmed = var.trim();
if trimmed.starts_with('\'') || trimmed.starts_with('"') {
continue;
}
let mut env = Environment::new();
env.set_undefined_behavior(UndefinedBehavior::Lenient);
@@ -280,5 +286,13 @@ description: "A test recipe"
assert!(result.contains(r#"name: "Simple Recipe""#));
}
#[test]
fn test_jinja_escape_syntax() {
let content = r#"{{'{{param_key}}'}}"#;
let params = HashMap::from([("recipe_dir".to_string(), "test_dir".to_string())]);
let result = render_recipe_content_with_params(content, &params).unwrap();
assert_eq!(result, "{{param_key}}");
}
}
}
+18 -18
View File
@@ -42,27 +42,27 @@ Now that the recipes are imported, to quickly invoke them in-session [add custom
| Recipe | Slash Command |
|--------|---------------|
| RPI Research Codebase | `research` |
| RPI Create Plan | `plan` |
| RPI Implement Plan | `implement` |
| RPI Iterate | `iterate` |
| RPI Research Codebase | `research_codebase`|
| RPI Create Plan | `create_plan` |
| RPI Implement Plan | `implement_plan` |
| RPI Iterate | `iterate_plan` |
</details>
## 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:
<details>
<summary>./thoughts/research/2025-12-22-llm-tool-selection-strategy.md</summary>
@@ -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).
@@ -83,7 +83,11 @@ export const SwitchModelModal = ({
const [provider, setProvider] = useState<string | null>(
initialProvider || currentProvider || null
);
const [model, setModel] = useState<string>(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<string>(
initialProvider && initialProvider !== currentProvider ? '' : currentModel || ''
);
const [isCustomModel, setIsCustomModel] = useState(false);
const [validationErrors, setValidationErrors] = useState({
provider: '',
@@ -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;
@@ -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<Record<string, string>>({});
@@ -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({
)}
<div>
<div className="flex items-center space-x-2 mb-2">
<Checkbox
id="no-auth-required"
checked={noAuthRequired}
onCheckedChange={handleNoAuthChange}
<label className="block text-sm font-medium text-textStandard mb-2">Authentication</label>
<p className="text-sm text-textSubtle mb-3">
Local LLMs like Ollama typically don't require an API key.
</p>
<div className="flex items-center space-x-2">
<input
type="checkbox"
id="requires-api-key"
checked={requiresApiKey}
onChange={(e) => handleRequiresApiKeyChange(e.target.checked)}
className="rounded border-borderStandard"
/>
<label
htmlFor="no-auth-required"
className="text-sm font-medium leading-none peer-disabled:cursor-not-allowed peer-disabled:opacity-70 text-textSubtle"
>
No authentication required
<label htmlFor="requires-api-key" className="text-sm text-textSubtle">
This provider requires an API key
</label>
</div>
{!noAuthRequired && (
<>
{requiresApiKey && (
<div className="mt-3">
<label
htmlFor="api-key"
className="flex items-center text-sm font-medium text-textStandard mb-2"
@@ -212,7 +214,7 @@ export default function CustomProviderForm({
{validationErrors.apiKey}
</p>
)}
</>
</div>
)}
</div>
{isEditable && (
@@ -241,15 +243,14 @@ export default function CustomProviderForm({
)}
</div>
<div className="flex items-center space-x-2 mb-10">
<Checkbox
<input
type="checkbox"
id="supports-streaming"
checked={supportsStreaming}
onCheckedChange={(checked) => setSupportsStreaming(checked as boolean)}
onChange={(e) => setSupportsStreaming(e.target.checked)}
className="rounded border-borderStandard"
/>
<label
htmlFor="supports-streaming"
className="text-sm font-medium leading-none peer-disabled:cursor-not-allowed peer-disabled:opacity-70 text-textSubtle"
>
<label htmlFor="supports-streaming" className="text-sm text-textSubtle">
Provider supports streaming responses
</label>
</div>
+55 -2
View File
@@ -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) => {