fix: remove dead read handler from DeveloperClient#7821
fix: remove dead read handler from DeveloperClient#7821codefromthecrypt merged 1 commit intomainfrom
Conversation
read is only offered in ACP mode (injected by AcpTools::list_tools when fs caps are present). DeveloperClient::call_tool never receives a read call because the extension manager validates tool names against get_tools() first. Signed-off-by: Adrian Cole <[email protected]>
codefromthecrypt
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f62d94364
ℹ️ 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".
| match name { | ||
| "read" => match Self::parse_args::<FileReadParams>(arguments) { | ||
| Ok(params) => Ok(self.edit_tools.file_read_with_cwd(params, working_dir)), | ||
| Err(error) => Ok(CallToolResult::error(vec![Content::text(format!( | ||
| "Error: {error}" | ||
| )) | ||
| .with_priority(0.0)])), | ||
| }, | ||
| "shell" => match Self::parse_args::<ShellParams>(arguments) { |
There was a problem hiding this comment.
read dispatch in DeveloperClient
Removing the "read" arm changes behavior for valid prefixed calls that still reach this client: ExtensionManager::resolve_tool accepts any "<extension>__<tool>" name before consulting list_tools (see resolve_tool), and default extension configs treat an empty available_tools list as "all tools allowed", so a developer__read request can still be dispatched to DeveloperClient::call_tool. Before this change those requests succeeded via file_read_with_cwd; now they return Unknown tool, which is a regression for legacy/manual prefixed tool invocations.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
👎
Since read is no longer in get_tools(), the LLM never sees developer__read as an available tool. It can't call what it doesn't know exists.
your comment is the same as developer__poop yes LLM could hallucinate that.
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
* origin/main: Remove include from Cargo.toml in goose-mcp (#7838) Exit agent loop when tool call JSON fails to parse (#7840) chore: remove redundant husky prepare script (#7829) Add github actions workflow for unused deps (#7681) fix: prevent SSE connection drops from silently truncating responses (#7831) doc: Added notes in contribution guide for pnpm (#7833) add prefer-offline to pnpm config to skip unnecessary registry lookups (#7828) fix: remove dead read handler from DeveloperClient (#7821) fix: prevent SageMaker TGI from being marked configured when only Bedrock keys are set (#7284) fix: disable some computercontroller functionality when no $DISPLAY detected (#7824) test(acp): align provider and server test parity (#7822) fix(acp): register MCP extensions when resuming a session (#7806) # Conflicts: # ui/desktop/src/hooks/useChatStream.ts
* main: (191 commits) fix: add tool_choice and parallel_tool_calls to chatgpt_codex provider (#7867) fix: tool confirmation handling for multiple requests (#7856) Remove dead OllamaSetup onboarding flow (#7861) fix: resolve tokio::sync::Mutex deadlock in recipe retry path (#7832) Upgrade Electron 40.6.0 → 41.0.0 (#7851) Only show up to 50 lines of source code (#7578) fix: stop writing without error when hitting broken pipe for goose session list (#7858) feat(acp): add session/set_mode handler (#7801) Keep messages in sync (#7850) More acp tools (#7843) fix: skip upgrade-insecure-requests CSP for external HTTP backends (#7714) fix(shell): prevent hang when command backgrounds a child process (#7689) Remove include from Cargo.toml in goose-mcp (#7838) Exit agent loop when tool call JSON fails to parse (#7840) chore: remove redundant husky prepare script (#7829) Add github actions workflow for unused deps (#7681) fix: prevent SSE connection drops from silently truncating responses (#7831) doc: Added notes in contribution guide for pnpm (#7833) add prefer-offline to pnpm config to skip unnecessary registry lookups (#7828) fix: remove dead read handler from DeveloperClient (#7821) ...
2 participants
Summary
Remove unreachable
readhandler fromDeveloperClient::call_tool.readis only offered in ACP mode viaAcpTools::list_tools; the extension manager validates tool names againstget_tools(), so this handler was never called.Type of Change
AI Assistance
Testing
Existing tests pass —
developer_tools_are_flatalready asserts["write", "edit", "shell", "tree"].Related Issues
Follow-up to #7668