Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 72 additions & 31 deletions cmd/crq/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,21 +93,35 @@ func run(ctx context.Context, args []string) int {
fmt.Printf("export CRQ_STATE_REF=%q\n", result.StateRef)
return 0
case "status":
if bad, found := unknownFlag(args[1:], "--line"); found {
fatal(fmt.Errorf("unknown flag %s (usage: crq status [--line])", bad))
return 1
}
// status takes no target: silently ignoring one would let `crq status 41`
// look like it reported on that PR.
if extra := positional(args[1:]); len(extra) > 0 {
fatal(fmt.Errorf("crq status takes no arguments, got %q (usage: crq status [--line])", extra[0]))
return 1
}
if err := cfg.RequireState(); err != nil {
fatal(err)
return 1
}
_, dashboard, err := service.Status(ctx)
state, dashboard, err := service.Status(ctx)
if err != nil {
fatal(err)
return 1
}
if hasFlag(args[1:], "--line") {
fmt.Println(crq.StatusLine(state, cfg))
return 0
}
fmt.Print(dashboard)
return 0
case "feedback":
repo, pr, ok := repoPR(args[1:])
if !ok {
fatal(errors.New("usage: crq feedback <repo> <pr>"))
repo, pr, err := target(ctx, service, args[1:], "crq feedback [<repo> <pr>]")
if err != nil {
fatal(err)
return 1
}
report, err := service.Feedback(ctx, repo, pr)
Expand All @@ -122,17 +136,16 @@ func run(ctx context.Context, args []string) int {
fatal(fmt.Errorf("unknown flag %s (usage: crq next <repo> <pr> [--wait])", bad))
return 1
}
repo, pr, ok := repoPR(positional(args[1:]))
if !ok {
fatal(errors.New("usage: crq next <repo> <pr> [--wait]"))
if err := cfg.RequireState(); err != nil {
fatal(err)
return 1
}
if err := cfg.RequireState(); err != nil {
repo, pr, err := target(ctx, service, positional(args[1:]), "crq next [<repo> <pr>] [--wait]")
if err != nil {
fatal(err)
return 1
}
var report crq.NextReport
var err error
if hasFlag(args[1:], "--wait") {
report, err = service.NextWaiting(ctx, repo, pr)
} else {
Expand All @@ -147,12 +160,12 @@ func run(ctx context.Context, args []string) int {
// caller never has to interpret two things at once.
return 0
case "wait":
repo, pr, ok := repoPR(args[1:])
if !ok {
fatal(errors.New("usage: crq wait <repo> <pr>"))
if err := cfg.RequireState(); err != nil {
fatal(err)
return 1
}
if err := cfg.RequireState(); err != nil {
repo, pr, err := target(ctx, service, args[1:], "crq wait [<repo> <pr>]")
if err != nil {
fatal(err)
return 1
}
Expand All @@ -165,9 +178,9 @@ func run(ctx context.Context, args []string) int {
// Same contract as next: the action is the answer, the exit code is not.
return 0
case "loop":
repo, pr, ok := repoPR(args[1:])
if !ok {
fatal(errors.New("usage: crq loop <repo> <pr>"))
repo, pr, err := target(ctx, service, args[1:], "crq loop [<repo> <pr>]")
if err != nil {
fatal(err)
return 1
}
if err := cfg.RequireState(); err != nil {
Expand Down Expand Up @@ -229,9 +242,9 @@ func run(ctx context.Context, args []string) int {
}
return 0
case "cancel":
repo, pr, ok := repoPR(args[1:])
if !ok {
fatal(errors.New("usage: crq cancel <repo> <pr>"))
repo, pr, err := target(ctx, service, args[1:], "crq cancel [<repo> <pr>]")
if err != nil {
fatal(err)
return 1
}
if err := cfg.RequireState(); err != nil {
Expand Down Expand Up @@ -263,9 +276,9 @@ func debug(ctx context.Context, service *crq.Service, store crq.StateStore, cfg
}
switch args[0] {
case "enqueue":
repo, pr, ok := repoPR(args[1:])
if !ok {
fatal(errors.New("usage: crq debug enqueue <repo> <pr>"))
repo, pr, err := target(ctx, service, args[1:], "crq debug enqueue [<repo> <pr>]")
if err != nil {
fatal(err)
return 1
}
result, err := service.Enqueue(ctx, repo, pr)
Expand Down Expand Up @@ -309,11 +322,11 @@ func usage() {
fmt.Print(`crq - CodeRabbit review queue for humans and automation

QUEUE WORKFLOWS
crq next <repo> <pr> ask what to do next about a PR (the agent loop)
crq wait <repo> <pr> block until there IS something to do, then say what
crq next [<repo> <pr>] ask what to do next about a PR (the agent loop)
crq wait [<repo> <pr>] block until there IS something to do, then say what
crq loop <repo> <pr> queue one PR review round, then emit JSON feedback
crq autoreview keep open PRs reviewed through the same queue
crq status show the queue, in-flight review, and quota state
crq status [--line] show the queue, in-flight review, and quota state

DRIVING A PR REVIEW
Call crq next, do exactly what .action says, call it again. That is the whole loop.
Expand All @@ -332,10 +345,12 @@ DRIVING A PR REVIEW

USAGE
crq init initialize state in CRQ_REPO
crq next <repo> <pr> [--wait] emit the single next action as JSON (--wait blocks)
crq next [<repo> <pr>] [--wait] emit the single next action as JSON (--wait blocks)
omit the target inside a checkout: crq reads the
remote and branch to find the pull request
Comment on lines +348 to +350

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 Badge Update the authoritative CLI docs for the new forms

The runtime now accepts inferred targets, but the command-specific help for next, wait, loop, feedback, and cancel still prints mandatory <repo> <pr> forms, the general usage block still shows wait as mandatory, and the README command contract remains unchanged (including omitting status --line). This leaves the repository's designated contract and usage surfaces contradicting the new CLI, so update them alongside these new optional forms.

AGENTS.md reference: AGENTS.md:L5-L6

Useful? React with 👍 / 👎.

crq wait <repo> <pr> block until actionable, then emit that action as JSON
crq loop <repo> <pr> coordinated trigger -> wait -> JSON feedback/convergence
crq feedback <repo> <pr> emit normalized actionable review findings as JSON
crq loop [<repo> <pr>] coordinated trigger -> wait -> JSON feedback/convergence
crq feedback [<repo> <pr>] emit normalized actionable review findings as JSON
crq resolve <thread-id> [<thread-id>...]
resolve addressed GitHub review threads
crq decline <thread-id> [...] --reason "<why>" [--keep-open]
Expand All @@ -346,8 +361,8 @@ USAGE
crq preflight [--type all|committed|uncommitted] [--base <branch>]
local CodeRabbit CLI pre-push review as JSON
crq doctor emit JSON readiness report for agents and humans
crq status print the dashboard
crq cancel <repo> <pr> remove queued/in-flight state for a PR
crq status [--line] print the dashboard, or one line for a status bar
crq cancel [<repo> <pr>] remove queued/in-flight state for a PR
crq debug <enqueue|pump|refresh|state>
maintenance tools; not for normal review loops

Expand Down Expand Up @@ -560,7 +575,18 @@ Checks include:
Use this before a human-run loop, background watcher, or autonomous agent.
`)
case "status":
fmt.Print("crq status\n\nPrint the dashboard rendered from the CAS state ref.\n")
fmt.Print(`crq status [--line]

Print the dashboard rendered from the CAS state ref.

--line prints ONE line instead, for a harness status bar:

crq 🔬 #41 reviewing · 2 queued

That answers "is it still going?" continuously, which is otherwise the question a
session spends the most tool calls on. It names the next PR only when the queue
actually knows which one that is.
`)
case "cancel":
fmt.Print("crq cancel <repo> <pr>\n\nRemove a PR from queued/in-flight crq state.\n")
case "debug":
Expand Down Expand Up @@ -629,6 +655,21 @@ func hasFlag(args []string, flag string) bool {
return false
}

// target resolves which PR a command is about: the two arguments if given, and
// otherwise the checkout the caller is standing in. Agents drive the loop from
// inside the repository, so making them carry a repo and number they are already
// standing in is two chances to name the wrong PR silently.
func target(ctx context.Context, service *crq.Service, args []string, usage string) (string, int, error) {
switch repo, pr, ok := repoPR(args); {
case ok:
return repo, pr, nil
case len(args) == 0:
return service.InferTarget(ctx)
Comment thread
kristofferR marked this conversation as resolved.
default:
return "", 0, fmt.Errorf("usage: %s", usage)
}
}

// unknownFlag returns the first flag in args that is not in allowed.
//
// positional() drops anything starting with "-", so without this a mistyped
Expand Down
4 changes: 4 additions & 0 deletions internal/crq/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"errors"
"fmt"
"io"
"net/url"
"sort"
"strconv"
"strings"
Expand Down Expand Up @@ -37,6 +38,9 @@ type GitHubAPI interface {
SearchOpenPRs(context.Context, string, bool, int) ([]ghapi.SearchPR, error)
EachOpenPR(context.Context, string, bool, func(ghapi.SearchPR) (bool, error)) error
GraphQL(context.Context, string, map[string]any, any) error
// ListPulls finds pull requests, filtered by the query. crq uses it to map a
// checkout's branch to the PR it belongs to.
ListPulls(context.Context, string, url.Values) ([]ghapi.Pull, error)
// GetRef reads a ref's SHA. It is the cheapest "did anything change?" probe
// crq has — a conditional GET that costs no quota while the ref is
// unchanged — which is what `crq wait` idles on.
Expand Down
29 changes: 29 additions & 0 deletions internal/crq/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"context"
"encoding/json"
"errors"
"net/url"
"sort"
"strconv"
"strings"
"sync"
Expand Down Expand Up @@ -235,6 +237,33 @@ func (f *fakeGitHub) EachOpenPR(_ context.Context, _ string, _ bool, fn func(gha
return nil
}

// ListPulls answers the branch->PR lookup target inference uses.
//
// It honours the head filter, because a fake that ignores it would let a caller
// asking for the wrong branch pass: every PR in the repository would come back
// and a single-result assertion would still hold.
func (f *fakeGitHub) ListPulls(_ context.Context, repo string, query url.Values) ([]ghapi.Pull, error) {
f.mu.Lock()
defer f.mu.Unlock()
wantRef := ""
if head := query.Get("head"); head != "" {
_, wantRef, _ = strings.Cut(head, ":") // "owner:branch"
}
var out []ghapi.Pull
for key, p := range f.pulls {
if !strings.HasPrefix(key, strings.ToLower(repo)+"#") {
continue
}
if wantRef != "" && p.Head.Ref != wantRef {
continue
}
out = append(out, p)
}
// Deterministic: map iteration order must not decide what a test sees.
sort.Slice(out, func(i, j int) bool { return out[i].Number < out[j].Number })
return out, nil
}

// GetRef reports the fake state ref. It is what `crq wait` idles on, so tests
// count the reads to assert the wait stays cheap.
func (f *fakeGitHub) GetRef(_ context.Context, _, _ string) (string, error) {
Expand Down
6 changes: 6 additions & 0 deletions internal/crq/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,12 @@ func renderDashboard(st State, cfg Config) string {
func renderTitle(st State, cfg Config) string {
return crqstate.RenderTitle(st, cfg.storeConfig())
}

// StatusLine renders the queue as a single line for a harness status bar.
func StatusLine(st State, cfg Config) string {
return crqstate.StatusLine(st, cfg.storeConfig())
}

func issueBody(st State, cfg Config) (string, error) {
return crqstate.IssueBody(st, cfg.storeConfig())
}
Expand Down
Loading