multica/server/internal/handler/comment_list_test.go

package handler

import (
	"context"
	"encoding/json"
	"net/http"
	"net/http/httptest"
	"net/url"
	"strconv"
	"strings"
	"testing"
	"time"
	"unicode/utf8"

	"github.com/google/uuid"
	"github.com/jackc/pgx/v5/pgtype"
	db "github.com/multica-ai/multica/server/pkg/db/generated"
)

// cursorQuery builds a properly URL-encoded query string for the recent +
// thread-cursor path. RFC3339Nano timestamps contain `:` and may contain `+`,
// both of which need escaping so they survive `(*url.URL).Query()` parsing on
// the server side.
//
// `before` and `beforeID` here name a *thread* (last_activity_at, root_id),
// not a single row — the recent path is thread-grouped (#2340).
func cursorQuery(recent int, before, beforeID string) string {
	v := url.Values{}
	if recent > 0 {
		v.Set("recent", strconv.Itoa(recent))
	}
	if before != "" {
		v.Set("before", before)
	}
	if beforeID != "" {
		v.Set("before_id", beforeID)
	}
	return v.Encode()
}

// nextThreadCursor reads the (before, before-id) headers the recent path
// emits when there is likely an older page to scroll to. Empty pair means
// the server signalled "no more threads".
func nextThreadCursor(w *httptest.ResponseRecorder) (string, string) {
	return w.Header().Get("X-Multica-Next-Before"), w.Header().Get("X-Multica-Next-Before-Id")
}

// commentListFixture seeds an issue with a known comment graph for the
// thread / recent / cursor tests. The shape:
//
//	root1 (oldest)
//	├── r1a
//	└── r1b
//	    └── r1b1   (nested reply — defends Elon's point 2: recursive root walk)
//	root2 (newer, separate thread)
//	├── r2a
//	└── r2b (newest overall)
//
// Each comment is inserted with an explicit created_at so ordering and
// cursor behavior are deterministic.
type commentListFixture struct {
	IssueID string
	Root1   string
	R1a     string
	R1b     string
	R1b1    string
	Root2   string
	R2a     string
	R2b     string
	Base    time.Time
}

func newCommentListFixture(t *testing.T) commentListFixture {
	t.Helper()
	ctx := context.Background()

	var issueID string
	if err := testPool.QueryRow(ctx, `
		INSERT INTO issue (workspace_id, creator_type, creator_id, title)
		VALUES ($1, 'member', $2, $3)
		RETURNING id
	`, testWorkspaceID, testUserID, "comment list fixture").Scan(&issueID); err != nil {
		t.Fatalf("create issue: %v", err)
	}
	t.Cleanup(func() {
		testPool.Exec(context.Background(), `DELETE FROM issue WHERE id = $1`, issueID)
	})

	base := time.Now().UTC().Add(-1 * time.Hour).Truncate(time.Second)

	insert := func(parent *string, offset time.Duration, body string) string {
		t.Helper()
		var id string
		if err := testPool.QueryRow(ctx, `
			INSERT INTO comment (issue_id, workspace_id, author_type, author_id, content, type, parent_id, created_at)
			VALUES ($1, $2, 'member', $3, $4, 'comment', $5, $6)
			RETURNING id
		`, issueID, testWorkspaceID, testUserID, body, parent, base.Add(offset)).Scan(&id); err != nil {
			t.Fatalf("insert comment %q: %v", body, err)
		}
		return id
	}

	root1 := insert(nil, 0, "root1")
	r1a := insert(&root1, 1*time.Minute, "r1a")
	r1b := insert(&root1, 2*time.Minute, "r1b")
	r1b1 := insert(&r1b, 3*time.Minute, "r1b1") // nested reply: parent is a reply, not a root
	root2 := insert(nil, 10*time.Minute, "root2")
	r2a := insert(&root2, 11*time.Minute, "r2a")
	r2b := insert(&root2, 12*time.Minute, "r2b")

	return commentListFixture{
		IssueID: issueID,
		Root1:   root1, R1a: r1a, R1b: r1b, R1b1: r1b1,
		Root2: root2, R2a: r2a, R2b: r2b,
		Base: base,
	}
}

func decodeComments(t *testing.T, body []byte) []CommentResponse {
	t.Helper()
	var resp []CommentResponse
	if err := json.Unmarshal(body, &resp); err != nil {
		t.Fatalf("decode comments: %v", err)
	}
	return resp
}

func listComments(t *testing.T, issueID, query string) (*httptest.ResponseRecorder, []CommentResponse) {
	t.Helper()
	w := httptest.NewRecorder()
	url := "/api/issues/" + issueID + "/comments"
	if query != "" {
		url += "?" + query
	}
	r := newRequest("GET", url, nil)
	r = withURLParam(r, "id", issueID)
	testHandler.ListComments(w, r)
	if w.Code != http.StatusOK {
		return w, nil
	}
	return w, decodeComments(t, w.Body.Bytes())
}

func ids(rows []CommentResponse) []string {
	out := make([]string, len(rows))
	for i, c := range rows {
		out[i] = c.ID
	}
	return out
}

func eqIDs(t *testing.T, got, want []string, ctx string) {
	t.Helper()
	if len(got) != len(want) {
		t.Fatalf("%s: ids len got=%d want=%d\ngot=%v\nwant=%v", ctx, len(got), len(want), got, want)
	}
	for i := range got {
		if got[i] != want[i] {
			t.Fatalf("%s: ids[%d] got=%s want=%s\ngot=%v\nwant=%v", ctx, i, got[i], want[i], got, want)
		}
	}
}

// TestListComments_DefaultPreservesChronologicalOrder is a guard against
// silent regressions in the unparameterized list path — agents and the UI
// both depend on chronological order.
func TestListComments_DefaultPreservesChronologicalOrder(t *testing.T) {
	if testHandler == nil || testPool == nil {
		t.Skip("database not available")
	}
	fx := newCommentListFixture(t)

	_, rows := listComments(t, fx.IssueID, "")
	want := []string{fx.Root1, fx.R1a, fx.R1b, fx.R1b1, fx.Root2, fx.R2a, fx.R2b}
	eqIDs(t, ids(rows), want, "default order")
}

// TestListComments_RootsOnlyReturnsTopLevelComments pins the #3164 contract:
// issue-level orientation should fetch only root comments, leaving replies for
// a later thread-scoped read if the caller needs detail.
func TestListComments_RootsOnlyReturnsTopLevelComments(t *testing.T) {
	if testHandler == nil || testPool == nil {
		t.Skip("database not available")
	}
	fx := newCommentListFixture(t)

	for _, tc := range []struct {
		name  string
		query string
	}{
		{name: "underscore query", query: "roots_only=true"},
		{name: "hyphenated alias", query: "roots-only=true"},
	} {
		t.Run(tc.name, func(t *testing.T) {
			w, rows := listComments(t, fx.IssueID, tc.query)
			eqIDs(t, ids(rows), []string{fx.Root1, fx.Root2}, tc.name)
			for _, row := range rows {
				if row.ParentID != nil {
					t.Fatalf("%s: expected root comment %s to have nil parent_id, got %q", tc.name, row.ID, *row.ParentID)
				}
			}
			nb, nbid := nextThreadCursor(w)
			if nb != "" || nbid != "" {
				t.Fatalf("%s: roots-only list should not emit cursor headers, got before=%q before_id=%q", tc.name, nb, nbid)
			}
		})
	}
}

// TestListComments_RootsOnlyWithSinceReturnsNewTopLevelComments proves the
// one allowed roots-only combination: `since` narrows the root list without
// pulling newer replies into the response.
func TestListComments_RootsOnlyWithSinceReturnsNewTopLevelComments(t *testing.T) {
	if testHandler == nil || testPool == nil {
		t.Skip("database not available")
	}
	fx := newCommentListFixture(t)

	v := url.Values{}
	v.Set("roots_only", "true")
	v.Set("since", fx.Base.Add(5*time.Minute).UTC().Format(time.RFC3339Nano))
	_, rows := listComments(t, fx.IssueID, v.Encode())
	eqIDs(t, ids(rows), []string{fx.Root2}, "roots_only + since")
	for _, row := range rows {
		if row.ParentID != nil {
			t.Fatalf("roots_only + since: expected root comment %s to have nil parent_id, got %q", row.ID, *row.ParentID)
		}
	}
}

// TestListComments_TimestampParamsAcceptSecondPrecision pins that RFC3339
// timestamps without fractional seconds still parse after the single
// RFC3339Nano parse, and that malformed values keep their 400 responses.
func TestListComments_TimestampParamsAcceptSecondPrecision(t *testing.T) {
	if testHandler == nil || testPool == nil {
		t.Skip("database not available")
	}
	fx := newCommentListFixture(t)

	v := url.Values{}
	v.Set("roots_only", "true")
	v.Set("since", fx.Base.Add(5*time.Minute).UTC().Format(time.RFC3339))
	w, rows := listComments(t, fx.IssueID, v.Encode())
	if w.Code != http.StatusOK {
		t.Fatalf("second-precision since: status %d body %s", w.Code, w.Body.String())
	}
	eqIDs(t, ids(rows), []string{fx.Root2}, "second-precision since")

	for _, tc := range []struct {
		name, query, wantMessage string
	}{
		{"since", "since=2026-01-02+03:04:05", "invalid since parameter"},
		{"before", cursorQuery(1, "2026-01-02T03:04", fx.Root1), "invalid before parameter"},
	} {
		w, _ := listComments(t, fx.IssueID, tc.query)
		if w.Code != http.StatusBadRequest || !strings.Contains(w.Body.String(), tc.wantMessage) {
			t.Fatalf("invalid %s: status %d body %s", tc.name, w.Code, w.Body.String())
		}
	}
}

// TestListComments_RootsOnlyReturnsThreadStats pins the orientation metadata:
// each root returned by roots_only carries reply_count (recursive descendant
// count) and last_activity_at (MAX created_at over the subtree), so an agent
// can triage which thread to open without fetching any replies. The fixture's
// root1 has a nested reply (r1b1 → r1b → root1), proving the count walks deeper
// than one level. Stats are roots-only — the default list must not carry them.
func TestListComments_RootsOnlyReturnsThreadStats(t *testing.T) {
	if testHandler == nil || testPool == nil {
		t.Skip("database not available")
	}
	fx := newCommentListFixture(t)

	_, rows := listComments(t, fx.IssueID, "roots_only=true")
	eqIDs(t, ids(rows), []string{fx.Root1, fx.Root2}, "roots stats order")

	byID := map[string]CommentResponse{}
	for _, r := range rows {
		byID[r.ID] = r
	}
	// root1: r1a, r1b, r1b1 (nested) → 3 replies; newest activity is r1b1 at +3m.
	assertRootStat(t, byID[fx.Root1], "root1", 3, fx.Base.Add(3*time.Minute))
	// root2: r2a, r2b → 2 replies; newest activity is r2b at +12m.
	assertRootStat(t, byID[fx.Root2], "root2", 2, fx.Base.Add(12*time.Minute))

	// The default (non-roots) list must NOT leak the roots-only stats.
	_, all := listComments(t, fx.IssueID, "")
	for _, r := range all {
		if r.ReplyCount != nil || r.LastActivityAt != nil {
			t.Fatalf("default list leaked roots-only stats on %s (reply_count=%v last_activity=%v)", r.ID, r.ReplyCount, r.LastActivityAt)
		}
	}
}

// TestListComments_RootAndThreadViewsPreserveAttributionFields guards explicit
// SQL projections from drifting behind the comment table. Both fields affect
// UI rendering and task lineage, so silently zeroing them changes behavior.
func TestListComments_RootAndThreadViewsPreserveAttributionFields(t *testing.T) {
	if testHandler == nil || testPool == nil {
		t.Skip("database not available")
	}
	fx := newCommentListFixture(t)

	sourceTaskID := "11111111-2222-3333-4444-555555555555"
	quickActionID := "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"
	for _, commentID := range []string{fx.Root1, fx.R1a} {
		if _, err := testPool.Exec(context.Background(), `
			UPDATE comment
			SET source_task_id = $1, quick_action_id = $2
			WHERE id = $3
		`, sourceTaskID, quickActionID, commentID); err != nil {
			t.Fatalf("stamp attribution fields on %s: %v", commentID, err)
		}
	}

	assertFields := func(label string, row CommentResponse) {
		t.Helper()
		if row.SourceTaskID == nil || *row.SourceTaskID != sourceTaskID {
			t.Errorf("%s source_task_id = %v, want %s", label, row.SourceTaskID, sourceTaskID)
		}
		if row.QuickActionID == nil || *row.QuickActionID != quickActionID {
			t.Errorf("%s quick_action_id = %v, want %s", label, row.QuickActionID, quickActionID)
		}
	}

	_, roots := listComments(t, fx.IssueID, "roots_only=true")
	assertFields("roots-only root", byID(roots)[fx.Root1])

	_, thread := listComments(t, fx.IssueID, "thread="+fx.R1a)
	threadByID := byID(thread)
	assertFields("thread root", threadByID[fx.Root1])
	assertFields("thread reply", threadByID[fx.R1a])
}

func assertRootStat(t *testing.T, c CommentResponse, label string, wantReplies int, wantLastActivity time.Time) {
	t.Helper()
	if c.ReplyCount == nil {
		t.Fatalf("%s: reply_count missing", label)
	}
	if *c.ReplyCount != wantReplies {
		t.Fatalf("%s: reply_count got=%d want=%d", label, *c.ReplyCount, wantReplies)
	}
	if c.LastActivityAt == nil {
		t.Fatalf("%s: last_activity_at missing", label)
	}
	got, err := time.Parse(time.RFC3339, *c.LastActivityAt)
	if err != nil {
		t.Fatalf("%s: last_activity_at parse %q: %v", label, *c.LastActivityAt, err)
	}
	if !got.Equal(wantLastActivity) {
		t.Fatalf("%s: last_activity_at got=%s want=%s", label, got.UTC(), wantLastActivity.UTC())
	}
}

// TestListComments_SummaryClipsContent pins the summary projection: with
// summary=true a long body is clipped to summaryContentRunes (+ellipsis) and
// flagged content_truncated=true, a short body is returned verbatim with
// content_truncated=false, and without summary the field is omitted entirely so
// the default response shape is unchanged.
func TestListComments_SummaryClipsContent(t *testing.T) {
	if testHandler == nil || testPool == nil {
		t.Skip("database not available")
	}
	ctx := context.Background()

	var issueID string
	if err := testPool.QueryRow(ctx, `
		INSERT INTO issue (workspace_id, creator_type, creator_id, title)
		VALUES ($1, 'member', $2, $3)
		RETURNING id
	`, testWorkspaceID, testUserID, "summary fixture").Scan(&issueID); err != nil {
		t.Fatalf("create issue: %v", err)
	}
	t.Cleanup(func() {
		testPool.Exec(context.Background(), `DELETE FROM issue WHERE id = $1`, issueID)
	})

	base := time.Now().UTC().Add(-time.Hour).Truncate(time.Second)
	insert := func(body string, offset time.Duration) string {
		t.Helper()
		var id string
		if err := testPool.QueryRow(ctx, `
			INSERT INTO comment (issue_id, workspace_id, author_type, author_id, content, type, created_at)
			VALUES ($1, $2, 'member', $3, $4, 'comment', $5)
			RETURNING id
		`, issueID, testWorkspaceID, testUserID, body, base.Add(offset)).Scan(&id); err != nil {
			t.Fatalf("insert comment: %v", err)
		}
		return id
	}
	longID := insert(strings.Repeat("x", 500), 0)
	shortID := insert("short", time.Minute)
	// Multi-byte body: clipping must land on a rune boundary, never mid-rune.
	cjkID := insert(strings.Repeat("你", 300), 2*time.Minute)

	// Baseline: no summary → full content, content_truncated omitted.
	_, full := listComments(t, issueID, "")
	for _, c := range full {
		if c.ContentTruncated != nil {
			t.Fatalf("baseline: content_truncated should be nil, got %v on %s", *c.ContentTruncated, c.ID)
		}
		if c.ID == longID && utf8.RuneCountInString(c.Content) != 500 {
			t.Fatalf("baseline: long content len got=%d want=500", utf8.RuneCountInString(c.Content))
		}
	}

	// summary=true → long clipped + truncated true; short verbatim + false.
	_, sum := listComments(t, issueID, "summary=true")
	byID := map[string]CommentResponse{}
	for _, c := range sum {
		byID[c.ID] = c
	}
	long := byID[longID]
	if long.ContentTruncated == nil || !*long.ContentTruncated {
		t.Fatalf("summary: long comment should be truncated, got %v", long.ContentTruncated)
	}
	if rc := utf8.RuneCountInString(long.Content); rc != summaryContentRunes+1 { // +1 for the ellipsis
		t.Fatalf("summary: long content rune count got=%d want=%d", rc, summaryContentRunes+1)
	}
	if !strings.HasSuffix(long.Content, "…") {
		t.Fatalf("summary: long content should end with ellipsis, got %q", long.Content)
	}
	short := byID[shortID]
	if short.ContentTruncated == nil || *short.ContentTruncated {
		t.Fatalf("summary: short comment should be untruncated (false), got %v", short.ContentTruncated)
	}
	if short.Content != "short" {
		t.Fatalf("summary: short content got=%q want=short", short.Content)
	}
	cjk := byID[cjkID]
	if cjk.ContentTruncated == nil || !*cjk.ContentTruncated {
		t.Fatalf("summary: cjk comment should be truncated, got %v", cjk.ContentTruncated)
	}
	if !utf8.ValidString(cjk.Content) {
		t.Fatalf("summary: cjk content was clipped mid-rune (invalid UTF-8): %q", cjk.Content)
	}
	if rc := utf8.RuneCountInString(cjk.Content); rc != summaryContentRunes+1 { // 200 CJK runes + ellipsis
		t.Fatalf("summary: cjk content rune count got=%d want=%d", rc, summaryContentRunes+1)
	}
	if !strings.HasSuffix(cjk.Content, "你…") {
		t.Fatalf("summary: cjk content should be whole 你-runes then ellipsis, got %q", cjk.Content)
	}
}

// TestListComments_RootsOnlySummaryComposes pins the spec's headline
// "table of contents" read: --roots-only --summary together must (a) clip each
// root's content and flag content_truncated, AND (b) still carry the roots-only
// orientation stats (reply_count over the full subtree, last_activity_at). The
// summary projection composes with roots_only rather than replacing its stats;
// a future refactor that moved the clip into a per-mode branch would silently
// break this composition, so both halves are asserted on the same response.
func TestListComments_RootsOnlySummaryComposes(t *testing.T) {
	if testHandler == nil || testPool == nil {
		t.Skip("database not available")
	}
	ctx := context.Background()

	var issueID string
	if err := testPool.QueryRow(ctx, `
		INSERT INTO issue (workspace_id, creator_type, creator_id, title)
		VALUES ($1, 'member', $2, $3)
		RETURNING id
	`, testWorkspaceID, testUserID, "roots+summary fixture").Scan(&issueID); err != nil {
		t.Fatalf("create issue: %v", err)
	}
	t.Cleanup(func() {
		testPool.Exec(context.Background(), `DELETE FROM issue WHERE id = $1`, issueID)
	})

	base := time.Now().UTC().Add(-time.Hour).Truncate(time.Second)
	insert := func(parent *string, body string, offset time.Duration) string {
		t.Helper()
		var id string
		if err := testPool.QueryRow(ctx, `
			INSERT INTO comment (issue_id, workspace_id, author_type, author_id, content, type, parent_id, created_at)
			VALUES ($1, $2, 'member', $3, $4, 'comment', $5, $6)
			RETURNING id
		`, issueID, testWorkspaceID, testUserID, body, parent, base.Add(offset)).Scan(&id); err != nil {
			t.Fatalf("insert comment: %v", err)
		}
		return id
	}
	rootID := insert(nil, strings.Repeat("x", 500), 0)
	insert(&rootID, "a reply", 5*time.Minute) // pushes last_activity past the root's own time

	_, rows := listComments(t, issueID, "roots_only=true&summary=true")
	eqIDs(t, ids(rows), []string{rootID}, "roots_only+summary returns only the root")

	root := rows[0]
	// Summary half: the long root content is clipped + flagged.
	if root.ContentTruncated == nil || !*root.ContentTruncated {
		t.Fatalf("roots+summary: root should be truncated, got %v", root.ContentTruncated)
	}
	if rc := utf8.RuneCountInString(root.Content); rc != summaryContentRunes+1 { // +1 for the ellipsis
		t.Fatalf("roots+summary: clipped content rune count got=%d want=%d", rc, summaryContentRunes+1)
	}
	// Stats half: orientation metadata survives the summary projection. The
	// reply (1 descendant, +5m) drives both reply_count and last_activity_at.
	assertRootStat(t, root, "root", 1, base.Add(5*time.Minute))
}

// TestListComments_ThreadResolvesFromAnyAnchor proves Elon's point 2:
// regardless of whether the anchor is a root, a direct reply, or a nested
// reply (parent_id points at another reply), the server walks up to the
// thread root and returns root + every descendant.
func TestListComments_ThreadResolvesFromAnyAnchor(t *testing.T) {
	if testHandler == nil || testPool == nil {
		t.Skip("database not available")
	}
	fx := newCommentListFixture(t)

	wantThread1 := []string{fx.Root1, fx.R1a, fx.R1b, fx.R1b1}

	t.Run("anchor is root", func(t *testing.T) {
		_, rows := listComments(t, fx.IssueID, "thread="+fx.Root1)
		eqIDs(t, ids(rows), wantThread1, "anchor=root1")
	})

	t.Run("anchor is direct reply", func(t *testing.T) {
		_, rows := listComments(t, fx.IssueID, "thread="+fx.R1a)
		eqIDs(t, ids(rows), wantThread1, "anchor=r1a (direct reply)")
	})

	t.Run("anchor is nested reply", func(t *testing.T) {
		// r1b1.parent_id = r1b, which itself is a reply. The recursive CTE
		// must climb root1 → r1b → r1b1 to resolve the root.
		_, rows := listComments(t, fx.IssueID, "thread="+fx.R1b1)
		eqIDs(t, ids(rows), wantThread1, "anchor=r1b1 (nested reply)")
	})

	t.Run("anchor in other thread returns only that thread", func(t *testing.T) {
		_, rows := listComments(t, fx.IssueID, "thread="+fx.R2a)
		eqIDs(t, ids(rows), []string{fx.Root2, fx.R2a, fx.R2b}, "anchor=r2a")
	})
}

// TestListComments_ThreadAnchorErrors covers the user-facing error surface
// for the thread path. The unknown-anchor case is what catches the typical
// "agent pasted a stale UUID" footgun — the server returns 404 instead of
// silently returning an empty list (which would otherwise be
// indistinguishable from a deleted thread).
func TestListComments_ThreadAnchorErrors(t *testing.T) {
	if testHandler == nil || testPool == nil {
		t.Skip("database not available")
	}
	fx := newCommentListFixture(t)

	t.Run("non-uuid thread returns 400", func(t *testing.T) {
		w, _ := listComments(t, fx.IssueID, "thread=not-a-uuid")
		if w.Code != http.StatusBadRequest {
			t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String())
		}
	})

	t.Run("unknown thread anchor returns 404", func(t *testing.T) {
		w, _ := listComments(t, fx.IssueID, "thread=00000000-0000-0000-0000-000000000001")
		if w.Code != http.StatusNotFound {
			t.Fatalf("expected 404, got %d: %s", w.Code, w.Body.String())
		}
	})
}

// TestListComments_ThreadRootWalkFailsClosedAcrossIssueBoundary constructs the
// anomalous shape the schema permits: a local reply whose parent belongs to
// another issue. Every recursive root-walk level must keep the issue predicate,
// so the foreign root is neither returned nor used to open a thread.
func TestListComments_ThreadRootWalkFailsClosedAcrossIssueBoundary(t *testing.T) {
	if testHandler == nil || testPool == nil {
		t.Skip("database not available")
	}
	foreignIssueID := createIssueForTimeline(t, "foreign thread root issue")
	issueID := createIssueForTimeline(t, "local thread reply issue")
	base := time.Now().UTC().Add(-time.Hour).Truncate(time.Second)
	foreignRootID := seedCommentRow(t, foreignIssueID, base, "foreign root", nil, nil)
	localReplyID := seedCommentRow(t, issueID, base.Add(time.Second), "local anomalous reply", &foreignRootID, nil)

	w, rows := listComments(t, issueID, "thread="+localReplyID)
	if w.Code != http.StatusNotFound {
		t.Fatalf("cross-issue root walk status = %d, want 404: %s", w.Code, w.Body.String())
	}
	if rows != nil {
		t.Fatalf("cross-issue root walk returned rows: %v", ids(rows))
	}
}

// TestListComments_RecentReturnsMostRecentlyActiveThreads pins the
// thread-grouped semantics from #2340. Row-based "newest N comments" would
// have surfaced [root2, r2a, r2b] for N=3 — a single thread's tail. The
// thread-grouped path treats the unit as a thread (root + descendants) and
// ranks threads by MAX(created_at) over the subtree, so:
//
//   - recent=1 → the single freshest-active thread (root2 thread) fully
//     expanded, oldest-active thread suppressed.
//   - recent=2 → both threads, with the older-active thread first so the
//     freshest sits at the prompt tail (closest to "now").
func TestListComments_RecentReturnsMostRecentlyActiveThreads(t *testing.T) {
	if testHandler == nil || testPool == nil {
		t.Skip("database not available")
	}
	fx := newCommentListFixture(t)

	t.Run("recent=1 returns the freshest-active thread fully", func(t *testing.T) {
		_, rows := listComments(t, fx.IssueID, "recent=1")
		eqIDs(t, ids(rows), []string{fx.Root2, fx.R2a, fx.R2b}, "recent=1")
	})

	t.Run("recent=2 returns both threads, older-active first", func(t *testing.T) {
		_, rows := listComments(t, fx.IssueID, "recent=2")
		// Threads sorted by (last_activity_at ASC, root_id ASC):
		//   root1 thread (last_activity = base + 3m via r1b1) FIRST
		//   root2 thread (last_activity = base + 12m via r2b) SECOND
		// In-thread ordering is chronological.
		want := []string{fx.Root1, fx.R1a, fx.R1b, fx.R1b1, fx.Root2, fx.R2a, fx.R2b}
		eqIDs(t, ids(rows), want, "recent=2")
	})
}

// TestListComments_RecentRanksStaleThreadAheadIfRecentlyReplied makes the
// MAX(created_at) ranking explicit: a thread whose root is old but which has
// a fresh reply must outrank a thread whose root is newer but quiet. Without
// this, "recent" decays into "most recent root" and misses the very signal
// that thread-grouping was meant to surface.
func TestListComments_RecentRanksStaleThreadAheadIfRecentlyReplied(t *testing.T) {
	if testHandler == nil || testPool == nil {
		t.Skip("database not available")
	}
	ctx := context.Background()

	var issueID string
	if err := testPool.QueryRow(ctx, `
		INSERT INTO issue (workspace_id, creator_type, creator_id, title)
		VALUES ($1, 'member', $2, $3) RETURNING id
	`, testWorkspaceID, testUserID, "stale-but-fresh fixture").Scan(&issueID); err != nil {
		t.Fatalf("create issue: %v", err)
	}
	t.Cleanup(func() {
		testPool.Exec(context.Background(), `DELETE FROM issue WHERE id = $1`, issueID)
	})

	base := time.Now().UTC().Add(-1 * time.Hour).Truncate(time.Second)
	insert := func(parent *string, offset time.Duration, body string) string {
		var id string
		if err := testPool.QueryRow(ctx, `
			INSERT INTO comment (issue_id, workspace_id, author_type, author_id, content, type, parent_id, created_at)
			VALUES ($1, $2, 'member', $3, $4, 'comment', $5, $6) RETURNING id
		`, issueID, testWorkspaceID, testUserID, body, parent, base.Add(offset)).Scan(&id); err != nil {
			t.Fatalf("insert: %v", err)
		}
		return id
	}

	// Stale-then-fresh: oldRoot was created at t=0 but received a reply at
	// t=30m. quietRoot was created at t=15m and never replied.
	oldRoot := insert(nil, 0, "oldRoot")
	quietRoot := insert(nil, 15*time.Minute, "quietRoot")
	freshReply := insert(&oldRoot, 30*time.Minute, "freshReply")

	_, rows := listComments(t, issueID, "recent=1")
	// Expected: only the oldRoot thread (oldRoot + freshReply). The
	// quietRoot thread is suppressed because its last_activity_at is older
	// than oldRoot's, even though its root was created later.
	eqIDs(t, ids(rows), []string{oldRoot, freshReply}, "recent=1 picks freshly replied stale thread")
	_ = quietRoot
}

// TestListComments_RecentEmitsThreadCursorWhenPageFull pins the header
// contract: a full page (threads in response == --recent N) emits the
// next-page cursor; an underfilled page emits nothing. The cursor points at
// the OLDEST thread in the page — that is the upper bound for the next
// (older) page.
func TestListComments_RecentEmitsThreadCursorWhenPageFull(t *testing.T) {
	if testHandler == nil || testPool == nil {
		t.Skip("database not available")
	}
	fx := newCommentListFixture(t)

	t.Run("underfilled page emits no cursor", func(t *testing.T) {
		// recent=5 with only 2 threads available — server has nothing older
		// to offer. No cursor headers means the client stops paginating.
		w, _ := listComments(t, fx.IssueID, "recent=5")
		nb, nbid := nextThreadCursor(w)
		if nb != "" || nbid != "" {
			t.Fatalf("expected no cursor, got before=%q before_id=%q", nb, nbid)
		}
	})

	t.Run("full page emits cursor pointing at oldest thread in page", func(t *testing.T) {
		// recent=1: page is full (1 thread returned, 1 requested). Cursor
		// must point at the (last_activity_at, root_id) of the only thread
		// in the page so the next request can fetch older threads.
		w, _ := listComments(t, fx.IssueID, "recent=1")
		nb, nbid := nextThreadCursor(w)
		if nbid != fx.Root2 {
			t.Fatalf("cursor before_id = %q, want %q (root2 — newest thread)", nbid, fx.Root2)
		}
		if nb == "" {
			t.Fatalf("cursor before is empty; expected RFC3339Nano timestamp")
		}
		if _, err := time.Parse(time.RFC3339Nano, nb); err != nil {
			t.Fatalf("cursor before = %q is not RFC3339Nano: %v", nb, err)
		}
	})
}

// TestListComments_RecentWithThreadCursorScrollsOlderThreads walks the issue
// thread-by-thread using the cursor the server emits. Pinning this avoids
// the row-based regression where a "newest N comments" cursor would interleave
// rows from multiple threads and skip thread membership across pages.
func TestListComments_RecentWithThreadCursorScrollsOlderThreads(t *testing.T) {
	if testHandler == nil || testPool == nil {
		t.Skip("database not available")
	}
	fx := newCommentListFixture(t)

	// Page 1: newest thread = root2.
	w1, page1 := listComments(t, fx.IssueID, "recent=1")
	eqIDs(t, ids(page1), []string{fx.Root2, fx.R2a, fx.R2b}, "page1 = root2 thread")
	nb, nbid := nextThreadCursor(w1)
	if nb == "" || nbid != fx.Root2 {
		t.Fatalf("page1 cursor = (%q, %q), want (non-empty, %q)", nb, nbid, fx.Root2)
	}

	// Page 2: cursor points at root2 → server returns the next older thread
	// (root1). All of root1's descendants come along.
	w2, page2 := listComments(t, fx.IssueID, cursorQuery(1, nb, nbid))
	eqIDs(t, ids(page2), []string{fx.Root1, fx.R1a, fx.R1b, fx.R1b1}, "page2 = root1 thread")

	// Page 3: cursor points at root1 → no older threads exist, page is
	// empty AND no cursor is emitted.
	nb2, nbid2 := nextThreadCursor(w2)
	if nb2 == "" || nbid2 != fx.Root1 {
		t.Fatalf("page2 cursor = (%q, %q), want (non-empty, %q)", nb2, nbid2, fx.Root1)
	}
	w3, page3 := listComments(t, fx.IssueID, cursorQuery(1, nb2, nbid2))
	if len(page3) != 0 {
		t.Fatalf("page3: expected empty (no older threads), got %d rows: %v", len(page3), ids(page3))
	}
	nb3, nbid3 := nextThreadCursor(w3)
	if nb3 != "" || nbid3 != "" {
		t.Fatalf("page3 cursor = (%q, %q), want both empty (end-of-list)", nb3, nbid3)
	}
}

// TestListComments_ThreadCursorStableUnderSameLastActivity locks the
// tie-break invariant for the thread cursor. Three threads with identical
// last_activity_at must paginate one-at-a-time without skips or duplicates,
// because (last_activity_at, root_id) — not just last_activity_at — is the
// total order. A timestamp-only cursor would either drop one thread or
// surface the same thread twice when ties land in the same microsecond.
func TestListComments_ThreadCursorStableUnderSameLastActivity(t *testing.T) {
	if testHandler == nil || testPool == nil {
		t.Skip("database not available")
	}
	ctx := context.Background()

	var issueID string
	if err := testPool.QueryRow(ctx, `
		INSERT INTO issue (workspace_id, creator_type, creator_id, title)
		VALUES ($1, 'member', $2, $3) RETURNING id
	`, testWorkspaceID, testUserID, "thread tie-break fixture").Scan(&issueID); err != nil {
		t.Fatalf("create issue: %v", err)
	}
	t.Cleanup(func() {
		testPool.Exec(context.Background(), `DELETE FROM issue WHERE id = $1`, issueID)
	})

	ts := time.Now().UTC().Add(-30 * time.Minute).Truncate(time.Millisecond)
	insertRoot := func(body string) string {
		var id string
		if err := testPool.QueryRow(ctx, `
			INSERT INTO comment (issue_id, workspace_id, author_type, author_id, content, type, created_at)
			VALUES ($1, $2, 'member', $3, $4, 'comment', $5) RETURNING id
		`, issueID, testWorkspaceID, testUserID, body, ts).Scan(&id); err != nil {
			t.Fatalf("insert: %v", err)
		}
		return id
	}
	a := insertRoot("a")
	b := insertRoot("b")
	c := insertRoot("c")

	// All three threads have last_activity_at = ts (each root is also the
	// thread's only comment). Order is (ts, root_id) — UUID lex tie-break.
	// Build canonical order by sorting the root ids and reversing (the SQL
	// orders DESC for selection).
	sorted := []string{a, b, c}
	for i := 0; i < len(sorted); i++ {
		for j := i + 1; j < len(sorted); j++ {
			if sorted[i] > sorted[j] {
				sorted[i], sorted[j] = sorted[j], sorted[i]
			}
		}
	}
	// Newest-first selection walks UUIDs DESC, so the page-1 thread is the
	// largest UUID; response is then ordered ASC (oldest-active first =
	// smallest-UUID-among-current-page) — with recent=1 there's only one
	// thread per page so the body shows that thread alone.
	wantOrder := []string{sorted[2], sorted[1], sorted[0]}

	var got []string
	w, page := listComments(t, issueID, "recent=1")
	if len(page) != 1 {
		t.Fatalf("page1: expected 1 thread (1 row), got %d", len(page))
	}
	got = append(got, page[0].ID)

	for i := 0; i < 2; i++ {
		nb, nbid := nextThreadCursor(w)
		if nb == "" || nbid == "" {
			t.Fatalf("page %d: missing cursor headers", i+1)
		}
		w, page = listComments(t, issueID, cursorQuery(1, nb, nbid))
		if len(page) != 1 {
			t.Fatalf("page %d: expected 1 thread (1 row), got %d", i+2, len(page))
		}
		got = append(got, page[0].ID)
	}

	eqIDs(t, got, wantOrder, "paginated walk")
}

// TestListComments_FlagCombinationRules locks Elon's point 4. The matrix is
// tiny on purpose — the goal is to ensure conflicting flags are rejected
// loudly at the API surface so the CLI's local validation cannot drift.
func TestListComments_FlagCombinationRules(t *testing.T) {
	if testHandler == nil || testPool == nil {
		t.Skip("database not available")
	}
	fx := newCommentListFixture(t)

	cases := []struct {
		name   string
		query  string
		status int
	}{
		{
			name:   "thread + recent rejected",
			query:  "thread=" + fx.Root1 + "&recent=5",
			status: http.StatusBadRequest,
		},
		{
			name: "thread + before rejected",
			query: (func() string {
				v := url.Values{}
				v.Set("thread", fx.Root1)
				v.Set("before", time.Now().UTC().Format(time.RFC3339))
				v.Set("before_id", uuid.NewString())
				return v.Encode()
			})(),
			status: http.StatusBadRequest,
		},
		{
			name: "before without before_id rejected",
			query: (func() string {
				v := url.Values{}
				v.Set("recent", "5")
				v.Set("before", time.Now().UTC().Format(time.RFC3339))
				return v.Encode()
			})(),
			status: http.StatusBadRequest,
		},
		{
			name: "before_id without before rejected",
			query: (func() string {
				v := url.Values{}
				v.Set("recent", "5")
				v.Set("before_id", uuid.NewString())
				return v.Encode()
			})(),
			status: http.StatusBadRequest,
		},
		{
			name: "before + before_id without recent rejected",
			// Cursor without --recent used to fall through to the default /
			// since path and silently return the full timeline (the gap Elon
			// called out in the PR #2787 second review). The 400 here pins
			// the documented "cursor scrolls within a recent window" rule.
			query: (func() string {
				v := url.Values{}
				v.Set("before", time.Now().UTC().Format(time.RFC3339))
				v.Set("before_id", uuid.NewString())
				return v.Encode()
			})(),
			status: http.StatusBadRequest,
		},
		{
			name:   "zero recent rejected",
			query:  "recent=0",
			status: http.StatusBadRequest,
		},
		{
			name:   "negative recent rejected",
			query:  "recent=-3",
			status: http.StatusBadRequest,
		},
		{
			name:   "non-numeric recent rejected",
			query:  "recent=lots",
			status: http.StatusBadRequest,
		},
		{
			name:   "non-boolean roots_only rejected",
			query:  "roots_only=yes",
			status: http.StatusBadRequest,
		},
		{
			name:   "non-boolean summary rejected",
			query:  "summary=yes",
			status: http.StatusBadRequest,
		},
		{
			name:   "roots_only + thread rejected",
			query:  "roots_only=true&thread=" + fx.Root1,
			status: http.StatusBadRequest,
		},
		{
			name:   "roots_only + recent rejected",
			query:  "roots_only=true&recent=1",
			status: http.StatusBadRequest,
		},
		{
			name:   "roots_only + tail rejected",
			query:  "roots_only=true&tail=1",
			status: http.StatusBadRequest,
		},
		{
			name: "roots_only + cursor rejected",
			query: (func() string {
				v := url.Values{}
				v.Set("roots_only", "true")
				v.Set("before", time.Now().UTC().Format(time.RFC3339))
				v.Set("before_id", uuid.NewString())
				return v.Encode()
			})(),
			status: http.StatusBadRequest,
		},
	}
	for _, tc := range cases {
		t.Run(tc.name, func(t *testing.T) {
			w, _ := listComments(t, fx.IssueID, tc.query)
			if w.Code != tc.status {
				t.Fatalf("query=%q\n  got=%d want=%d body=%s", tc.query, w.Code, tc.status, w.Body.String())
			}
		})
	}
}

// TestListComments_RecentWithSinceFilteredEmptySuppressesCursor pins
// Elon's nit on PR #2787 / MUL-2340: a `recent + since` page whose every
// row gets dropped by the `since` filter must NOT emit a next-page cursor.
//
// Pagination walks threads in strictly decreasing last_activity_at. If the
// `since` filter empties this page, every comment in this page is <= since
// ⇒ head.last_activity_at <= since ⇒ every older thread (strictly less
// recent than head) also has last_activity_at < since ⇒ guaranteed empty.
// Emitting a cursor in that case would invite the caller into a wasted
// walk of pages that can never produce a row.
func TestListComments_RecentWithSinceFilteredEmptySuppressesCursor(t *testing.T) {
	if testHandler == nil || testPool == nil {
		t.Skip("database not available")
	}
	fx := newCommentListFixture(t)

	// `since` = base + 1h (i.e. AFTER every comment in the fixture, where the
	// freshest row sits at base + 12m). With recent=1 the page is technically
	// "full" (1 thread out of 1 requested) so the legacy `len(seen) >= N`
	// check would emit a cursor — but every row in that page is <= since, so
	// the body is empty AND no older page can ever yield a >since row.
	v := url.Values{}
	v.Set("recent", "1")
	v.Set("since", fx.Base.Add(1*time.Hour).UTC().Format(time.RFC3339Nano))
	w, rows := listComments(t, fx.IssueID, v.Encode())
	if len(rows) != 0 {
		t.Fatalf("expected empty page after since-filter, got %d rows: %v", len(rows), ids(rows))
	}
	nb, nbid := nextThreadCursor(w)
	if nb != "" || nbid != "" {
		t.Fatalf("recent+since empty page must NOT emit cursor; got before=%q before_id=%q (this walks the caller into a guaranteed-empty pagination loop)", nb, nbid)
	}
}

// TestListComments_RecentWithSinceKeepsCursorWhenPageHasRows is the
// counterpoint to TestListComments_RecentWithSinceFilteredEmptySuppressesCursor:
// if the `since` filter leaves at least one row in the page, the cursor must
// still be emitted (the suppression rule is narrowly scoped to the empty
// case so it can't accidentally swallow legitimate next-page hints).
func TestListComments_RecentWithSinceKeepsCursorWhenPageHasRows(t *testing.T) {
	if testHandler == nil || testPool == nil {
		t.Skip("database not available")
	}
	fx := newCommentListFixture(t)

	// since = base + 11m30s drops everything from root1 (last_activity = base+3m)
	// and from root2 except r2b (base+12m). With recent=1 (the freshest-active
	// thread, root2), the page keeps r2b and the cursor still points at root2
	// so the caller can scroll older threads if they want.
	v := url.Values{}
	v.Set("recent", "1")
	v.Set("since", fx.Base.Add(11*time.Minute+30*time.Second).UTC().Format(time.RFC3339Nano))
	w, rows := listComments(t, fx.IssueID, v.Encode())
	eqIDs(t, ids(rows), []string{fx.R2b}, "recent=1 + since keeps r2b")
	nb, nbid := nextThreadCursor(w)
	if nb == "" || nbid != fx.Root2 {
		t.Fatalf("non-empty recent+since page must keep cursor; got before=%q before_id=%q want root_id=%q", nb, nbid, fx.Root2)
	}
}

// TestListComments_RecentWithSinceSuppressesCursorWhenHeadPastSince covers
// the mixed case from Elon's #2787 second review: the page is full (so the
// legacy `len(seen) >= N` check would emit a cursor) AND the `since` filter
// keeps rows from fresher threads (so the legacy `len(comments) == 0`
// suppression does NOT trip), but the head (oldest-active) thread already
// sits at or before `since`. Older pages walk strictly less-recent threads,
// so none of them can produce a `> since` comment — emitting a cursor here
// would still drag the caller into a guaranteed-empty next-page fetch.
//
// Fixture: root1 last_activity = base+3m, root2 last_activity = base+12m.
// recent=2 ⇒ both threads on the page, head = root1. since = base+5m drops
// everything in root1, keeps root2 entirely. Expect body = root2 thread,
// no cursor.
func TestListComments_RecentWithSinceSuppressesCursorWhenHeadPastSince(t *testing.T) {
	if testHandler == nil || testPool == nil {
		t.Skip("database not available")
	}
	fx := newCommentListFixture(t)

	v := url.Values{}
	v.Set("recent", "2")
	v.Set("since", fx.Base.Add(5*time.Minute).UTC().Format(time.RFC3339Nano))
	w, rows := listComments(t, fx.IssueID, v.Encode())
	eqIDs(t, ids(rows), []string{fx.Root2, fx.R2a, fx.R2b}, "recent=2 + since keeps root2 thread")
	nb, nbid := nextThreadCursor(w)
	if nb != "" || nbid != "" {
		t.Fatalf("head thread (root1, last_activity = base+3m) is already <= since (base+5m); older pages can't beat it. cursor must be suppressed, got before=%q before_id=%q", nb, nbid)
	}
}

// TestListComments_ThreadWithSinceFiltersWithinThread proves the allowed
// combination from the rules: `thread + since` returns only comments in
// that thread newer than `since`. The since filter is applied in-memory
// after the thread CTE so the root membership semantics stay intact.
func TestListComments_ThreadWithSinceFiltersWithinThread(t *testing.T) {
	if testHandler == nil || testPool == nil {
		t.Skip("database not available")
	}
	fx := newCommentListFixture(t)

	// since = base+1m30s → drop root1, r1a; keep r1b, r1b1.
	v := url.Values{}
	v.Set("thread", fx.Root1)
	v.Set("since", fx.Base.Add(90*time.Second).UTC().Format(time.RFC3339Nano))
	_, rows := listComments(t, fx.IssueID, v.Encode())
	eqIDs(t, ids(rows), []string{fx.R1b, fx.R1b1}, "thread+since")
}

// nextReplyCursor reads the (before, before-id) headers the thread + tail
// path emits when there is likely an older page of replies inside the same
// thread. Same wire shape as the thread-cursor headers — context decides
// which (the caller knows whether they used --recent or --tail).
func nextReplyCursor(w *httptest.ResponseRecorder) (string, string) {
	return w.Header().Get("X-Multica-Next-Before"), w.Header().Get("X-Multica-Next-Before-Id")
}

// TestListComments_ThreadTailReturnsRootPlusNewestReplies pins the core
// MUL-2421 contract: `--thread X --tail N` returns the thread root + the N
// most recent replies in that thread. Body stays chronological, so the
// root sits at the head and the freshest reply at the tail (closest to
// "now" in an agent prompt).
func TestListComments_ThreadTailReturnsRootPlusNewestReplies(t *testing.T) {
	if testHandler == nil || testPool == nil {
		t.Skip("database not available")
	}
	fx := newCommentListFixture(t)

	t.Run("tail=2 keeps newest 2 replies + root", func(t *testing.T) {
		v := url.Values{}
		v.Set("thread", fx.Root1)
		v.Set("tail", "2")
		// root1 has r1a, r1b, r1b1 in order. Newest 2 = r1b, r1b1.
		_, rows := listComments(t, fx.IssueID, v.Encode())
		eqIDs(t, ids(rows), []string{fx.Root1, fx.R1b, fx.R1b1}, "tail=2")
	})

	t.Run("tail larger than reply count returns full thread", func(t *testing.T) {
		v := url.Values{}
		v.Set("thread", fx.Root1)
		v.Set("tail", "99")
		_, rows := listComments(t, fx.IssueID, v.Encode())
		eqIDs(t, ids(rows), []string{fx.Root1, fx.R1a, fx.R1b, fx.R1b1}, "tail=99 (oversized)")
	})

	t.Run("tail=0 returns root only", func(t *testing.T) {
		// Per the issue: root must never be elided — even tail=0 keeps it,
		// so a reader landing on a long thread still gets the "what is this
		// about" context without dragging any replies into prompt context.
		v := url.Values{}
		v.Set("thread", fx.Root1)
		v.Set("tail", "0")
		_, rows := listComments(t, fx.IssueID, v.Encode())
		eqIDs(t, ids(rows), []string{fx.Root1}, "tail=0 (root only)")
	})

	t.Run("anchor on a nested reply still walks up to the root", func(t *testing.T) {
		// r1b1 is a reply-of-reply (parent_id = r1b, itself a reply).
		// The recursive root walk must climb to root1 regardless. tail
		// applies to *that* thread's replies, not to whichever subtree
		// the anchor happens to be in.
		v := url.Values{}
		v.Set("thread", fx.R1b1)
		v.Set("tail", "1")
		_, rows := listComments(t, fx.IssueID, v.Encode())
		eqIDs(t, ids(rows), []string{fx.Root1, fx.R1b1}, "tail=1 anchored at nested reply")
	})
}

// TestListComments_ThreadTailEmitsReplyCursorWhenPageFull pins the cursor
// header contract for the thread + tail path. A full page (replyCount ==
// tail) emits a reply cursor pointing at the oldest reply in the page; an
// underfilled page emits nothing so the caller stops paginating.
func TestListComments_ThreadTailEmitsReplyCursorWhenPageFull(t *testing.T) {
	if testHandler == nil || testPool == nil {
		t.Skip("database not available")
	}
	fx := newCommentListFixture(t)

	t.Run("underfilled page emits no cursor", func(t *testing.T) {
		// tail=5 on a thread with 3 replies (r1a, r1b, r1b1). The SQL
		// returns all 3 — no older page exists.
		v := url.Values{}
		v.Set("thread", fx.Root1)
		v.Set("tail", "5")
		w, _ := listComments(t, fx.IssueID, v.Encode())
		nb, nbid := nextReplyCursor(w)
		if nb != "" || nbid != "" {
			t.Fatalf("expected no cursor, got before=%q before_id=%q", nb, nbid)
		}
	})

	t.Run("tail=0 emits no cursor (no replies were requested)", func(t *testing.T) {
		// tail=0 is the "I just want the root context" mode. There is no
		// reply page to scroll, so the server must not invite the caller
		// to walk one.
		v := url.Values{}
		v.Set("thread", fx.Root1)
		v.Set("tail", "0")
		w, _ := listComments(t, fx.IssueID, v.Encode())
		nb, nbid := nextReplyCursor(w)
		if nb != "" || nbid != "" {
			t.Fatalf("tail=0 must not emit cursor, got before=%q before_id=%q", nb, nbid)
		}
	})

	t.Run("full page emits cursor pointing at oldest reply", func(t *testing.T) {
		// tail=2 on root1 ⇒ page = r1b, r1b1. Oldest reply on page = r1b.
		v := url.Values{}
		v.Set("thread", fx.Root1)
		v.Set("tail", "2")
		w, _ := listComments(t, fx.IssueID, v.Encode())
		nb, nbid := nextReplyCursor(w)
		if nbid != fx.R1b {
			t.Fatalf("cursor before_id = %q, want %q (r1b — oldest reply on page)", nbid, fx.R1b)
		}
		if nb == "" {
			t.Fatalf("cursor before is empty; expected RFC3339Nano timestamp")
		}
		if _, err := time.Parse(time.RFC3339Nano, nb); err != nil {
			t.Fatalf("cursor before = %q is not RFC3339Nano: %v", nb, err)
		}
	})

	t.Run("exact-boundary page emits no cursor", func(t *testing.T) {
		// root1 has exactly 3 replies (r1a, r1b, r1b1). Requesting --tail 3
		// returns the entire reply set; there is nothing older to scroll
		// to, so no cursor must be emitted. Pre-fix the server used
		// `replyCount >= tail` and falsely sent a cursor here — the next
		// page then returned just the root, wasting a round-trip.
		// (MUL-2421 review fix.)
		v := url.Values{}
		v.Set("thread", fx.Root1)
		v.Set("tail", "3")
		w, rows := listComments(t, fx.IssueID, v.Encode())
		eqIDs(t, ids(rows), []string{fx.Root1, fx.R1a, fx.R1b, fx.R1b1}, "tail==replyCount returns full thread")
		nb, nbid := nextReplyCursor(w)
		if nb != "" || nbid != "" {
			t.Fatalf("exact-boundary page must not emit cursor, got before=%q before_id=%q", nb, nbid)
		}
	})
}

// TestListComments_ThreadTailCursorScrollsOlderReplies walks a long thread
// reply-by-reply via the cursor the server emits. This is the analogue of
// TestListComments_RecentWithThreadCursorScrollsOlderThreads but inside
// one thread: cursor returns are reply cursors, not thread cursors, and
// the root must keep showing up on every page.
func TestListComments_ThreadTailCursorScrollsOlderReplies(t *testing.T) {
	if testHandler == nil || testPool == nil {
		t.Skip("database not available")
	}
	fx := newCommentListFixture(t)

	// Page 1: tail=1 on root1 → newest reply = r1b1.
	v := url.Values{}
	v.Set("thread", fx.Root1)
	v.Set("tail", "1")
	w1, page1 := listComments(t, fx.IssueID, v.Encode())
	eqIDs(t, ids(page1), []string{fx.Root1, fx.R1b1}, "page1 = root + r1b1")
	nb, nbid := nextReplyCursor(w1)
	if nb == "" || nbid != fx.R1b1 {
		t.Fatalf("page1 cursor = (%q, %q), want (non-empty, %q)", nb, nbid, fx.R1b1)
	}

	// Page 2: cursor points at r1b1 → server returns the next older reply
	// (r1b). Root is re-emitted on every page so the agent never loses the
	// thread context.
	v.Set("before", nb)
	v.Set("before_id", nbid)
	w2, page2 := listComments(t, fx.IssueID, v.Encode())
	eqIDs(t, ids(page2), []string{fx.Root1, fx.R1b}, "page2 = root + r1b")
	nb2, nbid2 := nextReplyCursor(w2)
	if nb2 == "" || nbid2 != fx.R1b {
		t.Fatalf("page2 cursor = (%q, %q), want (non-empty, %q)", nb2, nbid2, fx.R1b)
	}

	// Page 3: cursor points at r1b → server returns r1a (the last reply).
	// r1a is the oldest reply in the thread, so the server must NOT emit a
	// cursor — the next page would return just the root, which is a wasted
	// round-trip. (MUL-2421 review: probe `reply_limit + 1` to detect
	// has-more instead of trusting replyCount >= tail.)
	v.Set("before", nb2)
	v.Set("before_id", nbid2)
	w3, page3 := listComments(t, fx.IssueID, v.Encode())
	eqIDs(t, ids(page3), []string{fx.Root1, fx.R1a}, "page3 = root + r1a (last reply)")
	nb3, nbid3 := nextReplyCursor(w3)
	if nb3 != "" || nbid3 != "" {
		t.Fatalf("page3 cursor = (%q, %q), want both empty (end-of-thread, no older replies after r1a)", nb3, nbid3)
	}
}

// TestListComments_ThreadTailWithSinceFiltersAfterTail proves the documented
// combination: `--thread + --tail + --since` applies `tail` first (newest N
// replies in the thread) and then drops anything <= since from that page.
// The root is exempt — it is always returned so the reader keeps the thread
// context even when the page would otherwise be empty.
func TestListComments_ThreadTailWithSinceFiltersAfterTail(t *testing.T) {
	if testHandler == nil || testPool == nil {
		t.Skip("database not available")
	}
	fx := newCommentListFixture(t)

	t.Run("since drops older replies, keeps root + fresher", func(t *testing.T) {
		// tail=3 on root1 → all 3 replies (r1a, r1b, r1b1) + root1.
		// since = base + 90s → drops r1a (base+1m); keeps r1b (base+2m),
		// r1b1 (base+3m).
		v := url.Values{}
		v.Set("thread", fx.Root1)
		v.Set("tail", "3")
		v.Set("since", fx.Base.Add(90*time.Second).UTC().Format(time.RFC3339Nano))
		_, rows := listComments(t, fx.IssueID, v.Encode())
		eqIDs(t, ids(rows), []string{fx.Root1, fx.R1b, fx.R1b1}, "tail=3+since")
	})

	t.Run("since drops every reply but root stays", func(t *testing.T) {
		// since past every comment in the fixture — root is still emitted
		// for context.
		v := url.Values{}
		v.Set("thread", fx.Root1)
		v.Set("tail", "3")
		v.Set("since", fx.Base.Add(1*time.Hour).UTC().Format(time.RFC3339Nano))
		_, rows := listComments(t, fx.IssueID, v.Encode())
		eqIDs(t, ids(rows), []string{fx.Root1}, "since past everything keeps root")
	})

	t.Run("tail overflow with since past oldest retained reply suppresses cursor", func(t *testing.T) {
		// Recreate Elon's MUL-2421 v2 case: long thread, tail=2 overflows
		// (3 replies exist, only top 2 kept), the page body still retains
		// a fresher reply (r1b1 at base+3m), but the oldest reply on the
		// retained page (r1b at base+2m) is already <= since (base+2m30s).
		// Older replies are strictly older than r1b → strictly older than
		// since → all guaranteed-filtered. Server must NOT emit a reply
		// cursor; otherwise the agent walks root-only pages until the
		// thread bottoms out.
		v := url.Values{}
		v.Set("thread", fx.Root1)
		v.Set("tail", "2")
		v.Set("since", fx.Base.Add(150*time.Second).UTC().Format(time.RFC3339Nano))
		w, rows := listComments(t, fx.IssueID, v.Encode())
		// Sanity-check the body precondition: we kept the fresher reply
		// (r1b1, base+3m) but dropped r1b (base+2m) via since. If this
		// assertion ever shifts, the cursor assertion below would be
		// testing a different shape than the bug Elon described.
		eqIDs(t, ids(rows), []string{fx.Root1, fx.R1b1}, "body keeps root + fresher reply only")
		nb, nbid := nextReplyCursor(w)
		if nb != "" || nbid != "" {
			t.Fatalf("expected no cursor (older page is guaranteed-empty under since), got before=%q before_id=%q", nb, nbid)
		}
	})
}

// TestListComments_ThreadTailFlagCombinationRules locks the API-surface
// rules for the new --tail flag. The matrix is narrow on purpose — the
// only legal combinations are: (thread + tail), (thread + tail + since),
// (thread + tail + before + before_id), and a thread/recent/cursor matrix
// that does NOT include tail.
func TestListComments_ThreadTailFlagCombinationRules(t *testing.T) {
	if testHandler == nil || testPool == nil {
		t.Skip("database not available")
	}
	fx := newCommentListFixture(t)

	cases := []struct {
		name   string
		query  string
		status int
	}{
		{
			// tail is a thread-scoped limit; outside of --thread there is
			// no defined behavior so it must be rejected at the API surface.
			name:   "tail without thread rejected",
			query:  "tail=5",
			status: http.StatusBadRequest,
		},
		{
			// Negative tail would round-trip to LIMIT -N which Postgres
			// flags as a syntax error. Catch it at the boundary.
			name:   "negative tail rejected",
			query:  "thread=" + fx.Root1 + "&tail=-1",
			status: http.StatusBadRequest,
		},
		{
			name:   "non-numeric tail rejected",
			query:  "thread=" + fx.Root1 + "&tail=lots",
			status: http.StatusBadRequest,
		},
		{
			// Cursor without --tail in the thread path used to be rejected
			// outright; now it requires --tail so the cursor's "scroll
			// older replies" meaning has somewhere to land. Without --tail
			// the server returns the whole thread anyway, so the cursor is
			// meaningless.
			name: "thread + before without tail rejected",
			query: (func() string {
				v := url.Values{}
				v.Set("thread", fx.Root1)
				v.Set("before", time.Now().UTC().Format(time.RFC3339))
				v.Set("before_id", uuid.NewString())
				return v.Encode()
			})(),
			status: http.StatusBadRequest,
		},
	}
	for _, tc := range cases {
		t.Run(tc.name, func(t *testing.T) {
			w, _ := listComments(t, fx.IssueID, tc.query)
			if w.Code != tc.status {
				t.Fatalf("query=%q\n  got=%d want=%d body=%s", tc.query, w.Code, tc.status, w.Body.String())
			}
		})
	}
}

// TestListComments_ThreadTailZeroReplyCountIsAllowed pins tail=0 as a valid
// caller intent (root-only). The split between "did not pass --tail" and
// "passed --tail 0" lives in the handler (ThreadTailSet bool); collapsing
// them into a single int would silently downgrade tail=0 to the full
// thread path.
func TestListComments_ThreadTailZeroReplyCountIsAllowed(t *testing.T) {
	if testHandler == nil || testPool == nil {
		t.Skip("database not available")
	}
	fx := newCommentListFixture(t)

	v := url.Values{}
	v.Set("thread", fx.Root1)
	v.Set("tail", "0")
	w, rows := listComments(t, fx.IssueID, v.Encode())
	if w.Code != http.StatusOK {
		t.Fatalf("tail=0 should succeed, got %d: %s", w.Code, w.Body.String())
	}
	eqIDs(t, ids(rows), []string{fx.Root1}, "tail=0 returns only root")
}

// TestListComments_ThreadTailNotFoundReturns404 makes the not-found surface
// of the paged path match the legacy thread path. A stale anchor is a
// realistic agent footgun (mention a comment that was later deleted), and
// returning [] would be indistinguishable from "the thread really does
// have no comments".
func TestListComments_ThreadTailNotFoundReturns404(t *testing.T) {
	if testHandler == nil || testPool == nil {
		t.Skip("database not available")
	}
	fx := newCommentListFixture(t)

	v := url.Values{}
	v.Set("thread", "00000000-0000-0000-0000-000000000001")
	v.Set("tail", "5")
	w, _ := listComments(t, fx.IssueID, v.Encode())
	if w.Code != http.StatusNotFound {
		t.Fatalf("expected 404 for unknown anchor, got %d: %s", w.Code, w.Body.String())
	}
	_ = fx
}

// resolveCommentRow marks a comment resolved directly in the DB (test helper —
// the public path goes through ResolveComment, but for list-filter tests we
// just need the column set).
func resolveCommentRow(t *testing.T, commentID string) {
	t.Helper()
	if _, err := testPool.Exec(context.Background(),
		`UPDATE comment SET resolved_at = now(), resolved_by_type = 'member', resolved_by_id = $2 WHERE id = $1`,
		commentID, testUserID,
	); err != nil {
		t.Fatalf("resolve comment row: %v", err)
	}
}

// TestCountNewCommentsSince_IssueWideExcludesAgentOwnAndTrigger pins the
// claim-side count query: it counts comments created after the given anchor
// ACROSS THE WHOLE ISSUE (every thread, not just the triggering one), excludes
// the triggering comment because it is injected into the prompt, and excludes
// the agent's own comments so a chatty agent does not inflate its own
// new-comment count.
func TestCountNewCommentsSince_IssueWideExcludesAgentOwnAndTrigger(t *testing.T) {
	if testHandler == nil || testPool == nil {
		t.Skip("database not available")
	}
	fx := newCommentListFixture(t)
	agentID := createHandlerTestAgent(t, "count-agent", []byte("[]"))

	if _, err := testPool.Exec(context.Background(), `
		INSERT INTO comment (issue_id, workspace_id, author_type, author_id, content, type, parent_id, created_at)
		VALUES ($1, $2, 'agent', $3, 'agent reply', 'comment', $4, $5)
	`, fx.IssueID, testWorkspaceID, agentID, fx.Root1, fx.Base.Add(4*time.Minute)); err != nil {
		t.Fatalf("insert agent comment: %v", err)
	}

	ctx := context.Background()

	// Anchor after r1a: root1/r1a are stale. Newer than the anchor are r1b
	// (trigger thread), r1b1 (the trigger itself — excluded), the agent reply
	// (excluded), and root2's whole thread root2/r2a/r2b (unrelated thread, now
	// counted because the count is issue-wide). So r1b + root2 + r2a + r2b = 4.
	got, err := testHandler.Queries.CountNewCommentsSince(ctx, db.CountNewCommentsSinceParams{
		AnchorID:    parseUUID(fx.R1b1),
		IssueID:     parseUUID(fx.IssueID),
		WorkspaceID: parseUUID(testWorkspaceID),
		Since:       pgtype.Timestamptz{Time: fx.Base.Add(90 * time.Second), Valid: true},
		AuthorID:    parseUUID(agentID),
	})
	if err != nil {
		t.Fatalf("count new since anchor: %v", err)
	}
	if got != 4 {
		t.Fatalf("expected issue-wide count of 4 (r1b + root2 + r2a + r2b), got %d", got)
	}

	// Anchor in the future: nothing is newer.
	got, err = testHandler.Queries.CountNewCommentsSince(ctx, db.CountNewCommentsSinceParams{
		AnchorID:    parseUUID(fx.R1b1),
		IssueID:     parseUUID(fx.IssueID),
		WorkspaceID: parseUUID(testWorkspaceID),
		Since:       pgtype.Timestamptz{Time: time.Now().Add(time.Hour), Valid: true},
		AuthorID:    parseUUID(agentID),
	})
	if err != nil {
		t.Fatalf("count new since future: %v", err)
	}
	if got != 0 {
		t.Fatalf("expected 0 new comments after a future anchor, got %d", got)
	}
}

// TestCreateCommentPreservesDirectParent pins the write-boundary invariant:
// parent_id stores the exact comment being replied to. Thread readers discover
// the root separately via recursive queries, so storing a reply-to-reply must
// not destroy the direct-parent signal that trigger logic needs.
func TestCreateCommentPreservesDirectParent(t *testing.T) {
	if testHandler == nil || testPool == nil {
		t.Skip("requires DB")
	}
	ctx := context.Background()

	var issueID string
	if err := testPool.QueryRow(ctx, `
		INSERT INTO issue (workspace_id, creator_type, creator_id, title)
		VALUES ($1, 'member', $2, $3)
		RETURNING id
	`, testWorkspaceID, testUserID, "direct parent fixture").Scan(&issueID); err != nil {
		t.Fatalf("create issue: %v", err)
	}
	t.Cleanup(func() {
		testPool.Exec(context.Background(), `DELETE FROM issue WHERE id = $1`, issueID)
	})

	create := func(parentID, body string) CommentResponse {
		t.Helper()
		payload := map[string]any{"content": body}
		if parentID != "" {
			payload["parent_id"] = parentID
		}
		w := httptest.NewRecorder()
		req := newRequest("POST", "/api/issues/"+issueID+"/comments", payload)
		req = withURLParam(req, "id", issueID)
		testHandler.CreateComment(w, req)
		if w.Code != http.StatusCreated {
			t.Fatalf("CreateComment(%q): expected 201, got %d: %s", body, w.Code, w.Body.String())
		}
		var resp CommentResponse
		if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
			t.Fatalf("decode created comment %q: %v", body, err)
		}
		return resp
	}

	root := create("", "root")
	if root.ParentID != nil {
		t.Fatalf("root comment must have nil parent_id, got %v", *root.ParentID)
	}

	// A direct reply to the root stays parented at the root.
	reply := create(root.ID, "reply")
	if reply.ParentID == nil || *reply.ParentID != root.ID {
		t.Fatalf("direct reply parent_id: want root %s, got %v", root.ID, reply.ParentID)
	}

	// A reply whose parent is itself a reply must keep that direct parent. The
	// thread root is recoverable through recursive thread reads; the direct
	// parent is not recoverable once overwritten.
	nested := create(reply.ID, "nested")
	if nested.ParentID == nil || *nested.ParentID != reply.ID {
		t.Fatalf("reply-to-reply parent_id: want direct parent %s, got %v (root was %s)", reply.ID, nested.ParentID, root.ID)
	}
}