Skip to content

fix: strip stray |CHAT2API| markers in tool call parsing#147

Open
Biogasbottle wants to merge 1 commit into
xiaoY233:mainfrom
Biogasbottle:main
Open

fix: strip stray |CHAT2API| markers in tool call parsing#147
Biogasbottle wants to merge 1 commit into
xiaoY233:mainfrom
Biogasbottle:main

Conversation

@Biogasbottle
Copy link
Copy Markdown

Problem

When using DeepSeek with managed XML tool calling (opencode client), |CHAT2API| markers leak into the response content, causing downstream tool execution to fail.

Root Cause

In both managedXml.parse() and ToolStreamParser.flush(), the parsed XML blocks were not being stripped from the content when:

  1. No valid tool calls were found (tool names not in allowed set)
  2. Stream flushing occurred with residual parsed buffer

The cleanup logic only ran when oolCalls.length > 0, leaving raw |CHAT2API| XML in the response text.

Fix

  • managedXml.ts / managedBracket.ts / anthropicToolUse.ts: Run
    awMatches.reduce(...) to strip matched blocks even when oolCalls.length === 0
  • ToolStreamParser.ts: Use parsed.content (already cleaned) instead of raw his.buffer in lush()

Additional

  • Added mitm/ to .gitignore
  • Added 4 DeepSeek E2E tests (non-stream + stream for deepseek-v4-pro-think-search and deepseek-v4-flash-think)

Copy link
Copy Markdown

@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: 6df9eb06cd

ℹ️ 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".


const shouldReleaseText = !this.emittedToolCall
const text = this.buffer
const text = parsed.content || this.buffer
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve empty parsed content in stream flush

Using parsed.content || this.buffer treats an intentionally empty cleaned result as falsy and falls back to the raw buffered payload. When the parser removes a malformed/unsupported tool block and no user-visible text remains, flush() emits the original |CHAT2API|... block back to clients, so the marker-leak bug still reproduces at end-of-stream. Use a nullish check (parsed.content ?? this.buffer) or equivalent to preserve valid empty strings.

Useful? React with 👍 / 👎.

Comment on lines +69 to +71
const cleanContent = rawMatches.length > 0
? rawMatches.reduce((acc, raw) => acc.replace(raw, ''), parseable).trim()
: content
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Avoid dropping fenced code when cleaning invalid tool blocks

This cleanup is computed from parseable, which is content after stripFencedCodeBlocks(). In the toolCalls.length === 0 path, responses that contain an invalid tool block plus any unrelated triple-backtick code lose the entire fenced section, which is a regression from previous behavior where non-tool content was preserved. Clean against the original content (or otherwise reinsert fenced segments) so only the matched tool block is removed.

Useful? React with 👍 / 👎.

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.

1 participant