mirror of
https://github.com/multica-ai/multica.git
synced 2026-07-28 05:46:58 +02:00
fix(comments): unescape \n in agent task-completion output
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
This commit is contained in:
@@ -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 `--<name>` flag value and a paired
|
||||
// `--<name>-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{
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
48
server/internal/util/text.go
Normal file
48
server/internal/util/text.go
Normal file
@@ -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()
|
||||
}
|
||||
49
server/internal/util/text_test.go
Normal file
49
server/internal/util/text_test.go
Normal file
@@ -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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user