Conversation
2719415 to
d09537a
Compare
299cc20 to
56e5144
Compare
Co-authored-by: Bennet Bo Fenner <bennetbo@gmx.de>
Co-authored-by: Bennet Bo Fenner <bennetbo@gmx.de>
Co-authored-by: Bennet Bo Fenner <bennetbo@gmx.de>
56e5144 to
bc91040
Compare
I don’t see how killing one of THE main features that made Claude Code integration usable in Zed (the edit-review experience) is brushed under the carpet as “some downsides.” Without the dedicated diff view for agent edits, the ACP integration is completely pointless as it stands. PLEASE REVERT. |
The disallowedTools merge fix from agentclientprotocol#295 was regressed by agentclientprotocol#316 when the options object was restructured. Restore the merge behavior and add regression tests for all three documented merge fields (disallowedTools, hooks, mcpServers). Fixes agentclientprotocol#334
|
Hi @nicojerome I understand you are frustrated, but being the main maintainer of this adapter for months, I didn't do this lightly, and if you look at the code we deleted, we were not only having to replicate entire tools, but also the complete permissions logic. This isn't tenable to maintain, so we won't be reverting. |
The disallowedTools merge fix from agentclientprotocol#295 was regressed by agentclientprotocol#316 when the options object was restructured. Restore the merge behavior and add regression tests for all three documented merge fields (disallowedTools, hooks, mcpServers). Fixes agentclientprotocol#334
The disallowedTools merge fix from agentclientprotocol#295 was regressed by agentclientprotocol#316 when the options object was restructured. Restore the merge behavior and add regression tests for all three documented merge fields (disallowedTools, hooks, mcpServers). Fixes agentclientprotocol#334
Without deep diff integration, what’s the actual value proposition of the Claude Agent ACP now? At this point, I can run Claude Code CLI in a split terminal and get essentially the same experience. Without proper diff separation and highlighting, the Zed chat interface becomes just a thin UI layer rather than a real integration. I’m genuinely curious what the intended benefit is going forward. |
The disallowedTools merge fix from agentclientprotocol#295 was regressed by agentclientprotocol#316 when the options object was restructured. Restore the merge behavior and add regression tests for all three documented merge fields (disallowedTools, hooks, mcpServers). Fixes agentclientprotocol#334
|
Not being able to get Anthropic onboard the ACP is a BIG fail for the project unfortunately. Although I understand the decision, I would rather have an older version of this running on Zed than to lose this functionality. If anyone wants to go back to the latest version with the edit-review experience on Zed, just download the latest version of it and point Zed to it.
Then on Zed settings just set the agent_servers
You can check the location with a |
|
Such a footgun to pull on claude users. :( |
|
This was not enough in my case, also needed to install 0.224.5 AND START IN OFFLINE – the first run self update seems to override my settings to not auto update!!
|
The plan text was accidentally dropped from the ExitPlanMode content array during the switch to built-in Claude Code tools (agentclientprotocol#316). This meant ACP clients could not display the plan when asking the user for approval — they only saw the title "Ready to code?" with no plan body. Restore the original `input.plan` → content mapping so the plan markdown is included in the tool_call and request_permission events. Fixes agentclientprotocol#450
## Summary The plan text was accidentally dropped from the `ExitPlanMode` content array during the switch to built-in Claude Code tools (#316, commit `be618f5`). This meant ACP clients could not display the plan when asking the user for approval — they only saw the title "Ready to code?" with no plan body. We noticed this in the Fabriqa app where our plan approval UI was rendering an empty card. ## What changed Restored the original `input.plan` → `content` mapping in `toolInfoFromToolUse()` so the plan markdown is included in the `tool_call` and `request_permission` events. **Before (broken):** ```typescript case "ExitPlanMode": { return { title: "Ready to code?", kind: "switch_mode", content: [], }; } ``` **After (fixed):** ```typescript case "ExitPlanMode": { const planInput = toolUse.input as { plan?: string }; return { title: "Ready to code?", kind: "switch_mode", content: planInput?.plan ? [{ type: "content" as const, content: { type: "text" as const, text: planInput.plan } }] : [], }; } ``` ## Where does `input.plan` come from? The Claude Agent SDK passes tool inputs to the `canUseTool` callback as `Record<string, unknown>` (see `Options.canUseTool` in `@anthropic-ai/claude-agent-sdk/sdk.d.ts`). When Claude calls `ExitPlanMode`, the model includes the plan text in the input. The formal `ExitPlanModeInput` type (in `@anthropic-ai/claude-agent-sdk/sdk-tools.d.ts`) only declares `allowedPrompts`, but it has a `[k: string]: unknown` index signature that allows additional properties. The model consistently sends `plan` (string) and `planFilePath` (string). SDK v0.2.76 explicitly added `planFilePath` to the tool input for hooks and SDK consumers, confirming these fields are intentional. The original implementation (commit `0289dd9`, "Support plan mode and all other permission modes" #40) relied on `input.plan` and worked correctly until the refactor in #316 dropped it. ### SDK history confirms `input.plan` is reliable The Claude Agent SDK itself had a regression where `ExitPlanMode` input was empty ([anthropics/claude-code#12288](anthropics/claude-code#12288)): | Version | Status | |---------|--------| | Claude Code 2.0.34 | `input.plan` populated | | Claude Code 2.0.51 | Regression — `input: {}` empty | | Agent SDK 0.1.54 | Fixed — `input.plan` restored | | Agent SDK 0.2.76 (current) | Working — also added `planFilePath` | The SDK team treated the empty input as a critical bug and fixed it promptly. The current SDK (0.2.76) reliably sends `input.plan`, so this adapter fix is safe to depend on. ## Test plan - [x] Added test: plan text is included in content when `input.plan` is provided - [x] Added test: content is empty when `input.plan` is not provided - [x] All existing tests pass Fixes #450
Given a number of recent issues stemming from Claude Code using more subagents that don't play nicely with our custom mcp server, plus some slowly diverging behavior, we made the decision to just switch back over to the built-in tools and do more processing on the tool input/output
There are some downsides, but also a lot of upsides, on the whole, it feels like a win for now.
Closes #310, #279, #305, #236, #94