Merge remote-tracking branch 'origin/main' into agent/j/d8e21756

server/internal/handler/comment.go

@@ -1139,7 +1139,13 @@ func (h *Handler) PreviewCommentTriggers(w http.ResponseWriter, r *http.Request)
 		parentComment = &parent
 	}
 
-	content := req.Content
+	// Normalize with the SAME entry point CreateComment/UpdateComment apply
+	// before persisting (sanitizeNullBytes), so the preview computes triggers on
+	// the exact content that will be stored and enqueued on submit. Otherwise a
+	// mention hidden behind a byte the DB strips (e.g. a NUL inside
+	// mention://agent/<uuid>) reads as inert in preview but enqueues the agent on
+	// submit — a preview/side-effect divergence (GH #5388 review).
+	content := sanitizeNullBytes(req.Content)
 	if content == "" {
 		writeJSON(w, http.StatusOK, CommentTriggerPreviewResponse{Agents: []CommentTriggerAgentResponse{}})
 		return
@@ -1200,6 +1206,15 @@ func (h *Handler) CreateComment(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// Strip bytes PostgreSQL's TEXT column rejects before the empty check. The
+	// case reachable over the JSON API is an embedded NUL (0x00, SQLSTATE
+	// 22021) that survives a JSON round trip; sanitizeNullBytes also drops
+	// invalid UTF-8 as defense-in-depth. A stray such byte in agent-written
+	// content (notably via --content-file) otherwise fails the INSERT with an
+	// opaque 500 the CLI renders as "server unavailable" and retries forever —
+	// the plausible cause of GH #5388. Mirrors the skill-import sanitization;
+	// normalizing first means all-NUL content is correctly treated as empty.
+	req.Content = sanitizeNullBytes(req.Content)
 	if req.Content == "" {
 		writeError(w, http.StatusBadRequest, "content is required")
 		return
@@ -2054,6 +2069,10 @@ func (h *Handler) UpdateComment(w http.ResponseWriter, r *http.Request) {
 		writeError(w, http.StatusBadRequest, "invalid request body")
 		return
 	}
+	// See CreateComment: strip NUL / invalid-UTF-8 bytes PostgreSQL's TEXT column
+	// rejects before the empty check, so an edit that introduces such a byte
+	// can't 500 (GH #5388).
+	req.Content = sanitizeNullBytes(req.Content)
 	if req.Content == "" {
 		writeError(w, http.StatusBadRequest, "content is required")
 		return

server/internal/handler/comment_content_sanitize_test.go

@@ -0,0 +1,116 @@
+package handler
+
+import (
+	"encoding/json"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/multica-ai/multica/server/internal/util"
+)
+
+// TestCreateComment_StripsNullBytesInsteadOf500 pins the fix for GH #5388.
+//
+// A comment whose content carries a byte PostgreSQL's TEXT type cannot store —
+// most commonly an embedded NUL (SQLSTATE 22021) that survives a JSON round
+// trip from `--content-file` — must post successfully with the offending byte
+// stripped, not fail the INSERT with an opaque 500 the CLI renders as a
+// generic "server unavailable" (and then retries forever).
+func TestCreateComment_StripsNullBytesInsteadOf500(t *testing.T) {
+	if testHandler == nil || testPool == nil {
+		t.Skip("database not available")
+	}
+
+	issueID := createTestIssue(t, "null-byte comment fixture (GH #5388)", "todo", "medium")
+	t.Cleanup(func() { deleteTestIssue(t, issueID) })
+
+	w := httptest.NewRecorder()
+	r := newRequest("POST", "/api/issues/"+issueID+"/comments", map[string]any{
+		"content": "diagnosis body\x00 with a stray NUL byte",
+	})
+	r = withURLParam(r, "id", issueID)
+
+	testHandler.CreateComment(w, r)
+	if w.Code != http.StatusCreated {
+		t.Fatalf("CreateComment with NUL byte: expected 201, got %d: %s", w.Code, w.Body.String())
+	}
+
+	var body map[string]any
+	if err := json.NewDecoder(w.Body).Decode(&body); err != nil {
+		t.Fatalf("decode response: %v", err)
+	}
+	got, _ := body["content"].(string)
+	if strings.ContainsRune(got, '\x00') {
+		t.Fatalf("stored content still contains a NUL byte: %q", got)
+	}
+	if want := "diagnosis body with a stray NUL byte"; got != want {
+		t.Fatalf("stored content: expected %q (NUL stripped), got %q", want, got)
+	}
+}
+
+// TestCommentTriggers_NullByteHiddenMention_PreviewMatchesEnqueue guards the
+// preview/side-effect divergence raised in review of the #5388 fix.
+//
+// CreateComment/UpdateComment sanitize content (strip NUL) before storing and
+// triggering, so a mention hidden behind a NUL — valid only AFTER the byte is
+// stripped — would enqueue an agent. PreviewCommentTriggers must sanitize with
+// the same entry point, or it would report an empty target set while submit
+// silently enqueues the agent. This pins parity for both the create and edit
+// paths (the fix touches both).
+func TestCommentTriggers_NullByteHiddenMention_PreviewMatchesEnqueue(t *testing.T) {
+	if testHandler == nil || testPool == nil {
+		t.Skip("database not available")
+	}
+
+	issueID := createCommentTriggerPreviewIssue(t, "NUL-hidden mention parity (GH #5388 review)", "", "")
+	// Distinct agents for create vs edit: a single (issue, agent) pair allows
+	// only one pending task (idx_one_pending_task_per_issue_agent), which would
+	// mask the second enqueue.
+	createTarget := createHandlerTestAgent(t, "Preview NUL Parity Create", nil)
+	editTarget := createHandlerTestAgent(t, "Preview NUL Parity Edit", nil)
+
+	// mentionWithHiddenNull places the NUL between the agent UUID and the
+	// closing ')', so MentionRe does not match the raw text but does match once
+	// the byte is stripped.
+	mentionWithHiddenNull := func(agentID string) string {
+		return fmt.Sprintf("please take a look [@Target](mention://agent/%s\x00)", agentID)
+	}
+	previewAgentIDs := func(resp CommentTriggerPreviewResponse) map[string]bool {
+		ids := make(map[string]bool, len(resp.Agents))
+		for _, a := range resp.Agents {
+			ids[a.ID] = true
+		}
+		return ids
+	}
+
+	// --- create path ---
+	createContent := mentionWithHiddenNull(createTarget)
+	if n := len(util.ParseMentions(createContent)); n != 0 {
+		t.Fatalf("precondition: raw NUL content should not parse as a mention, got %d", n)
+	}
+	preview := previewCommentTriggersForTest(t, issueID, map[string]any{"content": createContent})
+	if ids := previewAgentIDs(preview); len(ids) != 1 || !ids[createTarget] {
+		t.Fatalf("create preview targets = %v, want exactly {%s}", ids, createTarget)
+	}
+	postCommentForTriggerPreviewTest(t, issueID, map[string]any{"content": createContent})
+	if got := countQueuedCommentTriggerTasks(t, issueID, createTarget); got != 1 {
+		t.Fatalf("create enqueued %d tasks for the mentioned agent, want 1 (parity with preview)", got)
+	}
+
+	// --- edit path ---
+	editContent := mentionWithHiddenNull(editTarget)
+	plainID := postCommentForTriggerPreviewTest(t, issueID, map[string]any{"content": "no mentions here yet"})
+	editPreview := previewCommentTriggersForTest(t, issueID, map[string]any{
+		"content":            editContent,
+		"editing_comment_id": plainID,
+	})
+	if ids := previewAgentIDs(editPreview); len(ids) != 1 || !ids[editTarget] {
+		t.Fatalf("edit preview targets = %v, want exactly {%s}", ids, editTarget)
+	}
+	updateCommentForTriggerPreviewTest(t, plainID, map[string]any{"content": editContent})
+	if got := countQueuedCommentTriggerTasks(t, issueID, editTarget); got != 1 {
+		t.Fatalf("edit enqueued %d tasks for the mentioned agent, want 1 (parity with preview)", got)
+	}
+}