fix(ollama): strip inline kimi cloud reasoning leak#86286
fix(ollama): strip inline kimi cloud reasoning leak#86286jason-allen-oneal wants to merge 12 commits into
Conversation
|
Codex review: needs maintainer review before merge. Reviewed May 25, 2026, 9:23 AM ET / 13:23 UTC. Summary PR surface: Source +183, Tests +473. Total +656 across 6 files. Reproducibility: yes. source-reproducible: current main forwards Ollama message.content directly into visible text deltas and final messages, so the linked Kimi-cloud delimiter payload would be rendered. I did not run a live vendor reproduction in this read-only review. Review metrics: 2 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge Security Review detailsBest possible solution: Land a narrow provider-local Ollama Kimi-cloud sanitizer only after maintainers accept the bounded buffering behavior, delimiter assumptions, and regression coverage as sufficient for this security-sensitive leak. Do we have a high-confidence way to reproduce the issue? Yes, source-reproducible: current main forwards Ollama message.content directly into visible text deltas and final messages, so the linked Kimi-cloud delimiter payload would be rendered. I did not run a live vendor reproduction in this read-only review. Is this the best way to solve the issue? Yes, with maintainer risk acceptance: keeping the workaround scoped to the Ollama provider and Kimi-cloud model refs is narrower than a gateway-wide inline reasoning stripper. The main remaining question is proof/risk tolerance, not a clearer code repair. AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against e761eb8f3e1d. Label changesLabel justifications:
Evidence reviewedPR surface: Source +183, Tests +473. Total +656 across 6 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
There was a problem hiding this comment.
Pull request overview
This PR adds an Ollama-provider response sanitizer to prevent Kimi cloud models from leaking inline “reasoning” into user-visible assistant text, and introduces regression tests around buildAssistantMessage().
Changes:
- Add
stripKimiInlineReasoningFromVisibleText(...)forkimi-k*:cloudresponses and apply it when building final assistanttextcontent. - Add stream-runtime tests verifying stripping for
kimi-k2.6:cloudand non-stripping for non-Kimi models.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
extensions/ollama/src/stream.ts |
Introduces and applies a Kimi cloud inline-reasoning sanitizer during final assistant message construction. |
extensions/ollama/src/stream-runtime.test.ts |
Adds regression tests for the sanitizer behavior in buildAssistantMessage(). |
Comments suppressed due to low confidence (1)
extensions/ollama/src/stream.ts:987
- The sanitizer is only applied when building the final assistant message here; the streaming path above emits
text_deltaevents usingaccumulatedContentwithout any stripping. If downstream consumers render streaming deltas, the inline reasoning still leaks untildone. Consider applying the same stripping logic to the streaming partials/deltas (or maintaining a separate visible accumulator) so users never see the reasoning prefix during streaming.
const text = stripKimiInlineReasoningFromVisibleText({
modelId: modelInfo.id,
text: response.message.content || "",
});
if (text) {
| const KIMI_INLINE_REASONING_BOUNDARY = "️"; | ||
| const KIMI_INLINE_REASONING_MIN_PREFIX_CHARS = 80; | ||
| const KIMI_INLINE_REASONING_MIN_ANSWER_CHARS = 8; | ||
|
|
||
| function stripKimiInlineReasoningFromVisibleText(params: { |
There was a problem hiding this comment.
Addressed in the current head. The delimiter is now matched as whitespace/start + \uFE0F with optional following whitespace in the Kimi sanitizer module, and the old bare indexOf marker no longer exists in stream.ts.
| it("strips inline reasoning prefix from kimi cloud visible text", () => { | ||
| const response = { | ||
| model: "kimi-k2.6:cloud", | ||
| created_at: "2026-01-01T00:00:00Z", | ||
| message: { | ||
| role: "assistant" as const, | ||
| content: | ||
| "I should think privately and not leak this planning text in the answer. I need to keep deciding what to say next. ️Final answer only.", |
There was a problem hiding this comment.
Addressed in the current head with the regression does not treat emoji variation selectors as Kimi inline-reasoning boundaries.
|
ClawSweeper PR egg ✨ Hatched: 🌱 uncommon Neon Signal Puff Hatch commandComment Hatchability rules:
Rarity: 🌱 uncommon. What is this egg doing here?
|
|
Addressed the two blocking patch findings in this PR head (
Added regression tests in
Validation rerun after fix:
After-fix real behavior proof status (live Kimi Cloud): still unavailable in this runtime due subscription gating. Redacted local artifacts:
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Maintainer note: please do not merge this PR yet. The direction is right: this should stay as a narrow Ollama Kimi cloud fix, and the stream path must be fixed before The current implementation is still too brittle in two ways:
The cleaner production fix is:
That keeps the fix narrow and avoids guessing across other providers, while covering the actual leak shapes this PR is trying to address. |
|
Implementation and validation update for head What changed:
Validation run:
Remaining note: this is behavior proof through OpenClaw's actual Ollama stream path with a compatible test server, not a live vendor Kimi Cloud capture. The live vendor prompts I tried earlier did not reproduce the delimiter payload, but the issue-shaped payload is now covered in both unit tests and stream behavior proof. |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Final implementation/validation update for head What changed:
Validation:
PR checks:
|
|
Live vendor reproduction was not stable in our environment. We could call Ollama Cloud with the subscribed key, but current live responses from This PR still fixes the reported wire shape from the issue and has stream-path proof using OpenClaw's real Ollama handler against an Ollama-compatible stream. |
|
@clawsweeper automerge |
|
🦞🔧
Draft PRs stay fix-only until GitHub marks them ready for review. Pause with Automerge progress:
|
|
ClawSweeper 🐠 reef update Thanks for the work on this. ClawSweeper opened a replacement PR only because the source branch was not writable from the available bot permissions. branch tides, not contributor blame. Why replacement: ClawSweeper could not update the source PR branch directly; GitHub did not grant sufficient push rights to the bot for that branch.
fish notes: model gpt-5.5, reasoning high; reviewed against b709229. |
Summary
kimi-k*:cloudand boundary-delimited payloads only, with conservative guards to avoid clipping normal outputChanges
extensions/ollama/src/stream.tsstripKimiInlineReasoningFromVisibleText(...)buildAssistantMessage(...)before publishing visibletextblockstext_delta,text_end, partials, and finaldoneextensions/ollama/src/stream-runtime.test.tskimi-k2.6:cloudReal behavior proof
Behavior addressed: Ollama Kimi cloud responses can include private reasoning in the visible content stream before the final answer boundary. OpenClaw must not emit that prefix in live streaming events or in the saved final assistant message.
Real environment tested:
bob@isengard,/tmp/openclaw-86286-prat PR headaaddcf0a3e56bd3eb1afceea4546d93efc1bff24, running OpenClaw's realcreateOllamaStreamFnagainst a local Ollama-compatible HTTP NDJSON server. This exercised the actual OpenClaw Ollama stream parser and event builder rather than the final message constructor alone.Exact steps or command run after this patch:
/home/bob/.nvm/versions/node/v22.22.0/bin/node --import tsx /tmp/openclaw-86286-real-proof.mjsEvidence after fix: Terminal output from that command included only the cleaned answer in the emitted OpenClaw stream events:
Observed result after fix: The injected hidden prefix text (
The user is asking for a short answer...) did not appear intext_delta,text_end, partial content, or the finaldonemessage. The stream emitted onlyFinal answer only..What was not tested: A live vendor response that naturally reproduces the delimiter leak was not captured; direct calls to the available Ollama Kimi cloud key did not reproduce the delimiter leak during investigation, so the proof used a local Ollama-compatible stream with the observed leaked wire shape.
Validation
/home/bob/.nvm/versions/node/v22.22.0/bin/node --import tsx /tmp/openclaw-86286-real-proof.mjsnode scripts/run-vitest.mjs extensions/ollama/src/stream-runtime.test.tsnode scripts/run-vitest.mjs src/agents/model-catalog-visibility.test.tsFixes #86129