List the threads a push leaves behind

README.md

@@ -383,6 +383,7 @@ least 10 minutes of silence.
 crq next <repo> <pr>      # ⭐ the agent loop: emit the single next action as JSON (--wait blocks)
 crq loop <repo> <pr>      # blocking one-shot round: fire + wait + emit JSON findings
 crq feedback <repo> <pr>  # current normalized findings as JSON, WITHOUT triggering a review
+crq threads <repo> <pr>                                     # every unresolved thread, outdated included
 crq resolve <thread-id> [<thread-id>...]                    # resolve addressed review threads
 crq decline <thread-id> [...] --reason "<why>" [--resolve]  # record why a finding is declined
 crq dismiss <repo> <pr> <finding-id> [...] --reason "<why>"  # account for a finding with no thread

cmd/crq/main.go

@@ -211,6 +211,23 @@ func run(ctx context.Context, args []string) int {
 		}
 		printJSON(result)
 		return 0
+	case "threads":
+		repo, pr, ok := repoPR(args[1:])
+		if !ok {
+			fatal(errors.New("usage: crq threads <repo> <pr>"))
+			return 1
+		}
+		if err := cfg.RequireState(); err != nil {
+			fatal(err)
+			return 1
+		}
+		threads, terr := service.OpenThreads(ctx, repo, pr)
+		if terr != nil {
+			fatal(terr)
+			return 1
+		}
+		printJSON(threads)
+		return 0
 	case "decline":
 		threads, reason, resolve, ok := parseDeclineArgs(args[1:])
 		if !ok || len(threads) == 0 || strings.TrimSpace(reason) == "" {
@@ -385,6 +402,7 @@ USAGE
   crq decline <thread-id> [...] --reason "<why>" [--keep-open]
                                    reply on a thread to record why a finding is declined
                                    (resolves it; --keep-open leaves it open)
+  crq threads <repo> <pr>          list every unresolved review thread, outdated ones included
   crq dismiss <repo> <pr> <finding-id> [...] --reason "<why>"
                                    account for a finding GitHub gives you no thread to close
   crq autoreview [--once] [--no-incremental]
@@ -540,6 +558,20 @@ replies contesting the decline, crq re-surfaces that reply as its own finding.
 
 Pass --keep-open to leave it unresolved anyway (an on-the-record disagreement you
 intend to keep working). Thread IDs come from .findings[].thread_id.
+`)
+	case "threads":
+		fmt.Print(`crq threads <repo> <pr>
+
+List the PR's unresolved review threads as JSON, including the ones GitHub has
+marked OUTDATED, with .thread_id ready for crq resolve.
+
+Findings leave outdated threads out on purpose: the code they point at is gone,
+and anything carrying a thread ID blocks the round until it is resolved. But an
+outdated thread is still open on the PR — and after a push that is every thread
+from the previous head, so fixing and pushing used to leave no way to close them
+through crq at all.
+
+Read this, decide, then crq resolve (or crq decline) the ones you have answered.
 `)
 	case "dismiss":
 		fmt.Print(`crq dismiss <repo> <pr> <finding-id> [<finding-id>...] --reason "<why>"

internal/crq/threads.go

@@ -0,0 +1,115 @@
+package crq
+
+import (
+	"context"
+	"sort"
+
+	"github.com/kristofferR/coderabbit-queue/internal/dialect"
+)
+
+// OpenThread is one unresolved review thread, enough to decide about it and to
+// pass its ID to `crq resolve`.
+type OpenThread struct {
+	ID   string `json:"thread_id"`
+	Path string `json:"path,omitempty"`
+	Line int    `json:"line,omitempty"`
+	Bot  string `json:"bot,omitempty"`
+	// Author names a human reviewer, when the thread is not a bot's.
+	Author   string `json:"author,omitempty"`
+	Outdated bool   `json:"outdated"`
+	// Title is the thread's first line, so a list is readable without opening
+	// each one.
+	Title string `json:"title,omitempty"`
+	URL   string `json:"url,omitempty"`
+}
+
+// OpenThreads lists a PR's unresolved review threads, INCLUDING the ones GitHub
+// has marked outdated.
+//
+// Findings deliberately exclude outdated threads: the code they point at is
+// gone, so re-reporting them would be noise, and anything with a thread ID
+// blocks a round until it is resolved. But an outdated thread is still open on
+// the PR, and after a push that is every thread from the previous head — so an
+// agent that fixed and pushed had no way left to close them through crq at all.
+// The documented answer was to stop using crq and query GitHub's GraphQL API by
+// hand, which is the one thing the skill tells agents never to do.
+//
+// This is the read that closes that loop: list, decide, `crq resolve`.
+func (s *Service) OpenThreads(ctx context.Context, repo string, pr int) ([]OpenThread, error) {
+	threads, err := s.reviewThreads(ctx, repo, pr)
+	if err != nil {
+		return nil, err
+	}
+	out := make([]OpenThread, 0, len(threads))
+	for _, thread := range threads {
+		if thread.IsResolved {
+			continue
+		}
+		open := OpenThread{ID: thread.ID, Path: thread.Path, Line: thread.Line, Outdated: thread.IsOutdated}
+		if nodes := thread.Comments.Nodes; len(nodes) > 0 {
+			first := nodes[0]
+			// Only when the author is actually a reviewer bot. The command
+			// promises every unresolved thread, humans included, and labelling
+			// "alice" as a bot makes the output plainly wrong.
+			isBot := isReviewerBot(s.cfg, first.Author.Login)
+			if isBot {
+				open.Bot = dialect.NormalizeBotName(first.Author.Login)
+			} else {
+				open.Author = first.Author.Login
+			}
+			open.Title = dialect.ThreadTitle(isBot, first.Body)
+			open.URL = first.URL
+			if open.Path == "" {
+				open.Path = first.Path
+			}
+			if open.Line == 0 {
+				// GitHub clears both current lines on an outdated thread while
+				// keeping originalLine — and outdated threads are the whole reason
+				// this command exists.
+				open.Line = firstPositive(first.Line, first.OriginalLine)
+			}
+		}
+		out = append(out, open)
+	}
+	// Current threads first, then by location, so the ones still pointing at
+	// live code are what a reader sees first.
+	sort.SliceStable(out, func(i, j int) bool {
+		if out[i].Outdated != out[j].Outdated {
+			return !out[i].Outdated
+		}
+		if out[i].Path != out[j].Path {
+			return out[i].Path < out[j].Path
+		}
+		return out[i].Line < out[j].Line
+	})
+	return out, nil
+}
+
+// isReviewerBot reports whether login is a reviewer crq knows: the configured
+// primary, a configured feedback bot, a configured co-reviewer, or a registry
+// entry.
+func isReviewerBot(cfg Config, login string) bool {
+	key := dialect.NormalizeBotName(login)
+	if key == dialect.NormalizeBotName(cfg.Bot) {
+		return true
+	}
+	// Whatever CRQ_FEEDBACK_BOTS names is a bot to `crq feedback`, so it has to
+	// be one here too — otherwise the same thread is a bot's finding in one
+	// command and a human's comment in the other.
+	for _, bot := range cfg.FeedbackBots {
+		if dialect.NormalizeBotName(bot) == key {
+			return true
+		}
+	}
+	for _, cb := range cfg.CoBots {
+		if dialect.NormalizeBotName(cb.Login) == key {
+			return true
+		}
+	}
+	// By login. CoReviewerByName also matches the config NAME, so a human whose
+	// GitHub handle is "codex" or "bugbot" would be serialized as that bot.
+	if co, ok := dialect.CoReviewerByName(login); ok {
+		return dialect.NormalizeBotName(co.Login) == key
+	}
+	return false
+}

internal/crq/threads_test.go

@@ -0,0 +1,129 @@
+package crq
+
+import (
+	"context"
+	"encoding/json"
+	"strings"
+	"testing"
+)
+
+// The point of the command: after a push every thread from the previous head is
+// outdated, findings leave those out by design, and an agent then had no way to
+// close them through crq at all. They must be listed — and listed last, since
+// the threads still pointing at live code matter more.
+func TestOpenThreadsIncludesOutdatedAndOrdersThem(t *testing.T) {
+	gh := newFakeGitHub()
+	gh.graphQL = func(query string, _ map[string]any, out any) error {
+		if !strings.Contains(query, "reviewThreads") {
+			return nil
+		}
+		return json.Unmarshal([]byte(`{"repository":{"pullRequest":{"reviewThreads":{
+		  "pageInfo":{"hasNextPage":false,"endCursor":""},
+		  "nodes":[
+		    {"id":"T_outdated","isResolved":false,"isOutdated":true,"path":"a.go","line":1,
+		     "comments":{"totalCount":1,"nodes":[{"body":"**Stale point**","author":{"login":"coderabbitai[bot]"}}]}},
+		    {"id":"T_resolved","isResolved":true,"isOutdated":false,"path":"b.go","line":2,
+		     "comments":{"totalCount":1,"nodes":[{"body":"**Done**","author":{"login":"coderabbitai[bot]"}}]}},
+		    {"id":"T_current","isResolved":false,"isOutdated":false,"path":"c.go","line":3,
+		     "comments":{"totalCount":1,"nodes":[{"body":"**Live point**","author":{"login":"coderabbitai[bot]"}}]}}
+		  ]}}}}`), out)
+	}
+	svc := NewService(firingConfig(), gh, NewMemoryStore(firingConfig()), nil)
+
+	threads, err := svc.OpenThreads(context.Background(), "o/r", 1)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(threads) != 2 {
+		t.Fatalf("got %d threads, want the two unresolved ones", len(threads))
+	}
+	if threads[0].ID != "T_current" || threads[0].Outdated {
+		t.Errorf("first = %+v, want the current thread", threads[0])
+	}
+	if threads[1].ID != "T_outdated" || !threads[1].Outdated {
+		t.Errorf("second = %+v, want the outdated thread — omitting it is the bug", threads[1])
+	}
+	if threads[0].Title != "Live point" || threads[0].Bot != "coderabbitai" {
+		t.Errorf("thread = %+v, want a readable title and the bot named", threads[0])
+	}
+	// A human's thread is listed too — the command promises every unresolved one
+	// — but calling a person a bot makes the output plainly wrong.
+	if threads[0].Author != "" {
+		t.Errorf("thread = %+v, want no author on a bot's thread", threads[0])
+	}
+}
+
+// A login in CRQ_FEEDBACK_BOTS is a bot to `crq feedback`, which surfaces its
+// findings. Listing the same thread as a human's would disagree with the command
+// an agent reads next, and would parse the title with the human heuristic.
+func TestOpenThreadsNamesAConfiguredFeedbackBot(t *testing.T) {
+	gh := newFakeGitHub()
+	gh.graphQL = func(query string, _ map[string]any, out any) error {
+		if !strings.Contains(query, "reviewThreads") {
+			return nil
+		}
+		return json.Unmarshal([]byte(`{"repository":{"pullRequest":{"reviewThreads":{
+		  "pageInfo":{"hasNextPage":false,"endCursor":""},
+		  "nodes":[{"id":"T_custom","isResolved":false,"isOutdated":false,"path":"a.go","line":1,
+		    "comments":{"totalCount":1,"nodes":[{"body":"### Nil deref\n\n**High Severity**\n\nDetail.",
+		      "author":{"login":"reviewdog[bot]"}}]}}]
+		}}}}`), out)
+	}
+	cfg := firingConfig()
+	cfg.FeedbackBots = append(cfg.FeedbackBots, "reviewdog[bot]")
+	svc := NewService(cfg, gh, NewMemoryStore(cfg), nil)
+
+	threads, err := svc.OpenThreads(context.Background(), "o/r", 1)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(threads) != 1 {
+		t.Fatalf("got %d threads, want the custom reviewer's", len(threads))
+	}
+	if threads[0].Bot != "reviewdog" || threads[0].Author != "" {
+		t.Errorf("thread = %+v, want the configured feedback bot named as a bot", threads[0])
+	}
+	// Read as a bot's: the heading is the summary and the bold span is severity.
+	if threads[0].Title != "Nil deref" {
+		t.Errorf("title = %q, want the bot's heading rather than its severity", threads[0].Title)
+	}
+}
+
+// The command promises every unresolved thread, so it also gets humans'. Copying
+// the author into `bot` made the output say `"bot":"alice"`.
+func TestOpenThreadsDoesNotCallAPersonABot(t *testing.T) {
+	gh := newFakeGitHub()
+	gh.graphQL = func(query string, _ map[string]any, out any) error {
+		if !strings.Contains(query, "reviewThreads") {
+			return nil
+		}
+		return json.Unmarshal([]byte(`{"repository":{"pullRequest":{"reviewThreads":{
+		  "pageInfo":{"hasNextPage":false,"endCursor":""},
+		  "nodes":[{"id":"T_human","isResolved":false,"isOutdated":true,"path":"","line":0,
+		    "comments":{"totalCount":1,"nodes":[{"body":"Can we rename this?","originalLine":42,
+		      "path":"a.go","author":{"login":"alice"}}]}}]
+		}}}}`), out)
+	}
+	cfg := firingConfig()
+	svc := NewService(cfg, gh, NewMemoryStore(cfg), nil)
+
+	threads, err := svc.OpenThreads(context.Background(), "o/r", 1)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(threads) != 1 {
+		t.Fatalf("got %d threads, want the human's", len(threads))
+	}
+	if threads[0].Bot != "" {
+		t.Errorf("thread = %+v, want no bot on a human's thread", threads[0])
+	}
+	if threads[0].Author != "alice" {
+		t.Errorf("thread = %+v, want the human named as the author", threads[0])
+	}
+	// An outdated thread often keeps only originalLine, and those are the whole
+	// reason this command exists — dropping the location loses the only pointer
+	// to what the comment was about.
+	if threads[0].Line != 42 {
+		t.Errorf("line = %d, want the original line 42", threads[0].Line)
+	}
+}