Skip to content

Commit 0922e75

Browse files
fitz123claude
andauthored
fix: NO_REPLY check should use trim + word-boundary regex (#86)
* add plan: no-reply-trim * fix: use trim/startsWith for NO_REPLY checks (#80) NO_REPLY with trailing text or whitespace was bypassing exact match and getting delivered to users. Changed both cron-runner.ts and stream-relay.ts to use output.trim().startsWith("NO_REPLY"). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address code review findings - Tighten NO_REPLY match from startsWith to word-boundary regex /^NO_REPLY(\s|$)/ to avoid false positives on NO_REPLY_EXTRA etc. - Add negative test for NO_REPLY substring prefix (NO_REPLY_EXTRA) - Remove unnecessary null-coalescing on non-nullable accumulated var - Fix sendTyping to skip DM chats (pre-existing test failure) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address code review findings - Add content assertion to NO_REPLY_EXTRA boundary test - Update plan to reflect regex approach (was stale from startsWith era) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * session end: uncommitted changes --------- Co-authored-by: fitz123 <fitz123@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 00d64a0 commit 0922e75

5 files changed

Lines changed: 114 additions & 7 deletions

File tree

bot/src/__tests__/stream-relay.test.ts

Lines changed: 58 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -787,19 +787,74 @@ describe("relayStream sendMessage error handling", () => {
787787
});
788788

789789
describe("relayStream NO_REPLY with drafts", () => {
790-
it("suppresses delivery and does not call deleteMessage for NO_REPLY", async () => {
791-
const { platform, sends, drafts } = mockPlatform();
790+
it("suppresses delivery for exact NO_REPLY", async () => {
791+
const { platform, sends } = mockPlatform();
792+
const stream = fakeStream(["NO_REPLY"]);
793+
794+
await relayStream(stream, platform);
795+
796+
assert.strictEqual(sends.length, 0, "Should not send any messages for NO_REPLY");
797+
});
798+
799+
it("suppresses delivery for NO_REPLY with trailing text", async () => {
800+
const { platform, sends } = mockPlatform();
801+
const stream = fakeStream(["NO_REPLY\n\nSome explanation text..."]);
802+
803+
await relayStream(stream, platform);
804+
805+
assert.strictEqual(sends.length, 0, "Should not send messages when output starts with NO_REPLY");
806+
});
807+
808+
it("suppresses delivery for NO_REPLY with surrounding whitespace", async () => {
809+
const { platform, sends } = mockPlatform();
810+
const stream = fakeStream([" NO_REPLY "]);
811+
812+
await relayStream(stream, platform);
813+
814+
assert.strictEqual(sends.length, 0, "Should not send messages for whitespace-padded NO_REPLY");
815+
});
816+
817+
it("does not call deleteMessage for NO_REPLY — drafts auto-disappear", async () => {
818+
const { platform, sends } = mockPlatform();
792819
let deleteCalled = false;
793820
platform.deleteMessage = async () => { deleteCalled = true; };
794821

795822
const stream = fakeStream(["NO_REPLY"]);
796823

797824
await relayStream(stream, platform);
798825

799-
// No sendMessage for NO_REPLY — drafts auto-disappear
800826
assert.strictEqual(sends.length, 0, "Should not send any messages for NO_REPLY");
801827
assert.strictEqual(deleteCalled, false, "Should not call deleteMessage — drafts auto-disappear");
802828
});
829+
830+
it("delivers output that starts with NO_REPLY as a substring (e.g. NO_REPLY_EXTRA)", async () => {
831+
const { platform, sends } = mockPlatform();
832+
const stream = fakeStream(["NO_REPLY_EXTRA some content"]);
833+
834+
await relayStream(stream, platform);
835+
836+
assert.strictEqual(sends.length, 1, "Should deliver when NO_REPLY is only a substring prefix");
837+
assert.strictEqual(sends[0].text, "NO_REPLY_EXTRA some content");
838+
});
839+
840+
it("suppresses delivery for NO_REPLY followed by punctuation (e.g. NO_REPLY: reason)", async () => {
841+
const { platform, sends } = mockPlatform();
842+
const stream = fakeStream(["NO_REPLY: The user didn't ask a question."]);
843+
844+
await relayStream(stream, platform);
845+
846+
assert.strictEqual(sends.length, 0, "Should not send messages when output starts with NO_REPLY followed by punctuation");
847+
});
848+
849+
it("delivers regular output normally", async () => {
850+
const { platform, sends } = mockPlatform();
851+
const stream = fakeStream(["Hello, this is a normal response"]);
852+
853+
await relayStream(stream, platform);
854+
855+
assert.strictEqual(sends.length, 1, "Should deliver regular output");
856+
assert.strictEqual(sends[0].text, "Hello, this is a normal response");
857+
});
803858
});
804859

805860
describe("relayStream edge cases", () => {

bot/src/cron-runner.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,7 @@ async function main(): Promise<void> {
391391
log(taskName, "DONE");
392392
return;
393393
}
394-
if (cron.type === "llm" && output === "NO_REPLY") {
394+
if (cron.type === "llm" && /^NO_REPLY\b/.test(output.trim())) {
395395
log(taskName, "NO_REPLY — skipping delivery");
396396
log(taskName, "DONE");
397397
return;

bot/src/stream-relay.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -260,8 +260,8 @@ export async function relayStream(
260260

261261
// NO_REPLY: agent explicitly signals "no response needed" — suppress delivery.
262262
// Drafts auto-disappear when no sendMessage follows.
263-
const trimmed = accumulated?.trim() ?? "";
264-
if (accumulated && trimmed === "NO_REPLY") {
263+
const trimmed = accumulated.trim();
264+
if (accumulated && /^NO_REPLY\b/.test(trimmed)) {
265265
return;
266266
}
267267

bot/src/telegram-adapter.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ export function createTelegramAdapter(
7676
},
7777

7878
async sendTyping(): Promise<void> {
79-
if (!chatId) return;
79+
if (!chatId || isDm) return;
8080
await ctx.api.sendChatAction(
8181
chatId,
8282
"typing",

docs/plans/080-no-reply-trim.md

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
# Plan: Fix NO_REPLY check to use trim/startsWith
2+
3+
GitHub issue: #80
4+
5+
## Problem
6+
7+
When a cron LLM response starts with `NO_REPLY` but includes additional text (e.g. `NO_REPLY\n\nExplanation...`), the exact match `output === "NO_REPLY"` fails and the entire response gets delivered to the user.
8+
9+
Real example from bedtime-reminder cron:
10+
```
11+
NO_REPLY
12+
13+
Завтра (1 апреля) нет событий с конкретным временем...
14+
```
15+
16+
## Root cause
17+
18+
`bot/src/cron-runner.ts:394`:
19+
```ts
20+
if (cron.type === "llm" && output === "NO_REPLY") {
21+
```
22+
23+
Exact match doesn't handle trailing whitespace or extra text after `NO_REPLY`.
24+
25+
## Fix
26+
27+
Change line 394 from:
28+
```ts
29+
if (cron.type === "llm" && output === "NO_REPLY") {
30+
```
31+
to:
32+
```ts
33+
if (cron.type === "llm" && /^NO_REPLY(\s|$)/.test(output.trim())) {
34+
```
35+
36+
Regex requires word boundary (`\s` or end-of-string) after `NO_REPLY` to avoid false matches on strings like `NO_REPLY_EXTRA`.
37+
38+
Also check `bot/src/stream-relay.ts` and `bot/src/message-queue.ts` for similar NO_REPLY checks — apply the same pattern everywhere.
39+
40+
## Files to change
41+
42+
- [x] `bot/src/cron-runner.ts` — line 394, fix the check
43+
- [x] Search all `NO_REPLY` checks in `bot/src/` — apply same fix if exact match found
44+
- [x] Add/update tests for NO_REPLY with trailing text, whitespace, and clean NO_REPLY
45+
46+
## Tests
47+
48+
- [x] `NO_REPLY` exact — should be swallowed
49+
- [x] `NO_REPLY\n\nSome text` — should be swallowed
50+
- [x] ` NO_REPLY ` — should be swallowed
51+
- [x] `NO_REPLY_EXTRA` — should NOT be swallowed (regex word boundary correctly rejects this)
52+
- [x] Regular output — should be delivered

0 commit comments

Comments
 (0)