Skip to content

fix(feishu): strip leaked thinking content from streaming output#54898

Open
Vicky-v7 wants to merge 3 commits intoopenclaw:mainfrom
Vicky-v7:fix/27717-reply-upgrade-prompt
Open

fix(feishu): strip leaked thinking content from streaming output#54898
Vicky-v7 wants to merge 3 commits intoopenclaw:mainfrom
Vicky-v7:fix/27717-reply-upgrade-prompt

Conversation

@Vicky-v7
Copy link
Copy Markdown

@Vicky-v7 Vicky-v7 commented Mar 26, 2026

Closes #26466 - strips leaked thinking/thought/antthinking content from Feishu streaming output when tags split across chunks

@openclaw-barnacle openclaw-barnacle bot added channel: feishu Channel integration: feishu size: S labels Mar 26, 2026
@Vicky-v7 Vicky-v7 force-pushed the fix/27717-reply-upgrade-prompt branch from 81f9231 to 66cde86 Compare March 26, 2026 04:48
@Vicky-v7 Vicky-v7 force-pushed the fix/27717-reply-upgrade-prompt branch from 66cde86 to 60e8043 Compare March 26, 2026 04:51
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: 81f9231296

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

Comment on lines +499 to +503
const cleaned = stripLeakedThinkingContent(payload.text);
if (!cleaned) {
return;
}
queueStreamingUpdate(cleaned, {
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 Replace stream snapshot when sanitization shortens partial text

Passing cleaned into queueStreamingUpdate(..., { mode: "snapshot" }) can leave leaked tag fragments on screen because snapshot updates are merged with mergeStreamingText, which keeps the previous value when it starts with the new one. If a chunk first emits a partial opener like <thi and the next snapshot sanitizes down to a shorter string (e.g. stripping <thinking>...), the old <thi prefix is retained and never removed, so the fix still leaks internal thinking markers in split-tag streams.

Useful? React with 👍 / 👎.

Comment on lines +70 to +75
result = result.replace(
/`[^`]*`/g, // skip inline code
(match) => "\0CODE" + match + "CODE\0",
);
result = result.replace(/<(?!\/?(?:a|b|i|em|strong|br|p|div|span|img|at|code|pre)\b)[^>\n]{0,80}(?!>)$/gm, (m) => {
return m.replace(/</g, "\\<");
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 escaping < inside inline code spans

The inline-code guard in sanitizeCardKitMarkdown does not actually isolate code spans before angle-bracket escaping: it wraps matches with sentinel text but leaves the original `...` content in place, so the subsequent regex can still match and escape < inside code. A message like `if (a < b)` is rewritten to include \<, altering displayed/code text in streaming cards instead of preserving inline code verbatim.

Useful? React with 👍 / 👎.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 26, 2026

Greptile Summary

This PR adds a channel-level safety net in the Feishu extension to strip leaked thinking-tag content from streaming output, plus a sanitizeCardKitMarkdown helper to prevent Card Kit streaming truncation from unbalanced backticks/angle brackets, and a compatibility fallback for interactive cards in reply context.\n\nKey changes:\n- reply-dispatcher.ts: stripLeakedThinkingContent correctly removes fully-closed and unclosed <thinking>/<thought>/<antthinking> blocks from streaming snapshots. A minor gap exists for partial tag names at chunk boundaries.\n- streaming-card.ts: sanitizeCardKitMarkdown balances triple-backtick fences and inline backticks, then escapes bare angle brackets. The inline-code sentinel approach in step 3 does not fully protect code-span content — the angle-bracket regex can still match a < inside a backtick span at end-of-line, incorrectly escaping it.\n- send.ts: sendMarkdownCardFeishu now routes replies through sendMessageFeishu (post/md format) to avoid older Feishu clients showing an upgrade prompt for interactive cards in reply context.

Confidence Score: 4/5

Safe to merge with one targeted fix — the inline-code sentinel bug in sanitizeCardKitMarkdown can corrupt angle brackets in backtick-wrapped code at line boundaries.

Primary goal (stripping leaked thinking content) is correctly implemented. The send.ts fallback is intentional and well-documented. The P1 sentinel bug is a narrow cosmetic edge case with no security or data-loss risk.

extensions/feishu/src/streaming-card.ts — sentinel-based inline-code protection in sanitizeCardKitMarkdown (lines 70-77)

Important Files Changed

Filename Overview
extensions/feishu/src/reply-dispatcher.ts Adds stripLeakedThinkingContent and pipes streaming snapshots through it — correctly handles fully-closed and unclosed thinking blocks, with a minor gap for partial tag names at chunk boundaries.
extensions/feishu/src/streaming-card.ts Adds sanitizeCardKitMarkdown to prevent Card Kit streaming truncation. Fence and inline-backtick balancing look correct, but the inline-code sentinel approach in step 3 does not protect code span content from angle-bracket escaping at end-of-line.
extensions/feishu/src/send.ts Adds a reply-path fallback to sendMessageFeishu (post format) for older client compatibility. Intentional, well-documented, and replyToMessageId removal from sendCardFeishu is correct.

Comments Outside Diff (1)

  1. extensions/feishu/src/streaming-card.ts, line 70-77 (link)

    P1 Sentinel wrapping does not prevent angle-bracket escaping inside inline code spans

    The sentinel strings only sandwich the original content — they do not neutralize < characters inside it. The subsequent angle-bracket regex scans the entire string and can still match a < that falls at end-of-line inside a sentinel-wrapped span.

    Example: a line ending with `x < y` becomes `x \< y` after the pass — wrongly escaped content that renders as \< in code.

    A safer approach replaces each code span with a neutral placeholder, runs the escaping pass, then restores:

    const saved: string[] = [];
    result = result.replace(/`[^`]*`/g, (m) => { saved.push(m); return `SPAN${saved.length - 1}END`; });
    // angle-bracket escaping here
    result = result.replace(/SPAN(\d+)END/g, (_, i) => saved[Number(i)]);
    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: extensions/feishu/src/streaming-card.ts
    Line: 70-77
    
    Comment:
    **Sentinel wrapping does not prevent angle-bracket escaping inside inline code spans**
    
    The sentinel strings only sandwich the original content — they do not neutralize `<` characters inside it. The subsequent angle-bracket regex scans the entire string and can still match a `<` that falls at end-of-line inside a sentinel-wrapped span.
    
    Example: a line ending with `` `x < y` `` becomes `` `x \< y` `` after the pass — wrongly escaped content that renders as `\<` in code.
    
    A safer approach replaces each code span with a neutral placeholder, runs the escaping pass, then restores:
    ```ts
    const saved: string[] = [];
    result = result.replace(/`[^`]*`/g, (m) => { saved.push(m); return `SPAN${saved.length - 1}END`; });
    // angle-bracket escaping here
    result = result.replace(/SPAN(\d+)END/g, (_, i) => saved[Number(i)]);
    ```
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: extensions/feishu/src/streaming-card.ts
Line: 70-77

Comment:
**Sentinel wrapping does not prevent angle-bracket escaping inside inline code spans**

The sentinel strings only sandwich the original content — they do not neutralize `<` characters inside it. The subsequent angle-bracket regex scans the entire string and can still match a `<` that falls at end-of-line inside a sentinel-wrapped span.

Example: a line ending with `` `x < y` `` becomes `` `x \< y` `` after the pass — wrongly escaped content that renders as `\<` in code.

A safer approach replaces each code span with a neutral placeholder, runs the escaping pass, then restores:
```ts
const saved: string[] = [];
result = result.replace(/`[^`]*`/g, (m) => { saved.push(m); return `SPAN${saved.length - 1}END`; });
// angle-bracket escaping here
result = result.replace(/SPAN(\d+)END/g, (_, i) => saved[Number(i)]);
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: extensions/feishu/src/reply-dispatcher.ts
Line: 26-35

Comment:
**Partial tag names at chunk boundaries will not be stripped**

Both regexes require a complete opening tag (including `>`). A snapshot that ends mid-tag — e.g. `Hello <thinking` or `Hello <thi` — escapes both passes and leaks to the UI.

Adding a third pass covers this gap:
```ts
cleaned = cleaned.replace(/<(?:thinking|thought|antthinking)?[^>]*$/gi, "");
```

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "fix(feishu): strip leaked thinking conte..." | Re-trigger Greptile

Comment on lines +26 to +35
/**
* Strip leaked thinking/thought content that should never reach the user.
*/
function stripLeakedThinkingContent(text: string): string {
let cleaned = text.replace(
/<(thinking|thought|antthinking)\b[^>]*>[\s\S]*?<\/\1>/gi,
"",
);
cleaned = cleaned.replace(/<(thinking|thought|antthinking)\b[^>]*>[\s\S]*$/gi, "");
return cleaned;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Partial tag names at chunk boundaries will not be stripped

Both regexes require a complete opening tag (including >). A snapshot that ends mid-tag — e.g. Hello <thinking or Hello <thi — escapes both passes and leaks to the UI.

Adding a third pass covers this gap:

cleaned = cleaned.replace(/<(?:thinking|thought|antthinking)?[^>]*$/gi, "");
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/feishu/src/reply-dispatcher.ts
Line: 26-35

Comment:
**Partial tag names at chunk boundaries will not be stripped**

Both regexes require a complete opening tag (including `>`). A snapshot that ends mid-tag — e.g. `Hello <thinking` or `Hello <thi` — escapes both passes and leaks to the UI.

Adding a third pass covers this gap:
```ts
cleaned = cleaned.replace(/<(?:thinking|thought|antthinking)?[^>]*$/gi, "");
```

How can I resolve this? If you propose a fix, please make it concise.

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: 43f8d2312a

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

Comment on lines +493 to +495
if (!cleaned) {
return;
}
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 Clear stream state when sanitization drops a snapshot

When stripReasoningTagsFromText produces an empty string for a newer partial snapshot (for example after an earlier fragment like "<thi" was already queued), this early return skips any snapshot update and leaves the previously leaked prefix in streamText. In production streaming sessions this means internal reasoning tag fragments can remain visible to users instead of being removed by the sanitizer path introduced here.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

channel: feishu Channel integration: feishu size: XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Thinking content leaks to channel output when Think: off

2 participants