From 768f4cd977ecfc19cfe605976ad80f0955383a65 Mon Sep 17 00:00:00 2001 From: Jiang Bohan Date: Wed, 29 Apr 2026 16:56:26 +0800 Subject: [PATCH] fix(comments): unescape `\n` in agent task-completion output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #1744 fixed literal `\n\n` rendering for the CLI surfaces (`issue create / update --description`, `issue comment add --content`) but the agent-completion path bypasses the CLI entirely: the daemon POSTs the agent's stdout to `/api/daemon/tasks/:id/complete`, and `TaskService. CompleteTask` writes `payload.Output` straight into `createAgentComment` and `CreateChatMessage` without decoding. Models (e.g. Codex) routinely emit Python/JSON-style `\n` literals in their final output, which then land in the DB as the 4-char escape sequence and render as one wall of text in the issue/chat panel — exactly the bug report in #1820. - Move `unescapeFlagText` from `server/cmd/multica/cmd_issue.go` to `server/internal/util/text.go` as `UnescapeBackslashEscapes` so the CLI and the service layer share one implementation. The full contract-boundary test suite moves with it. - Apply `UnescapeBackslashEscapes` to `payload.Output` before it reaches `createAgentComment` and `CreateChatMessage` in `TaskService.CompleteTask`. Same `\n / \r / \t / \\` decoding as the CLI; other escape sequences (`\d`, `\w`, `\u`, etc.) pass through verbatim so regex/format strings in agent output survive. Closes #1820 --- server/cmd/multica/cmd_issue.go | 53 ++++------------------------ server/cmd/multica/cmd_issue_test.go | 46 ------------------------ server/internal/service/task.go | 13 +++++-- server/internal/util/text.go | 48 +++++++++++++++++++++++++ server/internal/util/text_test.go | 49 +++++++++++++++++++++++++ 5 files changed, 114 insertions(+), 95 deletions(-) create mode 100644 server/internal/util/text.go create mode 100644 server/internal/util/text_test.go diff --git a/server/cmd/multica/cmd_issue.go b/server/cmd/multica/cmd_issue.go index 39d6fdc675..f95954f5de 100644 --- a/server/cmd/multica/cmd_issue.go +++ b/server/cmd/multica/cmd_issue.go @@ -13,15 +13,16 @@ import ( "github.com/spf13/cobra" "github.com/multica-ai/multica/server/internal/cli" + "github.com/multica-ai/multica/server/internal/util" ) // resolveTextFlag picks between a `--` flag value and a paired // `---stdin` flag, mirroring the existing `--content` / `--content-stdin` // pattern. It returns the resolved string and an error when both are set or -// stdin is requested but produces no body. The resulting text is returned -// verbatim — callers decide whether to apply unescapeFlagText to the inline -// flag form (and never to the stdin form, which already preserves literal -// backslashes). +// stdin is requested but produces no body. Inline flag values are passed +// through util.UnescapeBackslashEscapes so bash-double-quoted `\n` becomes a +// real newline; stdin bodies are returned verbatim so literal backslashes +// survive intact. func resolveTextFlag(cmd *cobra.Command, flagName string) (string, bool, error) { stdinFlag := flagName + "-stdin" useStdin, _ := cmd.Flags().GetBool(stdinFlag) @@ -43,49 +44,7 @@ func resolveTextFlag(cmd *cobra.Command, flagName string) (string, bool, error) if inline == "" { return "", false, nil } - return unescapeFlagText(inline), true, nil -} - -// unescapeFlagText decodes the common backslash escape sequences (\n, \r, \t, -// \\) in a free-form string flag value. Shells like bash do not expand these -// inside double quotes, so an LLM agent that emits -// `--content "para1\n\npara2"` ends up sending the literal 4-char sequence to -// the CLI and then to storage, where it renders as text rather than as line -// breaks. Decoding here makes the flag behave the way callers intuit; users -// who genuinely need a literal backslash-n can write `\\n` or pipe the body -// via `--content-stdin` / `--description-stdin`, which bypass this path -// entirely. -func unescapeFlagText(s string) string { - if !strings.ContainsRune(s, '\\') { - return s - } - var b strings.Builder - b.Grow(len(s)) - for i := 0; i < len(s); i++ { - c := s[i] - if c == '\\' && i+1 < len(s) { - switch s[i+1] { - case 'n': - b.WriteByte('\n') - i++ - continue - case 'r': - b.WriteByte('\r') - i++ - continue - case 't': - b.WriteByte('\t') - i++ - continue - case '\\': - b.WriteByte('\\') - i++ - continue - } - } - b.WriteByte(c) - } - return b.String() + return util.UnescapeBackslashEscapes(inline), true, nil } var issueCmd = &cobra.Command{ diff --git a/server/cmd/multica/cmd_issue_test.go b/server/cmd/multica/cmd_issue_test.go index d3aa12d143..3d42e8087c 100644 --- a/server/cmd/multica/cmd_issue_test.go +++ b/server/cmd/multica/cmd_issue_test.go @@ -98,52 +98,6 @@ func TestResolveTextFlag(t *testing.T) { }) } -func TestUnescapeFlagText(t *testing.T) { - tests := []struct { - name string - in string - want string - }{ - {"empty", "", ""}, - {"no escapes", "hello world", "hello world"}, - {"single newline", `line1\nline2`, "line1\nline2"}, - {"double newline becomes paragraph", `para1\n\npara2`, "para1\n\npara2"}, - {"tab and carriage return", `a\tb\rc`, "a\tb\rc"}, - {"escaped backslash preserved as literal", `keep\\nliteral`, `keep\nliteral`}, - {"trailing lone backslash kept verbatim", `tail\`, `tail\`}, - {"unknown escape kept verbatim", `\x not touched`, `\x not touched`}, - {"mixed real and escaped newlines", "real\n" + `and\nescaped`, "real\nand\nescaped"}, - {"unicode untouched", `中文段落\n下一段`, "中文段落\n下一段"}, - // Contract boundary: only \n \r \t \\ are decoded. Common regex / - // path / formatter escape sequences such as \d, \w, \s, \u, \0 must - // pass through verbatim — this lets users paste regex snippets or - // printf-style format strings into --content without surprise - // mutation. Anyone who genuinely wants the literal characters \\n - // can either double the backslash or pipe the body via stdin. - {"regex digit class untouched", `\d+\s*\w+`, `\d+\s*\w+`}, - {"unicode escape untouched", `café`, `café`}, - {"null escape untouched", `\0 sentinel`, `\0 sentinel`}, - {"windows path no special chars", `C:\Users\bob`, `C:\Users\bob`}, - {"backslash-quote pair untouched", `quote\"inside`, `quote\"inside`}, - // Documented sharp edge of the contract: a path or string that - // embeds a literal backslash-n IS rewritten because the helper - // cannot distinguish "model emitted \n thinking it would become a - // newline" from "user pasted a path that happens to start with - // \new". Callers who need the literal sequence must double the - // backslash (`\\new`) or pipe the body via --content-stdin / - // --description-stdin. This test pins that intentional behavior. - {"path starting with backslash-n is mutated", `C:\new\folder`, "C:\new\\folder"}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := unescapeFlagText(tt.in) - if got != tt.want { - t.Errorf("unescapeFlagText(%q) = %q, want %q", tt.in, got, tt.want) - } - }) - } -} - func TestTruncateID(t *testing.T) { tests := []struct { name string diff --git a/server/internal/service/task.go b/server/internal/service/task.go index 648ed82169..704ae78189 100644 --- a/server/internal/service/task.go +++ b/server/internal/service/task.go @@ -621,7 +621,12 @@ func (s *TaskService) CompleteTask(ctx context.Context, taskID pgtype.UUID, resu var payload protocol.TaskCompletedPayload if err := json.Unmarshal(result, &payload); err == nil { if payload.Output != "" { - s.createAgentComment(ctx, task.IssueID, task.AgentID, redact.Text(payload.Output), "comment", task.TriggerCommentID) + // Match the CLI's --content / --description behavior: agents that + // emit literal `\n` 4-char sequences (Python/JSON-style) get them + // decoded into real newlines before the comment hits the DB. See + // util.UnescapeBackslashEscapes for the exact contract. + body := util.UnescapeBackslashEscapes(payload.Output) + s.createAgentComment(ctx, task.IssueID, task.AgentID, redact.Text(body), "comment", task.TriggerCommentID) } } } @@ -642,10 +647,14 @@ func (s *TaskService) CompleteTask(ctx context.Context, taskID pgtype.UUID, resu if task.ChatSessionID.Valid { var payload protocol.TaskCompletedPayload if err := json.Unmarshal(result, &payload); err == nil && payload.Output != "" { + // Same unescape as the issue-comment path above: literal `\n` from + // agent stdout becomes a real newline so the chat panel renders + // paragraph breaks instead of one wall of prose. + body := util.UnescapeBackslashEscapes(payload.Output) if _, err := s.Queries.CreateChatMessage(ctx, db.CreateChatMessageParams{ ChatSessionID: task.ChatSessionID, Role: "assistant", - Content: redact.Text(payload.Output), + Content: redact.Text(body), TaskID: task.ID, }); err != nil { slog.Error("failed to save assistant chat message", "task_id", util.UUIDToString(task.ID), "error", err) diff --git a/server/internal/util/text.go b/server/internal/util/text.go new file mode 100644 index 0000000000..00be979a70 --- /dev/null +++ b/server/internal/util/text.go @@ -0,0 +1,48 @@ +package util + +import "strings" + +// UnescapeBackslashEscapes decodes the common backslash escape sequences +// (\n, \r, \t, \\) that LLM agents routinely emit as 4-character literals +// because Python/JSON-style string conventions are their default. The same +// helper is used by the CLI to fix bash-double-quote bodies (where the shell +// doesn't expand \n) and by the daemon-task completion path to fix raw agent +// stdout that arrives with literal `\n\n` between paragraphs. +// +// Only \n / \r / \t / \\ are decoded. Other escape sequences (\d, \w, \s, +// \u, \0, \", etc.) pass through verbatim so regex literals and printf +// format strings survive without surprise mutation. Callers that need the +// literal 4-char sequence intact should bypass this helper entirely (the CLI +// exposes --content-stdin / --description-stdin for that case). +func UnescapeBackslashEscapes(s string) string { + if !strings.ContainsRune(s, '\\') { + return s + } + var b strings.Builder + b.Grow(len(s)) + for i := 0; i < len(s); i++ { + c := s[i] + if c == '\\' && i+1 < len(s) { + switch s[i+1] { + case 'n': + b.WriteByte('\n') + i++ + continue + case 'r': + b.WriteByte('\r') + i++ + continue + case 't': + b.WriteByte('\t') + i++ + continue + case '\\': + b.WriteByte('\\') + i++ + continue + } + } + b.WriteByte(c) + } + return b.String() +} diff --git a/server/internal/util/text_test.go b/server/internal/util/text_test.go new file mode 100644 index 0000000000..96b6619c47 --- /dev/null +++ b/server/internal/util/text_test.go @@ -0,0 +1,49 @@ +package util + +import "testing" + +func TestUnescapeBackslashEscapes(t *testing.T) { + tests := []struct { + name string + in string + want string + }{ + {"empty", "", ""}, + {"no escapes", "hello world", "hello world"}, + {"single newline", `line1\nline2`, "line1\nline2"}, + {"double newline becomes paragraph", `para1\n\npara2`, "para1\n\npara2"}, + {"tab and carriage return", `a\tb\rc`, "a\tb\rc"}, + {"escaped backslash preserved as literal", `keep\\nliteral`, `keep\nliteral`}, + {"trailing lone backslash kept verbatim", `tail\`, `tail\`}, + {"unknown escape kept verbatim", `\x not touched`, `\x not touched`}, + {"mixed real and escaped newlines", "real\n" + `and\nescaped`, "real\nand\nescaped"}, + {"unicode untouched", `中文段落\n下一段`, "中文段落\n下一段"}, + // Contract boundary: only \n \r \t \\ are decoded. Common regex / + // path / formatter escape sequences such as \d, \w, \s, \u, \0 must + // pass through verbatim — this lets users paste regex snippets or + // printf-style format strings into --content without surprise + // mutation. Anyone who genuinely wants the literal characters \\n + // can either double the backslash or pipe the body via stdin. + {"regex digit class untouched", `\d+\s*\w+`, `\d+\s*\w+`}, + {"unicode escape untouched", `café`, `café`}, + {"null escape untouched", `\0 sentinel`, `\0 sentinel`}, + {"windows path no special chars", `C:\Users\bob`, `C:\Users\bob`}, + {"backslash-quote pair untouched", `quote\"inside`, `quote\"inside`}, + // Documented sharp edge of the contract: a path or string that + // embeds a literal backslash-n IS rewritten because the helper + // cannot distinguish "model emitted \n thinking it would become a + // newline" from "user pasted a path that happens to start with + // \new". Callers who need the literal sequence must double the + // backslash (`\\new`) or pipe the body via --content-stdin / + // --description-stdin. This test pins that intentional behavior. + {"path starting with backslash-n is mutated", `C:\new\folder`, "C:\new\\folder"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := UnescapeBackslashEscapes(tt.in) + if got != tt.want { + t.Errorf("UnescapeBackslashEscapes(%q) = %q, want %q", tt.in, got, tt.want) + } + }) + } +}