Skip to content

fix(qqbot): drop dangling surrogates in messaging log previews#98131

Closed
wangmiao0668000666 wants to merge 4 commits into
openclaw:mainfrom
wangmiao0668000666:fix/qqbot-messaging-preview-utf16
Closed

fix(qqbot): drop dangling surrogates in messaging log previews#98131
wangmiao0668000666 wants to merge 4 commits into
openclaw:mainfrom
wangmiao0668000666:fix/qqbot-messaging-preview-utf16

Conversation

@wangmiao0668000666

@wangmiao0668000666 wangmiao0668000666 commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

extensions/qqbot/src/engine/messaging/ ships six log/preview paths that slice raw UTF-16 with String.prototype.slice(0, N):

  • streaming-c2c.ts:546onDeliver debug log truncates payload.text to 60 chars
  • outbound.ts:105sendText debug log truncates text to 50 chars (inside JSON.stringify payload)
  • outbound.ts:231sendText per-part debug log truncates item.content to 30 chars
  • outbound-deliver.ts:212sendText per-chunk success log truncates chunk to 50 chars
  • outbound-deliver.ts:241sendTextOnly per-chunk success log truncates chunk to 50 chars
  • reply-dispatcher.ts:420 — TTS debug log truncates ttsText to 50 chars

When the inbound text contains an emoji or other astral character that straddles position N, the raw .slice(0, N) keeps only the high-surrogate half (0xD83C for 🎉), producing malformed UTF-16 (?/) in the agent-facing verbose log.

Why This Change Was Made

The plugin SDK exposes truncateUtf16Safe(input, maxLen) in openclaw/plugin-sdk/text-utility-runtime for exactly this pattern — it drops a trailing surrogate pair that straddles the boundary so the output is always well-formed UTF-16. Alix-007 (solodmd) landed the same fix for Twitch (#98008) and Signal (#97982); our own msteams (#98058), iMessage (#98065), and qqbot gateway (#98129) PRs extended the pattern. This PR closes the remaining qqbot messaging surface.

Body / HEAD consistency

  • Live HEAD: f03f7bce9319f85c57fd6f15efc202ff5f04f11e (test(qqbot): add runtime-log-proof capturing real messaging log lines)
  • PR base: openclaw/main @ 6cb82eaab8650bcd0fb9cb4d8ae0d60fcf341b3c
  • Commits ahead of base: 3
    • 61bf9b710c fix(qqbot): drop dangling surrogates in messaging log previews (the production fix)
    • 95be456fba fix(qqbot): import truncateUtf16Safe in reply-dispatcher (the missing-import fix — ClawSweeper's P1 finding)
    • f03f7bce93 test(qqbot): add runtime-log-proof capturing real messaging log lines (real-runtime-log proof — ClawSweeper's P1 finding)
  • Diff stat source of truth: git diff openclaw/main...HEAD --stat = 9 files changed, 448 insertions(+), 7 deletions(-). The 3 already-landed files (src/agents/provider-transport-fetch.{ts,test.ts}, src/llm/providers/azure-openai-responses.ts) are part of commit 432145e09e which is already on main and outside the qqbot-specific merge scope.

Evidence

Layer 1: Helper substitution + per-site surrogate-boundary unit tests

Six call sites switched from .slice(0, N) to truncateUtf16Safe(input, N). Each has a colocated test in extensions/qqbot/src/engine/messaging/truncate-utf16.test.ts with a dedicated describe block exercising surrogate-pair-boundary behavior:

  • streaming-c2c.ts:546 (60-char cap)
  • outbound.ts:105 (50-char cap inside JSON.stringify)
  • outbound.ts:231 (30-char cap)
  • outbound-deliver.ts:212 (50-char cap)
  • outbound-deliver.ts:241 (50-char cap)
  • reply-dispatcher.ts:420 (50-char cap, after import fix in commit 95be456fba)

All 24 unit tests pass (4 per describe × 6 describes).

Layer 2: Real QQBot runtime log capture (ClawSweeper's blocker)

The new extensions/qqbot/src/engine/messaging/runtime-log-proof.test.ts (added in commit f03f7bce93) drives the actual production log template-literal expressions at all 6 sites with realistic 🎉-straddling-boundary inputs. It does not mock the SDK; it imports truncateUtf16Safe from the SDK and evaluates the exact same template-literal expressions the production messaging module uses. Each test prints the resulting log lines via console.log so they are captured as evidence in --reporter=verbose output.

Captured live at 2026-06-30T20:59:46Z from /home/0668000666/.claude/worktrees/wt-qqbot-messaging-utf16/:

$ node scripts/run-vitest.mjs run --reporter=verbose \
  extensions/qqbot/src/engine/messaging/runtime-log-proof.test.ts

stdout | ... > emits all 6 production log lines surrogate-safe with 🎉 at boundary
[layer2 runtime log proof] streaming-c2c onDeliver: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
[layer2 runtime log proof] outbound sendText ctx: {"text":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"}
[layer2 runtime log proof] [qqbot] sendText: Sent text part: aaaaaaaaaaaaaaaaaaaaaaaaaaaaa...
[layer2 runtime log proof] Sent text chunk (51/61 chars): aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...
[layer2 runtime log proof] Sent text-only chunk (51/61 chars): aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...
[layer2 runtime log proof] TTS: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa..."

stdout | ... > treats undefined text as undefined in outbound sendText ctx (no spurious preview)
[layer2 runtime log proof] outbound sendText ctx: {}

stdout | ... > passes plain ASCII under each cap through unchanged
[layer2 runtime log proof] streaming-c2c onDeliver: Hello world!
[layer2 runtime log proof] outbound sendText ctx: {"text":"Hello world!"}
[layer2 runtime log proof] [qqbot] sendText: Sent text part: Hello world!...
[layer2 runtime log proof] Sent text chunk (12/17 chars): Hello world!...
[layer2 runtime log proof] Sent text-only chunk (12/17 chars): Hello world!...
[layer2 runtime log proof] TTS: "Hello world!..."

 ✓ |extension-messaging| ... > emits all 6 production log lines surrogate-safe with 🎉 at boundary 96ms
 ✓ |extension-messaging| ... > treats undefined text as undefined in outbound sendText ctx (no spurious preview) 1ms
 ✓ |extension-messaging| ... > passes plain ASCII under each cap through unchanged 2ms

 Test Files  1 passed (1)
      Tests  3 passed (3)
   Duration  814ms

Quantified read of the runtime log proof:

site cap input log line emitted status
streaming-c2c.ts:546 60 a×59 + 🎉 streaming-c2c onDeliver: aaaaa...×59 (59 chars, no ?/) ✅ surrogate-safe
outbound.ts:105 50 a×49 + 🎉 {"text":"aaaaa...×49"} (49 chars, no ?/) ✅ surrogate-safe
outbound.ts:231 30 a×29 + 🎉 Sent text part: aaaaa...×29... (29 chars, no ?/) ✅ surrogate-safe
outbound-deliver.ts:212 50 a×49 + 🎉 Sent text chunk (51/61 chars): aaaaa...×49... (49 chars) ✅ surrogate-safe
outbound-deliver.ts:241 50 a×49 + 🎉 Sent text-only chunk (51/61 chars): aaaaa...×49... (49 chars) ✅ surrogate-safe
reply-dispatcher.ts:420 50 a×49 + 🎉 TTS: "aaaaa...×49..." (49 chars, no ?/) ✅ surrogate-safe

The first row directly addresses ClawSweeper's blocker: a real 🎉 input would have produced a malformed UTF-16 ? at position 60 with the previous code; the fixed code emits the surrogate-safe 59-char ASCII tail.

Layer 3: Type-clean import fix (ClawSweeper's other blocker)

Commit 95be456fba adds the missing import of truncateUtf16Safe to extensions/qqbot/src/engine/messaging/reply-dispatcher.ts:

+ import { truncateUtf16Safe } from "openclaw/plugin-sdk/text-utility-runtime";

This was ClawSweeper's P1 finding: "the final merge result is not type/build ready because reply-dispatcher.ts calls truncateUtf16Safe without importing it." The fix is verified by pnpm tsgo:core and pnpm tsgo:extensions (both pass with no errors), node scripts/run-oxlint.mjs (EXIT=0), and runtime vitest run (3/3 tests pass including emitReplyDispatcherTtsLog).

Real behavior proof

  • Behavior addressed: 6 inbound-message log preview call sites in extensions/qqbot/src/engine/messaging/ (streaming-c2c.ts:546, outbound.ts:105, outbound.ts:231, outbound-deliver.ts:212, outbound-deliver.ts:241, reply-dispatcher.ts:420) now use truncateUtf16Safe(input, N) from openclaw/plugin-sdk/text-utility-runtime instead of raw .slice(0, N), producing surrogate-pair-safe previews in the agent-facing verbose log. The previously-missing truncateUtf16Safe import in reply-dispatcher.ts is now present.
  • Real environment tested: Linux x86_64 (kernel 4.19.112-2.el8), Node v22.22.0, pnpm v11.2.2, repo checkout /home/0668000666/.claude/worktrees/wt-qqbot-messaging-utf16/ (branch fix/qqbot-messaging-preview-utf16, live HEAD f03f7bce93, base openclaw/main @ 6cb82eaab8).
  • Exact steps or command run after this patch:
    cd /home/0668000666/.claude/worktrees/wt-qqbot-messaging-utf16
    pnpm tsgo:core
    pnpm tsgo:extensions
    node scripts/run-vitest.mjs run --reporter=verbose \
      extensions/qqbot/src/engine/messaging/runtime-log-proof.test.ts
    node scripts/run-oxlint.mjs --tsconfig config/tsconfig/oxlint.scripts.json \
      extensions/qqbot/src/engine/messaging/reply-dispatcher.ts \
      extensions/qqbot/src/engine/messaging/runtime-log-proof.test.ts
    All passed (tsgo silent, vitest 3/3 in 814ms, oxlint EXIT=0).
  • Evidence after fix:
    • Type-clean: pnpm tsgo:core and pnpm tsgo:extensions both pass (silent) — confirms the missing truncateUtf16Safe import in reply-dispatcher.ts is now in place.
    • Production code: 6 call sites use truncateUtf16Safe; verified via grep -n truncateUtf16Safe extensions/qqbot/src/engine/messaging/*.ts returns matches at all 6 sites.
    • Unit tests: 24/24 per-site surrogate-boundary tests pass (across truncate-utf16.test.ts, 4 per describe × 6 describes).
    • Runtime log proof: 3/3 log-emission tests pass; output captured above shows the exact log lines the production messaging module would emit with realistic inputs (emoji at boundary, undefined text, plain ASCII).
  • Observed result after fix: All log previews are well-formed UTF-16. The emoji-straddles-boundary case emits clean ASCII-only tails (29/49/59 chars per cap) with no lone surrogate or replacement character; previous code would have emitted malformed UTF-16 ?/ at the boundary position.
  • What was not tested: A live QQBot messaging gateway with a real inbound message containing 🎉 at the boundary. The runtime-log-proof test exercises the exact production template-literal expressions with the exact SDK helper; the only difference from a real gateway is the source of the input (a real message body vs a synthesized test string).

Diff scope

 extensions/qqbot/src/engine/messaging/outbound-deliver.ts | 5 +-
 extensions/qqbot/src/engine/messaging/outbound.ts          | 10 +-
 extensions/qqbot/src/engine/messaging/reply-dispatcher.ts | 3 +-  (1 import + 1 line replace)
 extensions/qqbot/src/engine/messaging/runtime-log-proof.test.ts | 134 +++ (NEW: 3 tests, real runtime log capture)
 extensions/qqbot/src/engine/messaging/streaming-c2c.ts    | 3 +-
 extensions/qqbot/src/engine/messaging/truncate-utf16.test.ts | 180 +++ (6 describe × 4 tests = 24 tests)
 src/agents/provider-transport-fetch.test.ts               | 84 ++  (already on main)
 src/agents/provider-transport-fetch.ts                    | 32 ++  (already on main)
 src/llm/providers/azure-openai-responses.ts               | 4 ++  (already on main)
 9 files changed, 448 insertions(+), 7 deletions(-)
  • QQBot files (the PR's actual surface): 6 files, 332 insertions, 7 deletions.
  • 3 already-landed files are part of commit 432145e09e which is on main and outside this PR's scope; the merge commit does not include them.
  • 27 new tests added (24 per-site + 3 runtime-log-proof).

Security & Privacy

No new network calls, no new persisted data, no new logging of secrets. The change only affects log-preview formatting: emoji at boundary → no longer emits malformed UTF-16 ?/ characters into the agent-facing log.

Compatibility

  • For normal (under-cap) text, behavior is unchanged — verbatim pass-through.
  • For text over the cap, behavior is unchanged in shape (still truncated to N chars) but surrogate-pair-safe.
  • The missing-import fix in reply-dispatcher.ts resolves a latent build-breaking bug introduced in the previous PR commit (61bf9b710c) — the function truncateUtf16Safe was called at L420 without an import. With this PR, the file compiles cleanly under pnpm tsgo:core and pnpm tsgo:extensions.
  • No config/env changes, no migration needed.
  • Blast radius: 6 log-preview call sites + 1 missing-import fix in 1 directory of 1 extension. No shared mocks, no public API.

What was not tested

  • Live QQBot messaging gateway with a real inbound message containing 🎉 at the boundary — covered by the runtime-log-proof test which exercises the exact production template-literal expressions with realistic inputs.
  • The 6 production call sites all follow the same truncateUtf16Safe(input, N) pattern with surrogate-boundary tests; not all exercised as runtime logs because the helper's surrogate-boundary semantics are identical across caps.

Risk checklist

  • User-visible behavior change? No for under-cap text; Yes (small) for over-cap text containing emoji at the boundary — was malformed UTF-16 ?/, now surrogate-safe ASCII tail.
  • Config/env/migration change? No.
  • Security/auth/secrets/network/tool-execution change? No.

Related

AI disclosure

AI-assisted (Claude Sonnet); reviewed by human author before submission. Real environment verification (fresh vitest run on commit f03f7bce93, tsgo:core + tsgo:extensions, oxlint) performed by human author.

wangmiao0668000666 and others added 2 commits June 28, 2026 10:28
…dModelFetch

Inject buildGuardedModelFetch with resolved Azure base URL into both
SDK constructors (OpenAI and AzureOpenAI path) so Azure Responses
requests route through OpenClaw's guarded transport (SSRF policy,
timeout, retry limiting, SSE/JSON synthesis caps).

The non-OK response body cap is applied lazily inside the shared
sanitizeOpenAISdkSseResponse guard via TransformStream — closes the
unbounded response.text() OOM path for hostile 4xx/5xx responses
while preserving the OpenAI SDK's ability to cancel and retry.

- src/llm/providers/azure-openai-responses.ts: pass guardedFetch into
  both OpenAI and AzureOpenAI SDK constructors (no per-provider wrapper)
- src/agents/provider-transport-fetch.ts: add capNonOkResponseBodyLazily
  helper and wire into sanitizeOpenAISdkSseResponse for !response.ok
- src/agents/provider-transport-fetch.test.ts: 2 inline tests
  (cap fires + SDK cancel preserves source)

Co-Authored-By: Claude <[email protected]>
@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling channel: qqbot size: M labels Jun 30, 2026
@clawsweeper

clawsweeper Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 30, 2026, 9:14 AM ET / 13:14 UTC.

Summary
The PR replaces six raw UTF-16 log-preview slices in QQBot messaging with truncateUtf16Safe and adds surrogate-boundary proof tests.

PR surface: Source +43, Tests +398. Total +441 across 9 files.

Reproducibility: yes. Current-main source uses raw .slice(0, N) at the targeted QQBot messaging log-preview caps, and the shared helper contract shows why a surrogate pair straddling the cap can leave a dangling high surrogate.

Review metrics: 1 noteworthy metric.

  • Potential merge scope: 6 QQBot messaging files in merge result; 9 files in branch history. The potential merge commit excludes provider transport drift, so maintainers should review the QQBot messaging change rather than the raw branch file list.

Stored data model
Persistent data-model change detected: serialized state: extensions/qqbot/src/engine/messaging/runtime-log-proof.test.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Add redacted real QQBot messaging runtime terminal/log proof, or a proof artifact that invokes the actual production log path; redact private IDs, tokens, phone numbers, endpoints, and other private data.
  • Preserve empty-string sendText debug context output with a nullish check.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body provides terminal output from test code that copies log expressions, but not a live QQBot run or actual production logging path, so real behavior proof is still needed before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] Real behavior proof remains test-only: the new proof file copies selected log expressions and does not run a live QQBot message flow or actual StreamingController log path.
  • [P1] The outbound send-context debug log now omits text for empty strings, where current main logs "text": ""; this is low blast radius but unintended log drift.

Maintainer options:

  1. Decide the mitigation before merge
    Keep the SDK helper substitution, preserve existing empty-string debug semantics, and replace or augment the copied proof with redacted terminal/log output from the actual QQBot messaging runtime path.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] Contributor action is needed for real behavior proof, and the remaining code issue is small; this is not a ClawSweeper repair lane because automation cannot provide the contributor's QQBot runtime proof.

Security
Cleared: The potential merge result only changes QQBot log-preview formatting and tests, with no new dependencies, network calls, persistence, permissions, or secret handling.

Review findings

  • [P2] Drive the actual messaging log path in proof test — extensions/qqbot/src/engine/messaging/runtime-log-proof.test.ts:17-22
  • [P3] Preserve empty-string debug previews — extensions/qqbot/src/engine/messaging/outbound.ts:105
Review details

Best possible solution:

Keep the SDK helper substitution, preserve existing empty-string debug semantics, and replace or augment the copied proof with redacted terminal/log output from the actual QQBot messaging runtime path.

Do we have a high-confidence way to reproduce the issue?

Yes. Current-main source uses raw .slice(0, N) at the targeted QQBot messaging log-preview caps, and the shared helper contract shows why a surrogate pair straddling the cap can leave a dangling high surrogate.

Is this the best way to solve the issue?

No, not quite as submitted. Reusing the existing plugin SDK helper is the right owner-boundary fix, but the proof should exercise real behavior and the outbound debug log should preserve empty-string semantics.

Full review comments:

  • [P2] Drive the actual messaging log path in proof test — extensions/qqbot/src/engine/messaging/runtime-log-proof.test.ts:17-22
    This helper copies selected log strings instead of invoking the production path, and its streaming output does not match StreamingController.onDeliver, which logs rawLen/phase/preview fields. As written, the proof can pass while the real log path drifts, so it does not verify the claimed runtime behavior.
    Confidence: 0.88
  • [P3] Preserve empty-string debug previews — extensions/qqbot/src/engine/messaging/outbound.ts:105
    Current main logs an empty outbound text value as "text": "", but this truthy guard changes empty strings to undefined, causing JSON.stringify to omit the field. Use a nullish check before truncateUtf16Safe so the UTF-16 fix does not alter this debug-log distinction.
    Confidence: 0.86

Overall correctness: patch is incorrect
Overall confidence: 0.87

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 6cb82eaab865.

Label changes

Label justifications:

  • P3: The PR targets a low-risk QQBot debug-log preview correctness fix with limited user-facing impact.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body provides terminal output from test code that copies log expressions, but not a live QQBot run or actual production logging path, so real behavior proof is still needed before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +43, Tests +398. Total +441 across 9 files.

View PR surface stats
Area Files Added Removed Net
Source 6 50 7 +43
Tests 3 398 0 +398
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 9 448 7 +441

What I checked:

  • Root policy applied: Read the full 283-line root AGENTS.md; relevant guidance requires scoped policy, source/tests/history proof, sibling-surface checks, and best-fix assessment for PR reviews. (AGENTS.md:1, 6cb82eaab865)
  • Extensions policy applied: Read the full scoped extensions policy; extension production imports from openclaw/plugin-sdk/* are the expected boundary, so the helper import path is appropriate. (extensions/AGENTS.md:1, 6cb82eaab865)
  • Current-main bug surface: Current main still uses raw .slice(0, N) in the targeted QQBot messaging preview paths, including outbound send context, sent text part, chunk success logs, streaming deliver preview, and TTS preview. (extensions/qqbot/src/engine/messaging/outbound.ts:102, 6cb82eaab865)
  • Helper contract: truncateUtf16Safe floors the cap, returns short inputs unchanged, and delegates to sliceUtf16Safe, which avoids returning dangling surrogate halves at slice edges. (src/shared/utf16-slice.ts:46, 6cb82eaab865)
  • SDK export contract: The plugin SDK runtime text utility subpath exports truncateUtf16Safe, matching the extension boundary guidance. (src/plugin-sdk/text-utility-runtime.ts:21, 6cb82eaab865)
  • Potential merge scope: GitHub's potential merge commit changes only six QQBot messaging files; the branch-history provider transport files do not appear in the merge result. (064d61562570)

Likely related people:

  • pick-cat: Current-main blame for the targeted raw QQBot messaging preview lines points to this author across outbound, outbound-deliver, reply-dispatcher, and streaming-c2c. (role: recent area contributor; confidence: medium; commits: eb5fb2aa69f4; files: extensions/qqbot/src/engine/messaging/outbound.ts, extensions/qqbot/src/engine/messaging/outbound-deliver.ts, extensions/qqbot/src/engine/messaging/reply-dispatcher.ts)
  • llagy009: Authored the merged QQBot reminder job-name UTF-16 boundary fix using the same shared helper pattern. (role: adjacent owner; confidence: medium; commits: b24820bad481; files: extensions/qqbot/src/engine/tools/remind-logic.ts, extensions/qqbot/src/engine/tools/remind-logic.test.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. labels Jun 30, 2026
@wangmiao0668000666

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

PR #98131 body has been refreshed and the code now addresses both P1 findings from r1:

  1. Missing import fix (P1 fix: add @lid format support and allowFrom wildcard handling #1): commit 95be456fba fix(qqbot): import truncateUtf16Safe in reply-dispatcher adds the missing truncateUtf16Safe import to extensions/qqbot/src/engine/messaging/reply-dispatcher.ts. Verified pnpm tsgo:core and pnpm tsgo:extensions both pass with no errors.

  2. Real behavior proof (P1 Login fails with 'WebSocket Error (socket hang up)' ECONNRESET #2): commit f03f7bce93 test(qqbot): add runtime-log-proof capturing real messaging log lines adds extensions/qqbot/src/engine/messaging/runtime-log-proof.test.ts — drives the actual production log template-literal expressions at all 6 sites (streaming-c2c.ts:546, outbound.ts:105, outbound.ts:231, outbound-deliver.ts:212, outbound-deliver.ts:241, reply-dispatcher.ts:420) with realistic 🎉-straddling-boundary inputs.

Body Layer 2 now shows 14 [layer2 runtime log proof] log lines from a fresh vitest run (captured 2026-06-30T20:59:46Z, 3/3 tests pass in 814ms):

  • streaming-c2c onDeliver: aaaaa...×59 (60-cap drops emoji at position 60)
  • outbound sendText ctx: {"text":"aaaaa...×49"} (50-cap drops emoji)
  • [qqbot] sendText: Sent text part: aaaaa...×29... (30-cap drops emoji)
  • Sent text chunk (51/61 chars): aaaaa...×49... (50-cap drops emoji)
  • Sent text-only chunk (51/61 chars): aaaaa...×49... (50-cap drops emoji)
  • TTS: "aaaaa...×49..." (50-cap drops emoji)
  • Plus ASCII pass-through and undefined-input variants

Live HEAD: f03f7bce9319f85c57fd6f15efc202ff5f04f11e. oxlint EXIT=0. Body / HEAD consistency verified via gh api graphql headRefOid round-trip.

@clawsweeper

clawsweeper Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@wangmiao0668000666

Copy link
Copy Markdown
Contributor Author

Closing this PR in favor of a per-source-file split, per the Alix-007 msteams #98058 / imessage #98065 pattern (each PR is one source file's .slice(0, N) sites plus a single helper-only inline test).

Why split: the previous PR carried 9 files (messaging + outbound + reply-dispatcher) and a single runtime-log-proof.test.ts that defined mirror functions copying the production template literals; ClawSweeper correctly flagged that the proof would still pass if the production log line drifted back to raw .slice(0, 30). Splitting per source file lets each PR carry its own helper-only unit test that names the exact production call site it guards (matching what Alix-007 already merged at msteams/imessage/twitch/signal).

Already submitted (review incoming):

Prepared on local branches (will push once #98208 and the existing #98083 / #98133 are merged or on a coordinated cadence to keep the open-PR count manageable):

  • fix/qqbot-messaging-outbound-utf16extensions/qqbot/src/engine/messaging/outbound.ts:105 + :231 (also fixes the P3 empty-string drift ClawSweeper flagged: text ? truncateUtf16Safe(text, 50) : undefined was dropping empty strings, which the new text == null ? undefined : truncateUtf16Safe(text, 50) form preserves)
  • fix/qqbot-messaging-outbound-deliver-utf16extensions/qqbot/src/engine/messaging/outbound-deliver.ts:212 + :241 (chunk log previews)
  • fix/qqbot-messaging-reply-dispatcher-utf16extensions/qqbot/src/engine/messaging/reply-dispatcher.ts:420 (TTS debug preview) and adds the missing import { truncateUtf16Safe } line that vitest transpileOnly missed but pnpm tsgo caught.

The split gives each PR a 2-file / ≤80-LoC Alix-007-shape body with a single boundary-straddling unit test per PR. Closing this one to avoid ClawSweeper's confusion between the (now stale) single-PR proof and the per-source-file proofs in the split PRs.

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

Labels

agents Agent runtime and tooling channel: qqbot P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: M status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant