fix(comments): honest merge outcome — refused merge is blocked, not fake coalesced (MUL-4525)

A pending-task merge previously reported success (coalesced) even when it
was refused or failed: attribution fail-closed and unknown DB errors both
returned handled=true, so the caller recorded coalesced for a merge that
never happened. mergeCommentIntoPendingTask now returns a distinguishable
commentMergeResult; commentMergeTerminalOutcome maps a real merge to
coalesced, a fail-closed refusal to blocked/attribution_blocked, and any
other failure to blocked/internal_error. Only "no queued task to fold"
falls through to the active-task decision. Adds pure coverage of the
mapping plus a fail-closed regression asserting the non-success outcome
and unchanged task count.

Co-authored-by: multica-agent <github@multica.ai>

server/internal/handler/comment.go

@@ -1494,8 +1494,16 @@ func (h *Handler) enqueueCommentAgentTriggers(ctx context.Context, issue db.Issu
 			// queued (not-yet-claimed) task so a single run still covers every
 			// comment, re-stamping the run's originator/overlay to the new
 			// comment (mergeCommentIntoPendingTask).
-			if h.mergeCommentIntoPendingTask(ctx, issue, trigger, triggerCommentID) {
-				record(trigger, DispatchCoalesced, ReasonCoalesced)
+			//
+			// The merge reports HOW it resolved: a real merge is coalesced, a
+			// fail-closed / failed merge is blocked (attribution_blocked /
+			// internal_error) — never mislabeled as success (MUL-4525 §2, Elon
+			// round 5). Only "no queued task to fold into" falls through to the
+			// active-task decision below.
+			if status, reason, terminal := commentMergeTerminalOutcome(
+				h.mergeCommentIntoPendingTask(ctx, issue, trigger, triggerCommentID),
+			); terminal {
+				record(trigger, status, reason)
 				continue
 			}
 			// The merge found no queued task to fold into: the existing task
@@ -1640,36 +1648,62 @@ func decideSuppressedLeaderOutcome(active bool, activeErr error) (DispatchStatus
 	}
 }
 
-// mergeCommentIntoPendingTask folds a newly-arrived comment into an existing
-// not-yet-started task for (issue, agent) instead of dropping it (MUL-4195).
-// Returns true when the comment was HANDLED — either merged, or a non-fatal DB
-// error we deliberately do not convert into a duplicate task. Returns false
-// only when no pending task exists anymore (pgx.ErrNoRows: it was
-// claimed/started between the dedup check and now), so the caller enqueues a
-// fresh task and the deliberate comment is never lost.
-//
-// The merge is GATED on the originator being unchanged (MUL-4195 review
+// commentMergeResult distinguishes how a pending-task merge attempt resolved so
+// the caller can report an HONEST outcome (MUL-4525, Elon round 5). A real merge
+// is coalesced, but a REFUSED or FAILED merge — even when we correctly fail
+// closed by keeping the original task and not enqueuing a duplicate — must NOT
+// be reported as a success-shaped coalesced.
+type commentMergeResult int
+
+const (
+	// commentMergeSucceeded: the comment folded into the queued task → coalesced.
+	commentMergeSucceeded commentMergeResult = iota
+	// commentMergeNoPendingTask: no queued task to merge into anymore (it was
+	// claimed/started between the dedup check and now). The caller runs the
+	// active-task decision (defer vs fresh enqueue).
+	commentMergeNoPendingTask
+	// commentMergeAttributionBlocked: fail-closed attribution refused re-stamping
+	// the merge. The original task is kept and no fresh task is enqueued, but the
+	// re-attribution did NOT happen → outcome attribution_blocked, not success.
+	commentMergeAttributionBlocked
+	// commentMergeError: an unknown attribution/DB error. Fail closed (keep the
+	// task, no duplicate enqueue), but the merge did not complete → outcome
+	// internal_error, not success.
+	commentMergeError
+)
+
+// commentMergeTerminalOutcome maps a merge result that carries its own final
+// outcome (everything except commentMergeNoPendingTask, which needs the
+// active-task decision) to the reported (status, reason). terminal=false only
+// for commentMergeNoPendingTask.
+func commentMergeTerminalOutcome(result commentMergeResult) (status DispatchStatus, reason DispatchReasonCode, terminal bool) {
+	switch result {
+	case commentMergeSucceeded:
+		return DispatchCoalesced, ReasonCoalesced, true
+	case commentMergeAttributionBlocked:
+		return DispatchBlocked, ReasonAttributionBlocked, true
+	case commentMergeError:
+		return DispatchBlocked, ReasonInternalError, true
+	default: // commentMergeNoPendingTask
+		return "", "", false
+	}
+}
+
 // mergeCommentIntoPendingTask folds a newly-arrived comment into the existing
 // QUEUED (not-yet-claimed) task for (issue, agent) instead of dropping it
-// (MUL-4195). Returns true when the comment was handled (merged, or a
-// non-fatal DB error we deliberately do not turn into a duplicate). Returns
-// false only when no queued task exists to merge into (pgx.ErrNoRows) — the
-// existing task is already dispatched/running, or was just claimed — in which
-// case the caller decides between deferring to completion reconcile and a fresh
-// enqueue.
+// (MUL-4195). It reports HOW it resolved via commentMergeResult so the caller
+// never mislabels a refused/failed merge as success (MUL-4525 §2). No path here
+// enqueues a duplicate: on any failure the original task is kept intact, so the
+// comment is still read by that run and its instruction is not lost — only the
+// re-attribution / merge bookkeeping is declined, and that is surfaced honestly.
 //
-// Recompute-on-merge (MUL-4195 review must-fix #1): the run's
+// Recompute-on-merge (MUL-4195 review must-fix #1): on success the run's
 // originator_user_id, runtime_mcp_overlay and runtime_connected_apps are
 // re-stamped to the NEW trigger comment's originator, and trigger_summary is
-// refreshed. This is what lets a different member's comment safely fold into a
-// task another member created: the single coalescing run then carries the
-// latest instruction's originator and the matching connected-app overlay,
-// instead of answering the new comment under the original user's capabilities
-// and audit identity. It also replaces the earlier originator gate + fresh
-// fallback, which could not create a second pending task anyway (the
-// one-pending-per-(issue,agent) unique index) and so silently dropped the
-// mismatched-originator comment.
-func (h *Handler) mergeCommentIntoPendingTask(ctx context.Context, issue db.Issue, trigger commentAgentTrigger, newTriggerCommentID pgtype.UUID) bool {
+// refreshed — so a different member's comment safely folds into a task another
+// member created, the coalescing run carrying the latest instruction's
+// originator and matching connected-app overlay.
+func (h *Handler) mergeCommentIntoPendingTask(ctx context.Context, issue db.Issue, trigger commentAgentTrigger, newTriggerCommentID pgtype.UUID) commentMergeResult {
 	// Re-attribute the coalescing run to the new comment's human atomically: the
 	// whole attribution snapshot moves, not just the person columns (MUL-4302). An
 	// issue-assignee reaction is comment_source; a mention / thread-parent /
@@ -1677,19 +1711,20 @@ func (h *Handler) mergeCommentIntoPendingTask(ctx context.Context, issue db.Issu
 	isMention := trigger.Source != commentTriggerSourceIssueAssignee
 	attr, err := h.TaskService.AttributionForMergedComment(ctx, issue.WorkspaceID, newTriggerCommentID, isMention, trigger.Agent)
 	if err != nil {
-		// Fail-closed: the new comment cannot be precisely attributed and this
-		// workspace forbids the owner_fallback degrade. REFUSE the merge — keep the
-		// existing queued task on its original (precise) snapshot instead of
-		// re-stamping it to a degraded owner_fallback. The task still runs and reads
-		// the full thread, so the comment's instruction is not lost; only the audit
-		// re-attribution is declined (MUL-4302, Elon must-fix). Treat as handled so we
-		// never spawn a duplicate task under the one-pending-per-(issue,agent) index.
-		slog.Warn("refused comment merge: attribution fail-closed, keeping original task snapshot",
+		// The new comment cannot be re-attributed. REFUSE the merge — keep the
+		// existing queued task on its original (precise) snapshot rather than
+		// re-stamp it to a degraded owner_fallback, and never spawn a duplicate.
+		// A fail-closed refusal is a distinct, honest outcome (attribution_blocked);
+		// any other error is unclassified (internal_error) — neither is success.
+		slog.Warn("refused comment merge: attribution failed, keeping original task snapshot",
 			"issue_id", uuidToString(issue.ID),
 			"agent_id", uuidToString(trigger.Agent.ID),
 			"new_trigger_comment_id", uuidToString(newTriggerCommentID),
 			"error", err)
-		return true
+		if errors.Is(err, service.ErrAttributionFailClosed) {
+			return commentMergeAttributionBlocked
+		}
+		return commentMergeError
 	}
 	overlay, connectedApps := h.TaskService.BuildRuntimeMCPOverlayForMerge(ctx, attr.UserID, trigger.Agent)
 	row, err := h.Queries.MergeCommentIntoPendingTask(ctx, db.MergeCommentIntoPendingTaskParams{
@@ -1712,15 +1747,16 @@ func (h *Handler) mergeCommentIntoPendingTask(ctx context.Context, issue db.Issu
 			// No pre-claim (queued/deferred) task to merge into. The caller
 			// defers to completion reconcile when an active task exists, or
 			// enqueues fresh when none does.
-			return false
+			return commentMergeNoPendingTask
 		}
 		// Unknown error: the pending task most likely still exists, so do NOT
-		// risk enqueuing a duplicate. Log and treat as handled.
+		// risk enqueuing a duplicate — but the merge did not happen, so this is
+		// not a success.
 		slog.Warn("merge comment into pending task failed",
 			"issue_id", uuidToString(issue.ID),
 			"agent_id", uuidToString(trigger.Agent.ID),
 			"error", err)
-		return true
+		return commentMergeError
 	}
 	slog.Info("merged comment into pending task",
 		"task_id", uuidToString(row.ID),
@@ -1728,7 +1764,7 @@ func (h *Handler) mergeCommentIntoPendingTask(ctx context.Context, issue db.Issu
 		"agent_id", uuidToString(trigger.Agent.ID),
 		"new_trigger_comment_id", uuidToString(newTriggerCommentID),
 		"coalesced_count", len(row.CoalescedCommentIds))
-	return true
+	return commentMergeSucceeded
 }
 
 // enqueueSingleCommentTrigger creates a fresh task for one computed trigger.

server/internal/handler/comment_decision_test.go

@@ -41,6 +41,42 @@ func TestDecidePostMergeMiss(t *testing.T) {
 	})
 }
 
+// TestCommentMergeTerminalOutcome is Elon round-5 must-fix: a pending-task merge
+// must report an HONEST public outcome. Only a real merge is coalesced; a
+// fail-closed refusal is blocked/attribution_blocked and any other failure is
+// blocked/internal_error — never a fabricated coalesced success. Only
+// "no queued task to fold into" is non-terminal (the caller runs the active-task
+// decision). Pure-tested because a real DB/attribution fault cannot be forced
+// through valid handler inputs, and this mapping is what governs the outcome.
+func TestCommentMergeTerminalOutcome(t *testing.T) {
+	cases := []struct {
+		name         string
+		result       commentMergeResult
+		wantTerminal bool
+		wantStatus   DispatchStatus
+		wantReason   DispatchReasonCode
+	}{
+		{"real merge coalesces", commentMergeSucceeded, true, DispatchCoalesced, ReasonCoalesced},
+		{"attribution fail-closed is blocked, not success", commentMergeAttributionBlocked, true, DispatchBlocked, ReasonAttributionBlocked},
+		{"unknown error is blocked, not success", commentMergeError, true, DispatchBlocked, ReasonInternalError},
+		{"no pending task defers to active-task decision", commentMergeNoPendingTask, false, "", ""},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			status, reason, terminal := commentMergeTerminalOutcome(tc.result)
+			if terminal != tc.wantTerminal {
+				t.Errorf("terminal = %v, want %v", terminal, tc.wantTerminal)
+			}
+			if status != tc.wantStatus || reason != tc.wantReason {
+				t.Errorf("got %q/%q, want %q/%q", status, reason, tc.wantStatus, tc.wantReason)
+			}
+			if tc.wantTerminal && status != DispatchCoalesced && status != DispatchBlocked {
+				t.Errorf("terminal outcome %q is neither the coalesced success nor a blocked failure", status)
+			}
+		})
+	}
+}
+
 // TestDecideSuppressedLeaderOutcome: the self-trigger-suppressed squad leader's
 // active-task check must never fake success — a query error is a non-success
 // internal_error, a confirmed active run defers, and a confirmed-none is

server/internal/handler/comment_merge_failclosed_test.go

@@ -86,8 +86,25 @@ func TestMergeCommentIntoPendingTask_FailClosedKeepsOriginalSnapshot(t *testing.
 		testPool.Exec(context.Background(), `UPDATE workspace SET attribution_fail_closed = false WHERE id = $1`, testWorkspaceID)
 	})
 
-	if handled := testHandler.mergeCommentIntoPendingTask(ctx, issue, trigger, parseUUID(cidB)); !handled {
-		t.Fatal("fail-closed merge must be treated as handled (no duplicate task), got false")
+	countTasks := func() int {
+		var n int
+		if err := testPool.QueryRow(ctx, `SELECT count(*) FROM agent_task_queue WHERE issue_id = $1 AND agent_id = $2`, issueID, agentID).Scan(&n); err != nil {
+			t.Fatalf("count tasks: %v", err)
+		}
+		return n
+	}
+	before := countTasks()
+
+	if result := testHandler.mergeCommentIntoPendingTask(ctx, issue, trigger, parseUUID(cidB)); result != commentMergeAttributionBlocked {
+		t.Fatalf("fail-closed merge result = %d, want commentMergeAttributionBlocked (refused, non-success)", result)
+	}
+	if after := countTasks(); after != before {
+		t.Errorf("task count changed %d -> %d on refused merge; a fail-closed refusal must not spawn a task", before, after)
+	}
+	// The refused merge must surface as a non-success blocked/attribution_blocked
+	// outcome, never a fabricated coalesced success.
+	if status, reason, terminal := commentMergeTerminalOutcome(commentMergeAttributionBlocked); !terminal || status != DispatchBlocked || reason != ReasonAttributionBlocked {
+		t.Errorf("attribution-blocked outcome = %s/%s (terminal=%v), want blocked/attribution_blocked", status, reason, terminal)
 	}
 	tc, orig, acc, src := readSnapshot()
 	if tc != cidA {
@@ -104,8 +121,8 @@ func TestMergeCommentIntoPendingTask_FailClosedKeepsOriginalSnapshot(t *testing.
 	if _, err := testPool.Exec(ctx, `UPDATE workspace SET attribution_fail_closed = false WHERE id = $1`, testWorkspaceID); err != nil {
 		t.Fatalf("clear fail-closed: %v", err)
 	}
-	if handled := testHandler.mergeCommentIntoPendingTask(ctx, issue, trigger, parseUUID(cidB)); !handled {
-		t.Fatal("fail-open merge should be handled, got false")
+	if result := testHandler.mergeCommentIntoPendingTask(ctx, issue, trigger, parseUUID(cidB)); result != commentMergeSucceeded {
+		t.Fatalf("fail-open merge result = %d, want commentMergeSucceeded", result)
 	}
 	tc2, orig2, _, src2 := readSnapshot()
 	if tc2 != cidB {