From 24ea38dcb24f9451a33de926e338e96dd7d6688d Mon Sep 17 00:00:00 2001 From: Rusty Raven Date: Thu, 16 Jul 2026 14:24:47 +0800 Subject: [PATCH] fix(comments): safely bound completion fallback output (#5492) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(comments): cap completion-fallback comment synthesized from raw output (#5455) On long, tool-heavy runs some runtimes emit the entire raw execution stream (text deltas + a `tool call` line per tool_use) as the task's final Output. When the agent left no comment of its own, CompleteTask synthesizes a fallback comment from that Output and posted it verbatim, dumping 190-264KB raw streams onto the issue thread (author_type=agent, type=comment, source_task_id=NULL). Cap the synthesized body: redact then truncate to a head that comfortably fits a real final message, appending a marker naming the omitted length instead of the multi-hundred-KB tail. Normal short outputs are unchanged. * test(comments): pin completion-fallback comment cap (#5455) Covers passthrough for real short messages, exact-cap boundary, rune-based (not byte) counting for multibyte content, and the reporter's fingerprint — a narration head followed by hundreds of `tool call` lines — asserting the stored body is bounded, the head preserved, and a truncation marker appended. * fix(comments): replace oversized fallback output with safe notice Co-authored-by: multica-agent --------- Co-authored-by: Eve Co-authored-by: multica-agent --- .../service/fallback_comment_truncate_test.go | 84 +++++++++++++++++++ server/internal/service/task.go | 32 ++++++- 2 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 server/internal/service/fallback_comment_truncate_test.go diff --git a/server/internal/service/fallback_comment_truncate_test.go b/server/internal/service/fallback_comment_truncate_test.go new file mode 100644 index 000000000..5ff53416e --- /dev/null +++ b/server/internal/service/fallback_comment_truncate_test.go @@ -0,0 +1,84 @@ +package service + +import ( + "strings" + "testing" + "unicode/utf8" +) + +// TestTruncateFallbackCommentBody pins the completion-fallback comment cap that +// keeps a runaway raw-stream Output off the issue thread (GH #5455). +func TestTruncateFallbackCommentBody(t *testing.T) { + t.Parallel() + + t.Run("short body passes through unchanged", func(t *testing.T) { + t.Parallel() + // A real final message — multi-line, well under the cap — must be stored + // verbatim, newlines intact (unlike the summary flattening path). + body := "I fixed the bug in the parser.\n\n- root cause: off-by-one\n- added a regression test" + if got := truncateFallbackCommentBody(body, maxSynthesizedFallbackCommentRunes); got != body { + t.Fatalf("short body was altered:\n got: %q\nwant: %q", got, body) + } + }) + + t.Run("body exactly at the cap is untouched", func(t *testing.T) { + t.Parallel() + body := strings.Repeat("x", maxSynthesizedFallbackCommentRunes) + if got := truncateFallbackCommentBody(body, maxSynthesizedFallbackCommentRunes); got != body { + t.Fatalf("body at cap was truncated: len(got)=%d", utf8.RuneCountInString(got)) + } + }) + + t.Run("raw execution-stream dump is replaced with a safe notice", func(t *testing.T) { + t.Parallel() + // Reproduce the reporter's fingerprint: first-turn narration followed by + // hundreds of repeated `tool call` lines and a tail answer — the shape a + // 200KB+ dump takes. None of that untrusted output may reach the comment. + var b strings.Builder + b.WriteString("I'll start by reading the issue context and the relevant files.\n") + for i := 0; i < 40000; i++ { + b.WriteString("tool call\n") + } + b.WriteString("FINAL ANSWER THAT MUST NOT BE MISTAKEN FOR TRUSTED OUTPUT") + dump := b.String() + if utf8.RuneCountInString(dump) < 200_000 { + t.Fatalf("test fixture too small: %d runes", utf8.RuneCountInString(dump)) + } + + got := truncateFallbackCommentBody(dump, maxSynthesizedFallbackCommentRunes) + + if n := utf8.RuneCountInString(got); n > 256 { + t.Fatalf("safe notice is unexpectedly large: %d runes", n) + } + for _, leaked := range []string{"I'll start", "tool call", "FINAL ANSWER"} { + if strings.Contains(got, leaked) { + t.Fatalf("safe notice leaked raw output %q: %q", leaked, got) + } + } + if !strings.Contains(got, "not posted") { + t.Fatalf("safe notice does not explain that output was withheld: %q", got) + } + if !strings.Contains(got, "Execution log") { + t.Fatalf("safe notice does not direct the user to the task run: %q", got) + } + }) + + t.Run("cap counts runes not bytes for multibyte content", func(t *testing.T) { + t.Parallel() + // A body of maxRunes multibyte runes is > maxRunes bytes but == maxRunes + // runes, so it must NOT be truncated — the boundary is rune-based. + body := strings.Repeat("你", maxSynthesizedFallbackCommentRunes) + if got := truncateFallbackCommentBody(body, maxSynthesizedFallbackCommentRunes); got != body { + t.Fatalf("multibyte body at rune cap was wrongly truncated") + } + // One rune over the cap is replaced rather than leaking a raw excerpt. + over := strings.Repeat("你", maxSynthesizedFallbackCommentRunes+10) + got := truncateFallbackCommentBody(over, maxSynthesizedFallbackCommentRunes) + if !utf8.ValidString(got) { + t.Fatalf("safe notice is invalid UTF-8") + } + if strings.Contains(got, "你") { + t.Fatalf("safe notice leaked a multibyte raw-output excerpt") + } + }) +} diff --git a/server/internal/service/task.go b/server/internal/service/task.go index de320b2bb..377856b18 100644 --- a/server/internal/service/task.go +++ b/server/internal/service/task.go @@ -10,6 +10,7 @@ import ( "strings" "sync" "time" + "unicode/utf8" "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgtype" @@ -116,6 +117,32 @@ func truncateForSummary(s string, maxRunes int) string { return string(rs[:maxRunes]) + "…" } +// maxSynthesizedFallbackCommentRunes bounds the completion-fallback comment that +// CompleteTask synthesizes from a task's final output when the agent left no +// comment of its own during the run. A real final assistant message is at most +// a few thousand words; anything larger is a runaway raw-stream dump — every +// streamed text delta concatenated together plus a literal `tool call` line per +// tool_use event — which some runtimes/providers emit as the task's Output on +// long, tool-heavy runs. Such a dump (observed at 190–264 KB) must never be +// posted, even partially, to the issue thread (GH #5455). +const maxSynthesizedFallbackCommentRunes = 8000 + +const oversizedFallbackCommentNotice = "This task completed, but its output was too large to post safely. The raw output was not posted. Review the task in this issue's Execution log." + +// truncateFallbackCommentBody bounds a synthesized completion-fallback comment +// body. Unlike truncateForSummary (which flattens newlines for a one-line row +// snapshot), it preserves genuine final messages below the cap verbatim. Output +// above the cap is untrusted: the reported failure mode puts process narration +// and tool traces at the head, so retaining any excerpt can expose execution +// details and still discard the final answer. Replace the entire body with a +// fixed notice instead. Callers pass the already-redacted body. +func truncateFallbackCommentBody(body string, maxRunes int) string { + if utf8.RuneCountInString(body) <= maxRunes { + return body + } + return oversizedFallbackCommentNotice +} + const ( taskAnalyticsContextCacheMax = 4096 // claimResponseRecoveryWindow must exceed daemon client.Timeout for @@ -2672,7 +2699,10 @@ func (s *TaskService) CompleteTask(ctx context.Context, taskID pgtype.UUID, resu "agent_id", util.UUIDToString(task.AgentID), ) } else { - s.createAgentComment(ctx, task.IssueID, task.AgentID, redact.Text(body), "comment", task.TriggerCommentID, pgtype.UUID{}) + // Redact first, then bound: a runaway raw-stream Output (GH #5455) + // must never reach the issue thread, even as a clipped excerpt. + content := truncateFallbackCommentBody(redact.Text(body), maxSynthesizedFallbackCommentRunes) + s.createAgentComment(ctx, task.IssueID, task.AgentID, content, "comment", task.TriggerCommentID, task.ID) } } }