fix(feishu): strip leaked thinking content from streaming output#54898
fix(feishu): strip leaked thinking content from streaming output#54898Vicky-v7 wants to merge 3 commits intoopenclaw:mainfrom
Conversation
81f9231 to
66cde86
Compare
66cde86 to
60e8043
Compare
There was a problem hiding this comment.
💡 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".
| const cleaned = stripLeakedThinkingContent(payload.text); | ||
| if (!cleaned) { | ||
| return; | ||
| } | ||
| queueStreamingUpdate(cleaned, { |
There was a problem hiding this comment.
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 👍 / 👎.
| 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, "\\<"); |
There was a problem hiding this comment.
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 SummaryThis PR adds a channel-level safety net in the Feishu extension to strip leaked thinking-tag content from streaming output, plus a Confidence Score: 4/5Safe 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)
|
| 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)
-
extensions/feishu/src/streaming-card.ts, line 70-77 (link)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
| /** | ||
| * 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; |
There was a problem hiding this 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:
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.There was a problem hiding this comment.
💡 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".
| if (!cleaned) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
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 👍 / 👎.
Closes #26466 - strips leaked thinking/thought/antthinking content from Feishu streaming output when tags split across chunks