diff --git a/packages/core/issues/mutations.ts b/packages/core/issues/mutations.ts index 17f9b5019..e9098ae0f 100644 --- a/packages/core/issues/mutations.ts +++ b/packages/core/issues/mutations.ts @@ -327,9 +327,14 @@ export function useCreateComment(issueId: string) { created_at: comment.created_at, updated_at: comment.updated_at, }; - qc.setQueryData(issueKeys.timeline(issueId), (old) => - old ? [...old, entry] : [entry], - ); + // Dedupe by id: the `comment:created` WS event may have already added + // this entry from the broadcast path before this onSuccess fires. Skip + // the append if the entry is already in the cache. + qc.setQueryData(issueKeys.timeline(issueId), (old) => { + if (!old) return [entry]; + if (old.some((e) => e.id === entry.id)) return old; + return [...old, entry]; + }); }, onSettled: () => { qc.invalidateQueries({ queryKey: issueKeys.timeline(issueId) }); diff --git a/server/internal/daemon/execenv/runtime_config.go b/server/internal/daemon/execenv/runtime_config.go index 49c3cb440..eeebaff8b 100644 --- a/server/internal/daemon/execenv/runtime_config.go +++ b/server/internal/daemon/execenv/runtime_config.go @@ -106,7 +106,7 @@ func buildMetaSkillContent(provider string, ctx TaskContextForEnv) string { b.WriteString("### Read\n") b.WriteString("- `multica issue get --output json` — Get full issue details (title, description, status, priority, assignee)\n") b.WriteString("- `multica issue list [--status X] [--priority X] [--assignee X | --assignee-id ] [--limit N] [--offset N] [--full-id] [--output json]` — List issues in workspace (default limit: 50; table output uses routable issue keys; JSON output includes `total`, `has_more` — use offset to paginate when `has_more` is true). Prefer `--assignee-id ` when scripting from `multica workspace members --output json` / `multica agent list --output json`.\n") - b.WriteString("- `multica issue comment list [--limit N] [--offset N] [--since ] --output json` — List comments on an issue (supports pagination; includes id, parent_id for threading)\n") + b.WriteString("- `multica issue comment list [--since ] --output json` — List all comments on an issue (server caps at 2000 rows). Use `--since` for incremental polling.\n") b.WriteString("- `multica issue label list --output json` — List labels currently attached to an issue\n") b.WriteString("- `multica issue subscriber list --output json` — List members/agents subscribed to an issue\n") b.WriteString("- `multica label list --output json` — List all labels defined in the workspace (returns id + name + color)\n") @@ -246,8 +246,8 @@ 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. Run `multica issue comment list %s --output json` to read the conversation\n", ctx.IssueID) - b.WriteString(" - If the output is very large or truncated, use pagination: `--limit 30` to get the latest 30 comments, or `--since ` to fetch only recent ones\n") + fmt.Fprintf(&b, "2. Run `multica issue comment list %s --output json` to read the conversation (returns all comments, capped server-side at 2000)\n", ctx.IssueID) + b.WriteString(" - For incremental polling, use `--since ` to fetch only comments newer than a known cursor\n") fmt.Fprintf(&b, "3. Find the triggering comment (ID: `%s`) and understand what is being asked — do NOT confuse it with previous comments\n", ctx.TriggerCommentID) 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") b.WriteString("5. If a reply IS warranted: do any requested work first, then **decide whether to include any `@mention` link.** The default is NO mention. Only mention when you are escalating to a human owner who is not yet involved, delegating a concrete new sub-task to another agent for the first time, or the user explicitly asked you to loop someone in. Never @mention the agent you are replying to as a thank-you or sign-off.\n") @@ -258,8 +258,7 @@ func buildMetaSkillContent(provider string, ctx TaskContextForEnv) string { // Assignment-triggered: defer to agent Skills for workflow specifics. b.WriteString("You are responsible for managing the issue status throughout your work.\n\n") fmt.Fprintf(&b, "1. Run `multica issue get %s --output json` to understand your task\n", ctx.IssueID) - fmt.Fprintf(&b, "2. Run `multica issue comment list %s --output json` to read the full comment history — this is mandatory, not optional. Earlier comments often carry context the issue body lacks (e.g. which repo to work in, the prior agent's findings, the reason the issue was reassigned to you). Skipping this step is the most common cause of agents acting on stale or incomplete instructions.\n", ctx.IssueID) - fmt.Fprintf(&b, " - If the output is very large or truncated, use pagination: `--limit 30` to get the latest 30 comments, or `--since ` to fetch only recent ones\n") + fmt.Fprintf(&b, "2. Run `multica issue comment list %s --output json` to read the full comment history (returns all comments, capped server-side at 2000) — this is mandatory, not optional. Earlier comments often carry context the issue body lacks (e.g. which repo to work in, the prior agent's findings, the reason the issue was reassigned to you). Skipping this step is the most common cause of agents acting on stale or incomplete instructions.\n", ctx.IssueID) fmt.Fprintf(&b, "3. Run `multica issue status %s in_progress`\n", ctx.IssueID) b.WriteString("4. Follow your Skills and Agent Identity to complete the task (write code, investigate, etc.)\n") fmt.Fprintf(&b, "5. **Post your final results as a comment — this step is mandatory**: `multica issue comment add %s --content \"...\"`. Your results are only visible to the user if posted via this CLI call; text in your terminal or run logs is NOT delivered.\n", ctx.IssueID) diff --git a/server/internal/daemon/prompt.go b/server/internal/daemon/prompt.go index a199b17a3..92d1e7d93 100644 --- a/server/internal/daemon/prompt.go +++ b/server/internal/daemon/prompt.go @@ -27,7 +27,7 @@ func BuildPrompt(task Task) string { b.WriteString("You are running as a local coding agent for a Multica workspace.\n\n") fmt.Fprintf(&b, "Your assigned issue ID is: %s\n\n", task.IssueID) fmt.Fprintf(&b, "Start by running `multica issue get %s --output json` to understand your task, then complete it.\n", task.IssueID) - fmt.Fprintf(&b, "If you need comment history, `multica issue comment list %s` returns the latest 50 by default — pass --limit or --since to scope older windows. Long issues can have thousands of comments; do not fetch everything blindly.\n", task.IssueID) + fmt.Fprintf(&b, "If you need comment history, `multica issue comment list %s --output json` returns all comments for the issue (server caps at 2000). Pass `--since ` to fetch only comments newer than a known cursor.\n", task.IssueID) return b.String() } @@ -116,7 +116,7 @@ func buildCommentPrompt(task Task) 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, "If you need comment history, `multica issue comment list %s` returns the latest 50 by default — pass --limit or --since to scope older windows. Long issues can have thousands of comments; do not fetch everything blindly.\n\n", task.IssueID) + fmt.Fprintf(&b, "If you need comment history, `multica issue comment list %s --output json` returns all comments for the issue (server caps at 2000). Pass `--since ` to fetch only comments newer than a known cursor.\n\n", task.IssueID) b.WriteString(execenv.BuildCommentReplyInstructions(task.IssueID, task.TriggerCommentID)) return b.String() } diff --git a/server/internal/handler/activity.go b/server/internal/handler/activity.go index a3f4b355d..3800002d5 100644 --- a/server/internal/handler/activity.go +++ b/server/internal/handler/activity.go @@ -41,12 +41,37 @@ type TimelineEntry struct { // data-shape rationale (#1929). const timelineHardCap = 2000 -// ListTimeline returns the full issue timeline (comments + activities merged) -// in chronological order (oldest first). Comments and activities each carry -// their own hard cap to bound the response — paged delivery and cursors were -// removed in #1929 because time-based pagination splits reply threads at page -// boundaries, and the actual data shape (p99 ~30 comments per issue) made the -// cursor machinery pure overhead. +// timelinePaginatedResponse mirrors the wrapper shape produced by the prior +// cursor-paginated ListTimeline (#2128). It is preserved as a backward-compat +// surface for installed Desktop builds and stale Web bundles between #2128 and +// #1929 that send `?limit=`/`?before=`/`?after=`/`?around=` and parse the +// response with the old TimelinePageSchema (entries + cursors). Cursors are +// always nil and `has_more_*` are always false: the new server returns the +// whole timeline in one shot. +type timelinePaginatedResponse struct { + Entries []TimelineEntry `json:"entries"` + NextCursor *string `json:"next_cursor"` + PrevCursor *string `json:"prev_cursor"` + HasMoreBefore bool `json:"has_more_before"` + HasMoreAfter bool `json:"has_more_after"` + TargetIndex *int `json:"target_index,omitempty"` +} + +// ListTimeline returns the full issue timeline (comments + activities merged). +// Two response shapes coexist for boundary compatibility (#1929): +// +// - No pagination params → flat ASC `TimelineEntry[]`. Matches the legacy +// desktop contract (Multica.app ≤ v0.2.25) and the new client. +// - Any of `limit` / `before` / `after` / `around` present → wrapped object +// with DESC entries + null cursors + has_more_*=false. Matches what a +// stale v0.2.26+ build expects when it parses the response with +// TimelinePageSchema; cursor-walking is now a no-op so the client just +// sees a single full page. +// +// Both shapes carry the same set of entries — paging and ordering differ. +// Time-based pagination was removed because it split reply threads at page +// boundaries, and at observed data sizes (p99 ~30 comments per issue) the +// cursor machinery was pure overhead. func (h *Handler) ListTimeline(w http.ResponseWriter, r *http.Request) { id := chi.URLParam(r, "id") issue, ok := h.loadIssueForUser(w, r, id) @@ -73,17 +98,42 @@ func (h *Handler) ListTimeline(w http.ResponseWriter, r *http.Request) { return } - entries := h.mergeTimelineAsc(r, comments, activities) + q := r.URL.Query() + wantWrapped := q.Get("limit") != "" || q.Get("before") != "" || + q.Get("after") != "" || q.Get("around") != "" + + if wantWrapped { + entries := h.mergeTimeline(r, comments, activities, false) + if entries == nil { + entries = []TimelineEntry{} + } + resp := timelinePaginatedResponse{Entries: entries} + // `around=`: locate the anchor in the DESC slice so the legacy + // client can scroll-to-highlight without a follow-up request. + if anchor := q.Get("around"); anchor != "" { + for i, e := range entries { + if e.ID == anchor { + idx := i + resp.TargetIndex = &idx + break + } + } + } + writeJSON(w, http.StatusOK, resp) + return + } + + entries := h.mergeTimeline(r, comments, activities, true) if entries == nil { entries = []TimelineEntry{} } writeJSON(w, http.StatusOK, entries) } -// mergeTimelineAsc merges comments and activities into a single timeline -// ordered by (created_at, id) ascending — oldest first, matching the contract -// the desktop and web frontends both consume. -func (h *Handler) mergeTimelineAsc(r *http.Request, comments []db.Comment, activities []db.ActivityLog) []TimelineEntry { +// mergeTimeline merges comments and activities and returns them sorted by +// (created_at, id). When ascending=true, oldest first (the new flat-array +// contract); otherwise newest first (the wrapped legacy contract). +func (h *Handler) mergeTimeline(r *http.Request, comments []db.Comment, activities []db.ActivityLog, ascending bool) []TimelineEntry { out := make([]TimelineEntry, 0, len(comments)+len(activities)) out = append(out, h.commentsToEntries(r, comments)...) for _, a := range activities { @@ -91,9 +141,15 @@ func (h *Handler) mergeTimelineAsc(r *http.Request, comments []db.Comment, activ } sort.Slice(out, func(i, j int) bool { if out[i].CreatedAt != out[j].CreatedAt { - return out[i].CreatedAt < out[j].CreatedAt + if ascending { + return out[i].CreatedAt < out[j].CreatedAt + } + return out[i].CreatedAt > out[j].CreatedAt } - return out[i].ID < out[j].ID + if ascending { + return out[i].ID < out[j].ID + } + return out[i].ID > out[j].ID }) return out } diff --git a/server/internal/handler/activity_test.go b/server/internal/handler/activity_test.go index 27837f88d..1ef9542ff 100644 --- a/server/internal/handler/activity_test.go +++ b/server/internal/handler/activity_test.go @@ -134,6 +134,79 @@ func TestListTimeline_MergesCommentsAndActivities(t *testing.T) { } } +// fetchTimelineWrapped exercises the legacy wrapped response shape that +// stale Multica.app v0.2.26+ builds still expect — sending any of +// limit/before/after/around makes the server emit a TimelinePage-style +// object (entries DESC, null cursors, has_more_*=false) instead of the new +// flat array. Used to verify the boundary-compat path doesn't regress. +func fetchTimelineWrapped(t *testing.T, issueID, query string) (timelinePaginatedResponse, int) { + t.Helper() + w := httptest.NewRecorder() + req := newRequest("GET", "/api/issues/"+issueID+"/timeline?"+query, nil) + req = withURLParam(req, "id", issueID) + testHandler.ListTimeline(w, req) + var resp timelinePaginatedResponse + if w.Code == http.StatusOK { + json.NewDecoder(w.Body).Decode(&resp) + } + return resp, w.Code +} + +// Boundary-compat: a stale client between #2128 and #1929 sends ?limit=50 +// and parses the response with TimelinePageSchema. The handler must keep +// returning the wrapped object so that path doesn't fall back to an empty +// timeline. +func TestListTimeline_LegacyWrappedShape_OnPaginationParams(t *testing.T) { + issueID := createIssueForTimeline(t, "Legacy wrapped shape test") + commentIDs, _ := seedTimelineEntries(t, issueID, 3, 0) + + resp, status := fetchTimelineWrapped(t, issueID, "limit=50") + if status != http.StatusOK { + t.Fatalf("status = %d, want 200", status) + } + if resp.HasMoreBefore || resp.HasMoreAfter { + t.Errorf("has_more_*: want false/false, got before=%v after=%v", + resp.HasMoreBefore, resp.HasMoreAfter) + } + if resp.NextCursor != nil || resp.PrevCursor != nil { + t.Errorf("cursors: want nil/nil, got next=%v prev=%v", resp.NextCursor, resp.PrevCursor) + } + // DESC order: most recent comment first; activity from issue-creation + // sits at the bottom. + commentEntries := []TimelineEntry{} + for _, e := range resp.Entries { + if e.Type == "comment" { + commentEntries = append(commentEntries, e) + } + } + if got, want := len(commentEntries), len(commentIDs); got != want { + t.Fatalf("comment count = %d, want %d", got, want) + } + for i, e := range commentEntries { + want := commentIDs[len(commentIDs)-1-i] + if e.ID != want { + t.Errorf("DESC entry %d: id = %s, want %s", i, e.ID, want) + } + } +} + +func TestListTimeline_LegacyWrappedShape_AroundFillsTargetIndex(t *testing.T) { + issueID := createIssueForTimeline(t, "Around target index test") + commentIDs, _ := seedTimelineEntries(t, issueID, 5, 0) + anchor := commentIDs[2] // pick a middle comment + + resp, status := fetchTimelineWrapped(t, issueID, "around="+anchor) + if status != http.StatusOK { + t.Fatalf("status = %d, want 200", status) + } + if resp.TargetIndex == nil { + t.Fatalf("target_index: want non-nil for around mode") + } + if got := resp.Entries[*resp.TargetIndex].ID; got != anchor { + t.Errorf("target_index points at %s, want anchor %s", got, anchor) + } +} + func TestListTimeline_EmptyIssue(t *testing.T) { issueID := createIssueForTimeline(t, "Empty timeline test") entries, status := fetchTimeline(t, issueID)