Compare commits

...

2 Commits

Author SHA1 Message Date
Jiang Bohan
7978236884 fix(comments): address review feedback on newline-rendering PR
- Cover the issue panel: ReadonlyContent (used by every comment card and
  the issue description) has its own react-markdown wiring; add
  remark-breaks there too so the renderer fix actually applies to the
  surface the bug was reported on, not just the chat panel. Pinned by
  ReadonlyContent line-break tests.
- Make the prompt's `--description` guidance executable: add
  `--description-stdin` to `issue create` / `issue update`, refactor
  comment-add to share a single `resolveTextFlag` helper, and have the
  injected runtime config name the real flag instead of an imaginary
  "stdin / a tempfile" path. Pinned by the runtime-config guidance test.
- Document the unescape contract on each affected flag's help text and
  pin the precise boundary in tests: `\n / \r / \t / \\` are decoded;
  `\d / \w / \s / \u / \0` and other unrecognised escapes pass through
  verbatim, so regex literals and Windows paths survive intact unless
  they embed a literal `\n` / `\r` / `\t`. Callers that need the literal
  sequence have `--content-stdin` / `--description-stdin` as the escape
  hatch.
2026-04-27 17:11:48 +08:00
Jiang Bohan
7ff64af921 fix(comments): preserve newlines from agent CLI writes
Agents (e.g. Codex) routinely emit `multica issue comment add --content
"para1\n\npara2"` because Python/JSON-style string literals are their
default. Bash does not expand `\n` inside double quotes, so the literal
4-char sequence flowed through the CLI into the database and rendered
as text in the issue panel — comments came out as one wall of prose.

Three coordinated fixes so the platform behavior no longer depends on
whether a given model has strong bash-quoting intuition:

- CLI: decode `\n / \r / \t / \\` in `--content` and `--description` for
  `issue create / update / comment add` (callers needing a literal
  backslash still have `--content-stdin`).
- Agent prompt: rewrite the comment-add example in the injected runtime
  config to require `--content-stdin` + HEREDOC for any multi-line body,
  and call out the same rule for `--description`. The previous wording
  flagged stdin only for "backticks, quotes", which models read as
  irrelevant to plain paragraphs.
- Renderer: add `remark-breaks` to the shared Markdown plugin chain so a
  bare `\n` becomes a visible line break instead of a CommonMark soft
  break — protects against models that emit single newlines for
  formatting.

Tests: pin the new CLI helper, and pin the runtime-config guidance so
the multi-line wording cannot decay back into a footnote.
2026-04-27 16:54:05 +08:00
10 changed files with 318 additions and 30 deletions

View File

@@ -3,6 +3,7 @@ import ReactMarkdown, { type Components, defaultUrlTransform } from 'react-markd
import rehypeKatex from 'rehype-katex'
import rehypeRaw from 'rehype-raw'
import rehypeSanitize, { defaultSchema } from 'rehype-sanitize'
import remarkBreaks from 'remark-breaks'
import remarkGfm from 'remark-gfm'
import remarkMath from 'remark-math'
import { FileText, Download } from 'lucide-react'
@@ -404,7 +405,7 @@ export function Markdown({
return (
<div className={cn('markdown-content break-words', className)}>
<ReactMarkdown
remarkPlugins={[remarkMath, [remarkGfm, { singleTilde: false }]]}
remarkPlugins={[remarkMath, remarkBreaks, [remarkGfm, { singleTilde: false }]]}
rehypePlugins={[rehypeRaw, [rehypeSanitize, sanitizeSchema], rehypeKatex]}
urlTransform={urlTransform}
components={components}

View File

@@ -39,6 +39,7 @@
"recharts": "3.8.0",
"rehype-katex": "catalog:",
"rehype-raw": "^7.0.0",
"remark-breaks": "^4.0.0",
"remark-gfm": "^4.0.1",
"remark-math": "catalog:",
"shiki": "^3.21.0",

View File

@@ -55,3 +55,20 @@ describe("ReadonlyContent math rendering", () => {
expect(text).toContain("\\int_0^1 x^2 \\, dx");
});
});
describe("ReadonlyContent line breaks", () => {
// Issue panel comments are the primary user-visible surface for agent
// output. CommonMark's default soft-break behavior collapses single
// newlines into spaces; agent text often relies on a single newline as a
// visible break. remark-breaks must remain wired into ReadonlyContent's
// remark plugin chain or comments lose their formatting again.
it("converts a single newline into a <br>", () => {
const { container } = render(<ReadonlyContent content={"line one\nline two"} />);
expect(container.querySelector("br")).not.toBeNull();
});
it("renders a blank-line gap as separate paragraphs", () => {
const { container } = render(<ReadonlyContent content={"para one\n\npara two"} />);
expect(container.querySelectorAll("p").length).toBeGreaterThanOrEqual(2);
});
});

View File

@@ -22,6 +22,7 @@ import ReactMarkdown, {
type Components,
} from "react-markdown";
import rehypeKatex from "rehype-katex";
import remarkBreaks from "remark-breaks";
import remarkGfm from "remark-gfm";
import remarkMath from "remark-math";
import rehypeRaw from "rehype-raw";
@@ -297,7 +298,7 @@ export function ReadonlyContent({ content, className }: ReadonlyContentProps) {
return (
<div ref={wrapperRef} className={cn("rich-text-editor readonly text-sm", className)}>
<ReactMarkdown
remarkPlugins={[remarkMath, [remarkGfm, { singleTilde: false }]]}
remarkPlugins={[remarkMath, remarkBreaks, [remarkGfm, { singleTilde: false }]]}
rehypePlugins={[rehypeRaw, [rehypeSanitize, sanitizeSchema], rehypeKatex]}
urlTransform={urlTransform}
components={components}

View File

@@ -78,6 +78,7 @@
"recharts": "3.8.0",
"rehype-katex": "catalog:",
"rehype-raw": "^7.0.0",
"remark-breaks": "^4.0.0",
"remark-gfm": "^4.0.1",
"remark-math": "catalog:",
"sonner": "^2.0.7"

23
pnpm-lock.yaml generated
View File

@@ -582,6 +582,9 @@ importers:
rehype-raw:
specifier: ^7.0.0
version: 7.0.0
remark-breaks:
specifier: ^4.0.0
version: 4.0.0
remark-gfm:
specifier: ^4.0.1
version: 4.0.1
@@ -742,6 +745,9 @@ importers:
rehype-raw:
specifier: ^7.0.0
version: 7.0.0
remark-breaks:
specifier: ^4.0.0
version: 4.0.0
remark-gfm:
specifier: ^4.0.1
version: 4.0.1
@@ -5459,6 +5465,9 @@ packages:
mdast-util-mdxjs-esm@2.0.1:
resolution: {integrity: sha512-EcmOpxsZ96CvlP03NghtH1EsLtr0n9Tm4lPUJUBccV9RwUOneqSycg19n5HGzCf+10LozMRSObtVr3ee1WoHtg==}
mdast-util-newline-to-break@2.0.0:
resolution: {integrity: sha512-MbgeFca0hLYIEx/2zGsszCSEJJ1JSCdiY5xQxRcLDDGa8EPvlLPupJ4DSajbMPAnC0je8jfb9TiUATnxxrHUog==}
mdast-util-phrasing@4.1.0:
resolution: {integrity: sha512-TqICwyvJJpBwvGAMZjj4J2n0X8QWp21b9l0o7eXyVJ25YNWYbJDVIyD1bZXE6WtV6RmKJVYmQAKWa0zWOABz2w==}
@@ -6447,6 +6456,9 @@ packages:
rehype-sanitize@6.0.0:
resolution: {integrity: sha512-CsnhKNsyI8Tub6L4sm5ZFsme4puGfc6pYylvXo1AeqaGbjOYyzNv3qZPwvs0oMJ39eryyeOdmxwUIo94IpEhqg==}
remark-breaks@4.0.0:
resolution: {integrity: sha512-IjEjJOkH4FuJvHZVIW0QCDWxcG96kCq7An/KVH2NfJe6rKZU2AsHeB3OEjPNRxi4QC34Xdx7I2KGYn6IpT7gxQ==}
remark-gfm@4.0.1:
resolution: {integrity: sha512-1quofZ2RQ9EWdeN34S79+KExV1764+wCUGop5CPL1WGdD0ocPpu91lzPGbwWMECpEpd42kJGQwzRfyov9j4yNg==}
@@ -12618,6 +12630,11 @@ snapshots:
transitivePeerDependencies:
- supports-color
mdast-util-newline-to-break@2.0.0:
dependencies:
'@types/mdast': 4.0.4
mdast-util-find-and-replace: 3.0.2
mdast-util-phrasing@4.1.0:
dependencies:
'@types/mdast': 4.0.4
@@ -13957,6 +13974,12 @@ snapshots:
'@types/hast': 3.0.4
hast-util-sanitize: 5.0.2
remark-breaks@4.0.0:
dependencies:
'@types/mdast': 4.0.4
mdast-util-newline-to-break: 2.0.0
unified: 11.0.5
remark-gfm@4.0.1:
dependencies:
'@types/mdast': 4.0.4

View File

@@ -15,6 +15,79 @@ import (
"github.com/multica-ai/multica/server/internal/cli"
)
// 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).
func resolveTextFlag(cmd *cobra.Command, flagName string) (string, bool, error) {
stdinFlag := flagName + "-stdin"
useStdin, _ := cmd.Flags().GetBool(stdinFlag)
inline, _ := cmd.Flags().GetString(flagName)
if useStdin && inline != "" {
return "", false, fmt.Errorf("--%s and --%s are mutually exclusive", flagName, stdinFlag)
}
if useStdin {
data, err := io.ReadAll(os.Stdin)
if err != nil {
return "", false, fmt.Errorf("read stdin for --%s: %w", stdinFlag, err)
}
body := strings.TrimSuffix(string(data), "\n")
if body == "" {
return "", false, fmt.Errorf("stdin content for --%s is empty", stdinFlag)
}
return body, true, nil
}
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()
}
var issueCmd = &cobra.Command{
Use: "issue",
Short: "Work with issues",
@@ -186,7 +259,8 @@ func init() {
// issue create
issueCreateCmd.Flags().String("title", "", "Issue title (required)")
issueCreateCmd.Flags().String("description", "", "Issue description")
issueCreateCmd.Flags().String("description", "", "Issue description (decodes \\n, \\r, \\t, \\\\; pipe via --description-stdin to preserve literal backslashes)")
issueCreateCmd.Flags().Bool("description-stdin", false, "Read issue description from stdin (preserves multi-line content verbatim)")
issueCreateCmd.Flags().String("status", "", "Issue status")
issueCreateCmd.Flags().String("priority", "", "Issue priority")
issueCreateCmd.Flags().String("assignee", "", "Assignee name (member or agent)")
@@ -198,7 +272,8 @@ func init() {
// issue update
issueUpdateCmd.Flags().String("title", "", "New title")
issueUpdateCmd.Flags().String("description", "", "New description")
issueUpdateCmd.Flags().String("description", "", "New description (decodes \\n, \\r, \\t, \\\\; pipe via --description-stdin to preserve literal backslashes)")
issueUpdateCmd.Flags().Bool("description-stdin", false, "Read new description from stdin (preserves multi-line content verbatim)")
issueUpdateCmd.Flags().String("status", "", "New status")
issueUpdateCmd.Flags().String("priority", "", "New priority")
issueUpdateCmd.Flags().String("assignee", "", "New assignee name (member or agent)")
@@ -232,8 +307,8 @@ func init() {
issueRunMessagesCmd.Flags().Int("since", 0, "Only return messages after this sequence number")
// issue comment add
issueCommentAddCmd.Flags().String("content", "", "Comment content (required unless --content-stdin)")
issueCommentAddCmd.Flags().Bool("content-stdin", false, "Read comment content from stdin (avoids shell escaping issues)")
issueCommentAddCmd.Flags().String("content", "", "Comment content (decodes \\n, \\r, \\t, \\\\; pipe via --content-stdin for multi-line bodies or to preserve literal backslashes)")
issueCommentAddCmd.Flags().Bool("content-stdin", false, "Read comment content from stdin (preserves multi-line content verbatim)")
issueCommentAddCmd.Flags().String("parent", "", "Parent comment ID (reply to a specific comment)")
issueCommentAddCmd.Flags().StringSlice("attachment", nil, "File path(s) to attach (can be specified multiple times)")
issueCommentAddCmd.Flags().String("output", "json", "Output format: table or json")
@@ -411,8 +486,12 @@ func runIssueCreate(cmd *cobra.Command, _ []string) error {
defer cancel()
body := map[string]any{"title": title}
if v, _ := cmd.Flags().GetString("description"); v != "" {
body["description"] = v
desc, hasDesc, err := resolveTextFlag(cmd, "description")
if err != nil {
return err
}
if hasDesc {
body["description"] = desc
}
if v, _ := cmd.Flags().GetString("status"); v != "" {
body["status"] = v
@@ -486,9 +565,12 @@ func runIssueUpdate(cmd *cobra.Command, args []string) error {
v, _ := cmd.Flags().GetString("title")
body["title"] = v
}
if cmd.Flags().Changed("description") {
v, _ := cmd.Flags().GetString("description")
body["description"] = v
if cmd.Flags().Changed("description") || cmd.Flags().Changed("description-stdin") {
desc, _, err := resolveTextFlag(cmd, "description")
if err != nil {
return err
}
body["description"] = desc
}
if cmd.Flags().Changed("status") {
v, _ := cmd.Flags().GetString("status")
@@ -717,25 +799,11 @@ func runIssueCommentList(cmd *cobra.Command, args []string) error {
}
func runIssueCommentAdd(cmd *cobra.Command, args []string) error {
content, _ := cmd.Flags().GetString("content")
useStdin, _ := cmd.Flags().GetBool("content-stdin")
if content != "" && useStdin {
return fmt.Errorf("--content and --content-stdin are mutually exclusive")
content, hasContent, err := resolveTextFlag(cmd, "content")
if err != nil {
return err
}
if useStdin {
data, err := io.ReadAll(os.Stdin)
if err != nil {
return fmt.Errorf("read stdin: %w", err)
}
content = strings.TrimSuffix(string(data), "\n")
if content == "" {
return fmt.Errorf("stdin content is empty")
}
}
if content == "" {
if !hasContent {
return fmt.Errorf("--content or --content-stdin is required")
}

View File

@@ -5,12 +5,145 @@ import (
"encoding/json"
"net/http"
"net/http/httptest"
"os"
"strings"
"testing"
"github.com/spf13/cobra"
"github.com/multica-ai/multica/server/internal/cli"
)
// pipeStdin replaces os.Stdin with a pipe seeded by the given body for the
// duration of fn, so resolveTextFlag's --content-stdin / --description-stdin
// branch can be exercised in unit tests without spawning a subprocess.
func pipeStdin(t *testing.T, body string, fn func()) {
t.Helper()
r, w, err := os.Pipe()
if err != nil {
t.Fatalf("pipe: %v", err)
}
if _, err := w.WriteString(body); err != nil {
t.Fatalf("write pipe: %v", err)
}
if err := w.Close(); err != nil {
t.Fatalf("close pipe writer: %v", err)
}
orig := os.Stdin
os.Stdin = r
defer func() {
os.Stdin = orig
_ = r.Close()
}()
fn()
}
// newFlagTestCmd builds a throwaway cobra.Command carrying the inline +
// stdin flag pair that resolveTextFlag expects.
func newFlagTestCmd(name string) *cobra.Command {
c := &cobra.Command{Use: "test"}
c.Flags().String(name, "", "")
c.Flags().Bool(name+"-stdin", false, "")
return c
}
func TestResolveTextFlag(t *testing.T) {
t.Run("inline value is unescaped", func(t *testing.T) {
c := newFlagTestCmd("description")
_ = c.Flags().Set("description", `para1\n\npara2`)
got, ok, err := resolveTextFlag(c, "description")
if err != nil || !ok {
t.Fatalf("unexpected: ok=%v err=%v", ok, err)
}
if got != "para1\n\npara2" {
t.Errorf("got %q, want decoded paragraphs", got)
}
})
t.Run("stdin body is preserved verbatim", func(t *testing.T) {
c := newFlagTestCmd("description")
_ = c.Flags().Set("description-stdin", "true")
body := "first line\nsecond line with a literal \\n in it\n"
pipeStdin(t, body, func() {
got, ok, err := resolveTextFlag(c, "description")
if err != nil || !ok {
t.Fatalf("unexpected: ok=%v err=%v", ok, err)
}
// strings.TrimSuffix one trailing newline like content-stdin.
want := "first line\nsecond line with a literal \\n in it"
if got != want {
t.Errorf("got %q, want %q", got, want)
}
})
})
t.Run("inline + stdin is rejected", func(t *testing.T) {
c := newFlagTestCmd("description")
_ = c.Flags().Set("description", "inline")
_ = c.Flags().Set("description-stdin", "true")
if _, _, err := resolveTextFlag(c, "description"); err == nil {
t.Fatalf("expected mutually-exclusive error")
}
})
t.Run("missing both returns hasValue=false", func(t *testing.T) {
c := newFlagTestCmd("description")
got, ok, err := resolveTextFlag(c, "description")
if err != nil {
t.Fatalf("unexpected err: %v", err)
}
if ok || got != "" {
t.Errorf("expected absent flag to yield (\"\", false), got (%q, %v)", got, ok)
}
})
}
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

View File

@@ -771,6 +771,39 @@ func TestInjectRuntimeConfigRequiresExplicitCommentPost(t *testing.T) {
}
}
// TestInjectRuntimeConfigDirectsMultiLineWritesToStdin pins the guidance that
// any multi-line content for `multica issue comment add` must go through
// `--content-stdin` + a HEREDOC. Agents that reached for the inline
// `--content "...\n\n..."` form ended up with literal 4-char `\n` sequences
// in stored comments because bash does not expand backslash escapes inside
// double quotes; see MUL-1467. This test prevents the multi-line guidance
// from silently regressing back into a "for special characters" footnote.
func TestInjectRuntimeConfigDirectsMultiLineWritesToStdin(t *testing.T) {
t.Parallel()
dir := t.TempDir()
if err := InjectRuntimeConfig(dir, "claude", TaskContextForEnv{IssueID: "issue-1"}); err != nil {
t.Fatalf("InjectRuntimeConfig failed: %v", err)
}
data, err := os.ReadFile(filepath.Join(dir, "CLAUDE.md"))
if err != nil {
t.Fatalf("read CLAUDE.md: %v", err)
}
s := string(data)
for _, want := range []string{
"multi-line content",
"MUST pipe via stdin",
"--content-stdin",
"<<'COMMENT'",
"`--description`",
"--description-stdin",
} {
if !strings.Contains(s, want) {
t.Errorf("CLAUDE.md missing multi-line guidance %q\n---\n%s", want, s)
}
}
}
func TestInjectRuntimeConfigAutopilotRunOnlyNoIssueWorkflow(t *testing.T) {
t.Parallel()
dir := t.TempDir()

View File

@@ -86,7 +86,17 @@ func buildMetaSkillContent(provider string, ctx TaskContextForEnv) string {
b.WriteString("- `multica issue create --title \"...\" [--description \"...\"] [--priority X] [--assignee X] [--parent <issue-id>] [--status X]` — Create a new issue\n")
b.WriteString("- `multica issue assign <id> --to <name>` — Assign an issue to a member or agent by name (use --unassign to remove assignee)\n")
b.WriteString("- `multica issue comment add <issue-id> --content \"...\" [--parent <comment-id>]` — Post a comment (use --parent to reply to a specific comment)\n")
b.WriteString(" - For content with special characters (backticks, quotes), pipe via stdin: `cat <<'COMMENT' | multica issue comment add <issue-id> --content-stdin`\n")
b.WriteString(" - **For multi-line content (anything with line breaks, paragraphs, code blocks, backticks, or quotes), you MUST pipe via stdin** — bash does NOT expand `\\n` inside double quotes, so writing `--content \"para1\\n\\npara2\"` stores the literal 4-char sequence and the comment renders without line breaks. Use a HEREDOC instead:\n")
b.WriteString("\n")
b.WriteString(" ```\n")
b.WriteString(" cat <<'COMMENT' | multica issue comment add <issue-id> --content-stdin\n")
b.WriteString(" First paragraph.\n")
b.WriteString("\n")
b.WriteString(" Second paragraph with `code` and \"quotes\".\n")
b.WriteString(" COMMENT\n")
b.WriteString(" ```\n")
b.WriteString("\n")
b.WriteString(" - The same rule applies to `--description` on `multica issue create` and `multica issue update` — use `--description-stdin` and pipe a HEREDOC for any multi-line description; the inline `--description \"...\"` form is for short single-line text only.\n")
b.WriteString("- `multica issue comment delete <comment-id>` — Delete a comment\n")
b.WriteString("- `multica issue status <id> <status>` — Update issue status (todo, in_progress, in_review, done, blocked)\n")
b.WriteString("- `multica issue update <id> [--title X] [--description X] [--priority X]` — Update issue fields\n")