Skip to content

feat: simplify developer extension#7466

Merged
codefromthecrypt merged 4 commits intomainfrom
develop2
Feb 26, 2026
Merged

feat: simplify developer extension#7466
codefromthecrypt merged 4 commits intomainfrom
develop2

Conversation

@baxen
Copy link
Collaborator

@baxen baxen commented Feb 24, 2026

Summary

  • Simplifies the developer extension by removing ~11,400 lines of code across analyze, editor models, text editor, and rmcp developer modules
  • Replaces complex code analysis (analyze/), editor model integrations (editor_models/), and the large rmcp developer implementation with streamlined edit.rs, tree.rs, and shell.rs modules
  • Adds a new platform extension for developer functionality in goose/src/agents/platform_extensions/developer.rs
  • Updates configuration, security scanning, and session handling to support the simplified architecture

Copilot AI review requested due to automatic review settings February 24, 2026 06:20
@baxen
Copy link
Collaborator Author

baxen commented Feb 24, 2026

not ready to merge - i still need to test in the app and do some benchmarking. however creating the PR now to discuss and test

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR dramatically simplifies the developer extension by removing over 11,000 lines of complex code analysis, editor model integrations, and text editor functionality. It replaces these with streamlined implementations focused on core file operations (write, edit), shell command execution, and directory tree visualization.

Changes:

  • Removes entire analyze/ module with tree-sitter parsing, call graph analysis, and language-specific queries
  • Removes editor_models/ with AI-powered code editing (MorphLLM, OpenAI, Relace integrations)
  • Replaces complex text editor with simple edit.rs (exact string find/replace) and write.rs operations
  • Adds new platform extension architecture for developer tools with unprefixed tool names (shell, write, edit, tree)
  • Updates configuration, security scanning, session export, and CLI to handle new tool naming scheme

Reviewed changes

Copilot reviewed 61 out of 62 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
goose/src/agents/platform_extensions/developer.rs New platform extension implementation with simplified tool set
goose-mcp/src/developer/edit.rs Simple file write/edit operations replacing complex text editor
goose-mcp/src/developer/shell.rs Rewritten shell execution with output limiting
goose-mcp/src/developer/tree.rs New directory tree visualization tool
goose/src/security/scanner.rs Updated to recognize new unprefixed tool names
goose/src/config/extensions.rs Added normalization for platform extensions
goose-cli/src/session/export.rs Updated markdown export for new tool names
goose-cli/src/commands/configure.rs Updated to create Platform config for developer extension
Various test files Updated tool names from prefixed to unprefixed format

data: {"id":"chatcmpl-D64NZp69RkEyXdUoDaCBj7fSYll8J","object":"chat.completion.chunk","created":1770339173,"model":"gpt-5-nano-2025-08-07","service_tier":"default","system_fingerprint":null,"choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":" "}}]},"finish_reason":null}],"usage":null,"obfuscation":"XurvUHlgwc"}

data: {"id":"chatcmpl-D64NZp69RkEyXdUoDaCBj7fSYll8J","object":"chat.completion.chunk","created":1770339173,"model":"gpt-5-nano-2025-08-07","service_tier":"default","system_fingerprint":null,"choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":" command"}}]},"finish_reason":null}],"usage":null,"obfuscation":"ZsYLy"}
data: {"id":"chatcmpl-D64NZp69RkEyXdUoDaCBj7fSYll8J","object":"chat.completion.chunk","created":1770339173,"model":"gpt-5-nano-2025-08-07","service_tier":"default","system_fingerprint":null,"choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":" // command"}}]},"finish_reason":null}],"usage":null,"obfuscation":"ZsYLy"}
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

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

The test data file shows the tool call was changed from developer.textEditor to developer.write, but the comment on line 357 still says // command which doesn't match the new tool's parameter naming. The write tool uses content parameter, not command. This comment should likely be removed or updated to // content.

Suggested change
data: {"id":"chatcmpl-D64NZp69RkEyXdUoDaCBj7fSYll8J","object":"chat.completion.chunk","created":1770339173,"model":"gpt-5-nano-2025-08-07","service_tier":"default","system_fingerprint":null,"choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":" // command"}}]},"finish_reason":null}],"usage":null,"obfuscation":"ZsYLy"}
data: {"id":"chatcmpl-D64NZp69RkEyXdUoDaCBj7fSYll8J","object":"chat.completion.chunk","created":1770339173,"model":"gpt-5-nano-2025-08-07","service_tier":"default","system_fingerprint":null,"choices":[{"index":0,"delta":{"tool_calls":[{"index":0,"function":{"arguments":" // content"}}]},"finish_reason":null}],"usage":null,"obfuscation":"ZsYLy"}

Copilot uses AI. Check for mistakes.
Comment on lines 240 to 254
/// The file is created once and rewritten on each call with large output.
fn output_buffer_path() -> Result<PathBuf, String> {
static PATH: Mutex<Option<PathBuf>> = Mutex::new(None);
let mut guard = PATH.lock().map_err(|e| format!("Lock poisoned: {e}"))?;
if let Some(path) = guard.as_ref() {
return Ok(path.clone());
}
let temp_file = tempfile::NamedTempFile::new()
.map_err(|e| format!("Failed to create temp file: {e}"))?;
let (_, path) = temp_file
.keep()
.map_err(|e| format!("Failed to persist temp file: {}", e.error))?;
*guard = Some(path.clone());
Ok(path)
}
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

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

The output_buffer_path() function uses a static Mutex<Option<PathBuf>> to store a single reusable temp file path. This creates a potential race condition where concurrent shell commands could overwrite each other's output before it's read. If two shell commands with large output execute simultaneously, the second call to save_full_output() could overwrite the file while the first command is still reading or returning its path to the user.

Copilot uses AI. Check for mistakes.
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 88568c29f2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +373 to +374
fn is_shell_tool_name(name: &str) -> bool {
matches!(name, "shell")

Choose a reason for hiding this comment

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

P1 Badge Scan prefixed developer shell calls during migration

PromptInjectionScanner now treats only shell as scannable, so any developer__shell call is skipped as non-malicious. That still occurs when a session is created from recipe-provided builtin developer configs (for example goose-self-test.yaml uses type: builtin/name: developer, and recipe extensions bypass normalize_platform_extension in resolve_extensions_for_new_session), which means shell commands in those flows no longer receive prompt-injection analysis. Please accept both names (or normalize recipe extensions) until builtin developer configs are fully removed.

Useful? React with 👍 / 👎.

)
.await?
}
McpCommand::Developer => serve(DeveloperServer::new()).await?,

Choose a reason for hiding this comment

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

P2 Badge Restore PATH/bootstrap wiring for MCP developer server

The developer MCP entrypoint now serves DeveloperServer::new() directly, removing the prior shell bootstrap (extend_path_with_shell(true) and bash_env_file(...)). Because the new shell tool runs /bin/bash -c with inherited environment only, this regresses command resolution in launch contexts with a minimal PATH (e.g., GUI/service launches), leading to avoidable command not found failures for common tools like rg, git, or node.

Useful? React with 👍 / 👎.

This is a major rework focusing on token efficiency, cost, and speed.
We are *removing* a lot of tools, because i think modern models do
better without them.

tldr; we are moving down to just four tools

shell: the usual run a command tool with large output handling.
edit: standard find/replace but terse output
write: create or overwrite a file, also terse output
tree: a portable version of the tree command with line number counts

This also moves the tools out of the mcp cli and into platform, to save
some small but non-zero overhead and to make it easier to terminate long
running shell commands
…se crate

The developer extension is a platform extension that runs in-process,
not an MCP server. Move edit, shell, and tree tool implementations
from goose-mcp/src/developer/ into goose/src/agents/platform_extensions/developer/
where they belong.

This removes the goose -> goose-mcp dependency and consolidates the
developer extension code in a single crate.
Copilot AI review requested due to automatic review settings February 24, 2026 18:25
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 65 out of 67 changed files in this pull request and generated 11 comments.

Comment on lines +105 to +132
fn is_developer_file_tool(tool_name: &str) -> bool {
matches!(tool_name, "write" | "edit")
}

fn extract_tool_locations(
tool_request: &goose::conversation::message::ToolRequest,
tool_response: &goose::conversation::message::ToolResponse,
) -> Vec<ToolCallLocation> {
let mut locations = Vec::new();

if let Ok(tool_call) = &tool_request.tool_call {
if tool_call.name != "developer__text_editor" {
if !is_developer_file_tool(tool_call.name.as_ref()) {
return locations;
}

let tool_name = tool_call.name.as_ref();
let path_str = tool_call
.arguments
.as_ref()
.and_then(|args| args.get("path"))
.and_then(|p| p.as_str());

if let Some(path_str) = path_str {
if matches!(tool_name, "write" | "edit") {
locations.push(create_tool_location(path_str, Some(1)));
return locations;
}

Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

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

extract_tool_locations() only recognizes unprefixed write|edit, but the codebase still contains/records tool calls with legacy names like developer__text_editor; consider accepting both legacy and new names so ACP can still extract locations from older sessions.

Copilot uses AI. Check for mistakes.
Comment on lines +154 to +176
fn build_shell_command(command_line: &str) -> tokio::process::Command {
#[cfg(windows)]
let mut command = {
let mut command = tokio::process::Command::new("cmd");
command.arg("/C").arg(command_line);
command
};

#[cfg(not(windows))]
let mut command = {
let shell = if PathBuf::from("/bin/bash").is_file() {
"/bin/bash".to_string()
} else {
std::env::var("SHELL").unwrap_or_else(|_| "sh".to_string())
};
let mut command = tokio::process::Command::new(shell);
command.arg("-c").arg(command_line);
command
};

command.set_no_window();
command
}
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

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

build_shell_command() only calls set_no_window() and never uses crate::subprocess::configure_subprocess(), so on Unix the shell command won’t be isolated into its own process group; timeouts/kills may leave child processes running (and Ctrl+C behavior can differ). Prefer calling configure_subprocess(&mut command) and ensure timeout cleanup terminates the whole process group.

Copilot uses AI. Check for mistakes.
Comment on lines +178 to +196
async fn collect_merged_output(
stdout: tokio::process::ChildStdout,
stderr: tokio::process::ChildStderr,
) -> Result<String, std::io::Error> {
let stdout = BufReader::new(stdout);
let stderr = BufReader::new(stderr);
let stdout = SplitStream::new(stdout.split(b'\n')).map(|line| ("stdout", line));
let stderr = SplitStream::new(stderr.split(b'\n')).map(|line| ("stderr", line));
let mut merged = stdout.merge(stderr);

let mut output = String::new();
while let Some((_stream, line)) = merged.next().await {
let mut line = line?;
line.push(b'\n');
output.push_str(&String::from_utf8_lossy(&line));
}

Ok(output.trim_end_matches('\n').to_string())
}
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

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

collect_merged_output() accumulates the full stdout/stderr into a String before truncation, so a command with very large output can cause high memory usage/OOM even though render_output() later enforces limits; consider streaming to a temp file once limits are exceeded (or enforcing a hard in-memory cap while collecting).

Copilot uses AI. Check for mistakes.
Comment on lines +538 to +542
matches!(name, "shell")
}

fn is_file_tool_name(name: &str) -> bool {
matches!(name, "write" | "edit")
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

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

is_shell_tool_name() / is_file_tool_name() only recognize unprefixed names, but there are still tool calls emitted as developer__shell in this repo; consider matching both shell and developer__shell (and likewise handling any legacy developer tool names) so older sessions/tool streams still render correctly.

Suggested change
matches!(name, "shell")
}
fn is_file_tool_name(name: &str) -> bool {
matches!(name, "write" | "edit")
matches!(name, "shell" | "developer__shell")
}
fn is_file_tool_name(name: &str) -> bool {
matches!(name, "write" | "edit" | "developer__write" | "developer__edit")

Copilot uses AI. Check for mistakes.
Comment on lines +115 to 136
fn is_shell_tool_name(tool_name: &str) -> bool {
matches!(tool_name, "shell")
}

fn is_developer_file_tool_name(tool_name: &str) -> bool {
matches!(tool_name, "write" | "edit")
}

pub fn tool_request_to_markdown(req: &ToolRequest, export_all_content: bool) -> String {
let mut md = String::new();
match &req.tool_call {
Ok(call) => {
let parts: Vec<_> = call.name.rsplitn(2, "__").collect();
let (namespace, tool_name_only) = if parts.len() == 2 {
(parts[1], parts[0])
} else if is_shell_tool_name(call.name.as_ref())
|| is_developer_file_tool_name(call.name.as_ref())
{
("developer", parts[0])
} else {
("Tool", parts[0])
};
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

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

tool_request_to_markdown() treats only shell and write|edit as “developer” tools when unprefixed; tree is also a developer tool (per the new Developer extension), so exports will label it under the generic “Tool” namespace. Consider extending the developer tool-name matcher to include tree (and ideally the legacy developer__* names for backward compatibility).

Copilot uses AI. Check for mistakes.
Comment on lines 122 to 382
@@ -377,6 +377,10 @@ impl PromptInjectionScanner {
}
}

fn is_shell_tool_name(name: &str) -> bool {
matches!(name, "shell")
}
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

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

is_shell_tool_name() only matches "shell", but this repo still emits tool calls named "developer__shell" (e.g. local inference and provider format tests), so prompt-injection scanning will be skipped for those shell commands; update the matcher to include both legacy and new names (or normalize tool names before scanning).

Copilot uses AI. Check for mistakes.
Comment on lines +207 to +212
fn count_file_lines(path: &Path) -> usize {
match fs::read_to_string(path) {
Ok(content) => content.lines().count(),
Err(_) => 0,
}
}
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

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

count_file_lines() reads each file fully into memory (read_to_string) just to count lines, which can be very expensive for large files and can fail on non-UTF8/binary content; use buffered reading (e.g., BufRead::lines) and/or skip/size-cap large files to avoid large allocations.

Copilot uses AI. Check for mistakes.
Comment on lines +236 to +255
fn output_buffer_path() -> Result<PathBuf, String> {
static PATH: Mutex<Option<PathBuf>> = Mutex::new(None);
let mut guard = PATH.lock().map_err(|e| format!("Lock poisoned: {e}"))?;
if let Some(path) = guard.as_ref() {
return Ok(path.clone());
}
let temp_file =
tempfile::NamedTempFile::new().map_err(|e| format!("Failed to create temp file: {e}"))?;
let (_, path) = temp_file
.keep()
.map_err(|e| format!("Failed to persist temp file: {}", e.error))?;
*guard = Some(path.clone());
Ok(path)
}

fn save_full_output(output: &str) -> Result<PathBuf, String> {
let path = output_buffer_path()?;
std::fs::write(&path, output).map_err(|e| format!("Failed to write output buffer: {e}"))?;
Ok(path)
}
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

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

output_buffer_path() uses a single static temp file for all calls, so concurrent shell invocations can overwrite each other’s saved output (and the path persists for the process lifetime); consider generating a per-invocation temp file (or adding a write lock + unique filenames) to avoid races and confusing/cross-request output.

Copilot uses AI. Check for mistakes.
Comment on lines +106 to +115
map.insert(
developer::EXTENSION_NAME,
PlatformExtensionDef {
name: developer::EXTENSION_NAME,
display_name: "Developer",
description: "Write and edit files, and execute shell commands",
default_enabled: true,
unprefixed_tools: true,
client_factory: |ctx| Box::new(developer::DeveloperClient::new(ctx).unwrap()),
},
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

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

client_factory uses DeveloperClient::new(ctx).unwrap(), which will panic the whole process if construction ever becomes fallible; either make DeveloperClient::new infallible (return Self) or handle the error without panicking (e.g., log and disable the extension).

Copilot uses AI. Check for mistakes.
Comment on lines +272 to 286
const isShellTool = data.tool_name === 'shell';
const isDeveloperFileTool = [
'read',
'write',
'edit'
].includes(data.tool_name);

// Format the arguments
if (data.tool_name === 'developer__shell' && data.arguments.command) {
if (isShellTool && data.arguments.command) {
contentDiv.innerHTML = `<pre><code>${escapeHtml(data.arguments.command)}</code></pre>`;
} else if (data.tool_name === 'developer__text_editor') {
const action = data.arguments.command || 'unknown';
} else if (isDeveloperFileTool) {
const action = data.arguments.command || data.tool_name;
const path = data.arguments.path || 'unknown';
contentDiv.innerHTML = `<div class="tool-param"><strong>action:</strong> ${action}</div>`;
contentDiv.innerHTML += `<div class="tool-param"><strong>path:</strong> ${escapeHtml(path)}</div>`;
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

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

The UI tool-name handling still treats read as a developer file tool, but the new Developer extension tools are write, edit, shell, and tree; keeping read here can mis-render tool calls and hide tree requests. Update the developer-tool lists to reflect the actual tool names.

Copilot uses AI. Check for mistakes.
@alexhancock alexhancock self-requested a review February 24, 2026 19:11
Copy link
Collaborator

@alexhancock alexhancock left a comment

Choose a reason for hiding this comment

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

I have been using this all day so far and it's working well. I say go for it!

Copilot AI review requested due to automatic review settings February 24, 2026 20:42
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 65 out of 67 changed files in this pull request and generated no new comments.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9a63ff9469

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +171 to +174
_cancellation_token: CancellationToken,
) -> Result<CallToolResult, Error> {
let working_dir = working_dir.map(Path::new);
match name {

Choose a reason for hiding this comment

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

P1 Badge Honor cancellation token for shell tool calls

DeveloperClient::call_tool receives a cancellation_token but never uses it when dispatching shell, so cancelling a long-running command (for example tail -f without a timeout) will not interrupt execution and the request can remain stuck until the process exits naturally. This is a regression for interactive sessions where users rely on cancellation to recover from accidental long-running commands.

Useful? React with 👍 / 👎.

Comment on lines +133 to +135
timed_out = true;
let _ = child.start_kill();
let _ = child.wait().await;

Choose a reason for hiding this comment

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

P1 Badge Kill entire process tree when shell timeout expires

When a timeout fires, the code only calls child.start_kill() on the parent shell process, but does not terminate its subprocess tree. Commands that spawn children (e.g. background jobs or nested scripts) can therefore survive the timeout and continue running after the tool reports completion, leaking resources and causing cross-command interference.

Useful? React with 👍 / 👎.

Comment on lines +241 to +245
fn output_buffer_path() -> Result<PathBuf, String> {
static PATH: Mutex<Option<PathBuf>> = Mutex::new(None);
let mut guard = PATH.lock().map_err(|e| format!("Lock poisoned: {e}"))?;
if let Some(path) = guard.as_ref() {
return Ok(path.clone());

Choose a reason for hiding this comment

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

P1 Badge Isolate oversized shell output files per call

output_buffer_path stores one global temp path in a static and reuses it for every truncated output. In multi-session/server environments, later commands overwrite the same file path returned to earlier users, so a user reading their “saved full output” can see another session’s data and lose their own output unexpectedly.

Useful? React with 👍 / 👎.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 64e7dabe47

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines 56 to 59
HashMap::from([
builtin!(developer, DeveloperServer),
builtin!(autovisualiser, AutoVisualiserRouter),
builtin!(computercontroller, ComputerControllerServer),
builtin!(memory, MemoryServer),

Choose a reason for hiding this comment

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

P1 Badge Preserve developer builtin compatibility for recipe execution

Dropping developer from BUILTIN_EXTENSIONS breaks legacy recipes that still declare type: builtin/name: developer (including existing in-repo recipes). Those recipe extension configs are still deserialized as Builtin and passed through session startup paths, so extension loading now fails for developer and runs continue without the required file/shell tools. Keep a compatibility alias here (or normalize recipe extensions before load) so recipe-based workflows do not silently lose developer capabilities.

Useful? React with 👍 / 👎.

Comment on lines +269 to +270
let preview_start = total_lines.saturating_sub(OUTPUT_PREVIEW_LINES);
let preview = lines[preview_start..].join("\n");

Choose a reason for hiding this comment

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

P2 Badge Enforce byte cap on truncated shell preview

When output crosses the byte threshold, the preview is still built from the last 50 full lines with no byte truncation. A command that emits very long lines can therefore return an oversized response even after hitting the byte limit, which defeats the intended guardrail and can inflate context/token usage. Clamp preview size by bytes/chars before returning it.

Useful? React with 👍 / 👎.

@alexhancock alexhancock added this pull request to the merge queue Feb 25, 2026
github-merge-queue bot pushed a commit that referenced this pull request Feb 25, 2026
Co-authored-by: Alex Hancock <alexhancock@block.xyz>
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 25, 2026
@codefromthecrypt codefromthecrypt added this pull request to the merge queue Feb 26, 2026
Merged via the queue into main with commit ced5c1b Feb 26, 2026
27 of 28 checks passed
@codefromthecrypt codefromthecrypt deleted the develop2 branch February 26, 2026 11:43
tlongwell-block added a commit that referenced this pull request Feb 27, 2026
…patible

* origin/main: (70 commits)
  feat: allow goose askai bot to search goose codebase (#7508)
  Revert "Reapply "fix: prevent crashes in long-running Electron sessions""
  Reapply "fix: prevent crashes in long-running Electron sessions"
  Revert "fix: prevent crashes in long-running Electron sessions"
  fix: replace unwrap() with graceful error in scheduler execute_job (#7436)
  fix: Dictation API error message shows incorrect limit (#7423)
  fix(acp): Use ACP schema types for session/list (#7409)
  fix(desktop): make bundle and updater asset naming configurable (#7337)
  fix(openai): preserve order in Responses API history (#7500)
  Use the correct Goose emoji 🪿 instead of Swan in README.md (#7485)
  feat(ui): implement fullscreen and pip display modes for MCP Apps (#7312)
  fix: prevent crashes in long-running Electron sessions
  Disable tool pair summarization (#7481)
  fix: New Recipe Warning does not close on cancel (#7524)
  The client is not the source of truth (#7438)
  feat: support Anthropic adaptive thinking (#7356)
  copilot instructions: reword no prerelease docs (#7101)
  fix(acp): don't fail session creation when model listing is unavailable (#7484)
  feat: simplify developer extension (#7466)
  feat: add goose-powered release notes generator workflow (#7503)
  ...

# Conflicts:
#	Cargo.lock
tlongwell-block added a commit that referenced this pull request Feb 27, 2026
Replace MCP dispatch + GOOSE_HOOK_EXIT workaround with direct subprocess
spawning for hook commands. Leverages PR #7466's new platform developer
shell (build_shell_command, user_login_path) for native exit codes, stdin
piping, and separate stdout/stderr.

New hooks/shell.rs:
- Deadlock-safe I/O: stdout + stderr drain concurrently before stdin write
- Process group isolation (unix) so Ctrl+C doesn't kill hooks
- Timeout + cancellation support with secondary drain timeouts
- 30s stdin write timeout prevents hanging on uncooperative children

Rewritten hooks/mod.rs:
- Split execute_action into execute_command (direct subprocess) and
  execute_mcp_tool (MCP dispatch, unchanged behavior)
- New parse_command_output with native exit code handling:
  exit 0 = JSON or plain text context, exit 2 = block with stderr reason,
  other = fail-open
- Removed GOOSE_HOOK_EXIT marker parsing entirely
- Zero-timeout guard (0 -> 600s) on both Command and McpTool paths

HooksOutcome.reason:
- Added reason field to propagate hook block reasons
- All 3 call sites (agent.rs x2, tool_execution.rs x1) now surface
  the hook's stderr message instead of hardcoded strings

Tests (11 total):
- 9 pure-logic tests for parse_command_output covering every exit code
  path, JSON/non-JSON parsing, truncation, blockable/non-blockable
- 2 real-subprocess tests for run_hook_command: stdin piping via jq,
  exit-2 stderr capture (Claude Code block protocol)

Crossfire reviewed: R1 6/10 -> R2 8/10 -> R3 9/10 APPROVE (Opus+Codex)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants