fix(line): strip internal tool-trace banners from outbound text#101708
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 7, 2026, 10:42 AM ET / 14:42 UTC. Summary PR surface: Source +2, Tests +19. Total +21 across 2 files. Reproducibility: yes. Current main is source-reproducible because the LINE adapter lacks Review metrics: 1 noteworthy metric.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. 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 the narrow LINE hook after normal maintainer review and required checks; keep the remaining cross-channel cleanup tracked on #90684. Do we have a high-confidence way to reproduce the issue? Yes. Current main is source-reproducible because the LINE adapter lacks Is this the best way to solve the issue? Yes. This is the narrowest maintainable LINE-specific fix because it uses the existing plugin SDK hook and canonical sanitizer already accepted by sibling channels; a broader shared-boundary sanitizer can remain on the umbrella issue. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against c8d95da14c5b. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +2, Tests +19. Total +21 across 2 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
Review history (1 earlier review cycle)
|
clawsweeper
Bot
added
rating: 🧂 unranked krab
|
@clawsweeper re-review — Evidence updated with negative control + direct |
|
🦞🧹 I asked ClawSweeper to review this item again. |
clawsweeper
Bot
added
proof: sufficient
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 80489d4894
ℹ️ 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".
| if (replySucceeded) { | ||
| throw err; |
There was a problem hiding this comment.
When a later push fails after the initial reply batch succeeded (the new >5-chunk path), this rethrow means deliverLineAutoReply never returns the { replyTokenUsed: true } state for monitor.ts to adopt. The shared reply dispatcher catches delivery errors and continues the delivery chain, so any subsequent block/final payload in the same LINE turn will still think the reply token is unused and will try to reply with an already-consumed token instead of going straight to push. Surface the error only after the caller has a way to retain the consumed-token state.
Useful? React with 👍 / 👎.
7a1901a to
e7ea781
Compare
|
Land-ready at exact head What changed:
Remote proof on Blacksmith Testbox Targeted Oxfmt passed for all eight changed files. Fresh whole-PR autoreview found no actionable issues. Live loopback HTTP proof exercised production LINE auto-reply orchestration: sanitized visible text reached the wire; the reply batch succeeded; the next push failed with HTTP 503; exactly two wire attempts occurred; the reply batch was not replayed; internal-only output made zero requests. {"scenario":"LINE reply batch then failed push over HTTP","sanitizedChunkInput":"Visible answer.","wireAttemptsBeforeFailure":2,"replayedReplyBatch":false,"internalOnlyRequests":0}Hosted exact-head merge-ref CI: https://github.com/openclaw/openclaw/actions/runs/29373316329 Known proof gap: no credentialed round-trip against the external LINE service. The loopback proof used the production orchestration and real HTTP request/failure semantics. |
|
Merged via squash.
|
…claw#101708) * fix(line): strip internal tool-trace banners from outbound text * fix(line): sanitize inbound assistant replies * chore: retrigger PR CI --------- Co-authored-by: Pick-cat <[email protected]> Co-authored-by: Peter Steinberger <[email protected]>
2 participants
Part of #90684
What Problem This Solves
LINE replies could expose internal assistant tool-trace banners to end users. The normal inbound auto-reply path also bypassed the generic outbound adapter, so adding the adapter hook alone did not cover the primary LINE reply path.
Why This Change Was Made
This applies the shared
sanitizeAssistantVisibleTextboundary in both LINE delivery paths: the standard outbound adapter and inbound auto-reply delivery before Markdown conversion. Internal-only text is suppressed without consuming a reply token, while rich content and fenced examples remain intact.The delivery result now carries whether visible output was actually sent, so dispatch can distinguish suppressed output from a user-visible reply. The reply-chunk fallback also tracks a successful reply-token batch and will not replay it if a later push chunk fails.
User Impact
LINE users no longer see internal tool-trace banners in bot replies. Normal prose, fenced examples, rich content, reply-token fallback, and chunked delivery remain supported. A later push failure no longer duplicates an already-delivered reply batch.
Evidence
tbx_01kxhbsb7dvp09f3qx22bjaxbcpnpm check:changed: production TypeScript, lint, formatting, SDK baselines, storage/media/runtime guards, and import-cycle checks passed; extension-test TypeScript reproduced the untouchedmainfailure fixed by fix(nextcloud-talk): restore replay test typecheck #107808Live proof output:
{"scenario":"LINE reply batch then failed push over HTTP","sanitizedChunkInput":"Visible answer.","wireAttemptsBeforeFailure":2,"replayedReplyBatch":false,"internalOnlyRequests":0}Known gap: no credentialed round-trip against the external LINE service. The loopback proof exercised the production delivery orchestration and real HTTP request/failure semantics.