fix: render local-command-stdout messages instead of dropping them (#649)

AI disclaimer: I identified this bug after noticing that custom skills
produced no output within the Zed Thread/Claude Agent UI. I used Claude
to investigate the issue and develop the fix. I have manually verified
the equivalent fix against an installed
`@zed-industries/claude-code-acp@0.16.1` (the deprecated package Zed
currently ships): custom slash commands that previously produced empty
threads now render the model's response correctly after Zed restart.

## Summary

The live `case "user" / "assistant"` handler in `src/acp-agent.ts` drops
every SDK message whose string content contains
`<local-command-stdout>`. The branch was added to tolerate `/compact`'s
malformed output, but its side-effect is silencing every successful
slash command invocation — custom user-defined skills (whose expanded
bodies arrive wrapped in `<command-*>` + `<local-command-stdout>`
markers) and built-in commands that emit textual output through these
tags.

## Symptom in ACP clients

User types `/some-command` (custom skill, or one of the affected
built-ins). Thread renders the user's `/some-command` bubble at the top,
then nothing — no expanded skill body, no model response. Under the hood
the model has actually received the expansion and produced a turn, but
the user message carrying the expansion never reaches the UI, and the
cascade of subsequent assistant content gets visually decoupled.

In CLI Claude Code the same expansion is printed to the terminal, so the
asymmetry is purely client-side.

## Fix

This repo already ships `stripLocalCommandMetadata` (and its underlying
`stripMarkerTags` helper) — they return `null` for marker-only payloads
and the stripped prose otherwise. They are already wired into session
replay/load (`acp-agent.ts:1385`) but not into the live handler.

The change wires the same helper into the live handler: strip the
markers and route what remains through the existing `toAcpNotifications`
path, preserving the message's role. Pure-marker payloads (e.g.
`/compact`'s) still no-op via the `null` branch, so the existing
`/compact works` integration test in `src/tests/acp-agent.test.ts`
continues to pass.

## Why the previous behaviour was conservative-but-wrong

The inline comment notes the filter exists for `/compact`'s SDK-side
malformed output. That tradeoff swallows a much larger class of
legitimate output (anything that uses `<local-command-stdout>` to wrap
real prose) to avoid one specific command's noise. With
`stripLocalCommandMetadata` available, the conservative behaviour can
target exactly the empty-after-strip case (still `null` → no-op) without
affecting messages that have real prose alongside markers — which is
exactly the regression case the helper's tests were written to cover
(see `src/tests/acp-agent.test.ts:1145-1163`, *"in the original bug
report the entire /model preamble and the user's real 'hi' prompt were
concatenated into a single message. We want to strip the marker tags and
preserve the real prose, not drop the whole message."*).

The fix applies that same principle to the live handler, where the
helper is most needed.

## Refs

- `zed-industries/claude-code-acp#624` — "'Skill' Session Updates
missing when invoked via slash command". The reporter correctly
diagnoses the mechanism: *"Claude just attaches the skill directly to
context when the /command is used, which means the skill loader isn't
used."* The skill body arrives wrapped in markers and gets dropped by
this filter.
- `zed-industries/claude-code-acp#642` — built-in slash commands
(`/usage`, `/status`, `/model`, `/memory`, `/permissions`, `/agents`,
`/mcp`) emit no output when invoked. Same root cause: their output
arrives in messages tagged with `<local-command-stdout>` and is filtered
out.

Both upstream issues remain open against the old repo
`zed-industries/claude-code-acp` and have no comments / linked PRs yet.
Cross-linking here so this PR can be the closing reference for both.

## Test plan

- [x] `npm run check` (lint + format) — passes
- [x] `npm run build` (tsc) — passes
- [x] `npx vitest run src/tests/acp-agent.test.ts -t
"stripLocalCommandMetadata"` — 8 passed
- [ ] Existing `/compact works` integration test — needs a CLI
environment to run; relies on the `null` branch preserving the no-op
behaviour for marker-only payloads, which the helper tests already cover

🤖 Generated with [Claude Code](https://claude.com/claude-code)
This commit is contained in:
Wandering Consciousness
2026-05-17 09:31:05 +12:00
committed by GitHub
parent eb1259cf88
commit 3b9b7d5a56

View File

@@ -1118,11 +1118,37 @@ export class ClaudeAcpAgent implements Agent {
// Slash commands like /compact can generate invalid output... doesn't match
// their own docs: https://docs.anthropic.com/en/docs/claude-code/sdk/sdk-slash-commands#%2Fcompact-compact-conversation-history
//
// Strip local-command marker tags from the content and render whatever
// real prose remains, so that custom slash commands / user-defined
// skills (whose bodies arrive wrapped in <command-*> / <local-command-stdout>
// markers) and built-in commands that emit textual output reach the UI.
// Previously this branch dropped every message containing a stdout
// marker, which silently swallowed every successful skill invocation
// and any built-in command that emits output through these tags.
// strip-and-render is safe because stripLocalCommandMetadata returns
// null when nothing renderable remains (e.g. pure-marker /compact
// payloads), so the no-render no-op path is preserved for those cases.
//
// Refs zed-industries/claude-code-acp#624, #642.
if (
typeof message.message.content === "string" &&
message.message.content.includes("<local-command-stdout>")
) {
this.logger.log(message.message.content);
const stripped = stripLocalCommandMetadata(message.message.content);
if (typeof stripped === "string") {
for (const notification of toAcpNotifications(
stripped,
message.message.role,
params.sessionId,
this.toolUseCache,
this.client,
this.logger,
)) {
await this.client.sessionUpdate(notification);
}
}
break;
}