From 6c8ad12858d9a3c754c1624ea0b876814ed2db19 Mon Sep 17 00:00:00 2001 From: nguyenha935 Date: Sun, 21 Jun 2026 06:03:08 +0700 Subject: [PATCH] fix: unify NO_REPLY detection (#1233) Co-authored-by: GoClaw Operator --- cmd/gateway_cron.go | 25 +-------- cmd/gateway_cron_test.go | 2 + internal/agent/sanitize.go | 57 +++++++++++--------- internal/agent/sanitize_silent_reply_test.go | 8 ++- internal/pipeline/stages_test.go | 29 ++++++++++ 5 files changed, 71 insertions(+), 50 deletions(-) diff --git a/cmd/gateway_cron.go b/cmd/gateway_cron.go index 0ee72308..3b2b10d9 100644 --- a/cmd/gateway_cron.go +++ b/cmd/gateway_cron.go @@ -172,30 +172,7 @@ func makeCronJobHandler(sched *scheduler.Scheduler, msgBus *bus.MessageBus, cfg } func cronOutputContainsNoReplySentinel(content string) bool { - text := strings.TrimSpace(content) - if text == "" { - return false - } - - const token = "NO_REPLY" - for i := 0; i+len(token) <= len(text); i++ { - if !strings.EqualFold(text[i:i+len(token)], token) { - continue - } - beforeOK := i == 0 || !cronNoReplyAlphaNumByte(text[i-1]) - after := i + len(token) - afterOK := after == len(text) || !cronNoReplyAlphaNumByte(text[after]) - if beforeOK && afterOK { - return true - } - } - return false -} - -func cronNoReplyAlphaNumByte(b byte) bool { - return (b >= 'a' && b <= 'z') || - (b >= 'A' && b <= 'Z') || - (b >= '0' && b <= '9') + return agent.IsSilentReply(content) } // resolveCronPeerKind infers peer kind from the cron job's user ID. diff --git a/cmd/gateway_cron_test.go b/cmd/gateway_cron_test.go index cd3f4735..a9d7e10b 100644 --- a/cmd/gateway_cron_test.go +++ b/cmd/gateway_cron_test.go @@ -76,6 +76,8 @@ func TestCronOutputContainsNoReplySentinel(t *testing.T) { {name: "exact", in: "NO_REPLY", want: true}, {name: "prefix explanation", in: "NO_REPLY - nothing to report", want: true}, {name: "suffix", in: "No relevant update. NO_REPLY", want: true}, + {name: "terminal glued punctuation", in: "This is directed at Bảo Ly Content, not me. I should stay silent.NO_REPLY", want: true}, + {name: "standalone suffix after space", in: "This is not for me. NO_REPLY", want: true}, {name: "mid sentence", in: "No changes found. NO_REPLY for this run.", want: true}, {name: "lowercase", in: "no_reply", want: true}, {name: "decorative underscore", in: "NO_REPLY_", want: true}, diff --git a/internal/agent/sanitize.go b/internal/agent/sanitize.go index a5e77841..2d2e9233 100644 --- a/internal/agent/sanitize.go +++ b/internal/agent/sanitize.go @@ -12,15 +12,14 @@ // - collapseConsecutiveDuplicateBlocks() // // Additional Go-specific: -// 5. stripEchoedSystemMessages() → strip hallucinated [System Message] blocks -// 6. stripGarbledToolXML() → strip garbled XML from models like DeepSeek +// 5. stripEchoedSystemMessages() → strip hallucinated [System Message] blocks +// 6. stripGarbledToolXML() → strip garbled XML from models like DeepSeek package agent import ( "log/slog" "regexp" "strings" - "unicode/utf8" "github.com/nextlevelbuilder/goclaw/internal/store" ) @@ -170,8 +169,10 @@ func stripDowngradedToolCallText(content string) string { // Matches TS stripThinkingTagsFromText() with strict mode. // Strips: ..., ..., -// ..., ..., -// ... +// +// ..., ..., +// ... +// // Go regexp doesn't support backreferences, so we use separate patterns. var thinkingTagPatterns = []*regexp.Regexp{ regexp.MustCompile(`(?is)]*>.*?`), @@ -378,17 +379,12 @@ func StripConfigLeak(content, agentType string) string { // --- NO_REPLY detection --- -// IsSilentReply checks if the text begins with a NO_REPLY token. +// IsSilentReply checks if the text contains a standalone NO_REPLY token. // -// Divergent from TS isSilentReplyText() (exact-match only) — we match broadly: -// decorative wrappers (`NO_REPLY_`, `"NO_REPLY"`, `**NO_REPLY**`) AND trailing -// explanations (`NO_REPLY because offline`, `NO_REPLY: note`) suppress delivery. -// Only requirement: the token is not glued to another word (`NO_REPLYING` is NOT silent). -// Case-insensitive. -// -// Trade-off vs upstream #19537: upstream guards against suppressing substantive -// replies that end in NO_REPLY. We accept that risk because observed model output -// leans toward "NO_REPLY + reason" rather than "real reply ending in NO_REPLY". +// Divergent from TS isSilentReplyText() (exact-match only) — we match broadly so +// both prefix forms (`NO_REPLY because offline`) and terminal sentinel forms +// (`not for me. NO_REPLY`) suppress delivery. The token must not be glued to +// another word (`NO_REPLYING` and `XNO_REPLY` are not silent). Case-insensitive. func IsSilentReply(text string) bool { trimmed := strings.TrimSpace(text) if trimmed == "" { @@ -397,18 +393,31 @@ func IsSilentReply(text string) bool { // Strip decorative wrappers from both ends (quotes, markdown emphasis, punctuation). stripped := strings.Trim(trimmed, "_ \t\n\r.,:;!?\"'`*~#>-()[]{}") const token = "NO_REPLY" - if len(stripped) < len(token) { + return containsStandaloneNoReplyToken(stripped, token) +} + +func containsStandaloneNoReplyToken(text, token string) bool { + if len(text) < len(token) { return false } - if !strings.EqualFold(stripped[:len(token)], token) { - return false + for i := 0; i+len(token) <= len(text); i++ { + if !strings.EqualFold(text[i:i+len(token)], token) { + continue + } + beforeOK := i == 0 || !isAlphaNumByte(text[i-1]) + after := i + len(token) + afterOK := after == len(text) || !isAlphaNumByte(text[after]) + if beforeOK && afterOK { + return true + } } - if len(stripped) == len(token) { - return true - } - // Token must not be glued to another word — next rune must be non-alphanumeric. - next, _ := utf8.DecodeRuneInString(stripped[len(token):]) - return !isAlphaNum(next) + return false +} + +func isAlphaNumByte(b byte) bool { + return (b >= 'a' && b <= 'z') || + (b >= 'A' && b <= 'Z') || + (b >= '0' && b <= '9') } func isAlphaNum(r rune) bool { diff --git a/internal/agent/sanitize_silent_reply_test.go b/internal/agent/sanitize_silent_reply_test.go index 05694026..900815e9 100644 --- a/internal/agent/sanitize_silent_reply_test.go +++ b/internal/agent/sanitize_silent_reply_test.go @@ -31,10 +31,14 @@ func TestIsSilentReply(t *testing.T) { {"prefix + space + content", "NO_REPLY hello", true}, {"prefix + colon + content", "NO_REPLY: offline", true}, {"prefix + because", "NO_REPLY because user is away", true}, + {"reason then terminal sentinel", "This is directed at Bảo Ly Content, not me. I should stay silent.NO_REPLY", true}, + {"suffix with whitespace", "This is not for me. NO_REPLY", true}, + {"suffix with newline", "This is not for me.\nNO_REPLY", true}, // NOT silent — token glued to another word, or not at start. {"embedded word", "NO_REPLYING", false}, - {"trailing after content", "Here you go. NO_REPLY", false}, - {"token mid-sentence", "Hello NO_REPLY world", false}, + {"glued suffix stays non-silent", "This is not for me.NO_REPLYING", false}, + {"trailing after content", "Here you go. NO_REPLY", true}, + {"token mid-sentence", "Hello NO_REPLY world", true}, {"empty", "", false}, {"whitespace only", " ", false}, {"unrelated text", "no reply needed", false}, diff --git a/internal/pipeline/stages_test.go b/internal/pipeline/stages_test.go index 75b90e1c..eb7087f3 100644 --- a/internal/pipeline/stages_test.go +++ b/internal/pipeline/stages_test.go @@ -2440,6 +2440,35 @@ func TestFinalizeStage_SanitizesContent(t *testing.T) { } } +func TestFinalizeStage_SuppressesSilentReplyAfterFlush(t *testing.T) { + t.Parallel() + finalContent := "This is directed at Bảo Ly Content, not me. I should stay silent.NO_REPLY" + var flushed []providers.Message + deps := &PipelineDeps{ + IsSilentReply: func(content string) bool { + return content == finalContent + }, + FlushMessages: func(_ context.Context, _ string, messages []providers.Message) error { + flushed = append(flushed, messages...) + return nil + }, + } + stage := NewFinalizeStage(deps) + state := defaultState() + state.Observe.FinalContent = finalContent + + err := stage.Execute(context.Background(), state) + if err != nil { + t.Fatalf("Execute() error: %v", err) + } + if state.Observe.FinalContent != "" { + t.Fatalf("FinalContent = %q, want empty after silent reply suppression", state.Observe.FinalContent) + } + if len(flushed) == 0 || flushed[len(flushed)-1].Content != finalContent { + t.Fatalf("flushed final content = %#v, want original silent reply persisted before suppression", flushed) + } +} + func TestFinalizeStage_DeduplicatesMediaByPath(t *testing.T) { t.Parallel() deps := &PipelineDeps{}