Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…transcript follow-up to 35e6a66 (#11712): looksLikeRawFieldTranscript fired on any replyText: line anywhere in a reply, so a legitimate diagnosis that QUOTES a leaked shouldRespond:/replyText: transcript (this repo's own daily debugging workflow) was silently replaced at the send boundary by the QUOTED replyText tail — the whole answer dropped with only a logger.warn. same hijack on the text-mode path: parseMessageHandlerFieldTranscript claimed any prose with a replyText: line and discarded every preamble line. structural rule instead of contains-check: a raw envelope echo IS the message — its first substantive line outside code fences is a known field line, with a shouldRespond:/replyText: hallmark at top level. prose preamble or fenced field lines mean the reply QUOTES a transcript and ships intact. parseFieldTranscript now treats fenced field lines as value content, so a real leak whose replyText quotes an envelope in a fence is no longer split at the quoted lines. the text-mode claim is gated on the same detector, and the comment/code mismatch (comment said routing AND reply field, code was OR) is resolved in favor of the documented-correct OR (a lone shouldRespond: IGNORE echo must stay claimable). genuine leak shape from #11712 (leading skeleton) is still detected, blocked, and recovered — existing regression tests unchanged and green.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
LifeOps Benchmark —
|
LifeOps Benchmark —
|
2 participants
Defect
Follow-up to 35e6a66 (#11712). The fail-closed raw-transcript guard and the text-mode transcript parser silently rewrote legitimate replies that QUOTE a
shouldRespond:/replyText:transcript, dropping the actual answer:packages/core/src/services/message.tsfinal_replyguard):looksLikeRawFieldTranscript(reply)matched/(^|\n)\s*replyText\s*:/anywhere in the reply — including quoted lines and lines inside code fences. A user pastes a leaked transcript into Discord and asks the bot to diagnose it (this repo's own daily workflow); Stage 1 answers correctly in the canonical JSON envelope withreplyText= diagnostic prose that quotes the transcript. The guard fired on the quotedreplyText:line and replaced the whole reply withextractReplyTextFromTranscript()output — only the text after the QUOTED marker — destroying the entire diagnosis with nothing but alogger.warn.packages/core/src/runtime/message-handler.tsparseMessageHandlerFieldTranscript): claimed any non-JSON prose containing areplyText:line and discarded every preamble line ("lines before the first field marker are preamble; ignore them") — same silent content loss for plain-prose answers that quote a transcript.!hasShouldRespond && !hasReplyText).Fix (structural, not contains-check)
A genuine text-mode envelope echo IS the message — its first substantive line (outside code fences) is a known field line, with a
shouldRespond:/replyText:hallmark at top level. A reply that opens with prose, or whose field lines sit inside a code fence, QUOTES a transcript and is content:looksLikeRawFieldTranscriptis now a fence-aware line scan implementing that rule. Quoting replies ship intact at the send boundary and through the plain-text synthesizer; the genuine raw RESPONSE_HANDLER structured output (shouldRespond:/replyText:/contexts:/topics:/emotion:) sent verbatim to discord when field parse falls through #11712 leak shape (leading skeleton) is still detected, blocked, and recovered.parseFieldTranscripttreats field lines inside ``` / ~~~ fences as value content, so a real leak whosereplyTextvalue quotes an envelope in a fence is no longer split at the quoted lines.parseMessageHandlerFieldTranscriptis gated on the same detector, so prose-with-preamble falls through to the tolerant plain-text handler with the full answer intact.shouldRespond: IGNOREecho must remain claimable; the comment now says so explicitly.Trade-off made explicit: a hypothetical leak prefixed by scaffold prose would now ship as visible (ugly but complete) text instead of being rewritten — visible skeleton beats silent destruction of a correct answer, and the observed #11712 leak shape (leading skeleton) remains fully fail-closed.
Reproduction / evidence
Re-confirmed on develop tip
16b69a6edfbefore the fix — 6 new tests failed exactly as the defect describes:After the fix:
packages/core/src/runtime/__tests__/response-field-transcript.test.ts— 22/22 (all 16 pre-existing raw RESPONSE_HANDLER structured output (shouldRespond:/replyText:/contexts:/topics:/emotion:) sent verbatim to discord when field parse falls through #11712 regression tests untouched and green, + new quoting/fence coverage)packages/core/src/__tests__/message-routing-live-regression.test.ts— new end-to-end scenario: canonical JSON envelope quoting a transcript →routeMessageHandlerOutput→final_reply→ guard predicate false → diagnosis ships verbatim; plus fail-closed regression for the genuine leakpackages/coresuite: 2670 passed / 11 skipped; the single failure (link-extraction.test.ts, live example.com fetch) reproduces on clean HEAD with this change stashed — pre-existing and unrelatedbun run --cwd packages/core typecheckcleanN/A — UI/screenshots/video: runtime parser/guard change, no user-facing surface beyond message text integrity, fully covered by the routing-level tests above.
N/A — live-LLM trajectory: the defect is deterministic post-model-output plumbing; the failing-then-passing tests drive the exact routed path (
parseMessageHandlerOutput→routeMessageHandlerOutput→ send-boundary predicate) with the confirmed real-world payload shape.