Skip to content

fix(acp): correct Claude refusal attribution - #774

Merged
ewen-poch merged 2 commits into
mainfrom
fix/claude-refusal-attribution
Aug 5, 2026
Merged

fix(acp): correct Claude refusal attribution#774
ewen-poch merged 2 commits into
mainfrom
fix/claude-refusal-attribution

Conversation

@ewen-poch

Copy link
Copy Markdown
Member

Problem

A live Claude Code session using a custom Anthropic-compatible provider completed two native WebSearch calls and produced a useful partial answer, then ended with stopReason: "refusal". Claude Code appended fixed Anthropic AUP wording, misleadingly attributing the selected provider refusal to Anthropic.

Proposed change

Replace only the fixed Claude Code AUP prefix with provider-neutral wording while normalizing visible ACP assistant events. Scope the rewrite to Claude Code sessions and preserve the remaining message, tool results, partial answer, and actual stop reason.

Scope and non-goals

  • No provider response or stop-reason rewriting
  • No automatic retry or safety bypass
  • No architecture, persistence, schema, data-model, or data-relationship changes
  • User messages and non-Claude frameworks remain unchanged

Acceptance criteria and validation

All checks below ran after the last material source edit.

  • Targeted behavior: npm test -- src/main/acp/runtime-events.test.ts src/main/acp/session-update-projector.test.ts — 25 passed
  • Node types: npm run typecheck:node — passed
  • Lint: npm run lint — passed with 0 errors and 17 pre-existing warnings outside changed files
  • Formatting: Prettier check on all changed files — passed
  • Full portable suite: npm test — 767 files passed, 14 skipped; 11,359 tests passed, 184 skipped

Renderer consumer coverage is excluded because the event shape is unchanged; only an exact assistant-text prefix is normalized. Platform/E2E lanes are excluded because the change does not touch platform-specific behavior, persistence, or build configuration.

Known residual risk: if Claude Code changes its refusal wording, exact-prefix normalization intentionally stops matching until the prefix is updated.

Review focus

Please verify exact-prefix matching, Claude-only routing, and preservation of user/non-Claude messages.

@github-actions github-actions Bot added the bug Something isn't working label Aug 5, 2026
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codex Review

Verdict: needs changes

[P2] Normalize refusals across streamed message chunks

src/main/acp/runtime-events.ts:134

Impact: If the provider splits the Claude refusal prefix across chunks, no chunk matches the full prefix and the final visible message still exposes Claude Code attribution.

Recommendation: Normalize at the message-stream boundary or retain minimal per-message prefix state; add a split-prefix regression test.

Summary: Static inspection found one user-visible streaming edge case: refusal normalization matches each ACP chunk independently, so a prefix split across valid chunks is not rewritten after reassembly.

@ewen-poch

Copy link
Copy Markdown
Member Author

Thanks. I traced the refusal producer in the pinned @agentclientprotocol/claude-agent-acp@0.60.0. When stop_reason === "refusal", the adapter sends the complete lastRefusalExplanation in one sendUpdate call as a single agent_message_chunk; that value is captured whole from stop_details.explanation. Regular assistant deltas can be split, but this adapter-authored policy message does not use that path.

Adding per-message buffering would introduce delayed output plus flush and lifecycle state for an input the current producer cannot emit, so the exact per-chunk normalization is intentionally kept stateless.

@ewen-poch
ewen-poch force-pushed the fix/claude-refusal-attribution branch from fab8b41 to 24495b4 Compare August 5, 2026 10:45
@ewen-poch

Copy link
Copy Markdown
Member Author

Rebased onto the latest main without conflicts. The source diff is unchanged. Post-rebase validation passes: 26 targeted ACP tests and npm run typecheck:node.

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codex Review

Verdict: needs changes

[P2] Normalize refusals across streamed message chunks

src/main/acp/runtime-events.ts:134

Impact: The check is performed independently on each agent_message_chunk. If ACP splits the fixed Claude refusal prefix across chunks, no chunk starts with the full prefix, so the visible and persisted response still exposes the Claude/Anthropic attribution.

Recommendation: Normalize after assembling the complete assistant message by messageId, or maintain bounded per-message prefix state so the replacement also works when the refusal prefix spans chunks.

Summary: Static review found one concrete stream-handling defect in the refusal normalization.

@ewen-poch

Copy link
Copy Markdown
Member Author

Addressed in 960a274. The exact-prefix normalizer is now shared and re-applied after complete stream assembly in both the renderer session store and TaskRunner output. A regression test splits the prefix across two chunks and verifies both the visible message and persisted projection use the provider-neutral wording.

This keeps the producer fast path while avoiding per-message buffering or lifecycle state. Validation: 124 targeted tests, Node/Web typechecks, lint with 0 errors, formatting, and the full portable suite with 11,385 tests passed.

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codex Review

Verdict: mergeable

No actionable findings.

Summary: No concrete merge-blocking defects found. Branch and pull request title prechecks passed.

@github-actions github-actions Bot added the ready-to-merge All completed AI reviewers found this pull request mergeable. label Aug 5, 2026
@ewen-poch
ewen-poch merged commit dce57c9 into main Aug 5, 2026
23 checks passed
@ewen-poch
ewen-poch deleted the fix/claude-refusal-attribution branch August 5, 2026 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ready-to-merge All completed AI reviewers found this pull request mergeable.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant