mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-11 03:13:24 +00:00
Merge pull request #179 from digitopvn/codex/issue-141-cron-no-reply-contains
fix(cron): suppress deliveries containing NO_REPLY
This commit is contained in:
3 files changed
+177
-9
No files matched your search
+46
-9
@@ -128,16 +128,26 @@ func makeCronJobHandler(sched *scheduler.Scheduler, msgBus *bus.MessageBus, cfg
|
||||
|
||||
// If job wants delivery to a channel, send the agent response to the target chat.
|
||||
if job.Deliver && job.DeliverChannel != "" && job.DeliverTo != "" {
|
||||
outMsg := bus.OutboundMessage{
|
||||
Channel: job.DeliverChannel,
|
||||
ChatID: job.DeliverTo,
|
||||
Content: result.Content,
|
||||
if cronOutputContainsNoReplySentinel(result.Content) {
|
||||
slog.Info("cron: suppressed delivery because output contained NO_REPLY",
|
||||
"job_id", job.ID,
|
||||
"job_name", job.Name,
|
||||
"channel", job.DeliverChannel,
|
||||
"to", job.DeliverTo,
|
||||
"content_len", len(result.Content),
|
||||
)
|
||||
} else {
|
||||
outMsg := bus.OutboundMessage{
|
||||
Channel: job.DeliverChannel,
|
||||
ChatID: job.DeliverTo,
|
||||
Content: result.Content,
|
||||
}
|
||||
if peerKind == "group" {
|
||||
outMsg.Metadata = map[string]string{"group_id": job.DeliverTo}
|
||||
}
|
||||
appendMediaToOutbound(&outMsg, result.Media)
|
||||
msgBus.PublishOutbound(outMsg)
|
||||
}
|
||||
if peerKind == "group" {
|
||||
outMsg.Metadata = map[string]string{"group_id": job.DeliverTo}
|
||||
}
|
||||
appendMediaToOutbound(&outMsg, result.Media)
|
||||
msgBus.PublishOutbound(outMsg)
|
||||
} else if job.Deliver {
|
||||
slog.Warn("cron: delivery configured but channel/chatID missing — output discarded",
|
||||
"job_id", job.ID, "job_name", job.Name, "channel", job.DeliverChannel, "to", job.DeliverTo)
|
||||
@@ -161,6 +171,33 @@ 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')
|
||||
}
|
||||
|
||||
// resolveCronPeerKind infers peer kind from the cron job's user ID.
|
||||
// Group cron jobs have userID prefixed with "group:" or "guild:" (set during job creation).
|
||||
func resolveCronPeerKind(job *store.CronJob) string {
|
||||
|
||||
@@ -3,10 +3,12 @@ package cmd
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/google/uuid"
|
||||
|
||||
"github.com/nextlevelbuilder/goclaw/internal/agent"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/bus"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/config"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/scheduler"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/store"
|
||||
@@ -64,3 +66,114 @@ func TestCronJobHandlerInjectsPayloadCredentialUserID(t *testing.T) {
|
||||
t.Fatalf("credential user ID in scheduled context = %q, want %q", gotCredentialUserID, wantCredentialUserID)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCronOutputContainsNoReplySentinel(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
in string
|
||||
want bool
|
||||
}{
|
||||
{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: "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},
|
||||
{name: "glued suffix", in: "NO_REPLYING", want: false},
|
||||
{name: "glued prefix", in: "XNO_REPLY", want: false},
|
||||
{name: "empty", in: "", want: false},
|
||||
{name: "unrelated", in: "no reply needed", want: false},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
if got := cronOutputContainsNoReplySentinel(tt.in); got != tt.want {
|
||||
t.Fatalf("cronOutputContainsNoReplySentinel(%q) = %v, want %v", tt.in, got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestCronJobHandlerSuppressesNoReplyDelivery(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
content string
|
||||
wantPublish bool
|
||||
}{
|
||||
{name: "normal content", content: "daily report ready", wantPublish: true},
|
||||
{name: "exact no reply", content: "NO_REPLY", wantPublish: false},
|
||||
{name: "suffix no reply", content: "No relevant update. NO_REPLY", wantPublish: false},
|
||||
{name: "prefix no reply", content: "NO_REPLY - nothing to report", wantPublish: false},
|
||||
{name: "decorative underscore no reply", content: "NO_REPLY_", wantPublish: false},
|
||||
{name: "glued token still delivers", content: "NO_REPLYING is a different word", wantPublish: true},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
mb := bus.New()
|
||||
defer mb.Close()
|
||||
|
||||
sched := scheduler.NewScheduler(
|
||||
scheduler.DefaultLanes(),
|
||||
scheduler.QueueConfig{
|
||||
Mode: scheduler.QueueModeQueue,
|
||||
Cap: 1,
|
||||
Drop: scheduler.DropOld,
|
||||
DebounceMs: 0,
|
||||
MaxConcurrent: 1,
|
||||
},
|
||||
func(context.Context, agent.RunRequest) (*agent.RunResult, error) {
|
||||
return &agent.RunResult{Content: tt.content}, nil
|
||||
},
|
||||
)
|
||||
defer sched.Stop()
|
||||
|
||||
handler := makeCronJobHandler(
|
||||
sched,
|
||||
mb,
|
||||
&config.Config{},
|
||||
nil,
|
||||
nil,
|
||||
nil,
|
||||
)
|
||||
|
||||
result, err := handler(&store.CronJob{
|
||||
ID: uuid.NewString(),
|
||||
TenantID: uuid.New(),
|
||||
Name: "delivery-report",
|
||||
AgentID: "reporter",
|
||||
UserID: "user-1",
|
||||
Stateless: true,
|
||||
Deliver: true,
|
||||
DeliverChannel: "telegram",
|
||||
DeliverTo: "chat-1",
|
||||
Payload: store.CronPayload{
|
||||
Kind: "agent_turn",
|
||||
Message: "daily report",
|
||||
},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("cron handler returned error: %v", err)
|
||||
}
|
||||
if result == nil || result.Content != tt.content {
|
||||
t.Fatalf("cron result = %#v, want content %q", result, tt.content)
|
||||
}
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond)
|
||||
defer cancel()
|
||||
got, ok := mb.SubscribeOutbound(ctx)
|
||||
if !tt.wantPublish {
|
||||
if ok {
|
||||
t.Fatalf("unexpected outbound message: %#v", got)
|
||||
}
|
||||
return
|
||||
}
|
||||
if !ok {
|
||||
t.Fatal("expected outbound message")
|
||||
}
|
||||
if got.Content != tt.content || got.Channel != "telegram" || got.ChatID != "chat-1" {
|
||||
t.Fatalf("outbound message = %#v, want channel telegram chat chat-1 content %q", got, tt.content)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -4,6 +4,24 @@ Significant changes, features, and fixes in reverse chronological order.
|
||||
|
||||
---
|
||||
|
||||
## 2026-06-13
|
||||
|
||||
### Cron NO_REPLY delivery suppression (issue #141)
|
||||
|
||||
**Fixes**
|
||||
|
||||
- Suppressed configured cron channel delivery when final agent output contains a
|
||||
standalone `NO_REPLY` sentinel anywhere in the text, while keeping normal chat
|
||||
silent-reply behavior unchanged.
|
||||
- Added suppression logging with cron job/channel metadata for debugging.
|
||||
|
||||
**Tests**
|
||||
|
||||
- Added cron handler regression coverage for exact, prefix, suffix,
|
||||
mid-sentence, decorative, and glued-token `NO_REPLY` outputs.
|
||||
|
||||
---
|
||||
|
||||
## 2026-06-12
|
||||
|
||||
### Cron scheduler shutdown drain (post-merge CI follow-up)
|
||||
|
||||
Reference in new issue
Block a user