mirror of
https://github.com/multica-ai/multica.git
synced 2026-06-30 10:59:31 +02:00
Compare commits
5 Commits
agent/lamb
...
agent/j/b0
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
62aff58555 | ||
|
|
5e30c75ffa | ||
|
|
ba93b2a072 | ||
|
|
1fe0e2e24c | ||
|
|
9a1f2cf299 |
@@ -373,9 +373,44 @@ Valid statuses: `backlog`, `todo`, `in_progress`, `in_review`, `done`, `blocked`
|
||||
### Comments
|
||||
|
||||
```bash
|
||||
# List comments
|
||||
# List comments — flat timeline, chronological. Hard cap of 2000 rows; on
|
||||
# long-running issues prefer one of the thread-aware reads below to keep
|
||||
# context windows tight.
|
||||
multica issue comment list <issue-id>
|
||||
|
||||
# Single thread (root + every descendant). Anchor may be the root itself
|
||||
# or any reply inside the thread — the server walks up to the root.
|
||||
multica issue comment list <issue-id> --thread <comment-id>
|
||||
|
||||
# Single thread, capped to the N most recent replies. The thread root is
|
||||
# always included (even with --tail 0), so an agent landing on a long
|
||||
# thread keeps the "what is this about" context without dragging hundreds
|
||||
# of replies into its prompt.
|
||||
multica issue comment list <issue-id> --thread <comment-id> --tail 30
|
||||
|
||||
# Scroll older replies inside the same thread. --before / --before-id are
|
||||
# the reply cursor that the previous response emitted on stderr as
|
||||
# `Next reply cursor: --before <ts> --before-id <reply-id>`.
|
||||
multica issue comment list <issue-id> --thread <comment-id> --tail 30 \
|
||||
--before <ts> --before-id <reply-id>
|
||||
|
||||
# Most recently active threads (root + every descendant), grouped by
|
||||
# thread. Returns N complete conversational arcs, oldest-active first so
|
||||
# the freshest thread sits closest to "now" in an agent prompt.
|
||||
multica issue comment list <issue-id> --recent 20
|
||||
|
||||
# Scroll older threads. Under --recent, --before / --before-id are a
|
||||
# THREAD cursor (thread last_activity_at + root id), emitted on stderr as
|
||||
# `Next thread cursor: --before <ts> --before-id <root-id>`.
|
||||
multica issue comment list <issue-id> --recent 20 \
|
||||
--before <ts> --before-id <root-id>
|
||||
|
||||
# Incremental polling. Combines with --thread or --recent; filters out
|
||||
# replies created on or before <ts> from the page (the thread root is
|
||||
# exempt so the agent always gets context).
|
||||
multica issue comment list <issue-id> --thread <comment-id> --tail 30 \
|
||||
--since <RFC3339-timestamp>
|
||||
|
||||
# Add a comment
|
||||
multica issue comment add <issue-id> --content "Looks good, merging now"
|
||||
|
||||
@@ -386,6 +421,29 @@ multica issue comment add <issue-id> --parent <comment-id> --content "Thanks!"
|
||||
multica issue comment delete <comment-id>
|
||||
```
|
||||
|
||||
**`--before` / `--before-id` semantics depend on the paging mode**, by
|
||||
design — same flag, different scope:
|
||||
|
||||
| Mode | What the cursor walks | stderr label |
|
||||
| --- | --- | --- |
|
||||
| `--recent N` | Older *threads* (last_activity_at, root_id) | `Next thread cursor` |
|
||||
| `--thread <id> --tail N` | Older *replies* inside that thread (created_at, id) | `Next reply cursor` |
|
||||
|
||||
Outside those two modes (`--thread` without `--tail`, or no `--thread`
|
||||
and no `--recent`) the cursor flags are rejected so they cannot silently
|
||||
no-op. The server emits the cursor headers (`X-Multica-Next-Before` /
|
||||
`X-Multica-Next-Before-Id`) only when an older page actually exists —
|
||||
exact-boundary pages (e.g. `--tail 3` on a thread with exactly 3
|
||||
replies) intentionally return no cursor so callers stop paginating.
|
||||
|
||||
When `--since` is combined with `--recent` or `--thread --tail`, the
|
||||
server additionally suppresses the cursor once the cursor target itself
|
||||
is older than `since`. Older pages walk strictly older rows, so they
|
||||
cannot satisfy `> since` either — emitting a cursor there would just
|
||||
hand back root-only pages until the caller reaches the start of the
|
||||
thread / issue. Incremental polling stops at the first page whose
|
||||
cursor target falls before the watermark.
|
||||
|
||||
### Subscribers
|
||||
|
||||
```bash
|
||||
|
||||
@@ -310,9 +310,10 @@ func init() {
|
||||
issueCommentListCmd.Flags().String("output", "table", "Output format: table or json")
|
||||
issueCommentListCmd.Flags().String("since", "", "Only return comments created after this timestamp (RFC3339)")
|
||||
issueCommentListCmd.Flags().String("thread", "", "Comment UUID — return the thread containing this comment (root + every descendant). May be a root or a reply id.")
|
||||
issueCommentListCmd.Flags().Int("tail", 0, "Only valid with --thread. Cap reply count to the N most recent replies; the thread root is always included (even with --tail 0). Use --before/--before-id to scroll to older replies.")
|
||||
issueCommentListCmd.Flags().Int("recent", 0, "Return the N most recently active threads (root + descendants per thread). Use --before/--before-id from the previous response to scroll to older threads.")
|
||||
issueCommentListCmd.Flags().String("before", "", "Thread cursor: last_activity_at (RFC3339Nano). Read from the X-Multica-Next-Before response header; must be paired with --before-id.")
|
||||
issueCommentListCmd.Flags().String("before-id", "", "Thread cursor: root comment UUID. Read from the X-Multica-Next-Before-Id response header; must be paired with --before.")
|
||||
issueCommentListCmd.Flags().String("before", "", "Cursor (RFC3339Nano timestamp). With --recent: thread cursor (last_activity_at). With --thread + --tail: reply cursor (reply created_at). Read from the X-Multica-Next-Before response header; must be paired with --before-id.")
|
||||
issueCommentListCmd.Flags().String("before-id", "", "Cursor UUID. With --recent: thread root UUID. With --thread + --tail: oldest reply UUID. Read from the X-Multica-Next-Before-Id response header; must be paired with --before.")
|
||||
|
||||
// issue runs
|
||||
issueRunsCmd.Flags().String("output", "table", "Output format: table or json")
|
||||
@@ -928,13 +929,16 @@ func runIssueCommentList(cmd *cobra.Command, args []string) error {
|
||||
since, _ := cmd.Flags().GetString("since")
|
||||
thread, _ := cmd.Flags().GetString("thread")
|
||||
recent, _ := cmd.Flags().GetInt("recent")
|
||||
tail, _ := cmd.Flags().GetInt("tail")
|
||||
// Flags().Changed distinguishes "user did not pass --recent" from
|
||||
// "user explicitly passed --recent 0" (or a negative value). The
|
||||
// GetInt zero-value collapses both cases, which would otherwise
|
||||
// cause us to silently drop an invalid value and fall back to the
|
||||
// default unparameterized list — exactly the drift Elon flagged in
|
||||
// the PR #2787 second review.
|
||||
// the PR #2787 second review. --tail follows the same pattern, and
|
||||
// also keeps "--tail 0" (root-only) distinguishable from "no --tail".
|
||||
recentSet := cmd.Flags().Changed("recent")
|
||||
tailSet := cmd.Flags().Changed("tail")
|
||||
before, _ := cmd.Flags().GetString("before")
|
||||
beforeID, _ := cmd.Flags().GetString("before-id")
|
||||
|
||||
@@ -944,17 +948,20 @@ func runIssueCommentList(cmd *cobra.Command, args []string) error {
|
||||
if recentSet && recent <= 0 {
|
||||
return fmt.Errorf("--recent must be a positive integer")
|
||||
}
|
||||
if tailSet && tail < 0 {
|
||||
return fmt.Errorf("--tail must be a non-negative integer (0 returns just the thread root)")
|
||||
}
|
||||
if thread != "" && recentSet {
|
||||
return fmt.Errorf("--thread and --recent are mutually exclusive")
|
||||
}
|
||||
if thread != "" && (before != "" || beforeID != "") {
|
||||
return fmt.Errorf("--thread cannot be combined with --before / --before-id")
|
||||
if tailSet && thread == "" {
|
||||
return fmt.Errorf("--tail requires --thread (it is a thread-scoped limit)")
|
||||
}
|
||||
if (before == "") != (beforeID == "") {
|
||||
return fmt.Errorf("--before and --before-id must be set together (composite cursor for stable pagination)")
|
||||
}
|
||||
if before != "" && !recentSet {
|
||||
return fmt.Errorf("--before / --before-id require --recent (cursor scrolls within a recent window)")
|
||||
if before != "" && !recentSet && !(thread != "" && tailSet) {
|
||||
return fmt.Errorf("--before / --before-id require --recent (thread cursor) or --thread + --tail (reply cursor)")
|
||||
}
|
||||
|
||||
params := url.Values{}
|
||||
@@ -964,6 +971,9 @@ func runIssueCommentList(cmd *cobra.Command, args []string) error {
|
||||
if thread != "" {
|
||||
params.Set("thread", thread)
|
||||
}
|
||||
if tailSet {
|
||||
params.Set("tail", fmt.Sprintf("%d", tail))
|
||||
}
|
||||
if recentSet {
|
||||
params.Set("recent", fmt.Sprintf("%d", recent))
|
||||
}
|
||||
@@ -983,13 +993,19 @@ func runIssueCommentList(cmd *cobra.Command, args []string) error {
|
||||
return fmt.Errorf("list comments: %w", err)
|
||||
}
|
||||
fmt.Fprintf(os.Stderr, "Showing %d comments.\n", len(comments))
|
||||
// Under --recent the server emits a thread cursor in headers when there
|
||||
// is likely an older page. Surface it on stderr so an operator (and the
|
||||
// agent prompt update that follows this PR) can scroll deeper without
|
||||
// having to dig into the raw HTTP response.
|
||||
// The server emits the next-page cursor in headers when there is likely
|
||||
// an older page. Surface it on stderr so an operator (and the agent
|
||||
// prompt update that follows this PR) can scroll deeper without having
|
||||
// to dig into the raw HTTP response. Label depends on which paging mode
|
||||
// the caller is in — under --recent the cursor is a thread cursor;
|
||||
// under --thread + --tail it is a reply cursor inside that thread.
|
||||
if nb := respHeaders.Get("X-Multica-Next-Before"); nb != "" {
|
||||
if nbid := respHeaders.Get("X-Multica-Next-Before-Id"); nbid != "" {
|
||||
fmt.Fprintf(os.Stderr, "Next thread cursor: --before %s --before-id %s\n", nb, nbid)
|
||||
label := "Next thread cursor"
|
||||
if thread != "" && tailSet {
|
||||
label = "Next reply cursor"
|
||||
}
|
||||
fmt.Fprintf(os.Stderr, "%s: --before %s --before-id %s\n", label, nb, nbid)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -4,8 +4,10 @@ import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
"os"
|
||||
"strconv"
|
||||
"strings"
|
||||
@@ -16,6 +18,50 @@ import (
|
||||
"github.com/multica-ai/multica/server/internal/cli"
|
||||
)
|
||||
|
||||
// stderrCapture redirects os.Stderr through a pipe so a test can assert on
|
||||
// the human-facing strings runIssueCommentList prints alongside its JSON
|
||||
// output. Read() drains the pipe and is safe to call multiple times.
|
||||
type stderrCapture struct {
|
||||
t *testing.T
|
||||
orig *os.File
|
||||
r, w *os.File
|
||||
out strings.Builder
|
||||
doneCh chan struct{}
|
||||
stopped bool
|
||||
}
|
||||
|
||||
func captureStderr(t *testing.T) *stderrCapture {
|
||||
t.Helper()
|
||||
r, w, err := os.Pipe()
|
||||
if err != nil {
|
||||
t.Fatalf("pipe: %v", err)
|
||||
}
|
||||
c := &stderrCapture{t: t, orig: os.Stderr, r: r, w: w, doneCh: make(chan struct{})}
|
||||
os.Stderr = w
|
||||
go func() {
|
||||
buf, _ := io.ReadAll(r)
|
||||
c.out.Write(buf)
|
||||
close(c.doneCh)
|
||||
}()
|
||||
return c
|
||||
}
|
||||
|
||||
func (c *stderrCapture) restore() {
|
||||
if c.stopped {
|
||||
return
|
||||
}
|
||||
c.stopped = true
|
||||
os.Stderr = c.orig
|
||||
_ = c.w.Close()
|
||||
<-c.doneCh
|
||||
_ = c.r.Close()
|
||||
}
|
||||
|
||||
func (c *stderrCapture) read() string {
|
||||
c.restore()
|
||||
return c.out.String()
|
||||
}
|
||||
|
||||
// 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.
|
||||
@@ -1508,6 +1554,7 @@ func newIssueCommentListTestCmd() *cobra.Command {
|
||||
cmd.Flags().String("since", "", "")
|
||||
cmd.Flags().String("thread", "", "")
|
||||
cmd.Flags().Int("recent", 0, "")
|
||||
cmd.Flags().Int("tail", 0, "")
|
||||
cmd.Flags().String("before", "", "")
|
||||
cmd.Flags().String("before-id", "", "")
|
||||
return cmd
|
||||
@@ -1567,12 +1614,12 @@ func TestRunIssueCommentListFlagGuards(t *testing.T) {
|
||||
wantMsg: "--recent must be a positive integer",
|
||||
},
|
||||
{
|
||||
name: "before + before-id without recent rejected",
|
||||
name: "before + before-id without recent or thread+tail rejected",
|
||||
setup: func(c *cobra.Command) {
|
||||
_ = c.Flags().Set("before", "2026-01-01T00:00:00Z")
|
||||
_ = c.Flags().Set("before-id", "00000000-0000-0000-0000-000000000001")
|
||||
},
|
||||
wantMsg: "--before / --before-id require --recent",
|
||||
wantMsg: "require --recent",
|
||||
},
|
||||
{
|
||||
name: "thread + recent still rejected when --recent explicit zero", // also covers the Changed() path
|
||||
@@ -1582,6 +1629,39 @@ func TestRunIssueCommentListFlagGuards(t *testing.T) {
|
||||
},
|
||||
wantMsg: "--thread and --recent are mutually exclusive",
|
||||
},
|
||||
{
|
||||
// --tail is a thread-scoped limit. Outside of --thread it
|
||||
// would be silently dropped at the server, so the CLI rejects
|
||||
// it locally with a clear hint.
|
||||
name: "tail without thread rejected",
|
||||
setup: func(c *cobra.Command) {
|
||||
_ = c.Flags().Set("tail", "5")
|
||||
},
|
||||
wantMsg: "--tail requires --thread",
|
||||
},
|
||||
{
|
||||
// tail=0 is allowed (root-only) but a negative value would
|
||||
// round-trip to LIMIT -N on the server. Catch at the CLI so
|
||||
// the user sees a useful message instead of a 400.
|
||||
name: "negative tail rejected",
|
||||
setup: func(c *cobra.Command) {
|
||||
_ = c.Flags().Set("thread", "00000000-0000-0000-0000-000000000001")
|
||||
_ = c.Flags().Set("tail", "-2")
|
||||
},
|
||||
wantMsg: "--tail must be a non-negative integer",
|
||||
},
|
||||
{
|
||||
// --thread + --before without --tail used to be rejected
|
||||
// outright. Now it requires --tail so the cursor's "scroll
|
||||
// older replies" semantics has somewhere to land.
|
||||
name: "thread + before without tail rejected",
|
||||
setup: func(c *cobra.Command) {
|
||||
_ = c.Flags().Set("thread", "00000000-0000-0000-0000-000000000001")
|
||||
_ = c.Flags().Set("before", "2026-01-01T00:00:00Z")
|
||||
_ = c.Flags().Set("before-id", "00000000-0000-0000-0000-000000000002")
|
||||
},
|
||||
wantMsg: "require --recent",
|
||||
},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
@@ -1598,6 +1678,108 @@ func TestRunIssueCommentListFlagGuards(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestRunIssueCommentList_ThreadTailPassesThroughAndPrintsReplyCursor pins
|
||||
// the positive path for --thread + --tail: the CLI forwards `thread` +
|
||||
// `tail` query params and, on response, prints "Next reply cursor" (not
|
||||
// "Next thread cursor") so an operator can scroll older replies inside
|
||||
// the same thread without guessing which cursor model the server emitted.
|
||||
func TestRunIssueCommentList_ThreadTailPassesThroughAndPrintsReplyCursor(t *testing.T) {
|
||||
var gotQuery url.Values
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.Method == http.MethodGet && strings.HasPrefix(r.URL.Path, "/api/issues/") && !strings.Contains(r.URL.Path, "/comments") {
|
||||
json.NewEncoder(w).Encode(map[string]any{
|
||||
"id": "issue-1",
|
||||
"identifier": "MUL-1",
|
||||
})
|
||||
return
|
||||
}
|
||||
if r.Method == http.MethodGet && strings.HasSuffix(r.URL.Path, "/comments") {
|
||||
gotQuery = r.URL.Query()
|
||||
// Emit a cursor so we can prove the CLI labels it "reply"
|
||||
// when the call was a --thread + --tail combo.
|
||||
w.Header().Set("X-Multica-Next-Before", "2026-01-01T00:00:00.000000001Z")
|
||||
w.Header().Set("X-Multica-Next-Before-Id", "00000000-0000-0000-0000-000000000999")
|
||||
w.Write([]byte("[]"))
|
||||
return
|
||||
}
|
||||
t.Errorf("unexpected request: %s %s", r.Method, r.URL.String())
|
||||
http.Error(w, "unexpected", http.StatusInternalServerError)
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
t.Setenv("MULTICA_SERVER_URL", srv.URL)
|
||||
t.Setenv("MULTICA_WORKSPACE_ID", "ws-1")
|
||||
t.Setenv("MULTICA_TOKEN", "test-token")
|
||||
|
||||
// Redirect stderr so we can assert on the "Next reply cursor" line —
|
||||
// that's the user-visible signal that the CLI knew it was paging
|
||||
// within a thread, not across threads.
|
||||
stderr := captureStderr(t)
|
||||
defer stderr.restore()
|
||||
|
||||
cmd := newIssueCommentListTestCmd()
|
||||
if err := cmd.Flags().Set("thread", "00000000-0000-0000-0000-000000000001"); err != nil {
|
||||
t.Fatalf("set thread: %v", err)
|
||||
}
|
||||
if err := cmd.Flags().Set("tail", "5"); err != nil {
|
||||
t.Fatalf("set tail: %v", err)
|
||||
}
|
||||
if err := runIssueCommentList(cmd, []string{"MUL-1"}); err != nil {
|
||||
t.Fatalf("runIssueCommentList: %v", err)
|
||||
}
|
||||
|
||||
if got := gotQuery.Get("thread"); got != "00000000-0000-0000-0000-000000000001" {
|
||||
t.Errorf("thread query = %q, want the passed anchor", got)
|
||||
}
|
||||
if got := gotQuery.Get("tail"); got != "5" {
|
||||
t.Errorf("tail query = %q, want %q", got, "5")
|
||||
}
|
||||
out := stderr.read()
|
||||
if !strings.Contains(out, "Next reply cursor: --before 2026-01-01T00:00:00.000000001Z --before-id 00000000-0000-0000-0000-000000000999") {
|
||||
t.Errorf("stderr missing reply-cursor line, got: %q", out)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRunIssueCommentList_RecentStillLabelsCursorAsThread is the negative
|
||||
// counterpart: under --recent the CLI must keep printing "Next thread
|
||||
// cursor". A regression that printed "reply" here would mis-signal the
|
||||
// cursor semantics to anyone copy-pasting it.
|
||||
func TestRunIssueCommentList_RecentStillLabelsCursorAsThread(t *testing.T) {
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.Method == http.MethodGet && strings.HasPrefix(r.URL.Path, "/api/issues/") && !strings.Contains(r.URL.Path, "/comments") {
|
||||
json.NewEncoder(w).Encode(map[string]any{
|
||||
"id": "issue-1",
|
||||
"identifier": "MUL-1",
|
||||
})
|
||||
return
|
||||
}
|
||||
w.Header().Set("X-Multica-Next-Before", "2026-01-01T00:00:00.000000001Z")
|
||||
w.Header().Set("X-Multica-Next-Before-Id", "00000000-0000-0000-0000-000000000777")
|
||||
w.Write([]byte("[]"))
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
t.Setenv("MULTICA_SERVER_URL", srv.URL)
|
||||
t.Setenv("MULTICA_WORKSPACE_ID", "ws-1")
|
||||
t.Setenv("MULTICA_TOKEN", "test-token")
|
||||
|
||||
stderr := captureStderr(t)
|
||||
defer stderr.restore()
|
||||
|
||||
cmd := newIssueCommentListTestCmd()
|
||||
if err := cmd.Flags().Set("recent", "3"); err != nil {
|
||||
t.Fatalf("set recent: %v", err)
|
||||
}
|
||||
if err := runIssueCommentList(cmd, []string{"MUL-1"}); err != nil {
|
||||
t.Fatalf("runIssueCommentList: %v", err)
|
||||
}
|
||||
|
||||
out := stderr.read()
|
||||
if !strings.Contains(out, "Next thread cursor:") {
|
||||
t.Errorf("stderr missing thread-cursor line, got: %q", out)
|
||||
}
|
||||
}
|
||||
|
||||
func TestValidIssueStatuses(t *testing.T) {
|
||||
expected := map[string]bool{
|
||||
"backlog": true,
|
||||
|
||||
@@ -3083,13 +3083,13 @@ func TestBuildMetaSkillContentOmitsRequestingUserWhenEmpty(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestInjectRuntimeConfigCommentTriggerThreadFirstReads locks in MUL-2387:
|
||||
// the runtime config's comment-triggered Workflow section must steer the
|
||||
// agent at the thread-aware reads from PR #2787 first (--thread anchored on
|
||||
// the trigger comment id, then --recent N with the stderr cursor for
|
||||
// pagination) rather than the legacy "dump the whole comment list" recipe.
|
||||
// The Available Commands core line also has to surface the new flags so the
|
||||
// agent has a single place to discover them.
|
||||
// TestInjectRuntimeConfigCommentTriggerThreadFirstReads locks in
|
||||
// MUL-2387 + MUL-2421: the runtime config's comment-triggered Workflow
|
||||
// section must steer the agent at thread-aware reads first, default the
|
||||
// trigger thread to `--thread <id> --tail 30` (bounded), and explain the
|
||||
// reply-cursor walk for older replies. `--recent N` stays as the
|
||||
// cross-thread fallback. The Available Commands core line also has to
|
||||
// surface the `--tail` flag so the agent has a single place to discover it.
|
||||
func TestInjectRuntimeConfigCommentTriggerThreadFirstReads(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
@@ -3112,19 +3112,24 @@ func TestInjectRuntimeConfigCommentTriggerThreadFirstReads(t *testing.T) {
|
||||
s := string(data)
|
||||
|
||||
// Workflow step 2 must read the trigger's thread with --thread anchored
|
||||
// on the exact trigger comment id from this task.
|
||||
// on the exact trigger comment id from this task, bounded to --tail 30.
|
||||
for _, want := range []string{
|
||||
"--thread " + triggerID,
|
||||
"multica issue comment list " + issueID + " --thread " + triggerID + " --output json",
|
||||
// --recent fallback at the documented default N=20.
|
||||
"--tail 30",
|
||||
"multica issue comment list " + issueID + " --thread " + triggerID + " --tail 30 --output json",
|
||||
// Reply cursor walks older replies inside the same thread.
|
||||
"Next reply cursor:",
|
||||
"--before-id <reply-id>",
|
||||
// --recent fallback at the documented default N=20 for cross-thread context.
|
||||
"multica issue comment list " + issueID + " --recent 20 --output json",
|
||||
// Cursor walks via the stderr line the CLI emits, not invented flags.
|
||||
"Next thread cursor:",
|
||||
"Next thread cursor",
|
||||
"--before",
|
||||
"--before-id",
|
||||
// --since is still available and combinable.
|
||||
// --since is still available and combinable (now scoped to the
|
||||
// post-MUL-2421 mode names).
|
||||
"--since",
|
||||
"may combine with `--thread` or `--recent`",
|
||||
"may combine with `--thread --tail` or `--recent`",
|
||||
// Explicit pushback on the legacy full-dump recipe so the model has
|
||||
// no reason to fall back to it on long issues.
|
||||
"Avoid the unfiltered",
|
||||
@@ -3139,8 +3144,10 @@ func TestInjectRuntimeConfigCommentTriggerThreadFirstReads(t *testing.T) {
|
||||
// single discovery point for non-workflow CLI use cases).
|
||||
for _, want := range []string{
|
||||
"[--thread <comment-id>",
|
||||
"--recent N [--before <ts> --before-id <root-id>]",
|
||||
"Next thread cursor:",
|
||||
"--tail N",
|
||||
"--recent N",
|
||||
"Next reply cursor",
|
||||
"Next thread cursor",
|
||||
} {
|
||||
if !strings.Contains(s, want) {
|
||||
t.Errorf("Available Commands core line missing %q\n---\n%s", want, s)
|
||||
@@ -3151,6 +3158,11 @@ func TestInjectRuntimeConfigCommentTriggerThreadFirstReads(t *testing.T) {
|
||||
if strings.Contains(s, "read the conversation (returns all comments, capped server-side at 2000)") {
|
||||
t.Errorf("comment-triggered Workflow still carries the legacy full-dump phrasing\n---\n%s", s)
|
||||
}
|
||||
// The pre-MUL-2421 unbounded `--thread` recipe (no --tail) is also a
|
||||
// regression target: it dumps the entire thread on long threads.
|
||||
if strings.Contains(s, "multica issue comment list "+issueID+" --thread "+triggerID+" --output json") {
|
||||
t.Errorf("comment-triggered Workflow regressed to unbounded --thread recipe (no --tail) — long threads will overflow context\n---\n%s", s)
|
||||
}
|
||||
}
|
||||
|
||||
// TestInjectRuntimeConfigAssignmentTriggerMentionsRecent pins that the
|
||||
|
||||
@@ -189,7 +189,7 @@ func buildMetaSkillContent(provider string, ctx TaskContextForEnv) string {
|
||||
b.WriteString("The default brief includes the commands needed for the core agent loop and common issue create/update tasks. For everything else, run `multica --help`, `multica <command> --help`, or `multica <command> <subcommand> --help`; prefer `--output json` when the command supports it.\n\n")
|
||||
b.WriteString("### Core\n")
|
||||
b.WriteString("- `multica issue get <id> --output json` — Get full issue details.\n")
|
||||
b.WriteString("- `multica issue comment list <issue-id> [--thread <comment-id> | --recent N [--before <ts> --before-id <root-id>]] [--since <RFC3339>] --output json` — List comments on an issue. Default returns everything (server cap 2000). On busy issues prefer the thread-aware reads: `--thread <comment-id>` returns one conversation (root + every reply), `--recent N` returns the N most recently active threads. `--before` / `--before-id` pair (printed as a `Next thread cursor:` line on stderr after a `--recent` page) scrolls to older threads. `--since` is for incremental polling and may combine with `--thread` or `--recent`.\n")
|
||||
b.WriteString("- `multica issue comment list <issue-id> [--thread <comment-id> [--tail N] | --recent N] [--before <ts> --before-id <uuid>] [--since <RFC3339>] --output json` — List comments on an issue. Default returns the full flat timeline (server cap 2000). On busy issues prefer the thread-aware reads: `--thread <comment-id>` returns one conversation (root + every reply); `--thread <id> --tail N` caps replies to the N most recent (root is always included, even at `--tail 0`); `--recent N` returns the N most recently active threads. `--before` / `--before-id` walks older replies under `--thread --tail` (stderr label: `Next reply cursor`) or older threads under `--recent` (stderr label: `Next thread cursor`). `--since` is for incremental polling and may combine with `--thread --tail` or `--recent`.\n")
|
||||
b.WriteString("- `multica issue create --title \"...\" [--description \"...\" | --description-stdin | --description-file <path>] [--priority X] [--status X] [--assignee X | --assignee-id <uuid>] [--parent <issue-id>] [--project <project-id>] [--due-date <RFC3339>] [--attachment <path>]` — Create a new issue; `--attachment` may be repeated.\n")
|
||||
b.WriteString("- `multica issue update <id> [--title X] [--description X | --description-stdin | --description-file <path>] [--priority X] [--status X] [--assignee X | --assignee-id <uuid>] [--parent <issue-id>] [--project <project-id>] [--due-date <RFC3339>]` — Update issue fields; use `--parent \"\"` to clear parent.\n")
|
||||
b.WriteString("- `multica repo checkout <url> [--ref <branch-or-sha>]` — Check out a repository into the working directory (creates a git worktree with a dedicated branch; use `--ref` for review/QA on a specific branch, tag, or commit)\n")
|
||||
@@ -313,9 +313,10 @@ func buildMetaSkillContent(provider string, ctx TaskContextForEnv) string {
|
||||
// Comment-triggered: focus on reading and replying
|
||||
b.WriteString("**This task was triggered by a NEW comment.** Your primary job is to respond to THIS specific comment, even if you have handled similar requests before in this session.\n\n")
|
||||
fmt.Fprintf(&b, "1. Run `multica issue get %s --output json` to understand the issue context\n", ctx.IssueID)
|
||||
fmt.Fprintf(&b, "2. Read the triggering thread first — that is what this comment is actually about: `multica issue comment list %s --thread %s --output json` returns the root and every reply in the same thread as the trigger.\n", ctx.IssueID, ctx.TriggerCommentID)
|
||||
fmt.Fprintf(&b, " - If the thread alone is not enough context, pull the most recently active threads on the issue: `multica issue comment list %s --recent 20 --output json`. Each `--recent` page also prints a `Next thread cursor: --before <ts> --before-id <root-id>` line on stderr; pass the same pair back as `--before <ts> --before-id <root-id>` to scroll to older threads when 20 still isn't enough.\n", ctx.IssueID)
|
||||
b.WriteString(" - Avoid the unfiltered `multica issue comment list <issue-id> --output json` form on long-running issues — it dumps the entire flat timeline (cap 2000) and wastes context on chatter unrelated to the trigger. `--since <RFC3339-timestamp>` is still available for incremental polling against a known cursor and may combine with `--thread` or `--recent`.\n")
|
||||
fmt.Fprintf(&b, "2. Read the triggering thread first — that is what this comment is actually about. Default to the 30 most recent replies in that thread: `multica issue comment list %s --thread %s --tail 30 --output json` returns the root + the 30 newest replies (root is always included, even at `--tail 0`).\n", ctx.IssueID, ctx.TriggerCommentID)
|
||||
b.WriteString(" - If 30 replies aren't enough, walk older replies in the same thread one page at a time using the stderr `Next reply cursor: --before <ts> --before-id <reply-id>` line — pass the same pair back as `--before <ts> --before-id <reply-id>` on the next call. Under `--thread --tail` the cursor walks older *replies*, not older threads.\n")
|
||||
fmt.Fprintf(&b, " - If you also need cross-thread background, pull the most recently active threads on the issue: `multica issue comment list %s --recent 20 --output json`. Under `--recent` the same `--before` / `--before-id` flags walk older *threads* instead of older replies, and the stderr line is `Next thread cursor: --before <ts> --before-id <root-id>`. Pass the pair back to scroll to older threads when 20 still isn't enough.\n", ctx.IssueID)
|
||||
b.WriteString(" - Avoid the unfiltered `multica issue comment list <issue-id> --output json` form on long-running issues — it dumps the entire flat timeline (cap 2000) and wastes context on chatter unrelated to the trigger. `--since <RFC3339-timestamp>` is still available for incremental polling against a known cursor and may combine with `--thread --tail` or `--recent`.\n")
|
||||
fmt.Fprintf(&b, "3. Find the triggering comment (ID: `%s`) inside the thread you just read and understand what is being asked — do NOT confuse it with previous comments\n", ctx.TriggerCommentID)
|
||||
if ctx.IsSquadLeader {
|
||||
b.WriteString("4. **Decide whether a reply is warranted.** If you produced actual work this turn (investigated, fixed, answered a real question), post the result via step 6 — that is a normal reply, not a noise comment. If the triggering comment was a pure acknowledgment / thanks / sign-off from another agent AND you produced no work this turn, do NOT post a reply — and do NOT post a comment saying 'No reply needed' or similar. Simply exit with no output. Silence is a valid and preferred way to end agent-to-agent conversations.\n")
|
||||
|
||||
@@ -147,7 +147,7 @@ func buildCommentPrompt(task Task, provider string) string {
|
||||
}
|
||||
}
|
||||
fmt.Fprintf(&b, "Start by running `multica issue get %s --output json` to understand your task, then decide how to proceed.\n\n", task.IssueID)
|
||||
fmt.Fprintf(&b, "For comment history, read the triggering thread first: `multica issue comment list %s --thread %s --output json` returns the root and every reply in the same thread as the trigger comment. If you still need more context, `multica issue comment list %s --recent 20 --output json` pulls the 20 most recently active threads on the issue (each `--recent` page prints a `Next thread cursor: --before <ts> --before-id <root-id>` line on stderr — pass the same pair back to scroll older threads). Avoid the unfiltered `--output json` form on long-running issues; it dumps the full flat timeline (cap 2000) and wastes context. `--since <RFC3339>` is still available for incremental polling and may combine with `--thread` or `--recent`.\n\n", task.IssueID, task.TriggerCommentID, task.IssueID)
|
||||
fmt.Fprintf(&b, "For comment history, read the triggering thread first: `multica issue comment list %s --thread %s --tail 30 --output json` returns the root + the 30 most recent replies in that thread (root is always included, even at `--tail 0`, so you keep the \"what is this about\" context without dragging hundreds of replies into your prompt). If 30 replies aren't enough, walk older replies in the same thread one page at a time by passing the stderr `Next reply cursor: --before <ts> --before-id <reply-id>` line back as `--before <ts> --before-id <reply-id>` on the next call. If you also need cross-thread background, `multica issue comment list %s --recent 20 --output json` pulls the 20 most recently active threads on the issue; under `--recent` the same `--before` / `--before-id` flags walk older *threads* (stderr label: `Next thread cursor`) instead of older replies. Avoid the unfiltered `--output json` form on long-running issues; it dumps the full flat timeline (cap 2000) and wastes context. `--since <RFC3339>` is still available for incremental polling and may combine with `--thread --tail` or `--recent`.\n\n", task.IssueID, task.TriggerCommentID, task.IssueID)
|
||||
b.WriteString(execenv.BuildCommentReplyInstructions(provider, task.IssueID, task.TriggerCommentID))
|
||||
return b.String()
|
||||
}
|
||||
|
||||
@@ -200,12 +200,13 @@ func TestBuildPromptSquadLeaderNoActionForAgentTrigger(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestBuildPromptCommentTriggerPromotesThreadReads pins MUL-2387: the
|
||||
// per-turn prompt for a comment-triggered task must steer the agent at the
|
||||
// thread-aware reads first (--thread anchored on the trigger comment id,
|
||||
// then --recent N as a fallback with cursor guidance) instead of the legacy
|
||||
// "dump the entire comment list" recipe. Locking this in test stops the
|
||||
// guidance from decaying back to a full-flat-dump on prompt edits.
|
||||
// TestBuildPromptCommentTriggerPromotesThreadReads pins MUL-2387 + MUL-2421:
|
||||
// the per-turn prompt for a comment-triggered task must default the trigger
|
||||
// thread read to `--thread <id> --tail 30` (so long threads don't dump
|
||||
// hundreds of replies into the agent's context) and explain reply-cursor
|
||||
// pagination for older replies. --recent N stays as the cross-thread
|
||||
// fallback. Locking this in test stops the guidance from decaying back to
|
||||
// either the legacy full-flat-dump or the unbounded `--thread` recipe.
|
||||
func TestBuildPromptCommentTriggerPromotesThreadReads(t *testing.T) {
|
||||
const (
|
||||
issueID = "issue-thread-1"
|
||||
@@ -221,18 +222,23 @@ func TestBuildPromptCommentTriggerPromotesThreadReads(t *testing.T) {
|
||||
out := BuildPrompt(task, "claude")
|
||||
|
||||
mustContain := []string{
|
||||
// Thread-first read pinned by trigger comment id.
|
||||
// Thread-first read pinned by trigger comment id, capped via --tail 30.
|
||||
"--thread " + triggerID,
|
||||
"`multica issue comment list " + issueID + " --thread " + triggerID + " --output json`",
|
||||
// --recent fallback uses the documented default N=20.
|
||||
"--tail 30",
|
||||
"`multica issue comment list " + issueID + " --thread " + triggerID + " --tail 30 --output json`",
|
||||
// Reply cursor walks older replies inside the same thread.
|
||||
"Next reply cursor:",
|
||||
"--before-id <reply-id>",
|
||||
// --recent stays as the cross-thread background fallback.
|
||||
"--recent 20 --output json",
|
||||
// Cursor walks via the stderr line the CLI emits, not invented flags.
|
||||
"Next thread cursor:",
|
||||
"Next thread cursor",
|
||||
"--before",
|
||||
"--before-id",
|
||||
// --since is preserved as an additional, combinable knob.
|
||||
// --since is preserved as an additional, combinable knob (now scoped
|
||||
// to the post-MUL-2421 mode names).
|
||||
"--since",
|
||||
"may combine with `--thread` or `--recent`",
|
||||
"may combine with `--thread --tail` or `--recent`",
|
||||
// Discourage the unfiltered full dump on long-running issues.
|
||||
"Avoid the unfiltered",
|
||||
"wastes context",
|
||||
@@ -249,6 +255,12 @@ func TestBuildPromptCommentTriggerPromotesThreadReads(t *testing.T) {
|
||||
if strings.Contains(out, "returns all comments for the issue (server caps at 2000)") {
|
||||
t.Errorf("buildCommentPrompt still carries the legacy full-dump phrasing")
|
||||
}
|
||||
// The pre-MUL-2421 unbounded `--thread` recipe (no --tail) is also a
|
||||
// regression target: it dumps the entire thread on long threads, which
|
||||
// is exactly what --tail 30 is meant to bound.
|
||||
if strings.Contains(out, "--thread "+triggerID+" --output json") {
|
||||
t.Errorf("buildCommentPrompt regressed to unbounded --thread recipe (no --tail) — long threads will overflow context\n--- output ---\n%s", out)
|
||||
}
|
||||
}
|
||||
|
||||
// TestBuildPromptDefaultMentionsRecent pins that the catch-all fallback
|
||||
|
||||
@@ -69,7 +69,7 @@ const commentHardCap = 2000
|
||||
|
||||
// ListComments returns comments for an issue. The default behaviour is
|
||||
// unchanged — full chronological dump capped at commentHardCap — so existing
|
||||
// callers and the desktop UI keep working as-is. Three optional query params
|
||||
// callers and the desktop UI keep working as-is. Four optional query params
|
||||
// give agent-style readers a thread-aware view that scales to long issues
|
||||
// without dragging every prior comment into context:
|
||||
//
|
||||
@@ -77,28 +77,45 @@ const commentHardCap = 2000
|
||||
// comment plus every descendant. The anchor may be a root or any reply;
|
||||
// the server walks up to the root via a recursive CTE, so callers do not
|
||||
// need to know whether the id they have is a root.
|
||||
// - tail=<N> — only valid with thread. Cap the reply count at the N most
|
||||
// recent replies (per (created_at, id)). The thread root is always
|
||||
// returned, even when N=0, so the reader keeps the "what is this thread
|
||||
// about" context. Without tail, thread returns the entire thread (the
|
||||
// pre-MUL-2421 behavior).
|
||||
// - recent=<N> — return the N most recently active threads (root + every
|
||||
// descendant per thread). A thread's recency is MAX(created_at) across
|
||||
// the whole subtree, so a stale-but-recently-replied thread ranks ahead
|
||||
// of an active-but-quiet one. Row-based "newest N comments" is
|
||||
// deliberately NOT exposed — it surfaces unrelated thread tails and
|
||||
// hides relevant history (#2340).
|
||||
// - before=<RFC3339> + before-id=<uuid> — thread cursor. The pair points at
|
||||
// a thread's (last_activity_at, root_id); the next page returns threads
|
||||
// strictly less recent. Both must be set together — a timestamp-only
|
||||
// cursor cannot tie-break two threads whose last_activity falls in the
|
||||
// same microsecond. The cursor for the next page is emitted via the
|
||||
// X-Multica-Next-Before / X-Multica-Next-Before-Id response headers.
|
||||
// - before=<RFC3339> + before-id=<uuid> — cursor. The pair's meaning is
|
||||
// context-dependent so the flag surface stays small:
|
||||
//
|
||||
// - with recent: a *thread* cursor — (last_activity_at, root_id) — and
|
||||
// the next page returns threads strictly less recent.
|
||||
// - with thread + tail: a *reply* cursor — (created_at, id) — and the
|
||||
// next page returns replies in the same thread strictly older than
|
||||
// that reply.
|
||||
//
|
||||
// Both values must be set together so the cursor can tie-break entries
|
||||
// landing in the same microsecond. The cursor for the next page is
|
||||
// emitted via the X-Multica-Next-Before / X-Multica-Next-Before-Id
|
||||
// response headers.
|
||||
//
|
||||
// Combination rules (kept narrow on purpose — Elon flagged the matrix risk):
|
||||
//
|
||||
// - thread is exclusive with recent and before/before-id. Asking for "the
|
||||
// most recent N within thread X" or "thread X but only before T" mixes
|
||||
// two different navigation models and is rejected with 400.
|
||||
// - thread may combine with since (incremental polling of one thread).
|
||||
// - thread is exclusive with recent. Asking for "the most recent N within
|
||||
// thread X" mixes two different navigation models and is rejected.
|
||||
// - thread + before/before-id requires tail. Without tail, thread returns
|
||||
// the entire thread and a cursor would be ignored — reject loudly so
|
||||
// the documented "cursor scrolls within a tailed window" rule holds.
|
||||
// - tail requires thread (it is a thread-scoped limit; outside of thread
|
||||
// it has no defined behavior).
|
||||
// - thread may combine with since (incremental polling of one thread),
|
||||
// and the since filter is applied after the tail/cursor cut so the
|
||||
// thread root is still emitted but stale rows drop out.
|
||||
// - recent may combine with before/before-id (scroll older threads) and
|
||||
// with since (recent activity in a window).
|
||||
// - since may combine with anything except (thread + recent / before).
|
||||
//
|
||||
// The response body is always chronological (oldest → newest); under recent
|
||||
// that means threads are listed oldest-active first and the freshest thread
|
||||
@@ -128,6 +145,7 @@ func (h *Handler) ListComments(w http.ResponseWriter, r *http.Request) {
|
||||
|
||||
threadStr := q.Get("thread")
|
||||
recentStr := q.Get("recent")
|
||||
tailStr := q.Get("tail")
|
||||
beforeTimeStr := q.Get("before")
|
||||
beforeIDStr := q.Get("before_id")
|
||||
if beforeIDStr == "" {
|
||||
@@ -140,21 +158,21 @@ func (h *Handler) ListComments(w http.ResponseWriter, r *http.Request) {
|
||||
writeError(w, http.StatusBadRequest, "thread and recent are mutually exclusive")
|
||||
return
|
||||
}
|
||||
if threadStr != "" && (beforeTimeStr != "" || beforeIDStr != "") {
|
||||
writeError(w, http.StatusBadRequest, "thread cannot be combined with before / before_id")
|
||||
if tailStr != "" && threadStr == "" {
|
||||
writeError(w, http.StatusBadRequest, "tail requires thread (it is a thread-scoped limit)")
|
||||
return
|
||||
}
|
||||
if (beforeTimeStr == "") != (beforeIDStr == "") {
|
||||
writeError(w, http.StatusBadRequest, "before and before_id must be set together (composite cursor)")
|
||||
return
|
||||
}
|
||||
// Cursor only makes sense as "scroll older within a recent window". A
|
||||
// cursor without `recent` would otherwise be silently dropped by
|
||||
// fetchCommentsForList and fall back to the default / since path —
|
||||
// returning a full timeline that the caller did not ask for. Reject
|
||||
// loudly so the API surface matches the documented semantics.
|
||||
if beforeTimeStr != "" && recentStr == "" {
|
||||
writeError(w, http.StatusBadRequest, "before / before_id require recent (cursor scrolls within a recent window)")
|
||||
// Cursor needs either a recent window (thread cursor) or a tailed thread
|
||||
// (reply cursor). A bare cursor would otherwise fall through to the
|
||||
// default / since path — returning a full timeline that the caller did
|
||||
// not ask for. Reject loudly so the API surface matches the documented
|
||||
// semantics.
|
||||
if beforeTimeStr != "" && recentStr == "" && (threadStr == "" || tailStr == "") {
|
||||
writeError(w, http.StatusBadRequest, "before / before_id require recent (thread cursor) or thread + tail (reply cursor)")
|
||||
return
|
||||
}
|
||||
|
||||
@@ -194,14 +212,35 @@ func (h *Handler) ListComments(w http.ResponseWriter, r *http.Request) {
|
||||
recentN = n
|
||||
}
|
||||
|
||||
// tail=0 is allowed (returns root only — useful for "what is this thread
|
||||
// about" lookups without dragging any replies into context). Negative
|
||||
// values are rejected because they'd round-trip to LIMIT -N which
|
||||
// PostgreSQL flags as a syntax error.
|
||||
threadTail := -1
|
||||
threadTailSet := false
|
||||
if tailStr != "" {
|
||||
n, err := strconv.Atoi(tailStr)
|
||||
if err != nil || n < 0 {
|
||||
writeError(w, http.StatusBadRequest, "invalid tail parameter; expected non-negative integer")
|
||||
return
|
||||
}
|
||||
if n > commentHardCap {
|
||||
n = commentHardCap
|
||||
}
|
||||
threadTail = n
|
||||
threadTailSet = true
|
||||
}
|
||||
|
||||
result, err := h.fetchCommentsForList(r.Context(), fetchCommentsArgs{
|
||||
Issue: issue,
|
||||
Since: sinceTime,
|
||||
ThreadAnchor: threadStr,
|
||||
RecentN: recentN,
|
||||
HasCursor: hasCursor,
|
||||
BeforeAt: beforeCursor,
|
||||
BeforeID: beforeUUID,
|
||||
Issue: issue,
|
||||
Since: sinceTime,
|
||||
ThreadAnchor: threadStr,
|
||||
ThreadTail: threadTail,
|
||||
ThreadTailSet: threadTailSet,
|
||||
RecentN: recentN,
|
||||
HasCursor: hasCursor,
|
||||
BeforeAt: beforeCursor,
|
||||
BeforeID: beforeUUID,
|
||||
})
|
||||
if err != nil {
|
||||
switch err {
|
||||
@@ -230,10 +269,13 @@ func (h *Handler) ListComments(w http.ResponseWriter, r *http.Request) {
|
||||
resp[i] = commentToResponse(c, grouped[cid], groupedAtt[cid])
|
||||
}
|
||||
|
||||
// Emit the next thread cursor as response headers when the recent page is
|
||||
// likely not the last one (returned a full page of threads). Headers stay
|
||||
// out of the JSON body so the default flat-array response shape — which
|
||||
// the desktop UI and existing callers depend on — is unchanged.
|
||||
// Emit the next cursor as response headers when the page is likely not
|
||||
// the last one. The cursor's meaning is context-dependent: under recent
|
||||
// it points at the oldest thread in the page (next page = older threads);
|
||||
// under thread + tail it points at the oldest reply in the page (next
|
||||
// page = older replies in the same thread). Headers stay out of the JSON
|
||||
// body so the default flat-array response shape — which the desktop UI
|
||||
// and existing callers depend on — is unchanged.
|
||||
if result.NextBefore != "" && result.NextBeforeID != "" {
|
||||
w.Header().Set("X-Multica-Next-Before", result.NextBefore)
|
||||
w.Header().Set("X-Multica-Next-Before-Id", result.NextBeforeID)
|
||||
@@ -245,14 +287,21 @@ func (h *Handler) ListComments(w http.ResponseWriter, r *http.Request) {
|
||||
// fetchCommentsArgs bundles the parsed query params so fetchCommentsForList
|
||||
// stays readable. Sentinel errors below let the caller turn DB-layer outcomes
|
||||
// into the right HTTP status without leaking SQL details.
|
||||
//
|
||||
// ThreadTail is split into a value + a "set" flag because tail=0 is a
|
||||
// meaningful caller intent (return just the root). A bare int would collapse
|
||||
// "user did not pass --tail" and "user passed --tail 0" into the same state,
|
||||
// which would silently downgrade the latter to the full-thread path.
|
||||
type fetchCommentsArgs struct {
|
||||
Issue db.Issue
|
||||
Since pgtype.Timestamptz
|
||||
ThreadAnchor string
|
||||
RecentN int
|
||||
HasCursor bool
|
||||
BeforeAt pgtype.Timestamptz
|
||||
BeforeID pgtype.UUID
|
||||
Issue db.Issue
|
||||
Since pgtype.Timestamptz
|
||||
ThreadAnchor string
|
||||
ThreadTail int
|
||||
ThreadTailSet bool
|
||||
RecentN int
|
||||
HasCursor bool
|
||||
BeforeAt pgtype.Timestamptz
|
||||
BeforeID pgtype.UUID
|
||||
}
|
||||
|
||||
// fetchCommentsResult carries both the materialised comments and (for the
|
||||
@@ -284,6 +333,115 @@ func (h *Handler) fetchCommentsForList(ctx context.Context, args fetchCommentsAr
|
||||
if err != nil {
|
||||
return fetchCommentsResult{}, errCommentThreadBadID
|
||||
}
|
||||
// Tailed path: paged query that returns root + the @reply_limit
|
||||
// most recent replies (per (created_at, id)). The thread root is
|
||||
// always returned, so a reader can land on a long thread without
|
||||
// dragging hundreds of replies into context. The reply-internal
|
||||
// cursor (--before / --before-id under --thread + --tail) scrolls
|
||||
// to older replies inside the same thread.
|
||||
if args.ThreadTailSet {
|
||||
// Probe for has-more by asking the SQL for one extra reply
|
||||
// beyond what the caller wants. If we get back >tail replies
|
||||
// there is at least one older reply still on disk; if we get
|
||||
// back ≤tail the page is the tail of the thread and there is
|
||||
// nothing older to scroll to (so we must NOT emit a cursor —
|
||||
// otherwise the next page is wasted round-trip that returns
|
||||
// just the root). This is the exact-boundary fix called out
|
||||
// in the MUL-2421 review.
|
||||
rows, err := h.Queries.ListThreadCommentsForIssuePaged(ctx, db.ListThreadCommentsForIssuePagedParams{
|
||||
AnchorID: anchor,
|
||||
IssueID: issue.ID,
|
||||
WorkspaceID: issue.WorkspaceID,
|
||||
HasCursor: args.HasCursor,
|
||||
BeforeAt: args.BeforeAt,
|
||||
BeforeID: args.BeforeID,
|
||||
ReplyLimit: int32(args.ThreadTail) + 1,
|
||||
})
|
||||
if err != nil {
|
||||
return fetchCommentsResult{}, err
|
||||
}
|
||||
if len(rows) == 0 {
|
||||
return fetchCommentsResult{}, errCommentThreadNotFound
|
||||
}
|
||||
// Split the result into root + replies (ASC order preserved).
|
||||
// Root is identified by parent_id IS NULL and is always
|
||||
// present in the SQL output; we keep it out of the cursor /
|
||||
// tail-trim logic so the user always sees thread context.
|
||||
var rootComment *db.Comment
|
||||
replies := make([]db.Comment, 0, len(rows))
|
||||
for _, r := range rows {
|
||||
c := db.Comment{
|
||||
ID: r.ID,
|
||||
IssueID: r.IssueID,
|
||||
AuthorType: r.AuthorType,
|
||||
AuthorID: r.AuthorID,
|
||||
Content: r.Content,
|
||||
Type: r.Type,
|
||||
CreatedAt: r.CreatedAt,
|
||||
UpdatedAt: r.UpdatedAt,
|
||||
ParentID: r.ParentID,
|
||||
WorkspaceID: r.WorkspaceID,
|
||||
ResolvedAt: r.ResolvedAt,
|
||||
ResolvedByType: r.ResolvedByType,
|
||||
ResolvedByID: r.ResolvedByID,
|
||||
}
|
||||
if !r.ParentID.Valid {
|
||||
root := c
|
||||
rootComment = &root
|
||||
continue
|
||||
}
|
||||
replies = append(replies, c)
|
||||
}
|
||||
// Trim the probe overflow back to the caller's tail. The SQL
|
||||
// emits ASC, so the extra row is the oldest reply — dropping
|
||||
// it from the head is what aligns "newest N" with the user's
|
||||
// request.
|
||||
hasMore := len(replies) > args.ThreadTail
|
||||
if hasMore {
|
||||
replies = replies[1:]
|
||||
}
|
||||
out := make([]db.Comment, 0, len(replies)+1)
|
||||
if rootComment != nil {
|
||||
out = append(out, *rootComment)
|
||||
}
|
||||
for _, r := range replies {
|
||||
// since drops stale rows AFTER the tail / cursor cut.
|
||||
// The root is exempt (already appended above): a reader
|
||||
// who set --since to skip already-seen replies still
|
||||
// needs the root context if the page only contained
|
||||
// the root.
|
||||
if args.Since.Valid && !r.CreatedAt.Time.After(args.Since.Time) {
|
||||
continue
|
||||
}
|
||||
out = append(out, r)
|
||||
}
|
||||
// Emit a reply cursor only when we proved an older reply
|
||||
// exists (hasMore). On an exact-boundary page (replyCount
|
||||
// == tail with no overflow) hasMore is false and the cursor
|
||||
// stays empty.
|
||||
//
|
||||
// Additionally suppress the cursor when `since` is set and
|
||||
// the oldest retained reply on this page is already <= since.
|
||||
// The next page walks replies strictly older than that one,
|
||||
// so every older reply has created_at strictly less — if the
|
||||
// cursor target itself can't satisfy `> since`, no older
|
||||
// reply can either, and continuing to paginate would only
|
||||
// return root-only pages until the agent walks the entire
|
||||
// pre-`since` history. This mirrors the head-thread guard on
|
||||
// the recent + since path. Flagged by Elon's second review on
|
||||
// MUL-2421.
|
||||
res := fetchCommentsResult{Comments: out}
|
||||
emitCursor := hasMore && len(replies) > 0
|
||||
if emitCursor && args.Since.Valid && !replies[0].CreatedAt.Time.After(args.Since.Time) {
|
||||
emitCursor = false
|
||||
}
|
||||
if emitCursor {
|
||||
oldest := replies[0]
|
||||
res.NextBefore = oldest.CreatedAt.Time.UTC().Format(time.RFC3339Nano)
|
||||
res.NextBeforeID = uuidToString(oldest.ID)
|
||||
}
|
||||
return res, nil
|
||||
}
|
||||
rows, err := h.Queries.ListThreadCommentsForIssue(ctx, db.ListThreadCommentsForIssueParams{
|
||||
AnchorID: anchor,
|
||||
IssueID: issue.ID,
|
||||
|
||||
@@ -666,3 +666,350 @@ func TestListComments_ThreadWithSinceFiltersWithinThread(t *testing.T) {
|
||||
_, 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
|
||||
}
|
||||
|
||||
@@ -533,6 +533,147 @@ func (q *Queries) ListThreadCommentsForIssue(ctx context.Context, arg ListThread
|
||||
return items, nil
|
||||
}
|
||||
|
||||
const listThreadCommentsForIssuePaged = `-- name: ListThreadCommentsForIssuePaged :many
|
||||
WITH RECURSIVE root_of AS (
|
||||
SELECT c.id, c.parent_id
|
||||
FROM comment c
|
||||
WHERE c.id = $1 AND c.issue_id = $2 AND c.workspace_id = $3
|
||||
UNION ALL
|
||||
SELECT p.id, p.parent_id
|
||||
FROM comment p
|
||||
JOIN root_of r ON p.id = r.parent_id
|
||||
),
|
||||
thread_root AS (
|
||||
SELECT id FROM root_of WHERE parent_id IS NULL LIMIT 1
|
||||
),
|
||||
descendants AS (
|
||||
SELECT c.id, c.issue_id, c.author_type, c.author_id, c.content, c.type,
|
||||
c.created_at, c.updated_at, c.parent_id, c.workspace_id,
|
||||
c.resolved_at, c.resolved_by_type, c.resolved_by_id
|
||||
FROM comment c
|
||||
JOIN thread_root tr ON c.id = tr.id
|
||||
UNION
|
||||
SELECT c.id, c.issue_id, c.author_type, c.author_id, c.content, c.type,
|
||||
c.created_at, c.updated_at, c.parent_id, c.workspace_id,
|
||||
c.resolved_at, c.resolved_by_type, c.resolved_by_id
|
||||
FROM comment c
|
||||
JOIN descendants d ON c.parent_id = d.id
|
||||
WHERE c.issue_id = $2 AND c.workspace_id = $3
|
||||
),
|
||||
reply_page AS (
|
||||
SELECT d.id, d.issue_id, d.author_type, d.author_id, d.content, d.type,
|
||||
d.created_at, d.updated_at, d.parent_id, d.workspace_id,
|
||||
d.resolved_at, d.resolved_by_type, d.resolved_by_id
|
||||
FROM descendants d
|
||||
WHERE d.id NOT IN (SELECT id FROM thread_root)
|
||||
AND (
|
||||
$4::boolean = FALSE
|
||||
OR (d.created_at, d.id) < ($5::timestamptz, $6::uuid)
|
||||
)
|
||||
ORDER BY d.created_at DESC, d.id DESC
|
||||
LIMIT $7
|
||||
)
|
||||
SELECT id, issue_id, author_type, author_id, content, type,
|
||||
created_at, updated_at, parent_id, workspace_id,
|
||||
resolved_at, resolved_by_type, resolved_by_id
|
||||
FROM (
|
||||
SELECT d.id, d.issue_id, d.author_type, d.author_id, d.content, d.type,
|
||||
d.created_at, d.updated_at, d.parent_id, d.workspace_id,
|
||||
d.resolved_at, d.resolved_by_type, d.resolved_by_id
|
||||
FROM descendants d
|
||||
JOIN thread_root tr ON d.id = tr.id
|
||||
UNION ALL
|
||||
SELECT id, issue_id, author_type, author_id, content, type,
|
||||
created_at, updated_at, parent_id, workspace_id,
|
||||
resolved_at, resolved_by_type, resolved_by_id
|
||||
FROM reply_page
|
||||
) combined
|
||||
ORDER BY created_at ASC, id ASC
|
||||
`
|
||||
|
||||
type ListThreadCommentsForIssuePagedParams struct {
|
||||
AnchorID pgtype.UUID `json:"anchor_id"`
|
||||
IssueID pgtype.UUID `json:"issue_id"`
|
||||
WorkspaceID pgtype.UUID `json:"workspace_id"`
|
||||
HasCursor bool `json:"has_cursor"`
|
||||
BeforeAt pgtype.Timestamptz `json:"before_at"`
|
||||
BeforeID pgtype.UUID `json:"before_id"`
|
||||
ReplyLimit int32 `json:"reply_limit"`
|
||||
}
|
||||
|
||||
type ListThreadCommentsForIssuePagedRow struct {
|
||||
ID pgtype.UUID `json:"id"`
|
||||
IssueID pgtype.UUID `json:"issue_id"`
|
||||
AuthorType string `json:"author_type"`
|
||||
AuthorID pgtype.UUID `json:"author_id"`
|
||||
Content string `json:"content"`
|
||||
Type string `json:"type"`
|
||||
CreatedAt pgtype.Timestamptz `json:"created_at"`
|
||||
UpdatedAt pgtype.Timestamptz `json:"updated_at"`
|
||||
ParentID pgtype.UUID `json:"parent_id"`
|
||||
WorkspaceID pgtype.UUID `json:"workspace_id"`
|
||||
ResolvedAt pgtype.Timestamptz `json:"resolved_at"`
|
||||
ResolvedByType pgtype.Text `json:"resolved_by_type"`
|
||||
ResolvedByID pgtype.UUID `json:"resolved_by_id"`
|
||||
}
|
||||
|
||||
// Same root-walk + descendants expansion as ListThreadCommentsForIssue, but
|
||||
// returns root + only the @reply_limit most recent replies (per the
|
||||
// (created_at, id) composite key). When @has_cursor=TRUE only replies with
|
||||
// (created_at, id) < (@before_at, @before_id) are eligible — that is the
|
||||
// cursor for scrolling *within* a thread.
|
||||
//
|
||||
// Root is unconditional: it is included regardless of @reply_limit (even 0)
|
||||
// and regardless of the cursor. A reader landing on a long thread needs the
|
||||
// root for the "what is this thread about" context, even if every reply has
|
||||
// been paginated past.
|
||||
//
|
||||
// Reply selection happens DESC (newest replies first) so the cursor walks
|
||||
// toward older replies; the outer SELECT then re-sorts the combined output
|
||||
// ASC so the body stays chronological (oldest → newest), matching every
|
||||
// other comment list path.
|
||||
func (q *Queries) ListThreadCommentsForIssuePaged(ctx context.Context, arg ListThreadCommentsForIssuePagedParams) ([]ListThreadCommentsForIssuePagedRow, error) {
|
||||
rows, err := q.db.Query(ctx, listThreadCommentsForIssuePaged,
|
||||
arg.AnchorID,
|
||||
arg.IssueID,
|
||||
arg.WorkspaceID,
|
||||
arg.HasCursor,
|
||||
arg.BeforeAt,
|
||||
arg.BeforeID,
|
||||
arg.ReplyLimit,
|
||||
)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
defer rows.Close()
|
||||
items := []ListThreadCommentsForIssuePagedRow{}
|
||||
for rows.Next() {
|
||||
var i ListThreadCommentsForIssuePagedRow
|
||||
if err := rows.Scan(
|
||||
&i.ID,
|
||||
&i.IssueID,
|
||||
&i.AuthorType,
|
||||
&i.AuthorID,
|
||||
&i.Content,
|
||||
&i.Type,
|
||||
&i.CreatedAt,
|
||||
&i.UpdatedAt,
|
||||
&i.ParentID,
|
||||
&i.WorkspaceID,
|
||||
&i.ResolvedAt,
|
||||
&i.ResolvedByType,
|
||||
&i.ResolvedByID,
|
||||
); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
items = append(items, i)
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return items, nil
|
||||
}
|
||||
|
||||
const resolveComment = `-- name: ResolveComment :one
|
||||
UPDATE comment SET
|
||||
resolved_at = COALESCE(resolved_at, now()),
|
||||
|
||||
@@ -58,6 +58,78 @@ FROM descendants
|
||||
ORDER BY created_at ASC, id ASC
|
||||
LIMIT @row_limit;
|
||||
|
||||
-- name: ListThreadCommentsForIssuePaged :many
|
||||
-- Same root-walk + descendants expansion as ListThreadCommentsForIssue, but
|
||||
-- returns root + only the @reply_limit most recent replies (per the
|
||||
-- (created_at, id) composite key). When @has_cursor=TRUE only replies with
|
||||
-- (created_at, id) < (@before_at, @before_id) are eligible — that is the
|
||||
-- cursor for scrolling *within* a thread.
|
||||
--
|
||||
-- Root is unconditional: it is included regardless of @reply_limit (even 0)
|
||||
-- and regardless of the cursor. A reader landing on a long thread needs the
|
||||
-- root for the "what is this thread about" context, even if every reply has
|
||||
-- been paginated past.
|
||||
--
|
||||
-- Reply selection happens DESC (newest replies first) so the cursor walks
|
||||
-- toward older replies; the outer SELECT then re-sorts the combined output
|
||||
-- ASC so the body stays chronological (oldest → newest), matching every
|
||||
-- other comment list path.
|
||||
WITH RECURSIVE root_of AS (
|
||||
SELECT c.id, c.parent_id
|
||||
FROM comment c
|
||||
WHERE c.id = @anchor_id AND c.issue_id = @issue_id AND c.workspace_id = @workspace_id
|
||||
UNION ALL
|
||||
SELECT p.id, p.parent_id
|
||||
FROM comment p
|
||||
JOIN root_of r ON p.id = r.parent_id
|
||||
),
|
||||
thread_root AS (
|
||||
SELECT id FROM root_of WHERE parent_id IS NULL LIMIT 1
|
||||
),
|
||||
descendants AS (
|
||||
SELECT c.id, c.issue_id, c.author_type, c.author_id, c.content, c.type,
|
||||
c.created_at, c.updated_at, c.parent_id, c.workspace_id,
|
||||
c.resolved_at, c.resolved_by_type, c.resolved_by_id
|
||||
FROM comment c
|
||||
JOIN thread_root tr ON c.id = tr.id
|
||||
UNION
|
||||
SELECT c.id, c.issue_id, c.author_type, c.author_id, c.content, c.type,
|
||||
c.created_at, c.updated_at, c.parent_id, c.workspace_id,
|
||||
c.resolved_at, c.resolved_by_type, c.resolved_by_id
|
||||
FROM comment c
|
||||
JOIN descendants d ON c.parent_id = d.id
|
||||
WHERE c.issue_id = @issue_id AND c.workspace_id = @workspace_id
|
||||
),
|
||||
reply_page AS (
|
||||
SELECT d.id, d.issue_id, d.author_type, d.author_id, d.content, d.type,
|
||||
d.created_at, d.updated_at, d.parent_id, d.workspace_id,
|
||||
d.resolved_at, d.resolved_by_type, d.resolved_by_id
|
||||
FROM descendants d
|
||||
WHERE d.id NOT IN (SELECT id FROM thread_root)
|
||||
AND (
|
||||
@has_cursor::boolean = FALSE
|
||||
OR (d.created_at, d.id) < (@before_at::timestamptz, @before_id::uuid)
|
||||
)
|
||||
ORDER BY d.created_at DESC, d.id DESC
|
||||
LIMIT @reply_limit
|
||||
)
|
||||
SELECT id, issue_id, author_type, author_id, content, type,
|
||||
created_at, updated_at, parent_id, workspace_id,
|
||||
resolved_at, resolved_by_type, resolved_by_id
|
||||
FROM (
|
||||
SELECT d.id, d.issue_id, d.author_type, d.author_id, d.content, d.type,
|
||||
d.created_at, d.updated_at, d.parent_id, d.workspace_id,
|
||||
d.resolved_at, d.resolved_by_type, d.resolved_by_id
|
||||
FROM descendants d
|
||||
JOIN thread_root tr ON d.id = tr.id
|
||||
UNION ALL
|
||||
SELECT id, issue_id, author_type, author_id, content, type,
|
||||
created_at, updated_at, parent_id, workspace_id,
|
||||
resolved_at, resolved_by_type, resolved_by_id
|
||||
FROM reply_page
|
||||
) combined
|
||||
ORDER BY created_at ASC, id ASC;
|
||||
|
||||
-- name: ListRecentThreadCommentsForIssue :many
|
||||
-- Returns the N most recently active threads (root + every descendant) rather
|
||||
-- than the N most recent rows. A thread's "last activity" is MAX(created_at)
|
||||
|
||||
Reference in New Issue
Block a user